Re: [PATCH 03/16] vl: merge -accel processing into configure_accelerators

[Marc-André Lureau]

Hi

On Wed, Nov 13, 2019 at 6:42 PM Paolo Bonzini <address@hidden> wrote:
>
> The next step is to move the parsing of "-machine accel=..." into vl.c,
> unifying it with the configure_accelerators() function that has just
> been introduced.  This way, we will be able to desugar it into multiple
> "-accel" options, without polluting accel/accel.c.
>
> The CONFIG_TCG and CONFIG_KVM symbols are not available in vl.c, but
> we can use accel_find instead to find their value at runtime.  Once we
> know that the binary has one of TCG or KVM, the default accelerator
> can be expressed simply as "tcg:kvm", because TCG never fails to initialize.
>

Reviewed-by: Marc-André Lureau <address@hidden>

> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
>  accel/accel.c          | 63 
> ++------------------------------------------------
>  include/sysemu/accel.h |  4 +++-
>  vl.c                   | 62 +++++++++++++++++++++++++++++++++++++++++++++----
>  3 files changed, 62 insertions(+), 67 deletions(-)
>
> diff --git a/accel/accel.c b/accel/accel.c
> index 5fa3171..74eda68 100644
> --- a/accel/accel.c
> +++ b/accel/accel.c
> @@ -44,7 +44,7 @@ static const TypeInfo accel_type = {
>  };
>
>  /* Lookup AccelClass from opt_name. Returns NULL if not found */
> -static AccelClass *accel_find(const char *opt_name)
> +AccelClass *accel_find(const char *opt_name)
>  {
>      char *class_name = g_strdup_printf(ACCEL_CLASS_NAME("%s"), opt_name);
>      AccelClass *ac = ACCEL_CLASS(object_class_by_name(class_name));
> @@ -52,7 +52,7 @@ static AccelClass *accel_find(const char *opt_name)
>      return ac;
>  }
>
> -static int accel_init_machine(AccelClass *acc, MachineState *ms)
> +int accel_init_machine(AccelClass *acc, MachineState *ms)
>  {
>      ObjectClass *oc = OBJECT_CLASS(acc);
>      const char *cname = object_class_get_name(oc);
> @@ -71,65 +71,6 @@ static int accel_init_machine(AccelClass *acc, 
> MachineState *ms)
>      return ret;
>  }
>
> -void configure_accelerator(MachineState *ms, const char *progname)
> -{
> -    const char *accel;
> -    char **accel_list, **tmp;
> -    int ret;
> -    bool accel_initialised = false;
> -    bool init_failed = false;
> -    AccelClass *acc = NULL;
> -
> -    accel = qemu_opt_get(qemu_get_machine_opts(), "accel");
> -    if (accel == NULL) {
> -        /* Select the default accelerator */
> -        int pnlen = strlen(progname);
> -        if (pnlen >= 3 && g_str_equal(&progname[pnlen - 3], "kvm")) {
> -            /* If the program name ends with "kvm", we prefer KVM */
> -            accel = "kvm:tcg";
> -        } else {
> -#if defined(CONFIG_TCG)
> -            accel = "tcg";
> -#elif defined(CONFIG_KVM)
> -            accel = "kvm";
> -#else
> -            error_report("No accelerator selected and"
> -                         " no default accelerator available");
> -            exit(1);
> -#endif
> -        }
> -    }
> -
> -    accel_list = g_strsplit(accel, ":", 0);
> -
> -    for (tmp = accel_list; !accel_initialised && tmp && *tmp; tmp++) {
> -        acc = accel_find(*tmp);
> -        if (!acc) {
> -            continue;
> -        }
> -        ret = accel_init_machine(acc, ms);
> -        if (ret < 0) {
> -            init_failed = true;
> -            error_report("failed to initialize %s: %s",
> -                         acc->name, strerror(-ret));
> -        } else {
> -            accel_initialised = true;
> -        }
> -    }
> -    g_strfreev(accel_list);
> -
> -    if (!accel_initialised) {
> -        if (!init_failed) {
> -            error_report("-machine accel=%s: No accelerator found", accel);
> -        }
> -        exit(1);
> -    }
> -
> -    if (init_failed) {
> -        error_report("Back to %s accelerator", acc->name);
> -    }
> -}
> -
>  void accel_setup_post(MachineState *ms)
>  {
>      AccelState *accel = ms->accelerator;
> diff --git a/include/sysemu/accel.h b/include/sysemu/accel.h
> index 8eb60b8..90b6213 100644
> --- a/include/sysemu/accel.h
> +++ b/include/sysemu/accel.h
> @@ -66,7 +66,9 @@ typedef struct AccelClass {
>
>  extern unsigned long tcg_tb_size;
>
> -void configure_accelerator(MachineState *ms, const char *progname);
> +AccelClass *accel_find(const char *opt_name);
> +int accel_init_machine(AccelClass *acc, MachineState *ms);
> +
>  /* Called just before os_setup_post (ie just before drop OS privs) */
>  void accel_setup_post(MachineState *ms);
>
> diff --git a/vl.c b/vl.c
> index 5367f23..fc9e70f 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2845,8 +2845,62 @@ static int do_configure_accelerator(void *opaque, 
> QemuOpts *opts, Error **errp)
>      return 0;
>  }
>
> -static void configure_accelerators(void)
> +static void configure_accelerators(const char *progname)
>  {
> +    const char *accel;
> +    char **accel_list, **tmp;
> +    int ret;
> +    bool accel_initialised = false;
> +    bool init_failed = false;
> +    AccelClass *acc = NULL;
> +
> +    accel = qemu_opt_get(qemu_get_machine_opts(), "accel");
> +    if (accel == NULL) {
> +        /* Select the default accelerator */
> +        if (!accel_find("tcg") && !accel_find("kvm")) {
> +            error_report("No accelerator selected and"
> +                         " no default accelerator available");
> +            exit(1);
> +        } else {
> +            int pnlen = strlen(progname);
> +            if (pnlen >= 3 && g_str_equal(&progname[pnlen - 3], "kvm")) {
> +                /* If the program name ends with "kvm", we prefer KVM */
> +                accel = "kvm:tcg";
> +            } else {
> +                accel = "tcg:kvm";
> +            }
> +        }
> +    }
> +
> +    accel_list = g_strsplit(accel, ":", 0);
> +
> +    for (tmp = accel_list; !accel_initialised && tmp && *tmp; tmp++) {
> +        acc = accel_find(*tmp);
> +        if (!acc) {
> +            continue;
> +        }
> +        ret = accel_init_machine(acc, current_machine);
> +        if (ret < 0) {
> +            init_failed = true;
> +            error_report("failed to initialize %s: %s",
> +                         acc->name, strerror(-ret));
> +        } else {
> +            accel_initialised = true;
> +        }
> +    }
> +    g_strfreev(accel_list);
> +
> +    if (!accel_initialised) {
> +        if (!init_failed) {
> +            error_report("-machine accel=%s: No accelerator found", accel);
> +        }
> +        exit(1);
> +    }
> +
> +    if (init_failed) {
> +        error_report("Back to %s accelerator", acc->name);
> +    }
> +
>      qemu_opts_foreach(qemu_find_opts("icount"),
>                        do_configure_icount, NULL, &error_fatal);
>
> @@ -4183,7 +4237,8 @@ int main(int argc, char **argv, char **envp)
>       * Note: uses machine properties such as kernel-irqchip, must run
>       * after machine_set_property().
>       */
> -    configure_accelerator(current_machine, argv[0]);
> +    cpu_ticks_init();
> +    configure_accelerators(argv[0]);
>
>      /*
>       * Beware, QOM objects created before this point miss global and
> @@ -4267,9 +4322,6 @@ int main(int argc, char **argv, char **envp)
>      /* spice needs the timers to be initialized by this point */
>      qemu_spice_init();
>
> -    cpu_ticks_init();
> -    configure_accelerators();
> -
>      if (default_net) {
>          QemuOptsList *net = qemu_find_opts("net");
>          qemu_opts_set(net, NULL, "type", "nic", &error_abort);
> --
> 1.8.3.1
>
>
>


-- 
Marc-André Lureau