[libvirt] [PATCH 0/4] esx, vmware: conditionally allow newer virtualHW versions

With this one should be able to connect with e.g. 'esx://localhost?allow_unsafe=1' to newer ESX hypervisor than supported. Docs are in [1/4] and [2/4] with esx and vmware changes, respectively. Reason for the parameter is explained in [3/4]. Tests are in [4/4]. Martin Kletzander (4): esx: parse new URI param 'allow_unsafe' vmware: parse new URI param 'allow_unsafe' vmx: allow version safety bypass with allowUnsafe tests: vmx support for allowUnsafe docs/drvesx.html.in | 14 ++++++++++++++ docs/drvvmware.html.in | 35 +++++++++++++++++++++++++++++++++++ src/esx/esx_driver.c | 6 ++++-- src/esx/esx_util.c | 13 ++++++++++++- src/esx/esx_util.h | 2 ++ src/vmware/vmware_conf.c | 4 ++-- src/vmware/vmware_conf.h | 2 ++ src/vmware/vmware_driver.c | 20 ++++++++++++++++++-- src/vmx/vmx.c | 24 ++++++++++++++++++------ src/vmx/vmx.h | 4 +++- tests/vmx2xmldata/vmx2xml-unsafe.vmx | 2 ++ tests/vmx2xmldata/vmx2xml-unsafe.xml | 18 ++++++++++++++++++ tests/vmx2xmltest.c | 36 +++++++++++++++++++++++++++++++----- 13 files changed, 161 insertions(+), 19 deletions(-) create mode 100644 tests/vmx2xmldata/vmx2xml-unsafe.vmx create mode 100644 tests/vmx2xmldata/vmx2xml-unsafe.xml -- 1.9.2

Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- docs/drvesx.html.in | 14 ++++++++++++++ src/esx/esx_util.c | 13 ++++++++++++- src/esx/esx_util.h | 2 ++ 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/docs/drvesx.html.in b/docs/drvesx.html.in index 0816baf..d941763 100644 --- a/docs/drvesx.html.in +++ b/docs/drvesx.html.in @@ -185,6 +185,20 @@ vpx://example-vcenter.com/folder1/dc1/folder2/example-esx.com override the default port 1080. </td> </tr> + <tr> + <td> + <code>allow_unsafe</code> + </td> + <td> + <code>0</code> or <code>1</code> + </td> + <td> + If set to 1, this disables check for supported + virtualHW.version, so connections to newer versions + than supported is possible. The default value is 0. + <span class="since">Since 1.2.5</since> + </td> + </tr> </table> diff --git a/src/esx/esx_util.c b/src/esx/esx_util.c index f84ecd5..87bca64 100644 --- a/src/esx/esx_util.c +++ b/src/esx/esx_util.c @@ -1,7 +1,7 @@ /* * esx_util.c: utility functions for the VMware ESX driver * - * Copyright (C) 2010-2012 Red Hat, Inc. + * Copyright (C) 2010-2012, 2014 Red Hat, Inc. * Copyright (C) 2009-2011 Matthias Bolte <matthias.bolte@googlemail.com> * Copyright (C) 2009 Maximilian Wilhelm <max@rfc2324.org> * @@ -46,6 +46,7 @@ esxUtil_ParseUri(esxUtil_ParsedUri **parsedUri, virURIPtr uri) size_t i; int noVerify; int autoAnswer; + int allowUnsafe; char *tmp; if (!parsedUri || *parsedUri) { @@ -150,6 +151,16 @@ esxUtil_ParseUri(esxUtil_ParsedUri **parsedUri, virURIPtr uri) goto cleanup; } } + } else if (STRCASEEQ(queryParam->name, "allow_unsafe")) { + if (virStrToLong_i(queryParam->value, NULL, 10, &allowUnsafe) < 0 || + (allowUnsafe != 0 && allowUnsafe != 1)) { + virReportError(VIR_ERR_INVALID_ARG, + _("Query parameter 'allow_unsafe' has unexpected value " + "'%s' (should be 0 or 1)"), queryParam->value); + goto cleanup; + } + + (*parsedUri)->allowUnsafe = allowUnsafe; } else { VIR_WARN("Ignoring unexpected query parameter '%s'", queryParam->name); diff --git a/src/esx/esx_util.h b/src/esx/esx_util.h index c6f14bb..0846ece 100644 --- a/src/esx/esx_util.h +++ b/src/esx/esx_util.h @@ -1,6 +1,7 @@ /* * esx_util.h: utility functions for the VMware ESX driver * + * Copyright (C) 2014 Red Hat, Inc. * Copyright (C) 2009 Matthias Bolte <matthias.bolte@googlemail.com> * * This library is free software; you can redistribute it and/or @@ -38,6 +39,7 @@ struct _esxUtil_ParsedUri { char *proxy_hostname; int proxy_port; char *path; + bool allowUnsafe; }; int esxUtil_ParseUri(esxUtil_ParsedUri **parsedUri, virURIPtr uri); -- 1.9.2

