diff mbox series

[RFC,v4,06/15] tests/qtest: Add qtest_get_machine_args

Message ID 20230119135424.5417-7-farosas@suse.de
State New
Headers show
Series target/arm: Allow CONFIG_TCG=n builds | expand

Commit Message

Fabiano Rosas Jan. 19, 2023, 1:54 p.m. UTC
QEMU machines might not have a default value defined for the -cpu
option. Add a custom init function that takes care of selecting the
default cpu in case the test did not specify one. For the machines
that do not have a default, the value MUST be provided by the test.

Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 tests/qtest/libqtest.c | 99 ++++++++++++++++++++++++++++++++++++++++++
 tests/qtest/libqtest.h | 11 +++++
 2 files changed, 110 insertions(+)

Comments

Richard Henderson Jan. 19, 2023, 6:55 p.m. UTC | #1
On 1/19/23 03:54, Fabiano Rosas wrote:
> QEMU machines might not have a default value defined for the -cpu
> option. Add a custom init function that takes care of selecting the
> default cpu in case the test did not specify one. For the machines
> that do not have a default, the value MUST be provided by the test.
> 
> Signed-off-by: Fabiano Rosas<farosas@suse.de>
> ---
>   tests/qtest/libqtest.c | 99 ++++++++++++++++++++++++++++++++++++++++++
>   tests/qtest/libqtest.h | 11 +++++
>   2 files changed, 110 insertions(+)

Looks plausible.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~
Thomas Huth Jan. 20, 2023, 11:48 a.m. UTC | #2
On 19/01/2023 14.54, Fabiano Rosas wrote:
> QEMU machines might not have a default value defined for the -cpu
> option.

Which machines for example? ... I thought we'd have a default CPU everywhere?

  Thomas
Cornelia Huck Jan. 20, 2023, noon UTC | #3
On Fri, Jan 20 2023, Thomas Huth <thuth@redhat.com> wrote:

> On 19/01/2023 14.54, Fabiano Rosas wrote:
>> QEMU machines might not have a default value defined for the -cpu
>> option.
>
> Which machines for example? ... I thought we'd have a default CPU everywhere?

There's a patch further above that removes it for KVM on Arm... do you
think that's a bad idea? In that case, I'm not sure what the default for
that case should even be...
Thomas Huth Jan. 20, 2023, 12:12 p.m. UTC | #4
On 20/01/2023 13.00, Cornelia Huck wrote:
> On Fri, Jan 20 2023, Thomas Huth <thuth@redhat.com> wrote:
> 
>> On 19/01/2023 14.54, Fabiano Rosas wrote:
>>> QEMU machines might not have a default value defined for the -cpu
>>> option.
>>
>> Which machines for example? ... I thought we'd have a default CPU everywhere?
> 
> There's a patch further above that removes it for KVM on Arm... do you
> think that's a bad idea? In that case, I'm not sure what the default for
> that case should even be...

Well, if there is just one machine in the whole of QEMU that does not have a 
default CPU anymore, that calls for trouble, I think (as we can already see 
in this series where you have to rework a lot of qtests). It's likely better 
to set another CPU as default in that machine in that case. What about 
simply using "max" or "host" if TCG is disabled there?

  Thomas
diff mbox series

Patch

diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 5cb38f90da..db8a40f0c7 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -1265,8 +1265,57 @@  static bool qtest_is_old_versioned_machine(const char *mname)
 struct MachInfo {
     char *name;
     char *alias;
+    char *default_cpu;
 };
 
