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