On 05/05/2014 03:47 AM, Martin Kletzander wrote:
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- docs/drvesx.html.in | 14 ++++++++++++++ src/esx/esx_util.c | 13 ++++++++++++- src/esx/esx_util.h | 2 ++ 3 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/docs/drvesx.html.in b/docs/drvesx.html.in index 0816baf..d941763 100644 --- a/docs/drvesx.html.in +++ b/docs/drvesx.html.in @@ -185,6 +185,20 @@ vpx://example-vcenter.com/folder1/dc1/folder2/example-esx.com override the default port 1080. </td> </tr> + <tr> + <td> + <code>allow_unsafe</code> + </td> + <td> + <code>0</code> or <code>1</code> + </td> + <td> + If set to 1, this disables check for supported + virtualHW.version, so connections to newer versions + than supported is possible. The default value is 0.
"connections" is plural, so s/is possible/are possible/ On the surface this (and the rest of the series) makes sense - it allows older libvirt to be used with a newer ESX version which may or may not work, until such time as newer libvirt has been tested to explicitly work with that ESX. But I'd like an opinion from someone that actually uses esx:// URIs before checking this in. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On Mon, May 05, 2014 at 01:48:03PM -0600, Eric Blake wrote:
On 05/05/2014 03:47 AM, Martin Kletzander wrote:
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- docs/drvesx.html.in | 14 ++++++++++++++ src/esx/esx_util.c | 13 ++++++++++++- src/esx/esx_util.h | 2 ++ 3 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/docs/drvesx.html.in b/docs/drvesx.html.in index 0816baf..d941763 100644 --- a/docs/drvesx.html.in +++ b/docs/drvesx.html.in @@ -185,6 +185,20 @@ vpx://example-vcenter.com/folder1/dc1/folder2/example-esx.com override the default port 1080. </td> </tr> + <tr> + <td> + <code>allow_unsafe</code> + </td> + <td> + <code>0</code> or <code>1</code> + </td> + <td> + If set to 1, this disables check for supported + virtualHW.version, so connections to newer versions + than supported is possible. The default value is 0.
"connections" is plural, so s/is possible/are possible/
On the surface this (and the rest of the series) makes sense - it allows older libvirt to be used with a newer ESX version which may or may not work, until such time as newer libvirt has been tested to explicitly work with that ESX. But I'd like an opinion from someone that actually uses esx:// URIs before checking this in.
Since the vmx parsing was moved to vmx/vmx.c (so basically since it was introduced), we were blindly (or at least in some cases) adding "support" for newer virtualHW_versions (version 8 in commit 5759a5cc, 9 in e0eb672e and finally 10 now in 5cc3cce5). I tried getting someone to test the version I added, but since there's so few of us working with ESX (I had to ask Matthias if he can have a look at the code), we can only assume that it will work. I even remember someone filing a bug that the connection works and everything, but there's a warning. My thinking was that we have two options here: 1) continue messing the code with blindly adding versions of virtualHW or 2) conditionally allow anything the user wants, with the condition showing that we don't "support" it. My reasoning behind choosing version 2 was that the development around VMX-related backends is not that active as with other ones. So if anything doesn't work because of HW version it is somehow visible that the user is not using "supported" versions of vmx descriptors (or how to call the .vmx file). I designed the patch so there is no functional nor API change with current usage, it merely allows to check whether our code works with newer versions without touching the code (currently, when someone wants to try that, he needs to add the version to supported ones, obviously). And it still leaves the list of supported versions in place and whoever wants some version to be supported can add it to the list. I know it is impossible to force people fix everything for versions they added, but it could be at least a bit visible that we currently don't have the capacity for that. Martin