+static char *qtest_get_cpu_name(QString *type)
+{
+    QDict *response, *cpuinfo;
+    QList *list;
+    const QListEntry *p;
+    QObject *qobj;
+    QString *qstr;
+    QTestState *qts;
+    char *cname = NULL;
+
+    qts = qtest_init("-machine none");
+    response = qtest_qmp(qts, "{ 'execute': 'query-cpu-definitions' }");
+    g_assert(response);
+    list = qdict_get_qlist(response, "return");
+
+    if (!list) {
+        /* Not all architectures implement query-cpu-definitions */
+        goto out;
+    }
+
+    for (p = qlist_first(list); p; p = qlist_next(p)) {
+        cpuinfo = qobject_to(QDict, qlist_entry_obj(p));
+        g_assert(cpuinfo);
+
+        qobj = qdict_get(cpuinfo, "typename");
+        g_assert(qobj);
+        qstr = qobject_to(QString, qobj);
+        g_assert(qstr);
+
+        if (g_str_equal(qstring_get_str(qstr),
+                        qstring_get_str(type))) {
+            qobj = qdict_get(cpuinfo, "name");
+            g_assert(qobj);
+            qstr = qobject_to(QString, qobj);
+            g_assert(qstr);
+
+            cname = g_strdup(qstring_get_str(qstr));
+            break;
+        }
+    }
+
+out:
+    qtest_quit(qts);
+    qobject_unref(response);
+
+    return cname;
+}
+
 /*
  * Returns an array with pointers to the available machine names.
  * The terminating entry has the name set to NULL.
@@ -1312,6 +1361,15 @@  static struct MachInfo *qtest_get_machines(void)
         } else {
             machines[idx].alias = NULL;
         }
+
+        qobj = qdict_get(minfo, "default-cpu-type");
+        if (qobj) {                           /* The default cpu is optional */
+            qstr = qobject_to(QString, qobj);
+            g_assert(qstr);
+            machines[idx].default_cpu = qtest_get_cpu_name(qstr);
+        } else {
+            machines[idx].default_cpu = NULL;
+        }
     }
 
     qtest_quit(qts);
@@ -1321,6 +1379,47 @@  static struct MachInfo *qtest_get_machines(void)
     return machines;
 }
 
+static const char *qtest_get_default_cpu(const char* machine)
+{
+    struct MachInfo *machines;
+    char *cpu = NULL;
+    int i;
+
+    if (g_str_equal(machine, "none")) {
+        return cpu;
+    }
+
+    machines = qtest_get_machines();
+
+    for (i = 0; machines[i].name != NULL; i++) {
+        if (g_str_equal(machines[i].name, machine)) {
+            cpu = machines[i].default_cpu;
+            break;
+        }
+    }
+
+    return cpu;
+}
+
+char *qtest_get_machine_args(const char *mname, const char *cname,
+                             const char *extra_args)
+{
+    const char *cpu;
+
+    cpu = cname ?: qtest_get_default_cpu(mname);
+    if (!cpu) {
+        /*
+         * There is no default cpu and the test did not provide a cpu
+         * name for this architecture/machine combination. The QEMU
+         * binary might still know how to select a cpu, so leave the
+         * -cpu option out.
+         */
+        return g_strdup_printf("-machine %s %s", mname, extra_args ?: "");
+    }
+    return g_strdup_printf("-machine %s -cpu %s %s", mname, cpu,
+                           extra_args ?: "");
+}
+
 void qtest_cb_for_every_machine(void (*cb)(const char *machine),
                                 bool skip_old_versioned)
 {
diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
index fcf1c3c3b3..f86f876c17 100644
--- a/tests/qtest/libqtest.h
+++ b/tests/qtest/libqtest.h
@@ -75,6 +75,17 @@  QTestState *qtest_init_without_qmp_handshake(const char *extra_args);
  */
 QTestState *qtest_init_with_serial(const char *extra_args, int *sock_fd);
 
+/**
+ * qtest_get_machine_args:
+ * @mname: the machine name.
+ * @cname: the cpu name.
+ * @extra_args: other arguments to concatenated in the args string.
+ *
+ * Returns: pointer to args.
+ */
+char *qtest_get_machine_args(const char *mname, const char *cname,
+                             const char *extra_args);
+
 /**
  * qtest_wait_qemu:
  * @s: #QTestState instance to operate on.