qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 06/16] vl: configure accelerators from -accel options


From: Wainer dos Santos Moschetta
Subject: Re: [PATCH 06/16] vl: configure accelerators from -accel options
Date: Mon, 18 Nov 2019 15:59:11 -0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0


On 11/13/19 12:38 PM, Paolo Bonzini wrote:
Drop the "accel" property from MachineState, and instead desugar
"-machine accel=" to a list of "-accel" options.

This has a semantic change due to removing merge_lists from -accel.
For example:

- "-accel kvm -accel tcg" all but ignored "-accel kvm".  This is a bugfix.

- "-accel kvm -accel thread=single" ignored "thread=single", since it
   applied the option to KVM.  Now it fails due to not specifying the
   accelerator on "-accel thread=single".

- "-accel tcg -accel thread=single" chose single-threaded TCG, while now
   it will fail due to not specifying the accelerator on "-accel
   thread=single".

Also, "-machine accel" and "-accel" become incompatible.

Signed-off-by: Paolo Bonzini <address@hidden>
---
  hw/core/machine.c   | 21 ------------
  include/hw/boards.h |  1 -
  vl.c                | 93 +++++++++++++++++++++++++++++++----------------------
  3 files changed, 54 insertions(+), 61 deletions(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index 1689ad3..45ddfb6 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -173,21 +173,6 @@ GlobalProperty hw_compat_2_1[] = {
  };
  const size_t hw_compat_2_1_len = G_N_ELEMENTS(hw_compat_2_1);
-static char *machine_get_accel(Object *obj, Error **errp)
-{
-    MachineState *ms = MACHINE(obj);
-
-    return g_strdup(ms->accel);
-}
-
-static void machine_set_accel(Object *obj, const char *value, Error **errp)
-{
-    MachineState *ms = MACHINE(obj);
-
-    g_free(ms->accel);
-    ms->accel = g_strdup(value);
-}
-
  static void machine_set_kernel_irqchip(Object *obj, Visitor *v,
                                         const char *name, void *opaque,
                                         Error **errp)
@@ -808,11 +793,6 @@ static void machine_class_init(ObjectClass *oc, void *data)
      mc->numa_mem_align_shift = 23;
      mc->numa_auto_assign_ram = numa_default_auto_assign_ram;
- object_class_property_add_str(oc, "accel",
-        machine_get_accel, machine_set_accel, &error_abort);
-    object_class_property_set_description(oc, "accel",
-        "Accelerator list", &error_abort);
-
      object_class_property_add(oc, "kernel-irqchip", "on|off|split",
          NULL, machine_set_kernel_irqchip,
          NULL, NULL, &error_abort);
@@ -971,7 +951,6 @@ static void machine_finalize(Object *obj)
  {
      MachineState *ms = MACHINE(obj);
- g_free(ms->accel);
      g_free(ms->kernel_filename);
      g_free(ms->initrd_filename);
      g_free(ms->kernel_cmdline);
diff --git a/include/hw/boards.h b/include/hw/boards.h
index de45087..36fcbda 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -275,7 +275,6 @@ struct MachineState {
/*< public >*/ - char *accel;
      bool kernel_irqchip_allowed;
      bool kernel_irqchip_required;
      bool kernel_irqchip_split;
diff --git a/vl.c b/vl.c
index b93217d..dd895db 100644
--- a/vl.c
+++ b/vl.c
@@ -293,7 +293,6 @@ static QemuOptsList qemu_accel_opts = {
      .name = "accel",
      .implied_opt_name = "accel",
      .head = QTAILQ_HEAD_INITIALIZER(qemu_accel_opts.head),
-    .merge_lists = true,
      .desc = {
          {
              .name = "accel",
@@ -2651,6 +2650,11 @@ static int machine_set_property(void *opaque,
          }
      }
+ /* Legacy options do not correspond to MachineState properties. */
+    if (g_str_equal(qom_name, "accel")) {
+        return 0;
+    }
+
      r = object_parse_property_opt(opaque, name, value, "type", errp);
      g_free(qom_name);
      return r;
@@ -2843,74 +2847,88 @@ static int do_configure_icount(void *opaque, QemuOpts 
*opts, Error **errp)
static int do_configure_accelerator(void *opaque, QemuOpts *opts, Error **errp)
  {
+    bool *p_init_failed = opaque;
+    const char *acc = qemu_opt_get(opts, "accel");
+    AccelClass *ac = accel_find(acc);
+    int ret;
+
+    if (!ac) {
+        return 0;
+    }
+    ret = accel_init_machine(ac, current_machine);
+    if (ret < 0) {
+        *p_init_failed = true;
+        error_report("failed to initialize %s: %s",
+                     acc, strerror(-ret));
+        return 0;
+    }
+
      if (tcg_enabled()) {
          qemu_tcg_configure(opts, &error_fatal);
      }
-    return 0;
+    return 1;
  }
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;
qemu_opts_foreach(qemu_find_opts("icount"),
                        do_configure_icount, NULL, &error_fatal);
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");

Perhaps on patch 07 you can also clarify this message, mentioning what's the default accelerator.

-            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";
+    if (QTAILQ_EMPTY(&qemu_accel_opts.head)) {
+        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 {
-                accel = "tcg:kvm";
+                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);
+        accel_list = g_strsplit(accel, ":", 0);
- for (tmp = accel_list; !accel_initialised && tmp && *tmp; tmp++) {
-        acc = accel_find(*tmp);
-        if (!acc) {
-            continue;
+        for (tmp = accel_list; !accel_initialised && tmp && *tmp; tmp++) {
+            /*
+             * Filter invalid accelerators here, to prevent obscenities
+             * such as "-machine accel=tcg,,thread=single".
+             */
+            if (accel_find(*tmp)) {
+                qemu_opts_parse_noisily(qemu_find_opts("accel"), *tmp, true);
+            }
          }
-        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;
+    } else {
+        if (accel != NULL) {
+            error_report("The -accel and \"-machine accel=\" options are 
incompatible");
+            exit(1);
          }
      }
-    g_strfreev(accel_list);
- if (!accel_initialised) {
+    if (!qemu_opts_foreach(qemu_find_opts("accel"),
+                           do_configure_accelerator, &init_failed, 
&error_fatal)) {
          if (!init_failed) {
-            error_report("-machine accel=%s: No accelerator found", accel);
+            error_report("no accelerator found");

Likewise.

Thanks,

Wainer

          }
          exit(1);
      }
if (init_failed) {
-        error_report("Back to %s accelerator", acc->name);
+        AccelClass *ac = ACCEL_GET_CLASS(current_machine->accelerator);
+        error_report("Back to %s accelerator", ac->name);
      }
- qemu_opts_foreach(qemu_find_opts("accel"),
-                      do_configure_accelerator, NULL, &error_fatal);
-
      if (!tcg_enabled() && use_icount) {
          error_report("-icount is not allowed with hardware virtualization");
          exit(1);
@@ -3618,9 +3636,6 @@ int main(int argc, char **argv, char **envp)
                                   "use -M accel=... for now instead");
                      exit(1);
                  }
-                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");




reply via email to

[Prev in Thread] Current Thread [Next in Thread]