[libvirt] [PATCH] ESX: interface driver routines

Hi All, 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. # On branch master # Changes not staged for commit: # (use "git add <file>..." to update what will be committed) # (use "git checkout -- <file>..." to discard changes in working directory) # # modified: src/esx/esx_interface_driver.c # modified: src/esx/esx_vi.c # modified: src/esx/esx_vi.h # modified: src/esx/esx_vi_generator.input # modified: src/esx/esx_vi_generator.py # modified: src/esx/esx_vi_types.c Thanks! Ata Attachment: 1. esxInterface.diff

On Sat, Jul 7, 2012 at 10:22 PM, Ata Bohra <ata.husain@hotmail.com> wrote:
Hi All,
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.
# On branch master # Changes not staged for commit: # (use "git add <file>..." to update what will be committed) # (use "git checkout -- <file>..." to discard changes in working directory) # # modified: src/esx/esx_interface_driver.c # modified: src/esx/esx_vi.c # modified: src/esx/esx_vi.h # modified: src/esx/esx_vi_generator.input # modified: src/esx/esx_vi_generator.py # modified: src/esx/esx_vi_types.c
Thanks! Ata
Please commit these to a local branch and then use git-send-email to send it to the list. Thanks. -- Doug Goldstein

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.
+ + 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 for your inputs Daniel, please see inline.
Date: Mon, 9 Jul 2012 09:21:36 +0100 From: berrange@redhat.com To: ata.husain@hotmail.com CC: libvirt-list@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.
+ + 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

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@redhat.com To: ata.husain@hotmail.com CC: libvirt-list@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 :|
participants (3)
-
Ata Bohra
-
Daniel P. Berrange
-
Doug Goldstein