2014-05-06 10:10 GMT+02:00 Martin Kletzander <mkletzan@redhat.com>:
On Mon, May 05, 2014 at 01:48:03PM -0600, Eric Blake wrote:
On 05/05/2014 03:47 AM, Martin Kletzander wrote:
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- docs/drvesx.html.in | 14 ++++++++++++++ src/esx/esx_util.c | 13 ++++++++++++- src/esx/esx_util.h | 2 ++ 3 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/docs/drvesx.html.in b/docs/drvesx.html.in index 0816baf..d941763 100644 --- a/docs/drvesx.html.in +++ b/docs/drvesx.html.in @@ -185,6 +185,20 @@ vpx://example-vcenter.com/folder1/dc1/folder2/example-esx.com override the default port 1080. </td> </tr> + <tr> + <td> + <code>allow_unsafe</code> + </td> + <td> + <code>0</code> or <code>1</code> + </td> + <td> + If set to 1, this disables check for supported + virtualHW.version, so connections to newer versions + than supported is possible. The default value is 0.
"connections" is plural, so s/is possible/are possible/
On the surface this (and the rest of the series) makes sense - it allows older libvirt to be used with a newer ESX version which may or may not work, until such time as newer libvirt has been tested to explicitly work with that ESX. But I'd like an opinion from someone that actually uses esx:// URIs before checking this in.
Since the vmx parsing was moved to vmx/vmx.c (so basically since it was introduced), we were blindly (or at least in some cases) adding "support" for newer virtualHW_versions (version 8 in commit 5759a5cc, 9 in e0eb672e and finally 10 now in 5cc3cce5). I tried getting someone to test the version I added, but since there's so few of us working with ESX (I had to ask Matthias if he can have a look at the code), we can only assume that it will work. I even remember someone filing a bug that the connection works and everything, but there's a warning. My thinking was that we have two options here:
1) continue messing the code with blindly adding versions of virtualHW or
2) conditionally allow anything the user wants, with the condition showing that we don't "support" it.
My reasoning behind choosing version 2 was that the development around VMX-related backends is not that active as with other ones. So if anything doesn't work because of HW version it is somehow visible that the user is not using "supported" versions of vmx descriptors (or how to call the .vmx file). I designed the patch so there is no functional nor API change with current usage, it merely allows to check whether our code works with newer versions without touching the code (currently, when someone wants to try that, he needs to add the version to supported ones, obviously). And it still leaves the list of supported versions in place and whoever wants some version to be supported can add it to the list.
I know it is impossible to force people fix everything for versions they added, but it could be at least a bit visible that we currently don't have the capacity for that.
Martin
Let's take a step back and look at the complete picture. Initially it seemed to me that the virtualHW version would have a larger effect on the content and format of the VMX file than to actually turned out to have. That's why I added this strict check to the parser in the first place. But after two major vSphere version (4 and 5) and multiple virtualHW version came along since the initial implementation of the VMX parser I'm now pretty confident that the actual virtualHW version is not that important to the parser as I initially thought it would be. I see no gain in adding any extra logic here to allow "unsafe" versions. Actually, I dislike this word in this context, it gives the user a wrong impression about the situation. There is nothing unsafe and nothing to test for compatibility here, it's only me misjudging the importance and meaning of this config entry. I suggest a different approach to deal with future virtualHW versions: basically ignore the value, as it's not that important. It should have been that way from the beginning. Here's a patch for that: https://www.redhat.com/archives/libvir-list/2014-May/msg00313.html -- Matthias Bolte http://photron.blogspot.com

