Patchwork [for-1.3] vl.c: Fix broken -usb option

login
register
mail settings
Submitter Peter Maydell
Date Nov. 22, 2012, 4:48 p.m.
Message ID <1353602925-28887-1-git-send-email-peter.maydell@linaro.org>
Download mbox | patch
Permalink /patch/201085/
State New
Headers show

Comments

Peter Maydell - Nov. 22, 2012, 4:48 p.m.
Commit 094b287f0b accidentally broke the "-usb" command line
option, so it would have no effect if the user had not specified
any machine options at that point. (the return value from
'qemu_opts_find(qemu_find_opts("machine"), 0);' is NULL if there
are no user specified options, so it is only to be used for
looking up an option, not when trying to set one.) Similarly,
would '-usbdevice' no longer cause USB to default to enabled.

Fix this regression by using the same style of code for forcing
the usb=on machine option that we use for other aliases such as
'-enable-kvm'.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
For 1.3 as this is a regression which would presumably cause
breakage for migrations and existing working qemu command lines.

 vl.c |   12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)
Li Zhang - Nov. 22, 2012, 5:05 p.m.
On 2012年11月23日 00:48, Peter Maydell wrote:
> Commit 094b287f0b accidentally broke the "-usb" command line
> option, so it would have no effect if the user had not specified
> any machine options at that point. (the return value from
> 'qemu_opts_find(qemu_find_opts("machine"), 0);' is NULL if there
> are no user specified options, so it is only to be used for
> looking up an option, not when trying to set one.) Similarly,
> would '-usbdevice' no longer cause USB to default to enabled.
>
> Fix this regression by using the same style of code for forcing
> the usb=on machine option that we use for other aliases such as
> '-enable-kvm'.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> For 1.3 as this is a regression which would presumably cause
> breakage for migrations and existing working qemu command lines.
>
>   vl.c |   12 ++++--------
>   1 file changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/vl.c b/vl.c
> index c8e9c78..a3ab384 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3273,16 +3273,12 @@ int main(int argc, char **argv, char **envp)
>                   break;
>               }
>               case QEMU_OPTION_usb:
> -                machine_opts = qemu_opts_find(qemu_find_opts("machine"), 0);
> -                if (machine_opts) {
> -                    qemu_opt_set_bool(machine_opts, "usb", true);
> -                }
> +                olist = qemu_find_opts("machine");
> +                qemu_opts_parse(olist, "usb=on", 0);
>                   break;
>               case QEMU_OPTION_usbdevice:
> -                machine_opts = qemu_opts_find(qemu_find_opts("machine"), 0);
> -                if (machine_opts) {
> -                    qemu_opt_set_bool(machine_opts, "usb", true);
> -                }
> +                olist = qemu_find_opts("machine");
> +                qemu_opts_parse(olist, "usb=on", 0);
>                   add_device_config(DEV_USB, optarg);
>                   break;
>               case QEMU_OPTION_device:
Looks good to me.

Thanks.
Anthony Liguori - Nov. 26, 2012, 9:49 p.m.
Peter Maydell <peter.maydell@linaro.org> writes:

> Commit 094b287f0b accidentally broke the "-usb" command line
> option, so it would have no effect if the user had not specified
> any machine options at that point. (the return value from
> 'qemu_opts_find(qemu_find_opts("machine"), 0);' is NULL if there
> are no user specified options, so it is only to be used for
> looking up an option, not when trying to set one.) Similarly,
> would '-usbdevice' no longer cause USB to default to enabled.
>
> Fix this regression by using the same style of code for forcing
> the usb=on machine option that we use for other aliases such as
> '-enable-kvm'.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> For 1.3 as this is a regression which would presumably cause
> breakage for migrations and existing working qemu command lines.
>

Applied. Thanks.

Regards,

Anthony Liguori

>  vl.c |   12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/vl.c b/vl.c
> index c8e9c78..a3ab384 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3273,16 +3273,12 @@ int main(int argc, char **argv, char **envp)
>                  break;
>              }
>              case QEMU_OPTION_usb:
> -                machine_opts = qemu_opts_find(qemu_find_opts("machine"), 0);
> -                if (machine_opts) {
> -                    qemu_opt_set_bool(machine_opts, "usb", true);
> -                }
> +                olist = qemu_find_opts("machine");
> +                qemu_opts_parse(olist, "usb=on", 0);
>                  break;
>              case QEMU_OPTION_usbdevice:
> -                machine_opts = qemu_opts_find(qemu_find_opts("machine"), 0);
> -                if (machine_opts) {
> -                    qemu_opt_set_bool(machine_opts, "usb", true);
> -                }
> +                olist = qemu_find_opts("machine");
> +                qemu_opts_parse(olist, "usb=on", 0);
>                  add_device_config(DEV_USB, optarg);
>                  break;
>              case QEMU_OPTION_device:
> -- 
> 1.7.9.5

Patch

diff --git a/vl.c b/vl.c
index c8e9c78..a3ab384 100644
--- a/vl.c
+++ b/vl.c
@@ -3273,16 +3273,12 @@  int main(int argc, char **argv, char **envp)
                 break;
             }
             case QEMU_OPTION_usb:
-                machine_opts = qemu_opts_find(qemu_find_opts("machine"), 0);
-                if (machine_opts) {
-                    qemu_opt_set_bool(machine_opts, "usb", true);
-                }
+                olist = qemu_find_opts("machine");
+                qemu_opts_parse(olist, "usb=on", 0);
                 break;
             case QEMU_OPTION_usbdevice:
-                machine_opts = qemu_opts_find(qemu_find_opts("machine"), 0);
-                if (machine_opts) {
-                    qemu_opt_set_bool(machine_opts, "usb", true);
-                }
+                olist = qemu_find_opts("machine");
+                qemu_opts_parse(olist, "usb=on", 0);
                 add_device_config(DEV_USB, optarg);
                 break;
             case QEMU_OPTION_device: