[7/7] cpu: Add `machine` parameter to query-cpu-definitions
diff mbox series

Message ID 20191025022553.25298-8-ehabkost@redhat.com
State New
Headers show
Series
  • i386: Add `machine` parameter to query-cpu-definitions
Related show

Commit Message

Eduardo Habkost Oct. 25, 2019, 2:25 a.m. UTC
The new parameter can be used by management software to query for
CPU model alias information for multiple machines without
restarting QEMU.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 qapi/machine-target.json                   | 14 +++++++-
 target/arm/helper.c                        |  4 ++-
 target/i386/cpu.c                          | 16 +++++++--
 target/mips/helper.c                       |  4 ++-
 target/ppc/translate_init.inc.c            |  4 ++-
 target/s390x/cpu_models.c                  |  4 ++-
 tests/acceptance/x86_cpu_model_versions.py | 42 ++++++++++++++++++++++
 7 files changed, 81 insertions(+), 7 deletions(-)

Comments

Markus Armbruster Oct. 25, 2019, 6:36 a.m. UTC | #1
Eduardo Habkost <ehabkost@redhat.com> writes:

> The new parameter can be used by management software to query for
> CPU model alias information for multiple machines without
> restarting QEMU.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  qapi/machine-target.json                   | 14 +++++++-
>  target/arm/helper.c                        |  4 ++-
>  target/i386/cpu.c                          | 16 +++++++--
>  target/mips/helper.c                       |  4 ++-
>  target/ppc/translate_init.inc.c            |  4 ++-
>  target/s390x/cpu_models.c                  |  4 ++-
>  tests/acceptance/x86_cpu_model_versions.py | 42 ++++++++++++++++++++++
>  7 files changed, 81 insertions(+), 7 deletions(-)
>
> diff --git a/qapi/machine-target.json b/qapi/machine-target.json
> index 55310a6aa2..7bff3811fe 100644
> --- a/qapi/machine-target.json
> +++ b/qapi/machine-target.json
> @@ -281,6 +281,10 @@

This is CpuDefinitionInfo.

>  #
>  # @alias-of: Name of CPU model this model is an alias for.  The target of the
>  #            CPU model alias may change depending on the machine type.
> +#            If the @machine argument was provided to query-cpu-definitions,
> +#            alias information that machine type will be returned.

"for that machine type"

> +#            If @machine is not provided, alias information for
> +#            the current machine will be returned.
>  #            Management software is supposed to translate CPU model aliases
>  #            in the VM configuration, because aliases may stop being
>  #            migration-safe in the future (since 4.1)
> @@ -317,9 +321,17 @@
   ##
   # @query-cpu-definitions:
>  #
>  # Return a list of supported virtual CPU definitions
>  #
> +# @machine: Name of machine type.  The command returns some data
> +#           that machine-specific.  This overrides the machine type

"that is machine-specific"

