Re: [Qemu-devel] [PATCH] vl: Fix broken thread=xxx option of the --accel parameter

[Alex Bennée]

Thomas Huth <address@hidden> writes:

> Commit bde4d9205 ("Fix the -accel parameter and the documentation for
> 'hax'") introduced a regression by adding a new local accel_opts
> variable which shadows the variable with the same name that is
> declared at the beginning of the main() scope. This causes the
> qemu_tcg_configure() call later to be always called with NULL, so
> that the thread=xxx option gets ignored. Fix it by removing the
> local accel_opts variable and use "opts" instead, which is meant
> for storing temporary QemuOpts values.
> And while we're at it, also change the exit(1) here to exit(0)
> since asking for help is not an error.
>
> Fixes: bde4d9205ee9def98852ff6054cdef4efd74e1f8
> Reported-by: Markus Armbruster <address@hidden>
> Reported-by: Emilio G. Cota <address@hidden>
> Signed-off-by: Thomas Huth <address@hidden>

Reviewed-by: Alex Bennée <address@hidden>
Tested-by: Alex Bennée <address@hidden>

I'll leave the wider question of a better layout to the QemuOpts experts
(I was/am very much an amateur when I first added the thread option).

> ---
>  vl.c | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/vl.c b/vl.c
> index be4dcf2..5aba544 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3757,21 +3757,18 @@ int main(int argc, char **argv, char **envp)
>                  qdev_prop_register_global(&kvm_pit_lost_tick_policy);
>                  break;
>              }
> -            case QEMU_OPTION_accel: {
> -                QemuOpts *accel_opts;
> -
> +            case QEMU_OPTION_accel:
>                  accel_opts = qemu_opts_parse_noisily(qemu_find_opts("accel"),
>                                                       optarg, true);
>                  optarg = qemu_opt_get(accel_opts, "accel");
>                  if (!optarg || is_help_option(optarg)) {
>                      error_printf("Possible accelerators: kvm, xen, hax, 
> tcg\n");
> -                    exit(1);
> +                    exit(0);
>                  }
> -                accel_opts = qemu_opts_create(qemu_find_opts("machine"), 
> NULL,
> -                                              false, &error_abort);
> -                qemu_opt_set(accel_opts, "accel", optarg, &error_abort);
> +                opts = qemu_opts_create(qemu_find_opts("machine"), NULL,
> +                                        false, &error_abort);
> +                qemu_opt_set(opts, "accel", optarg, &error_abort);
>                  break;
> -            }
>              case QEMU_OPTION_usb:
>                  olist = qemu_find_opts("machine");
>                  qemu_opts_parse_noisily(olist, "usb=on", false);


--
Alex Bennée