Re: [libvirt] [PATCH v2 2/3] qemu: add support for dump-guest-core option

The "dump-guest-core' option is new option for the machine type (-machine pc,dump-guest-core) that controls whether the guest memory will be marked as dumpable. While testing this, I've found out that the value for the '-M' options is not parsed correctly when additional parameters are used. However, when '-machine' is used for the same options, it gets parsed as expected. That's why this patch also modifies the parsing and creating of the command line, so both '-M' and '-machine' are recognized. In QEMU's help there is only mention of the 'machine parameter now with no sign of the older '-M'. --- src/qemu/qemu_capabilities.c | 4 +++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 81 +++++++++++++++++++++++++++++++++++++++----- tests/qemuhelptest.c | 3 +- 4 files changed, 79 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 3582cbd..4b52dc5 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -182,6 +182,7 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST, "seccomp-sandbox", "reboot-timeout", /* 110 */ + "dump-guest-core", ); struct _qemuCaps { @@ -1243,6 +1244,9 @@ qemuCapsComputeCmdFlags(const char *help, if (strstr(help, "-no-shutdown") && (version < 14000 || version > 15000)) qemuCapsSet(caps, QEMU_CAPS_NO_SHUTDOWN); + if (strstr(help, "dump-guest-core=on|off")) + qemuCapsSet(caps, QEMU_CAPS_DUMP_GUEST_CORE); + /* * Handling of -incoming arg with varying features * -incoming tcp (kvm >= 79, qemu >= 0.10.0) diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 2201cb3..a069f42 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -146,6 +146,7 @@ enum qemuCapsFlags { QEMU_CAPS_SCSI_DISK_WWN = 108, /* Is scsi-disk.wwn available? */ QEMU_CAPS_SECCOMP_SANDBOX = 109, /* -sandbox */ QEMU_CAPS_REBOOT_TIMEOUT = 110, /* -boot reboot-timeout */ + QEMU_CAPS_DUMP_GUEST_CORE = 111, /* dump-guest-core-parameter */ QEMU_CAPS_LAST, /* this must always be the last item */ }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 3807596..52ad4d2 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4306,6 +4306,45 @@ no_memory: goto cleanup; } +static int +qemuBuildMachineArgStr(virCommandPtr cmd, + const virDomainDefPtr def, + qemuCapsPtr caps) +{ + /* This should *never* be NULL, since we always provide + * a machine in the capabilities data for QEMU. So this + * check is just here as a safety in case the unexpected + * happens */ + if (!def->os.machine) + return 0; + + if (!def->mem.dump_core) { + /* if no parameter to the machine type is needed, we still use + * '-M' to keep the most of the compatibility with older versions. + */ + virCommandAddArgList(cmd, "-M", def->os.machine, NULL);

+ } else { + if (!qemuCapsGet(caps, QEMU_CAPS_DUMP_GUEST_CORE)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("dump-guest-core is not available " + " with this QEMU binary")); + return -1; + } + + /* However, in case there is a parameter to be added, we need to + * use the "-machine" parameter because qemu is not parsing the + * "-M" correctly */ + virCommandAddArg(cmd, "-machine"); + virCommandAddArgFormat(cmd, + "%s,%s=%s", + def->os.machine, + "dump-guest-core", + virDomainMemDumpTypeToString(def->mem.dump_core));

+ } + + return 0; +} + static char * qemuBuildSmpArgStr(const virDomainDefPtr def, qemuCapsPtr caps) @@ -4498,12 +4537,8 @@ qemuBuildCommandLine(virConnectPtr conn, } virCommandAddArg(cmd, "-S"); /* freeze CPU */ - /* This should *never* be NULL, since we always provide - * a machine in the capabilities data for QEMU. So this - * check is just here as a safety in case the unexpected - * happens */ - if (def->os.machine) - virCommandAddArgList(cmd, "-M", def->os.machine, NULL); + if (qemuBuildMachineArgStr(cmd, def, caps) < 0) + goto error; if (qemuBuildCpuArgStr(driver, def, emulator, caps, &ut, &cpu, &hasHwVirt, !!migrateFrom) < 0) @@ -8320,10 +8355,38 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps, } if (STREQ(def->name, "")) VIR_FREE(def->name); - } else if (STREQ(arg, "-M")) { + } else if (STREQ(arg, "-M") || + STREQ(arg, "-machine")) { + char *params; WANT_VALUE(); - if (!(def->os.machine = strdup(val))) - goto no_memory; + params = strchr(val, ','); + if (params == NULL) { + if (!(def->os.machine = strdup(val))) + goto no_memory; + } else { + if (!(def->os.machine = strndup(val, params - val))) + goto no_memory; + + while(params++) { + /* prepared for more "-machine" parameters */ + char *tmp = params; + params = strchr(params, ','); + + if (STRPREFIX(tmp, "dump-guest-core=")) { + tmp += strlen("dump-guest-core="); + if (params) { + tmp = strndup(tmp, params - tmp); + if (tmp == NULL) + goto no_memory; + } + def->mem.dump_core = virDomainMemDumpTypeFromString(tmp); + if (def->mem.dump_core < 0)

+ def->mem.dump_core = VIR_DOMAIN_MEM_DUMP_DEFAULT; + if (params) + VIR_FREE(tmp); + } + } + } } else if (STREQ(arg, "-serial")) { WANT_VALUE(); if (STRNEQ(val, "none")) { diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c index a3d9656..079aef8 100644 --- a/tests/qemuhelptest.c +++ b/tests/qemuhelptest.c @@ -846,7 +846,8 @@ mymain(void) QEMU_CAPS_VIRTIO_SCSI_PCI, QEMU_CAPS_BLOCKIO, QEMU_CAPS_SCSI_DISK_WWN, - QEMU_CAPS_SECCOMP_SANDBOX); + QEMU_CAPS_SECCOMP_SANDBOX, + QEMU_CAPS_DUMP_GUEST_CORE); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; }