> +#           used to look up that information.  This can be used,
> +#           for example, to query machine-specific CPU model aliases
> +#           without restarting QEMU (since 4.2)
> +#
>  # Returns: a list of CpuDefInfo
>  #
>  # Since: 1.2.0
>  ##
> -{ 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'],
> +{ 'command': 'query-cpu-definitions',
> +  'data': { '*machine': 'str' },
> +  'returns': ['CpuDefinitionInfo'],
>    'if': 'defined(TARGET_PPC) || defined(TARGET_ARM) || defined(TARGET_I386) || defined(TARGET_S390X) || defined(TARGET_MIPS)' }

I'm afraid the doc comment is less than clear.  Before I can suggest
improvements, I have questions.

Looks like @alias-of is the only part of the return value that changes
when I re-run query-cpu-definitions with another @machine argument.
Correct?

How is this going to be used?  Will management software run
query-cpu-definitions for each machine type returned by query-machines?
Or just for a few of them?

> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 0d9a2d2ab7..96f9fe7fff 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -6965,7 +6965,9 @@ static void arm_cpu_add_definition(gpointer data, gpointer user_data)
>      *cpu_list = entry;
>  }
>  
> -CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
> +CpuDefinitionInfoList *qmp_query_cpu_definitions(bool has_machine,
> +                                                 const char *machine,
> +                                                 Error **errp)
>  {
>      CpuDefinitionInfoList *cpu_list = NULL;
>      GSList *list;
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 67d1eca4ed..ae633793ed 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -4078,11 +4078,23 @@ static void x86_cpu_definition_entry(gpointer data, gpointer user_data)
>      args->cpu_list = entry;
>  }
>  
> -CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
> +CpuDefinitionInfoList *qmp_query_cpu_definitions(bool has_machine,
> +                                                 const char *machine,
> +                                                 Error **errp)
>  {
>      X86CPUDefinitionArgs args = { .cpu_list = NULL };
>      GSList *list;
> -    MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine());
> +    MachineClass *mc;
> +
> +    if (!has_machine) {
> +        mc = MACHINE_GET_CLASS(qdev_get_machine());
> +    } else {
> +        mc = machine_find_class(machine);
> +        if (!mc) {
> +            error_setg(errp, "Machine type '%s' not found", machine);
> +            return NULL;
> +        }
> +    }
>  
>      args.default_version = default_cpu_version_for_machine(mc);
>      list = get_sorted_cpu_model_list();
> diff --git a/target/mips/helper.c b/target/mips/helper.c
> index a2b6459b05..a73c767462 100644
> --- a/target/mips/helper.c
> +++ b/target/mips/helper.c
> @@ -1481,7 +1481,9 @@ static void mips_cpu_add_definition(gpointer data, gpointer user_data)
>      *cpu_list = entry;
>  }
>  
> -CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
> +CpuDefinitionInfoList *qmp_query_cpu_definitions(bool has_machine,
> +                                                 const char *machine,
> +                                                 Error **errp)
>  {
>      CpuDefinitionInfoList *cpu_list = NULL;
>      GSList *list;
> diff --git a/target/ppc/translate_init.inc.c b/target/ppc/translate_init.inc.c
> index ba726dec4d..4493309c4c 100644
> --- a/target/ppc/translate_init.inc.c
> +++ b/target/ppc/translate_init.inc.c
> @@ -10350,7 +10350,9 @@ static void ppc_cpu_defs_entry(gpointer data, gpointer user_data)
>      *first = entry;
>  }
>  
> -CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
> +CpuDefinitionInfoList *qmp_query_cpu_definitions(bool has_machine,
> +                                                 const char *machine,
> +                                                 Error **errp)
>  {
>      CpuDefinitionInfoList *cpu_list = NULL;
>      GSList *list;
> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
> index 7e92fb2e15..e40f1f6bec 100644
> --- a/target/s390x/cpu_models.c
> +++ b/target/s390x/cpu_models.c
> @@ -456,7 +456,9 @@ static void create_cpu_model_list(ObjectClass *klass, void *opaque)
>      *cpu_list = entry;
>  }
>  
> -CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
> +CpuDefinitionInfoList *qmp_query_cpu_definitions(bool has_machine,
> +                                                 const char *machine,
> +                                                 Error **errp)
>  {
>      struct CpuDefinitionInfoListData list_data = {
>          .list = NULL,

Should the commit message explain why all implementations but one ignore
@machine?

> diff --git a/tests/acceptance/x86_cpu_model_versions.py b/tests/acceptance/x86_cpu_model_versions.py
> index 5fc9ca4bc6..79c5250243 100644
> --- a/tests/acceptance/x86_cpu_model_versions.py
> +++ b/tests/acceptance/x86_cpu_model_versions.py
> @@ -234,6 +234,48 @@ class X86CPUModelAliases(avocado_qemu.Test):
>  
>          self.validate_aliases(cpus)
>  
> +    def test_machine_arg_none(self):
> +        """Check if unversioned CPU model is an alias pointing to right version"""
> +        vm1 = self.get_vm()
> +        vm1.add_args('-S')
> +        vm1.set_machine('pc-i440fx-4.0')
> +        vm1.launch()
> +        cpus1 = dict((m['name'], m.get('alias-of')) for m in vm1.command('query-cpu-definitions', machine='none'))
> +        vm1.shutdown()
> +
> +        vm2 = self.get_vm()
> +        vm2.add_args('-S')
> +        vm2.set_machine('none')
> +        vm2.launch()
> +        cpus2 = dict((m['name'], m.get('alias-of')) for m in vm2.command('query-cpu-definitions'))
> +        vm1.shutdown()
> +
> +        self.assertEquals(cpus1, cpus2)
> +
> +    def test_machine_arg_4_1(self):
> +        """Check if unversioned CPU model is an alias pointing to right version"""
> +        vm1 = self.get_vm()
> +        vm1.add_args('-S')
> +        vm1.set_machine('pc-i440fx-4.0')
> +        vm1.launch()
> +        cpus1 = dict((m['name'], m.get('alias-of')) for m in vm1.command('query-cpu-definitions', machine='pc-i440fx-4.1'))
> +        vm1.shutdown()
> +
> +        vm2 = self.get_vm()
> +        vm2.add_args('-S')
> +        vm2.set_machine('pc-i440fx-4.1')
> +        vm2.launch()
> +        cpus2 = dict((m['name'], m.get('alias-of')) for m in vm2.command('query-cpu-definitions'))
> +        vm1.shutdown()
> +
> +        self.assertEquals(cpus1, cpus2)
> +
> +    def test_invalid_machine(self):
> +        self.vm.add_args('-S')
> +        self.vm.launch()
> +        r = self.vm.qmp('query-cpu-definitions', machine='invalid-machine-123')
> +        self.assertIn('error', r)
> +
>  
>  class CascadelakeArchCapabilities(avocado_qemu.Test):
>      """

Tests look good to me.

I admit to being confused on when to use the tests/acceptance/ framework
and when to use qtest.
Eduardo Habkost Oct. 25, 2019, 1:22 p.m. UTC | #2
CCing libvir-list.

On Fri, Oct 25, 2019 at 08:36:59AM +0200, Markus Armbruster wrote:
> Eduardo Habkost <ehabkost@redhat.com> writes:
> > The new parameter can be used by management software to query for
> > CPU model alias information for multiple machines without
> > restarting QEMU.
[...]
> > @@ -317,9 +321,17 @@
>    ##
>    # @query-cpu-definitions:
> >  #
> >  # Return a list of supported virtual CPU definitions
> >  #
> > +# @machine: Name of machine type.  The command returns some data
> > +#           that machine-specific.  This overrides the machine type
> 
> "that is machine-specific"
> 
> > +#           used to look up that information.  This can be used,
> > +#           for example, to query machine-specific CPU model aliases
> > +#           without restarting QEMU (since 4.2)
> > +#
> >  # Returns: a list of CpuDefInfo
> >  #
> >  # Since: 1.2.0
> >  ##
> > -{ 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'],
> > +{ 'command': 'query-cpu-definitions',
> > +  'data': { '*machine': 'str' },
> > +  'returns': ['CpuDefinitionInfo'],
> >    'if': 'defined(TARGET_PPC) || defined(TARGET_ARM) || defined(TARGET_I386) || defined(TARGET_S390X) || defined(TARGET_MIPS)' }
> 
> I'm afraid the doc comment is less than clear.  Before I can suggest
> improvements, I have questions.
> 
> Looks like @alias-of is the only part of the return value that changes
> when I re-run query-cpu-definitions with another @machine argument.
> Correct?

Yes.

> 
> How is this going to be used?  Will management software run
> query-cpu-definitions for each machine type returned by query-machines?
> Or just for a few of them?

I don't know.  I'll let Jiri and other libvirt developers answer
that.

Patch
diff mbox series

diff --git a/qapi/machine-target.json b/qapi/machine-target.json
index 55310a6aa2..7bff3811fe 100644
--- a/qapi/machine-target.json
+++ b/qapi/machine-target.json
@@ -281,6 +281,10 @@ 
 #
 # @alias-of: Name of CPU model this model is an alias for.  The target of the
 #            CPU model alias may change depending on the machine type.
+#            If the @machine argument was provided to query-cpu-definitions,
+#            alias information that machine type will be returned.
+#            If @machine is not provided, alias information for
+#            the current machine will be returned.
 #            Management software is supposed to translate CPU model aliases
 #            in the VM configuration, because aliases may stop being
 #            migration-safe in the future (since 4.1)
@@ -317,9 +321,17 @@ 
 #
 # Return a list of supported virtual CPU definitions
 #
+# @machine: Name of machine type.  The command returns some data
+#           that machine-specific.  This overrides the machine type
+#           used to look up that information.  This can be used,
+#           for example, to query machine-specific CPU model aliases
+#           without restarting QEMU (since 4.2)
+#
 # Returns: a list of CpuDefInfo
 #
 # Since: 1.2.0
 ##
-{ 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'],
+{ 'command': 'query-cpu-definitions',
+  'data': { '*machine': 'str' },
+  'returns': ['CpuDefinitionInfo'],
   'if': 'defined(TARGET_PPC) || defined(TARGET_ARM) || defined(TARGET_I386) || defined(TARGET_S390X) || defined(TARGET_MIPS)' }
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 0d9a2d2ab7..96f9fe7fff 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -6965,7 +6965,9 @@  static void arm_cpu_add_definition(gpointer data, gpointer user_data)
     *cpu_list = entry;
 }
 
-CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
+CpuDefinitionInfoList *qmp_query_cpu_definitions(bool has_machine,
+                                                 const char *machine,
+                                                 Error **errp)
 {
     CpuDefinitionInfoList *cpu_list = NULL;
     GSList *list;
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 67d1eca4ed..ae633793ed 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -4078,11 +4078,23 @@  static void x86_cpu_definition_entry(gpointer data, gpointer user_data)
     args->cpu_list = entry;
 }
 
-CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
+CpuDefinitionInfoList *qmp_query_cpu_definitions(bool has_machine,
+                                                 const char *machine,
+                                                 Error **errp)
 {
     X86CPUDefinitionArgs args = { .cpu_list = NULL };
     GSList *list;
-    MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine());
+    MachineClass *mc;
+
+    if (!has_machine) {
+        mc = MACHINE_GET_CLASS(qdev_get_machine());
+    } else {
+        mc = machine_find_class(machine);
+        if (!mc) {
+            error_setg(errp, "Machine type '%s' not found", machine);
+            return NULL;
+        }
+    }
 
     args.default_version = default_cpu_version_for_machine(mc);
     list = get_sorted_cpu_model_list();
diff --git a/target/mips/helper.c b/target/mips/helper.c
index a2b6459b05..a73c767462 100644
--- a/target/mips/helper.c
+++ b/target/mips/helper.c
@@ -1481,7 +1481,9 @@  static void mips_cpu_add_definition(gpointer data, gpointer user_data)
     *cpu_list = entry;
 }
 
-CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
+CpuDefinitionInfoList *qmp_query_cpu_definitions(bool has_machine,
+                                                 const char *machine,
+                                                 Error **errp)
 {
     CpuDefinitionInfoList *cpu_list = NULL;
     GSList *list;
diff --git a/target/ppc/translate_init.inc.c b/target/ppc/translate_init.inc.c
index ba726dec4d..4493309c4c 100644
--- a/target/ppc/translate_init.inc.c
+++ b/target/ppc/translate_init.inc.c
@@ -10350,7 +10350,9 @@  static void ppc_cpu_defs_entry(gpointer data, gpointer user_data)
     *first = entry;
 }
 
-CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
+CpuDefinitionInfoList *qmp_query_cpu_definitions(bool has_machine,
+                                                 const char *machine,
+                                                 Error **errp)
 {
     CpuDefinitionInfoList *cpu_list = NULL;
     GSList *list;
diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index 7e92fb2e15..e40f1f6bec 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -456,7 +456,9 @@  static void create_cpu_model_list(ObjectClass *klass, void *opaque)
     *cpu_list = entry;
 }
 
-CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
+CpuDefinitionInfoList *qmp_query_cpu_definitions(bool has_machine,
+                                                 const char *machine,
+                                                 Error **errp)
 {
     struct CpuDefinitionInfoListData list_data = {
         .list = NULL,
diff --git a/tests/acceptance/x86_cpu_model_versions.py b/tests/acceptance/x86_cpu_model_versions.py
index 5fc9ca4bc6..79c5250243 100644
--- a/tests/acceptance/x86_cpu_model_versions.py
+++ b/tests/acceptance/x86_cpu_model_versions.py
@@ -234,6 +234,48 @@  class X86CPUModelAliases(avocado_qemu.Test):
 
         self.validate_aliases(cpus)
 
+    def test_machine_arg_none(self):
+        """Check if unversioned CPU model is an alias pointing to right version"""
+        vm1 = self.get_vm()
+        vm1.add_args('-S')
+        vm1.set_machine('pc-i440fx-4.0')
+        vm1.launch()
+        cpus1 = dict((m['name'], m.get('alias-of')) for m in vm1.command('query-cpu-definitions', machine='none'))
+        vm1.shutdown()
+
+        vm2 = self.get_vm()
+        vm2.add_args('-S')
+        vm2.set_machine('none')
+        vm2.launch()
+        cpus2 = dict((m['name'], m.get('alias-of')) for m in vm2.command('query-cpu-definitions'))
+        vm1.shutdown()
+
+        self.assertEquals(cpus1, cpus2)
+
+    def test_machine_arg_4_1(self):
+        """Check if unversioned CPU model is an alias pointing to right version"""
+        vm1 = self.get_vm()
+        vm1.add_args('-S')
+        vm1.set_machine('pc-i440fx-4.0')
+        vm1.launch()
+        cpus1 = dict((m['name'], m.get('alias-of')) for m in vm1.command('query-cpu-definitions', machine='pc-i440fx-4.1'))
+        vm1.shutdown()
+
+        vm2 = self.get_vm()
+        vm2.add_args('-S')
+        vm2.set_machine('pc-i440fx-4.1')
+        vm2.launch()
+        cpus2 = dict((m['name'], m.get('alias-of')) for m in vm2.command('query-cpu-definitions'))
+        vm1.shutdown()
+
+        self.assertEquals(cpus1, cpus2)
+
+    def test_invalid_machine(self):
+        self.vm.add_args('-S')
+        self.vm.launch()
+        r = self.vm.qmp('query-cpu-definitions', machine='invalid-machine-123')
+        self.assertIn('error', r)
+
 
 class CascadelakeArchCapabilities(avocado_qemu.Test):
     """