[libvirt] [PATCH 0/2] VMware Workstation domainXMLFromNative support
These patches add domainXMLFromNative to the vmware driver and add support for more network configurations in vmx files Jean-Baptiste Rouault (2): vmware: implement domainXMLFromNative vmx: Better Workstation vmx handling src/vmware/vmware_driver.c | 31 +++++++++++++++++++++++++++++++ src/vmx/vmx.c | 29 ++++++++++++++++++----------- 2 files changed, 49 insertions(+), 11 deletions(-) -- 1.7.9
--- src/vmware/vmware_driver.c | 31 +++++++++++++++++++++++++++++++ 1 files changed, 31 insertions(+), 0 deletions(-) diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c index 56e9d2d..8f9d922 100644 --- a/src/vmware/vmware_driver.c +++ b/src/vmware/vmware_driver.c @@ -844,6 +844,36 @@ vmwareDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) return ret; } +static char * +vmwareDomainXMLFromNative(virConnectPtr conn, const char *nativeFormat, + const char *nativeConfig, + unsigned int flags) +{ + struct vmware_driver *driver = conn->privateData; + virVMXContext ctx; + virDomainDefPtr def = NULL; + char *xml = NULL; + + virCheckFlags(0, NULL); + + if (STRNEQ(nativeFormat, "vmware-vmx")) { + vmwareError(VIR_ERR_INVALID_ARG, + _("Unsupported config format '%s'"), nativeFormat); + return NULL; + } + + ctx.parseFileName = vmwareCopyVMXFileName; + + def = virVMXParseConfig(&ctx, driver->caps, nativeConfig); + + if (def != NULL) + xml = virDomainDefFormat(def, VIR_DOMAIN_XML_INACTIVE); + + virDomainDefFree(def); + + return xml; +} + static int vmwareNumDefinedDomains(virConnectPtr conn) { @@ -988,6 +1018,7 @@ static virDriver vmwareDriver = { .domainGetInfo = vmwareDomainGetInfo, /* 0.8.7 */ .domainGetState = vmwareDomainGetState, /* 0.9.2 */ .domainGetXMLDesc = vmwareDomainGetXMLDesc, /* 0.8.7 */ + .domainXMLFromNative = vmwareDomainXMLFromNative, /* 0.9.11 */ .listDefinedDomains = vmwareListDefinedDomains, /* 0.8.7 */ .numOfDefinedDomains = vmwareNumDefinedDomains, /* 0.8.7 */ .domainCreate = vmwareDomainCreate, /* 0.8.7 */ -- 1.7.9
This patch adds support for vmx files with empty networkName values (which is the case for vmx generated by Workstation). It also adds support for vmx containing NATed network interfaces. --- src/vmx/vmx.c | 29 ++++++++++++++++++----------- 1 files changed, 18 insertions(+), 11 deletions(-) diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 823d5df..3cc3b10 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -2418,12 +2418,20 @@ virVMXParseEthernet(virConfPtr conf, int controller, virDomainNetDefPtr *def) } /* vmx:networkName -> def:data.bridge.brname */ - if ((connectionType == NULL || - STRCASEEQ(connectionType, "bridged") || - STRCASEEQ(connectionType, "custom")) && - virVMXGetConfigString(conf, networkName_name, &networkName, - false) < 0) { - goto cleanup; + if (connectionType == NULL || + STRCASEEQ(connectionType, "bridged") || + STRCASEEQ(connectionType, "custom")) { + if (virVMXGetConfigString(conf, networkName_name, &networkName, + true) < 0) + goto cleanup; + + if (networkName == NULL) { + networkName = strdup(""); + if (networkName == NULL) { + virReportOOMError(); + goto cleanup; + } + } } /* vmx:vnet -> def:data.ifname */ @@ -2447,11 +2455,10 @@ virVMXParseEthernet(virConfPtr conf, int controller, virDomainNetDefPtr *def) connectionType, connectionType_name); goto cleanup; } else if (STRCASEEQ(connectionType, "nat")) { - /* FIXME */ - VMX_ERROR(VIR_ERR_INTERNAL_ERROR, - _("No yet handled value '%s' for VMX entry '%s'"), - connectionType, connectionType_name); - goto cleanup; + (*def)->type = VIR_DOMAIN_NET_TYPE_USER; + (*def)->model = virtualDev; + + virtualDev = NULL; } else if (STRCASEEQ(connectionType, "custom")) { (*def)->type = VIR_DOMAIN_NET_TYPE_BRIDGE; (*def)->model = virtualDev; -- 1.7.9
2012/2/22 Jean-Baptiste Rouault <jean-baptiste.rouault@diateam.net>:
This patch adds support for vmx files with empty networkName values (which is the case for vmx generated by Workstation).
It also adds support for vmx containing NATed network interfaces. --- src/vmx/vmx.c | 29 ++++++++++++++++++----------- 1 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 823d5df..3cc3b10 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -2418,12 +2418,20 @@ virVMXParseEthernet(virConfPtr conf, int controller, virDomainNetDefPtr *def) }
/* vmx:networkName -> def:data.bridge.brname */ - if ((connectionType == NULL || - STRCASEEQ(connectionType, "bridged") || - STRCASEEQ(connectionType, "custom")) && - virVMXGetConfigString(conf, networkName_name, &networkName, - false) < 0) { - goto cleanup; + if (connectionType == NULL || + STRCASEEQ(connectionType, "bridged") || + STRCASEEQ(connectionType, "custom")) { + if (virVMXGetConfigString(conf, networkName_name, &networkName, + true) < 0) + goto cleanup;
When networkName is NULL then there was no ethernet0.networkName entry in the VMX file at all. Setting it to an empty string here will result in an ethernet0.networkName = "" entry in the VMX file when the domain XML config is reformatted to VMX again. I'm not sure that this is correct. In general, a missing VMX entry means that the hypervisor should use the default value for this entry, but explicitly giving it an empty value will probably result in different behavior. As VIR_DOMAIN_NET_TYPE_BRIDGE requires a bridge name networkName cannot be NULL. So we need a special string to indicate that ethernet0.networkName is missing. This could be an empty string. To do this virVMXFormatEthernet needs to be changed to not add an ethernet0.networkName entry when def->data.bridge.brname is an empty string. A minor problem with this approach is that parsing and reformatting and VMX file with ethernet0.networkName = "" will result in removing this entry as "" is used as special value now.
+ if (networkName == NULL) { + networkName = strdup(""); + if (networkName == NULL) { + virReportOOMError(); + goto cleanup; + } + } }
/* vmx:vnet -> def:data.ifname */ @@ -2447,11 +2455,10 @@ virVMXParseEthernet(virConfPtr conf, int controller, virDomainNetDefPtr *def) connectionType, connectionType_name); goto cleanup; } else if (STRCASEEQ(connectionType, "nat")) { - /* FIXME */ - VMX_ERROR(VIR_ERR_INTERNAL_ERROR, - _("No yet handled value '%s' for VMX entry '%s'"), - connectionType, connectionType_name); - goto cleanup; + (*def)->type = VIR_DOMAIN_NET_TYPE_USER; + (*def)->model = virtualDev; + + virtualDev = NULL; } else if (STRCASEEQ(connectionType, "custom")) { (*def)->type = VIR_DOMAIN_NET_TYPE_BRIDGE; (*def)->model = virtualDev;
You're missing the counterpart in virVMXFormatEthernet to handle VIR_DOMAIN_NET_TYPE_USER. And this needs extra testcases in the vmx2xml and xml2vmx tests to cover this additions to the VMX parser before I can ACK it. A good start but it needs a v2. -- Matthias Bolte http://photron.blogspot.com
2012/2/22 Matthias Bolte <matthias.bolte@googlemail.com>:
And this needs extra testcases in the vmx2xml and xml2vmx tests to cover this additions to the VMX parser before I can ACK it.
The test suite already has some actual in-the-wild ESX and GSX VMX config files. I suggest you add some actual VMX files for Workstation that cover the new aspects in this patch. -- Matthias Bolte http://photron.blogspot.com
2012/2/22 Jean-Baptiste Rouault <jean-baptiste.rouault@diateam.net>:
--- src/vmware/vmware_driver.c | 31 +++++++++++++++++++++++++++++++ 1 files changed, 31 insertions(+), 0 deletions(-)
ACK. This one is good and independent from the second patch, so I push it now. -- Matthias Bolte http://photron.blogspot.com
participants (2)
-
Jean-Baptiste Rouault -
Matthias Bolte