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

Fabiano Rosas posted 15 patches 3 years ago
There is a newer version of this series
[RFC PATCH v4 06/15] tests/qtest: Add qtest_get_machine_args
Posted by Fabiano Rosas 3 years ago
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(+)

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.
-- 
2.35.3
Re: [RFC PATCH v4 06/15] tests/qtest: Add qtest_get_machine_args
Posted by Thomas Huth 3 years ago
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
Re: [RFC PATCH v4 06/15] tests/qtest: Add qtest_get_machine_args
Posted by Cornelia Huck 3 years ago
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...
Re: [RFC PATCH v4 06/15] tests/qtest: Add qtest_get_machine_args
Posted by Thomas Huth 3 years ago
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
Re: [RFC PATCH v4 06/15] tests/qtest: Add qtest_get_machine_args
Posted by Richard Henderson 3 years ago
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~