Thanks for your inputs Daniel, please see inline.
> Date: Mon, 9 Jul 2012 09:21:36 +0100 > From: berrange@xxxxxxxxxx > To: ata.husain@xxxxxxxxxxx > CC: libvirt-list@xxxxxxxxxx > Subject: Re: [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. > > > + > > + xmlNewProp(root, XML_CAST "type", XML_CAST "bridge"); > > + xmlNewProp(root, XML_CAST "name", XML_CAST virtualNic->device); > > + xmlDocSetRootElement(doc, root); > > + > > + /* define boot start mode */ > > + startNode = xmlNewChild(root, NULL, XML_CAST "start", NULL); > > + xmlNewProp(startNode, XML_CAST "mode", XML_CAST "onboot"); > > + > > + /* append mtu value */ > > + mtuNode = xmlNewChild(root, NULL, XML_CAST "mtu", NULL); > > + virBufferAsprintf(&item, "%d",virtualNic->spec->mtu && > > + virtualNic->spec->mtu->value ? > > + virtualNic->spec->mtu->value : > > + 1500); > > + const char *mtustr = virBufferContentAndReset(&item); > > + if (mtustr == NULL) { > > + virReportOOMError(); > > + goto cleanup; > > + } > > + xmlNewProp(mtuNode, XML_CAST "size", XML_CAST mtustr); > > + > > + /* append mac address field */ > > + if (!use_static && virtualNic->spec->mac) { > > + xmlNodePtr mac_node = xmlNewChild(root, NULL, XML_CAST "mac", NULL); > > + xmlNewProp(mac_node, XML_CAST "address", > > + XML_CAST virtualNic->spec->mac); > > + } > > + > > + /* TODO - Handle VLAN (via portgroup?) */ > > + if (virtualNic->spec->ip->subnetMask && > > + *virtualNic->spec->ip->subnetMask && > > + inet_aton(virtualNic->spec->ip->subnetMask, &addr) == 0) { > > + ESX_ERROR(VIR_ERR_INTERNAL_ERROR, "%s", > > + _("Error parsing netmask")); > > + goto cleanup; > > + } > > + > > + host_addr = ntohl(addr.s_addr); > > + /* Calculate masklen */ > > + for (i = 0; i < 32; ++i) { > > + if (host_addr & 0x01) { > > + break; > > + } > > + zero_count++; > > + host_addr >>= 1; > > + } > > + masklen = 32 - zero_count; > > + > > + /* append protocol field */ > > + /* TODO - Add IPv6 Support */ > > + protocolNode = xmlNewChild(root, NULL, XML_CAST "protocol", NULL); > > + xmlNewProp(protocolNode, XML_CAST "family", XML_CAST "ipv4"); > > + if (virtualNic->spec->ip->dhcp == 1) { > > + dhcpNode = xmlNewChild(protocolNode, NULL, XML_CAST "dhcp", NULL); > > + /* avoids compiler warning */ > > + VIR_DEBUG("dhcpNode name: %s", (char *)dhcpNode->name); > > + } > > + > > + if (virtualNic->spec->ip->dhcp != 1 || !use_static) { > > + if (virtualNic->spec->ip->ipAddress && > > + *virtualNic->spec->ip->ipAddress) { > > + xmlNodePtr ipAddrNode = > > + xmlNewChild(protocolNode, NULL, XML_CAST "ip", NULL); > > + xmlNewProp(ipAddrNode, XML_CAST "address", XML_CAST > > + virtualNic->spec->ip->ipAddress); > > + > > + virBufferAsprintf(&item, "%d", masklen); > > + const char *maskstr = virBufferContentAndReset(&item); > > + if (maskstr == NULL) { > > + virReportOOMError(); > > + goto cleanup; > > + } > > + xmlNewProp(ipAddrNode, XML_CAST "prefix", XML_CAST maskstr); > > + > > + xmlNodePtr routeNode = > > + xmlNewChild(protocolNode, NULL, XML_CAST "route", NULL); > > + xmlNewProp(routeNode, XML_CAST "gateway", > > + XML_CAST ipRouteConfig->defaultGateway); > > + } > > + } > > + > > + /* Add bridge information */ > > + bridgeNode = xmlNewChild(root, NULL, XML_CAST "bridge", NULL); > > + xmlNewProp(bridgeNode, XML_CAST "stp", XML_CAST "off"); > > + > > + for (physicalNic = physicalNicList; > > + physicalNic != NULL; > > + physicalNic = physicalNic->_next) { > > + xmlNodePtr bridgeIfaceNode = > > + xmlNewChild(bridgeNode, NULL, XML_CAST "interface", NULL); > > + xmlNewProp(bridgeIfaceNode, XML_CAST "type", XML_CAST "ethernet"); > > + xmlNewProp(bridgeIfaceNode, XML_CAST "name", > > + XML_CAST physicalNic->device); > > + > > + xmlNodePtr bridgeIfaceMacNode = > > + xmlNewChild(bridgeIfaceNode, NULL, XML_CAST "mac", NULL); > > + xmlNewProp(bridgeIfaceMacNode, XML_CAST "address", > > + XML_CAST physicalNic->mac); > > + } > > + > > + xmlDocDumpFormatMemory(doc, &xmlbuff, &bufferSize, 1); > > + if (xmlbuff == NULL) { > > + virReportOOMError(); > > + goto cleanup; > > + } > > + > > + ret = strdup((char *)xmlbuff); > > + if (ret == NULL) { > > + virReportOOMError(); > > + goto cleanup; > > + } > > + > > + cleanup: > > + VIR_FREE(mtustr); > > + if (xmlbuff != NULL) { > > + xmlFree(xmlbuff); > > + } > > + xmlFreeDoc(doc); > > + > > + return ret; > > +} > > +static int > > > Regards, > 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 :| Thanks! Ata |
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list