From 6700062fb00031c2c4fb8a561653a1a58246ba4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Mon, 13 Aug 2018 12:42:01 +0100 Subject: [PATCH] qemu: fix default machine for argv -> xml convertor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Historically the argv -> xml convertor wanted the same default machine as we'd set when parsing xml. The latter has now changed, however, to use a default defined by libvirt. The former needs fixing to again honour the default QEMU machine. This exposed a bug in handling for the aarch64 target, as QEMU does not define any default machine. Thus we should not having been accepting argv without a -machine provided. Reviewed-by: John Ferlan Signed-off-by: Daniel P. Berrangé --- src/qemu/qemu_capabilities.c | 12 ++++++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_driver.c | 6 ++- src/qemu/qemu_parse_command.c | 36 +++++++++++------ src/qemu/qemu_parse_command.h | 8 +++- tests/qemuargv2xmldata/nomachine-aarch64.args | 11 ----- tests/qemuargv2xmldata/nomachine-aarch64.xml | 40 ------------------- tests/qemuargv2xmldata/nomachine-ppc64.xml | 4 +- tests/qemuargv2xmldata/nomachine-x86_64.xml | 4 +- tests/qemuargv2xmldata/pseries-disk.xml | 4 +- tests/qemuargv2xmldata/pseries-nvram.xml | 4 +- tests/qemuargv2xmltest.c | 18 ++++++++- 12 files changed, 70 insertions(+), 78 deletions(-) delete mode 100644 tests/qemuargv2xmldata/nomachine-aarch64.args delete mode 100644 tests/qemuargv2xmldata/nomachine-aarch64.xml diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index a97023ce82..a075677421 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2069,6 +2069,18 @@ const char *virQEMUCapsGetCanonicalMachine(virQEMUCapsPtr qemuCaps, return name; } +const char * +virQEMUCapsGetDefaultMachine(virQEMUCapsPtr qemuCaps) +{ + size_t i; + + for (i = 0; i < qemuCaps->nmachineTypes; i++) { + if (qemuCaps->machineTypes[i].qemuDefault) + return qemuCaps->machineTypes[i].name; + } + + return NULL; +} int virQEMUCapsGetMachineMaxCpus(virQEMUCapsPtr qemuCaps, const char *name) diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index da017ff52b..3d3a978759 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -559,6 +559,7 @@ bool virQEMUCapsIsCPUModeSupported(virQEMUCapsPtr qemuCaps, virCPUMode mode); const char *virQEMUCapsGetCanonicalMachine(virQEMUCapsPtr qemuCaps, const char *name); +const char *virQEMUCapsGetDefaultMachine(virQEMUCapsPtr qemuCaps); int virQEMUCapsGetMachineMaxCpus(virQEMUCapsPtr qemuCaps, const char *name); bool virQEMUCapsGetMachineHotplugCpus(virQEMUCapsPtr qemuCaps, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index da8c4e8991..a0f7c71675 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7103,7 +7103,8 @@ static char *qemuConnectDomainXMLFromNative(virConnectPtr conn, if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto cleanup; - def = qemuParseCommandLineString(caps, driver->xmlopt, config, + def = qemuParseCommandLineString(driver->qemuCapsCache, + caps, driver->xmlopt, config, NULL, NULL, NULL); if (!def) goto cleanup; @@ -16606,7 +16607,8 @@ static virDomainPtr qemuDomainQemuAttach(virConnectPtr conn, if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto cleanup; - if (!(def = qemuParseCommandLinePid(caps, driver->xmlopt, pid, + if (!(def = qemuParseCommandLinePid(driver->qemuCapsCache, + caps, driver->xmlopt, pid, &pidfile, &monConfig, &monJSON))) goto cleanup; diff --git a/src/qemu/qemu_parse_command.c b/src/qemu/qemu_parse_command.c index fdc1d34068..c0bf27f800 100644 --- a/src/qemu/qemu_parse_command.c +++ b/src/qemu/qemu_parse_command.c @@ -1829,7 +1829,8 @@ qemuParseCommandLineBootDevs(virDomainDefPtr def, const char *str) * as is practical. This is not an exact science.... */ static virDomainDefPtr -qemuParseCommandLine(virCapsPtr caps, +qemuParseCommandLine(virFileCachePtr capsCache, + virCapsPtr caps, virDomainXMLOptionPtr xmlopt, char **progenv, char **progargv, @@ -1851,6 +1852,7 @@ qemuParseCommandLine(virCapsPtr caps, virDomainDiskDefPtr disk = NULL; const char *ceph_args = qemuFindEnv(progenv, "CEPH_ARGS"); bool have_sdl = false; + virQEMUCapsPtr qemuCaps; if (pidfile) *pidfile = NULL; @@ -1865,6 +1867,9 @@ qemuParseCommandLine(virCapsPtr caps, return NULL; } + if (!(qemuCaps = virQEMUCapsCacheLookup(capsCache, progargv[0]))) + goto error; + if (!(def = virDomainDefNew())) goto error; @@ -2017,17 +2022,18 @@ qemuParseCommandLine(virCapsPtr caps, /* If no machine type has been found among the arguments, then figure * out a reasonable value by using capabilities */ if (!def->os.machine) { - virCapsDomainDataPtr capsdata; + const char *mach = virQEMUCapsGetDefaultMachine(qemuCaps); - if (!(capsdata = virCapabilitiesDomainDataLookup(caps, def->os.type, - def->os.arch, def->virtType, NULL, NULL))) - goto error; - - if (VIR_STRDUP(def->os.machine, capsdata->machinetype) < 0) { - VIR_FREE(capsdata); + if (!mach) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Binary '%s' does not have a default machine type " + "and no '-machine' arg is present"), + progargv[0]); goto error; } - VIR_FREE(capsdata); + + if (VIR_STRDUP(def->os.machine, mach) < 0) + goto error; } /* Now the real processing loop */ @@ -2718,6 +2724,7 @@ qemuParseCommandLine(virCapsPtr caps, else qemuDomainCmdlineDefFree(cmd); + virObjectUnref(qemuCaps); return def; error: @@ -2732,11 +2739,13 @@ qemuParseCommandLine(virCapsPtr caps, } if (pidfile) VIR_FREE(*pidfile); + virObjectUnref(qemuCaps); return NULL; } -virDomainDefPtr qemuParseCommandLineString(virCapsPtr caps, +virDomainDefPtr qemuParseCommandLineString(virFileCachePtr capsCache, + virCapsPtr caps, virDomainXMLOptionPtr xmlopt, const char *args, char **pidfile, @@ -2750,7 +2759,7 @@ virDomainDefPtr qemuParseCommandLineString(virCapsPtr caps, if (qemuStringToArgvEnv(args, &progenv, &progargv) < 0) goto cleanup; - def = qemuParseCommandLine(caps, xmlopt, progenv, progargv, + def = qemuParseCommandLine(capsCache, caps, xmlopt, progenv, progargv, pidfile, monConfig, monJSON); cleanup: @@ -2808,7 +2817,8 @@ static int qemuParseProcFileStrings(int pid_value, return ret; } -virDomainDefPtr qemuParseCommandLinePid(virCapsPtr caps, +virDomainDefPtr qemuParseCommandLinePid(virFileCachePtr capsCache, + virCapsPtr caps, virDomainXMLOptionPtr xmlopt, pid_t pid, char **pidfile, @@ -2828,7 +2838,7 @@ virDomainDefPtr qemuParseCommandLinePid(virCapsPtr caps, qemuParseProcFileStrings(pid, "environ", &progenv) < 0) goto cleanup; - if (!(def = qemuParseCommandLine(caps, xmlopt, progenv, progargv, + if (!(def = qemuParseCommandLine(capsCache, caps, xmlopt, progenv, progargv, pidfile, monConfig, monJSON))) goto cleanup; diff --git a/src/qemu/qemu_parse_command.h b/src/qemu/qemu_parse_command.h index b3a950a420..a4db1a50ca 100644 --- a/src/qemu/qemu_parse_command.h +++ b/src/qemu/qemu_parse_command.h @@ -24,19 +24,23 @@ #ifndef __QEMU_PARSE_COMMAND_H__ # define __QEMU_PARSE_COMMAND_H__ +# include "virfilecache.h" + # define QEMU_QXL_VGAMEM_DEFAULT 16 * 1024 /* * NB: def->name can be NULL upon return and the caller * *must* decide how to fill in a name in this case */ -virDomainDefPtr qemuParseCommandLineString(virCapsPtr caps, +virDomainDefPtr qemuParseCommandLineString(virFileCachePtr capsCache, + virCapsPtr caps, virDomainXMLOptionPtr xmlopt, const char *args, char **pidfile, virDomainChrSourceDefPtr *monConfig, bool *monJSON); -virDomainDefPtr qemuParseCommandLinePid(virCapsPtr caps, +virDomainDefPtr qemuParseCommandLinePid(virFileCachePtr capsCache, + virCapsPtr caps, virDomainXMLOptionPtr xmlopt, pid_t pid, char **pidfile, diff --git a/tests/qemuargv2xmldata/nomachine-aarch64.args b/tests/qemuargv2xmldata/nomachine-aarch64.args deleted file mode 100644 index b17c0d0c23..0000000000 --- a/tests/qemuargv2xmldata/nomachine-aarch64.args +++ /dev/null @@ -1,11 +0,0 @@ -LC_ALL=C \ -PATH=/bin \ -HOME=/home/test \ -USER=test \ -LOGNAME=test \ -QEMU_AUDIO_DRV=none \ -/usr/bin/qemu-system-aarch64 \ --name QEMUGuest1 \ --m 512 \ --hda /dev/HostVG/QEMUGuest1 \ --cdrom /root/boot.iso diff --git a/tests/qemuargv2xmldata/nomachine-aarch64.xml b/tests/qemuargv2xmldata/nomachine-aarch64.xml deleted file mode 100644 index 9492423389..0000000000 --- a/tests/qemuargv2xmldata/nomachine-aarch64.xml +++ /dev/null @@ -1,40 +0,0 @@ - - QEMUGuest1 - c7a5fdbd-edaf-9455-926a-d65c16db1809 - 524288 - 524288 - 1 - - hvm - - - - - - - destroy - restart - destroy - - /usr/bin/qemu-system-aarch64 - - - - -
- - - - - - -
- - - - - - - diff --git a/tests/qemuargv2xmldata/nomachine-ppc64.xml b/tests/qemuargv2xmldata/nomachine-ppc64.xml index 1f15a950e3..18b0c5c20a 100644 --- a/tests/qemuargv2xmldata/nomachine-ppc64.xml +++ b/tests/qemuargv2xmldata/nomachine-ppc64.xml @@ -5,7 +5,7 @@ 524288 1 - hvm + hvm @@ -27,7 +27,7 @@
- +
diff --git a/tests/qemuargv2xmldata/nomachine-x86_64.xml b/tests/qemuargv2xmldata/nomachine-x86_64.xml index 33cde4c55a..6863c8f5f4 100644 --- a/tests/qemuargv2xmldata/nomachine-x86_64.xml +++ b/tests/qemuargv2xmldata/nomachine-x86_64.xml @@ -5,7 +5,7 @@ 524288 1 - hvm + hvm @@ -30,7 +30,7 @@
- +
diff --git a/tests/qemuargv2xmldata/pseries-disk.xml b/tests/qemuargv2xmldata/pseries-disk.xml index 1f15a950e3..18b0c5c20a 100644 --- a/tests/qemuargv2xmldata/pseries-disk.xml +++ b/tests/qemuargv2xmldata/pseries-disk.xml @@ -5,7 +5,7 @@ 524288 1 - hvm + hvm @@ -27,7 +27,7 @@
- +
diff --git a/tests/qemuargv2xmldata/pseries-nvram.xml b/tests/qemuargv2xmldata/pseries-nvram.xml index 7787847a90..500227afc0 100644 --- a/tests/qemuargv2xmldata/pseries-nvram.xml +++ b/tests/qemuargv2xmldata/pseries-nvram.xml @@ -5,7 +5,7 @@ 524288 1 - hvm + hvm @@ -14,7 +14,7 @@ destroy /usr/bin/qemu-system-ppc64 - +
diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c index cb010268c4..966e75ef5f 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -65,7 +65,8 @@ static int testCompareXMLToArgvFiles(const char *xmlfile, if (virTestLoadFile(cmdfile, &cmd) < 0) goto fail; - if (!(vmdef = qemuParseCommandLineString(driver.caps, driver.xmlopt, + if (!(vmdef = qemuParseCommandLineString(driver.qemuCapsCache, + driver.caps, driver.xmlopt, cmd, NULL, NULL, NULL))) goto fail; @@ -154,6 +155,20 @@ mymain(void) if (qemuTestDriverInit(&driver) < 0) return EXIT_FAILURE; +# define LOAD_CAPS(arch) \ + do { \ + virQEMUCapsPtr qemuCaps; \ + qemuCaps = qemuTestParseCapabilitiesArch(VIR_ARCH_X86_64, \ + "qemucapabilitiesdata/caps_2.12.0." arch ".xml"); \ + if (virFileCacheInsertData(driver.qemuCapsCache, \ + "/usr/bin/qemu-system-" arch, \ + qemuCaps) < 0) \ + return EXIT_FAILURE; \ + } while (0) + + LOAD_CAPS("x86_64"); + LOAD_CAPS("aarch64"); + LOAD_CAPS("ppc64"); # define DO_TEST_FULL(name, flags) \ do { \ @@ -290,7 +305,6 @@ mymain(void) DO_TEST("machine-keywrap-none-argv"); DO_TEST("nomachine-x86_64"); - DO_TEST("nomachine-aarch64"); DO_TEST("nomachine-ppc64"); qemuTestDriverFree(&driver);