Re: [Qemu-trivial] [RFC PATCH] vl: only display available accelerators

[Eduardo Habkost <ehabkost@redhat.com>]

On Mon, Oct 30, 2017 at 03:14:59PM -0300, Philippe Mathieu-Daudé wrote:
> examples configuring with '--enable-kvm --disable-tcg'
> 
> - before
> 
>   $ qemu-system-x86_64 -accel help
>   Possible accelerators: kvm, xen, hax, tcg
> 
>   $ qemu-system-x86_64 -accel tcg
>   qemu-system-x86_64: -machine accel=tcg: No accelerator found
> 
>   # qemu-system-x86_64 -accel hax
>   qemu-system-x86_64: -machine accel=hax: No accelerator found
> 
> - after
> 
>   $ qemu-system-x86_64 -accel help
>   Possible accelerators:
>     xen
>     kvm
> 
> Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> since RFC:
>   - use much cleaner object_class_get_list(TYPE_ACCEL, false)
> 
>  vl.c | 39 ++++++++++++++++++++++++++++++++++-----
>  1 file changed, 34 insertions(+), 5 deletions(-)
> 
> diff --git a/vl.c b/vl.c
> index ec299099ff..0f13641715 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2764,6 +2764,39 @@ static gint machine_class_cmp(gconstpointer a, gconstpointer b)
>      exit(!name || !is_help_option(name));
>  }
>  
> +static void accel_list_entry(gpointer data, gpointer user_data)
> +{
> +    ObjectClass *oc = data;
> +    const char *typename = object_class_get_name(oc);
> +    int len;
> +
> +    if (!qtest_driver() && !g_strcmp0(typename, ACCEL_CLASS_NAME("qtest"))) {
> +        return; /* used by test cases */
> +    }

I would prefer to not hardcode the accel name here.  If you want
qtest to have special behavior, you can add a new field to
AccelClass to implement that.

However, I don't think I agree we should hide qtest from the
user: it is accepted on the command-line, so why should we lie?
Why not add a description field and indicate that qtest is useful
only for testing?

> +
> +    len = strlen(typename) - strlen("-" TYPE_ACCEL);

You can use AccelClass::name here.

> +    if (len > 0) {
> +        error_printf("  %.*s\n", len, typename);
> +    }
> +}
> +
> +static void accel_parse(const char *name, QemuOpts *accel_opts)
> +{
> +    const char *optarg = qemu_opt_get(accel_opts, "accel");
> +    GSList *list;
> +
> +    if (!is_help_option(optarg)) {
> +        return;
> +    }
> +
> +    list = object_class_get_list(TYPE_ACCEL, false);
> +    error_printf("Possible accelerators:\n");
> +    g_slist_foreach(list, accel_list_entry, NULL);
> +    g_slist_free(list);
> +
> +    exit(0);
> +}
> +
>  void qemu_add_exit_notifier(Notifier *notify)
>  {
>      notifier_list_add(&exit_notifiers, notify);
> @@ -3881,11 +3914,7 @@ int main(int argc, char **argv, char **envp)
>              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(0);
> -                }
> +                accel_parse(optarg, accel_opts);
>                  opts = qemu_opts_create(qemu_find_opts("machine"), NULL,
>                                          false, &error_abort);
>                  qemu_opt_set(opts, "accel", optarg, &error_abort);
> -- 
> 2.15.0.rc2
> 

-- 
Eduardo