Re: [libvirt] [PATCH 2/2] vmx: Better Workstation vmx handling
2012/2/22 Jean-Baptiste Rouault <jean-baptiste.rouault(a)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