2012/2/22 Jean-Baptiste Rouault <jean-baptiste.rouault@xxxxxxxxxxx>:
> 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list