[libvirt] [PATCH] ESX: interface driver routines

[Daniel P. Berrange]

On Mon, Jul 09, 2012 at 05:55:47PM -0700, Ata Bohra wrote:
> 
> Thanks for your inputs Daniel, please see inline. 
> 
> > Date: Mon, 9 Jul 2012 09:21:36 +0100
> > From: berrange at redhat.com
> > To: ata.husain at hotmail.com
> > CC: libvirt-list at redhat.com
> > Subject: Re: [libvirt] [PATCH] ESX: interface driver routines
> > 
> > On Sun, Jul 08, 2012 at 08:52:01AM +0530, Ata Bohra wrote:
> > > Attached patch adds missing routines to esx interface driver, one of
> > > the missing routine is esxInterfaceDefineXML. I am currently working
> > > on it and will update patch soon. Also, patch addresses some of the
> > > changes needed in esx_vi_generator.py and esx_vi_types.c to deserialize
> > > "String List" types. 
> > 
> > Thanks for taking the time todo all this work !
> > 
> > > +/**
> > > + * Generates native XML descritpor for a given interface.
> > > + * For instance:
> > > + *    <interface type="bridge" name="%s">
> > > + *      <start mode="onboot"/>"
> > > + *      <mtu size="%d"/>"
> > > + *      <mac address="%s"/>
> > > + *      <protocol family="ipv4">
> > > + *         <dhcp/>
> > > + *         <ip address="%s" prefix="%d"/>
> > > + *         <route gateway="%s"/>
> > > + *      </protocol>
> > > + *      <bridge stp="off">
> > > + *         <interface type="ethernet" name="%s">
> > > + *            <mac address="%s"/>
> > > + *         </interface>
> > > + *      </bridge>
> > > + *    </interface>
> > > + */
> > > +static char*
> > > +esxGetNativeInterfaceXMLDesc(const esxVI_HostVirtualNic *virtualNic,
> > > +                             const esxVI_HostIpRouteConfig *ipRouteConfig,
> > > +                             const esxVI_PhysicalNic *physicalNicList,
> > > +                             const unsigned int flags)
> > > +{
> > > +    const esxVI_PhysicalNic *physicalNic = NULL;
> > > +    xmlDocPtr doc = NULL;
> > > +    xmlNodePtr root = NULL;
> > > +    xmlNodePtr startNode = NULL;
> > > +    xmlNodePtr mtuNode = NULL;
> > > +    xmlNodePtr protocolNode = NULL;
> > > +    xmlNodePtr bridgeNode = NULL;
> > > +    xmlNodePtr dhcpNode = NULL;
> > > +    xmlChar *xmlbuff = NULL;
> > > +    int use_static = 0;
> > > +    struct in_addr addr;
> > > +    uint32_t host_addr = 0;
> > > +    int zero_count = 0;
> > > +    int masklen = 0;
> > > +    int i = 0;
> > > +    virBuffer item = VIR_BUFFER_INITIALIZER;
> > > +    int bufferSize = 0;
> > > +    char *ret = NULL;
> > > +
> > > +    if (VIR_INTERFACE_XML_INACTIVE & flags) {
> > > +        use_static = 1;
> > > +    }
> > > +
> > > +    doc = xmlNewDoc(XML_CAST "1.0");
> > > +    root = xmlNewDocNode(doc, NULL, XML_CAST "interface", NULL);
> > 
> > You must not construct XML documents manually like this. We have
> > an internal data structure for representing the inteface configuration
> > that you should populate. This has an API for generating the correct
> > formed XML document.  See the structs & APIs in src/conf/interface_conf.h
> > 
> > The same applies for parsing XML too - use the inteface APIs.
> [AB]: I am using the predefined API provided by src/conf/interface_conf.h/c to generate the libvirt domain XML returned from this function (virInterfaceDefParseString). Analyzing the interface_conf.h, there are couple of ways to get interfaceDefPtr: virInterfaceDefParseString, virInterfaceDefParseFile or virInterfaceDefParseNode. So instead of preparing the native XML by using virBuffer technique I prepared XML document which is then given to virInterfaceDefParseString() to generate virInterfaceDef object and further used to generate libvirt domain interface XML file (virInterfaceDefFormat). While coding I found the XML one easy to read and follow. Please correct me if wrong. 


No, this isn't acceptable. None of the hypervisor drivers are
permitted to using the libxml functions for either generating
or parsing configuration. You must populate the virInterfaceDefPtr
object directly.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|