On Sat, May 10, 2014 at 05:06:27PM +0200, Matthias Bolte wrote:
2014-05-06 10:10 GMT+02:00 Martin Kletzander <mkletzan@redhat.com>:
On Mon, May 05, 2014 at 01:48:03PM -0600, Eric Blake wrote:
On 05/05/2014 03:47 AM, Martin Kletzander wrote:
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- docs/drvesx.html.in | 14 ++++++++++++++ src/esx/esx_util.c | 13 ++++++++++++- src/esx/esx_util.h | 2 ++ 3 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/docs/drvesx.html.in b/docs/drvesx.html.in index 0816baf..d941763 100644 --- a/docs/drvesx.html.in +++ b/docs/drvesx.html.in @@ -185,6 +185,20 @@ vpx://example-vcenter.com/folder1/dc1/folder2/example-esx.com override the default port 1080. </td> </tr> + <tr> + <td> + <code>allow_unsafe</code> + </td> + <td> + <code>0</code> or <code>1</code> + </td> + <td> + If set to 1, this disables check for supported + virtualHW.version, so connections to newer versions + than supported is possible. The default value is 0.
"connections" is plural, so s/is possible/are possible/
On the surface this (and the rest of the series) makes sense - it allows older libvirt to be used with a newer ESX version which may or may not work, until such time as newer libvirt has been tested to explicitly work with that ESX. But I'd like an opinion from someone that actually uses esx:// URIs before checking this in.
Since the vmx parsing was moved to vmx/vmx.c (so basically since it was introduced), we were blindly (or at least in some cases) adding "support" for newer virtualHW_versions (version 8 in commit 5759a5cc, 9 in e0eb672e and finally 10 now in 5cc3cce5). I tried getting someone to test the version I added, but since there's so few of us working with ESX (I had to ask Matthias if he can have a look at the code), we can only assume that it will work. I even remember someone filing a bug that the connection works and everything, but there's a warning. My thinking was that we have two options here:
1) continue messing the code with blindly adding versions of virtualHW or
2) conditionally allow anything the user wants, with the condition showing that we don't "support" it.
My reasoning behind choosing version 2 was that the development around VMX-related backends is not that active as with other ones. So if anything doesn't work because of HW version it is somehow visible that the user is not using "supported" versions of vmx descriptors (or how to call the .vmx file). I designed the patch so there is no functional nor API change with current usage, it merely allows to check whether our code works with newer versions without touching the code (currently, when someone wants to try that, he needs to add the version to supported ones, obviously). And it still leaves the list of supported versions in place and whoever wants some version to be supported can add it to the list.
I know it is impossible to force people fix everything for versions they added, but it could be at least a bit visible that we currently don't have the capacity for that.
Martin
Let's take a step back and look at the complete picture.
Initially it seemed to me that the virtualHW version would have a larger effect on the content and format of the VMX file than to actually turned out to have. That's why I added this strict check to the parser in the first place. But after two major vSphere version (4 and 5) and multiple virtualHW version came along since the initial implementation of the VMX parser I'm now pretty confident that the actual virtualHW version is not that important to the parser as I initially thought it would be.
I see no gain in adding any extra logic here to allow "unsafe" versions. Actually, I dislike this word in this context, it gives the user a wrong impression about the situation. There is nothing unsafe and nothing to test for compatibility here, it's only me misjudging the importance and meaning of this config entry.
I suggest a different approach to deal with future virtualHW versions: basically ignore the value, as it's not that important. It should have been that way from the beginning. Here's a patch for that:
https://www.redhat.com/archives/libvir-list/2014-May/msg00313.html
That patch has way better approach, so Self-NACK to this series. Martin

Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- docs/drvvmware.html.in | 35 +++++++++++++++++++++++++++++++++++ src/vmware/vmware_conf.h | 2 ++ src/vmware/vmware_driver.c | 17 ++++++++++++++++- 3 files changed, 53 insertions(+), 1 deletion(-) diff --git a/docs/drvvmware.html.in b/docs/drvvmware.html.in index 240afd0..c2f7d38 100644 --- a/docs/drvvmware.html.in +++ b/docs/drvvmware.html.in @@ -51,6 +51,41 @@ vmwarews+tcp://user@example.com/session (remote access to VMware Workstation, S vmwarews+ssh://user@example.com/session (remote access to VMware Workstation, SSH tunnelled) </pre> + <h4><a name="extraparams">Extra parameters</a></h4> + <p> + Extra parameters can be added to a URI as part of the query string + (the part following <code>?</code>). A single parameter is formed by a + <code>name=value</code> pair. Multiple parameters are separated by + <code>&</code>. + </p> +<pre> +?<span style="color: #E50000">allow_unsafe=1</span> +</pre> + <p> + The driver understands the extra parameters shown below. + </p> + <table class="top_table"> + <tr> + <th>Name</th> + <th>Values</th> + <th>Meaning</th> + </tr> + <tr> + <td> + <code>allow_unsafe</code> + </td> + <td> + <code>0</code> or <code>1</code> + </td> + <td> + If set to 1, this disables check for supported + virtualHW.version, so connections to newer versions + than supported is possible. The default value is 0. + <span class="since">Since 1.2.5</since> + </td> + </tr> + </table> + <h2><a name="xmlconfig">Example domain XML config</a></h2> <pre> diff --git a/src/vmware/vmware_conf.h b/src/vmware/vmware_conf.h index 1f3c41a..6ab838c 100644 --- a/src/vmware/vmware_conf.h +++ b/src/vmware/vmware_conf.h @@ -51,6 +51,8 @@ struct vmware_driver { unsigned long version; int type; char *vmrun; + + bool allowUnsafe; }; typedef struct _vmwareDomain { diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c index 6edc0bc..b0a3279 100644 --- a/src/vmware/vmware_driver.c +++ b/src/vmware/vmware_driver.c @@ -1,6 +1,6 @@ /*---------------------------------------------------------------------------*/ /* - * Copyright (C) 2011-2012 Red Hat, Inc. + * Copyright (C) 2011-2012, 2014 Red Hat, Inc. * Copyright 2010, diateam (www.diateam.net) * Copyright (C) 2013. Doug Goldstein <cardoe@cardoe.com> * @@ -99,6 +99,7 @@ vmwareConnectOpen(virConnectPtr conn, struct vmware_driver *driver; size_t i; char *tmp; + int allowUnsafe; virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR); @@ -157,6 +158,20 @@ vmwareConnectOpen(virConnectPtr conn, goto cleanup; } + for (i = 0; i < conn->uri->paramsCount; i++) { + virURIParamPtr queryParam = &conn->uri->params[i]; + if (STRCASEEQ(queryParam->name, "allow_unsafe")) { + if (virStrToLong_i(queryParam->value, NULL, 10, &allowUnsafe) < 0 || + (allowUnsafe != 0 && allowUnsafe != 1)) { + virReportError(VIR_ERR_INVALID_ARG, + _("Query parameter 'allow_unsafe' has unexpected value " + "'%s' (should be 0 or 1)"), queryParam->value); + goto cleanup; + } + driver->allowUnsafe = allowUnsafe; + } + } + /* Match the non-'vmware' part of the scheme as the driver backend */ driver->type = vmwareDriverTypeFromString(tmp); -- 1.9.2

