On 07/24/2018 11:23 PM, Cole Robinson wrote:
We should still make an effort to fill in data, just not raise an error if say an ostype/virttype combo disappeared from caps.
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/conf/domain_conf.c | 13 ++++++------- tests/qemuxml2argvdata/missing-machine.xml | 2 +- tests/qemuxml2argvtest.c | 3 +++ 3 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b7f6a22e20..78ee000857 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -19178,18 +19178,17 @@ virDomainDefParseCaps(virDomainDefPtr def, goto cleanup; }
- if (!(flags & VIR_DOMAIN_DEF_PARSE_SKIP_OSTYPE_CHECKS)) { - if (!(capsdata = virCapabilitiesDomainDataLookup(caps, - def->os.type, def->os.arch, def->virtType, - NULL, NULL))) + if (!(capsdata = virCapabilitiesDomainDataLookup(caps, def->os.type, + def->os.arch, def->virtType, NULL, NULL))) {
This looks misaligned ;-)
+ if (!(flags & VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE)) goto cleanup;
So you're changing the flag here even though I believe it belongs to the next patch. It helps downstream maintainers, but in the end the code will look the same.
- + virResetLastError(); + } else { if (!def->os.arch) def->os.arch = capsdata->arch; if ((!def->os.machine && - VIR_STRDUP(def->os.machine, capsdata->machinetype) < 0)) { + VIR_STRDUP(def->os.machine, capsdata->machinetype) < 0)) goto cleanup; - } }
ret = 0; diff --git a/tests/qemuxml2argvdata/missing-machine.xml b/tests/qemuxml2argvdata/missing-machine.xml index 4ce7b377a5..2900baec90 100644 --- a/tests/qemuxml2argvdata/missing-machine.xml +++ b/tests/qemuxml2argvdata/missing-machine.xml @@ -6,7 +6,7 @@ <currentMemory unit='KiB'>219100</currentMemory> <vcpu placement='static' cpuset='1'>1</vcpu> <os> - <type arch='i686'>hvm</type> + <type arch='alpha'>hvm</type>
Firstly I was wondering why is this change needed, but then I read the comment in the next hunk and it makes sense. We need to have non-existent pair of arch and os type so that the error is triggered.
<boot dev='hd'/> </os> <clock offset='utc'/> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 1a936faef1..03b6d92912 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2773,6 +2773,9 @@ mymain(void) QEMU_CAPS_OBJECT_GPEX, QEMU_CAPS_NEC_USB_XHCI);
+ /* VM XML has invalid arch/ostype/virttype combo, but the SKIP flag + * will avoid the error. Still, we expect qemu driver to complain about + * missing machine error, and not crash */ DO_TEST_PARSE_FLAGS_ERROR("missing-machine", VIR_DOMAIN_DEF_PARSE_SKIP_OSTYPE_CHECKS, NONE);
Michal