[libvirt] [PATCH v2 2/3] qemu: Add support for reboot-timeout

[Peter Krempa]

On 09/19/12 19:22, Martin Kletzander wrote:
> This patch adds support for "-boot reboot-timeout=rb_time" that is
> added in QEMU.
> ---
>   src/qemu/qemu_capabilities.c |  4 ++++
>   src/qemu/qemu_capabilities.h |  1 +
>   src/qemu/qemu_command.c      | 28 ++++++++++++++++++++++++++++
>   3 files changed, 33 insertions(+)
>
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 278b550..3582cbd 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -180,6 +180,8 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST,
>                 "ide-drive.wwn",
>                 "scsi-disk.wwn",
>                 "seccomp-sandbox",
> +
> +              "reboot-timeout", /* 110 */
>       );
>
>   struct _qemuCaps {
> @@ -1191,6 +1193,8 @@ qemuCapsComputeCmdFlags(const char *help,
>           qemuCapsSet(caps, QEMU_CAPS_NESTING);
>       if (strstr(help, ",menu=on"))
>           qemuCapsSet(caps, QEMU_CAPS_BOOT_MENU);
> +    if (strstr(help, ",reboot-timeout=rb_time"))
> +        qemuCapsSet(caps, QEMU_CAPS_REBOOT_TIMEOUT);
>       if ((fsdev = strstr(help, "-fsdev"))) {
>           qemuCapsSet(caps, QEMU_CAPS_FSDEV);
>           if (strstr(fsdev, "readonly"))
> diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
> index 4da2a29..2201cb3 100644
> --- a/src/qemu/qemu_capabilities.h
> +++ b/src/qemu/qemu_capabilities.h
> @@ -145,6 +145,7 @@ enum qemuCapsFlags {
>       QEMU_CAPS_IDE_DRIVE_WWN      = 107, /* Is ide-drive.wwn available? */
>       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_LAST,                   /* this must always be the last item */
>   };
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index f8012ec..3807596 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -4945,7 +4945,22 @@ qemuBuildCommandLine(virConnectPtr conn,
>                   VIR_WARN("bootmenu is enabled but not "
>                            "supported by this QEMU binary");
>               }
> +        }
> +
> +        if (def->os.bios.rt_set) {
> +            if (!qemuCapsGet(caps, QEMU_CAPS_REBOOT_TIMEOUT)) {
> +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                               _("reboot timeout is not supported "
> +                                 "by this QEMU binary"));
> +                goto error;
> +            }
> +
> +            if (boot_nparams++)
> +                virBufferAddChar(&boot_buf, ',');
>
> +            virBufferAsprintf(&boot_buf,
> +                              "reboot-timeout=%d",
> +                              def->os.bios.rt_delay);
>           }
>
>           if (boot_nparams > 0) {
> @@ -8271,6 +8286,19 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps,
>                           qemuParseCommandLineBootDevs(def, token);
>                       } else if (STRPREFIX(token, "menu=on")) {
>                           def->os.bootmenu = 1;
> +                    } else if (STRPREFIX(token, "reboot-timeout=")) {
> +                        int num;
> +                        char *endptr = strchr(token, ',');
> +                        if (virStrToLong_i(token + strlen("reboot-timeout="),
> +                                           &endptr, 0, &num) < 0) {

This doesn't seem ok. You assign endptr somewhere into the string and 
then virStrToLong_i rewrites it, but you never check the return value. I 
suppose you wanted to check if after the number is converted the next 
char is a comma. You need to do the check after virStrToLong_i.

> +                            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                                           _("cannot parse reboot-timeout value"));
> +                            goto error;
> +                        }
> +                        if (num > 65535)
> +                            num = 65535;
> +                        def->os.bios.rt_delay = num;
> +                        def->os.bios.rt_set = true;
>                       }
>                       token = strchr(token, ',');
>                       /* This incrementation has to be done here in order to make it
>

ACK when you fix the checking for the comma.

Peter