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

[Peter Krempa]

On 09/20/12 15:15, Martin Kletzander wrote:
> On 09/20/2012 12:22 PM, Peter Krempa wrote:
>>> @@ -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.
>>
>
> Yes, sorry for that, you're absolutely right. To be sure, is this alright?
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index f8012ec..4821910 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -8271,6 +8286,20 @@ 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;
> +                        if (virStrToLong_i(token + strlen("reboot-timeout="),
> +                                           &endptr, 10, &num) < 0) ||
> +                            endptr != strchr(token, ',') {
> +                            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                                           _("cannot parse reboot-timeout value"));
> +                            goto error;
> +                        }
> +                        if (num > 65535)
> +                            num = 65535;

You should probably check for the lower bound too here.

> +                        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 to this with the lower bound check.

Peter