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

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

[Peter Wu]

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);
             }
         }
-- 
2.24.1

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

[Paolo Bonzini]

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.