We were (almost) blindly adding new virtualHW versions to the check that was supposed to catch unsupported ones. Accepting such changes renders the check completely useless. In order to avoid that, new parameter, 'allowUnsafe', was added to the function. This parameter controls whether we error out with unsupported virtualHW version or not. Until there's somebody testing the code or an autotest setup for that, we should not be adding new versions to the mix and instead caller applications (like virt-v2v) should make use of this parameter. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/esx/esx_driver.c | 6 ++++-- src/vmware/vmware_conf.c | 4 ++-- src/vmware/vmware_driver.c | 3 ++- src/vmx/vmx.c | 24 ++++++++++++++++++------ src/vmx/vmx.h | 4 +++- 5 files changed, 29 insertions(+), 12 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index d082927..f7d40b2 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -2735,7 +2735,8 @@ esxDomainGetXMLDesc(virDomainPtr domain, unsigned int flags) ctx.formatFileName = NULL; ctx.autodetectSCSIControllerModel = NULL; - def = virVMXParseConfig(&ctx, priv->xmlopt, vmx); + def = virVMXParseConfig(&ctx, priv->xmlopt, vmx, + priv->parsedUri->allowUnsafe); if (def) { if (powerState != esxVI_VirtualMachinePowerState_PoweredOff) { @@ -2794,7 +2795,8 @@ esxConnectDomainXMLFromNative(virConnectPtr conn, const char *nativeFormat, ctx.formatFileName = NULL; ctx.autodetectSCSIControllerModel = NULL; - def = virVMXParseConfig(&ctx, priv->xmlopt, nativeConfig); + def = virVMXParseConfig(&ctx, priv->xmlopt, nativeConfig, + priv->parsedUri->allowUnsafe); if (def) { xml = virDomainDefFormat(def, VIR_DOMAIN_XML_INACTIVE); diff --git a/src/vmware/vmware_conf.c b/src/vmware/vmware_conf.c index 29ca322..5a435e4 100644 --- a/src/vmware/vmware_conf.c +++ b/src/vmware/vmware_conf.c @@ -162,8 +162,8 @@ vmwareLoadDomains(struct vmware_driver *driver) if (virFileReadAll(vmxPath, 10000, &vmx) < 0) goto cleanup; - if ((vmdef = - virVMXParseConfig(&ctx, driver->xmlopt, vmx)) == NULL) { + if ((vmdef = virVMXParseConfig(&ctx, driver->xmlopt, vmx, + driver->allowUnsafe)) == NULL) { goto cleanup; } diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c index b0a3279..ff5c2de 100644 --- a/src/vmware/vmware_driver.c +++ b/src/vmware/vmware_driver.c @@ -999,7 +999,8 @@ vmwareConnectDomainXMLFromNative(virConnectPtr conn, const char *nativeFormat, ctx.parseFileName = vmwareCopyVMXFileName; - def = virVMXParseConfig(&ctx, driver->xmlopt, nativeConfig); + def = virVMXParseConfig(&ctx, driver->xmlopt, nativeConfig, + driver->allowUnsafe); if (def != NULL) xml = virDomainDefFormat(def, VIR_DOMAIN_XML_INACTIVE); diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 169440c..194fda8 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -1225,7 +1225,8 @@ virVMXGatherSCSIControllers(virVMXContext *ctx, virDomainDefPtr def, virDomainDefPtr virVMXParseConfig(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, - const char *vmx) + const char *vmx, + bool allowUnsafe) { bool success = false; virConfPtr conf = NULL; @@ -1316,14 +1317,25 @@ virVMXParseConfig(virVMXContext *ctx, goto cleanup; } + /* If you need to add another version in here, please consider + * using allow_unsafe = 1 in URI in the upper layer. We currently + * say we support these versions, but actually, we were just + * adding these versions with no sense of the related hypervisor + * changes whatsoever. */ if (virtualHW_version != 4 && virtualHW_version != 7 && virtualHW_version != 8 && virtualHW_version != 9 && virtualHW_version != 10) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Expecting VMX entry 'virtualHW.version' to be " - "4, 7, 8, 9 or 10 but found %lld"), - virtualHW_version); - goto cleanup; + if (allowUnsafe) { + VIR_DEBUG("Value %lld of VMX entry 'virtualHW.version' is not " + "supported, but explicitly allowed", + virtualHW_version); + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Expecting VMX entry 'virtualHW.version' to be " + "4, 7, 8, 9 or 10 but found %lld"), + virtualHW_version); + goto cleanup; + } } /* vmx:uuid.bios -> def:uuid */ diff --git a/src/vmx/vmx.h b/src/vmx/vmx.h index 6a68c8b..0525d0a 100644 --- a/src/vmx/vmx.h +++ b/src/vmx/vmx.h @@ -1,6 +1,7 @@ /* * vmx.h: VMware VMX parsing/formatting functions * + * Copyright (C) 2014 Red Hat, Inc. * Copyright (C) 2009-2010 Matthias Bolte <matthias.bolte@googlemail.com> * * This library is free software; you can redistribute it and/or @@ -80,7 +81,8 @@ char *virVMXConvertToUTF8(const char *encoding, const char *string); virDomainDefPtr virVMXParseConfig(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, - const char *vmx); + const char *vmx, + bool allowUnsafe); int virVMXParseVNC(virConfPtr conf, virDomainGraphicsDefPtr *def); -- 1.9.2

Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- tests/vmx2xmldata/vmx2xml-unsafe.vmx | 2 ++ tests/vmx2xmldata/vmx2xml-unsafe.xml | 18 ++++++++++++++++++ tests/vmx2xmltest.c | 36 +++++++++++++++++++++++++++++++----- 3 files changed, 51 insertions(+), 5 deletions(-) create mode 100644 tests/vmx2xmldata/vmx2xml-unsafe.vmx create mode 100644 tests/vmx2xmldata/vmx2xml-unsafe.xml diff --git a/tests/vmx2xmldata/vmx2xml-unsafe.vmx b/tests/vmx2xmldata/vmx2xml-unsafe.vmx new file mode 100644 index 0000000..a3998ed --- /dev/null +++ b/tests/vmx2xmldata/vmx2xml-unsafe.vmx @@ -0,0 +1,2 @@ +config.version = "8" +virtualHW.version = "11" diff --git a/tests/vmx2xmldata/vmx2xml-unsafe.xml b/tests/vmx2xmldata/vmx2xml-unsafe.xml new file mode 100644 index 0000000..38bcf43 --- /dev/null +++ b/tests/vmx2xmldata/vmx2xml-unsafe.xml @@ -0,0 +1,18 @@ +<domain type='vmware'> + <uuid>00000000-0000-0000-0000-000000000000</uuid> + <memory unit='KiB'>32768</memory> + <currentMemory unit='KiB'>32768</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686'>hvm</type> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <video> + <model type='vmvga' vram='4096'/> + </video> + </devices> +</domain> diff --git a/tests/vmx2xmltest.c b/tests/vmx2xmltest.c index 1d2e012..6877610 100644 --- a/tests/vmx2xmltest.c +++ b/tests/vmx2xmltest.c @@ -19,6 +19,10 @@ static virCapsPtr caps; static virDomainXMLOptionPtr xmlopt; static virVMXContext ctx; +typedef enum { + FLAG_ALLOW_UNSAFE = 1 << 0, + FLAG_EXPECT_ERROR = 1 << 1, +} testCompareHelperFlags; static void testCapsInit(void) @@ -71,7 +75,7 @@ testCapsInit(void) } static int -testCompareFiles(const char *vmx, const char *xml) +testCompareFiles(const char *vmx, const char *xml, bool allowUnsafe) { int ret = -1; char *vmxData = NULL; @@ -85,7 +89,7 @@ testCompareFiles(const char *vmx, const char *xml) if (virtTestLoadFile(xml, &xmlData) < 0) goto cleanup; - if (!(def = virVMXParseConfig(&ctx, xmlopt, vmxData))) + if (!(def = virVMXParseConfig(&ctx, xmlopt, vmxData, allowUnsafe))) goto cleanup; if (!virDomainDefCheckABIStability(def, def)) { @@ -115,6 +119,8 @@ testCompareFiles(const char *vmx, const char *xml) struct testInfo { const char *input; const char *output; + + unsigned int flags; }; static int @@ -132,7 +138,21 @@ testCompareHelper(const void *data) goto cleanup; } - ret = testCompareFiles(vmx, xml); + ret = testCompareFiles(vmx, xml, info->flags & FLAG_ALLOW_UNSAFE); + + if (info->flags & FLAG_EXPECT_ERROR) { + if (ret == 0) { + ret = -1; + if (virTestGetDebug()) + fprintf(stderr, "qemuBuildCommandLine should have failed\n"); + } else if (ret < 0) { + if (virTestGetDebug() > 1) + fprintf(stderr, "Got expected error: %s\n", + virGetLastErrorMessage()); + virResetLastError(); + ret = 0; + } + } cleanup: VIR_FREE(vmx); @@ -189,9 +209,9 @@ mymain(void) { int ret = 0; -# define DO_TEST(_in, _out) \ +# define DO_TEST_FULL(_in, _out, flags) \ do { \ - struct testInfo info = { _in, _out }; \ + struct testInfo info = { _in, _out, flags}; \ virResetLastError(); \ if (virtTestRun("VMware VMX-2-XML "_in" -> "_out, \ testCompareHelper, &info) < 0) { \ @@ -199,6 +219,9 @@ mymain(void) } \ } while (0) +# define DO_TEST(_in, _out) DO_TEST_FULL(_in, _out, 0) + + testCapsInit(); if (caps == NULL) { @@ -285,6 +308,9 @@ mymain(void) DO_TEST("annotation", "annotation"); + DO_TEST_FULL("unsafe", "unsafe", FLAG_ALLOW_UNSAFE); + DO_TEST_FULL("unsafe", "unsafe", FLAG_EXPECT_ERROR); + DO_TEST("smbios", "smbios"); DO_TEST("svga", "svga"); -- 1.9.2
participants (3)
-
Eric Blake
-
Martin Kletzander
-
Matthias Bolte