On 20.09.2012 10:58, Martin Kletzander wrote:
> 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);
I wonder if we should switch to -machine unconditionally on enough new
qemu (option was introduced in 0.15) and leave -M just for backwards
compatibility since qemu is (sort of) maintaining -M just because of us.
But that'd be a separate patch anyway.
> + } 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));
This looks weird to me:
s/%s,%s=%s/%s,dump-guest-core=%s/
> + }
> +
> + 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)
I guess qemu refuses to start with dump-guest-core=default but no one
knows. s/</<=/
> + 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;
> }
>
ACK with nits fixed.
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list