Re: [PATCH] hw/i386/pc: fix regression in parsing vga cmdline parameter

[Paolo Bonzini <pbonzini@redhat.com>]

On 21/12/19 17:21, Peter Wu wrote:
> When the 'vga=' parameter is succeeded by another parameter, QEMU 4.2.0
> would refuse to start with a rather cryptic message:
> 
>     $ qemu-system-x86_64 -kernel /boot/vmlinuz-linux -append 'vga=792 quiet'
>     qemu: can't parse 'vga' parameter: Invalid argument
> 
> It was not clear whether this applied to the '-vga std' parameter or the
> '-append' one. Fix the parsing regression and clarify the error.
> 
> Fixes: 133ef074bd ("hw/i386/pc: replace use of strtol with qemu_strtoui in x86_load_linux()")
> Cc: Sergio Lopez <slp@redhat.com>
> Signed-off-by: Peter Wu <peter@lekensteyn.nl>
> ---
> Hi,
> 
> This fixes a regression in QEMU 4.2.0 where my existing scripts would
> fail to boot while it worked fine with QEMU 4.1.1.
> 
> I do wonder whether QEMU has any business in strictly enforcing the
> contents of the kernel command line. Perhaps it should only warn about
> the issue, and not exit? Previously it would silently ignore bad values.
> 
> Kind regards,
> Peter
> ---
>  hw/i386/x86.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/i386/x86.c b/hw/i386/x86.c
> index 394edc2f72..121650ae51 100644
> --- a/hw/i386/x86.c
> +++ b/hw/i386/x86.c
> @@ -508,6 +508,7 @@ void x86_load_linux(X86MachineState *x86ms,
>      vmode = strstr(kernel_cmdline, "vga=");
>      if (vmode) {
>          unsigned int video_mode;
> +        const char *end;
>          int ret;
>          /* skip "vga=" */
>          vmode += 4;
> @@ -518,10 +519,9 @@ void x86_load_linux(X86MachineState *x86ms,
>          } else if (!strncmp(vmode, "ask", 3)) {
>              video_mode = 0xfffd;
>          } else {
> -            ret = qemu_strtoui(vmode, NULL, 0, &video_mode);
> -            if (ret != 0) {
> -                fprintf(stderr, "qemu: can't parse 'vga' parameter: %s\n",
> -                        strerror(-ret));
> +            ret = qemu_strtoui(vmode, &end, 0, &video_mode);
> +            if (ret != 0 || (*end && *end != ' ')) {
> +                fprintf(stderr, "qemu: invalid 'vga=' kernel parameter.\n");
>                  exit(1);
>              }
>          }
> 

Cc: qemu-stable@nongnu.org

Queued, thanks.