diff mbox series

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

Message ID 20191221162124.1159291-1-peter@lekensteyn.nl
State New
Headers show
Series hw/i386/pc: fix regression in parsing vga cmdline parameter | expand

Commit Message

Peter Wu Dec. 21, 2019, 4:21 p.m. UTC
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(-)

Comments

Paolo Bonzini Dec. 22, 2019, 12:17 p.m. UTC | #1
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.
diff mbox series

Patch

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