[Qemu-devel] [PATCH 0/7] qapi: add commands to remove the need to parse -help output

[Qemu-devel] [PATCH 0/7] qapi: add commands to remove the need to parse -help output

[Anthony Liguori]

This series implements the necessary commands to implements danpb's idea to
remove -help parsing in libvirt.  We would introduce all of these commands in
1.2 and then change the -help output starting in 1.3.

Here is Dan's plan from a previous thread:

<danpb>

 Basically I'd sum up my new idea as "just use QMP".

  * No new command line arguments like -capabilities

  * libvirt invokes something like

      $QEMUBINARY -qmp CHARDEV -nodefault -nodefconfig -nographics

  * libvirt then runs a number of  QMP commands to find out
    what it needs to know. I'd expect the following existing
    commands would be used

      - query-version             - already supported
      - query-commands            - already supported
      - query-events              - already supported
      - query-kvm                 - already supported
      - qom-{list,list-types,get} - already supported
      - query-spice/vnc           - already supported

 And add the following new commands

      - query-devices             - new, -device ?, and/or -device NAME,? data
 in QMP
      - query-machines            - new, -M ? in QMP
      - query-cpu-types           - new, -cpu ? in QMP

 The above would take care of probably 50% of the current libvirt
 capabilities probing, including a portion of the -help stuff. Then
 there is all the rest of the crap we detect from the -help. We could
 just take the view, that "as of 1.2", we assume everything we previously
 detected is just available by default, and thus don't need to probe
 it.  For stuff that is QOM based, I expect we'll be able to detect new
 features in the future using the qom-XXX monitor commands. For stuff
 that is non-qdev, and non-qom, libvirt can just do a plain version
 number check, unless we decide there is specific info worth exposing
 via other new 'query-XXX' monitor commands.
 Basically I'd sum up my new idea as "just use QMP".

  * No new command line arguments like -capabilities

  * libvirt invokes something like

      $QEMUBINARY -qmp CHARDEV -nodefault -nodefconfig -nographics

  * libvirt then runs a number of  QMP commands to find out
    what it needs to know. I'd expect the following existing
    commands would be used

      - query-version             - already supported
      - query-commands            - already supported
      - query-events              - already supported
      - query-kvm                 - already supported
      - qom-{list,list-types,get} - already supported
      - query-spice/vnc           - already supported

    And add the following new commands

      - query-devices             - new, -device ?, and/or -device NAME,? data
 in QMP
      - query-machines            - new, -M ? in QMP
      - query-cpu-types           - new, -cpu ? in QMP

 The above would take care of probably 50% of the current libvirt
 capabilities probing, including a portion of the -help stuff. Then
 there is all the rest of the crap we detect from the -help. We could
 just take the view, that "as of 1.2", we assume everything we previously
 detected is just available by default, and thus don't need to probe
 it.  For stuff that is QOM based, I expect we'll be able to detect new
 features in the future using the qom-XXX monitor commands. For stuff
 that is non-qdev, and non-qom, libvirt can just do a plain version
 number check, unless we decide there is specific info worth exposing
 via other new 'query-XXX' monitor commands.

</danpb>

The one thing to note is that I didn't add a query-devices command because you
can already do:

qmp query-devices --implements=device --abstract=False

To get the equivalent output of -device ?.  Instead, I added a command to list
specific properties of a device which is the equivalent of -device FOO,?

[Qemu-devel] [PATCH 1/7] qmp: introduce device-list-properties command

[Anthony Liguori]

This can be used in conjunction with qom-list-types to determine the supported
set of devices and their parameters.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 qapi-schema.json |   28 ++++++++++++++++++++++++++++
 qmp-commands.hx  |    7 +++++++
 qmp.c            |   50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 85 insertions(+), 0 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index bc55ed2..015a84a 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1727,6 +1727,34 @@
   'returns': [ 'ObjectTypeInfo' ] }
 
 ##
+# @DevicePropertyInfo:
+#
+# Information about device properties.
+#
+# @name: the name of the property
+# @type: the typename of the property
+#
+# Since: 1.2
+##
+{ 'type': 'DevicePropertyInfo',
+  'data': { 'name': 'str', 'type': 'str' } }
+
+##
+# @device-list-properties:
+#
+# List properties associated with a device.
+#
+# @typename: the type name of a device
+#
+# Returns: a list of DevicePropertyInfo describing a devices properties
+#
+# Since: 1.2
+##
+{ 'command': 'device-list-properties',
+  'data': { 'typename': 'str'},
+  'returns': [ 'DevicePropertyInfo' ] }
+
+##
 # @migrate
 #
 # Migrates the current running guest to another Virtual Machine.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index e3cf3c5..5c55528 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2215,3 +2215,10 @@ EQMP
         .args_type  = "implements:s?,abstract:b?",
         .mhandler.cmd_new = qmp_marshal_input_qom_list_types,
     },
+
+    {
+        .name       = "device-list-properties",
+        .args_type  = "typename:s",
+        .mhandler.cmd_new = qmp_marshal_input_device_list_properties,
+    },
+
diff --git a/qmp.c b/qmp.c
index fee9fb2..254a32f 100644
--- a/qmp.c
+++ b/qmp.c
@@ -417,3 +417,53 @@ ObjectTypeInfoList *qmp_qom_list_types(bool has_implements,
 
     return ret;
 }
+
+DevicePropertyInfoList *qmp_device_list_properties(const char *typename,
+                                                   Error **errp)
+{
+    ObjectClass *klass;
+    Property *prop;
+    DevicePropertyInfoList *prop_list = NULL;
+
+    klass = object_class_by_name(typename);
+    if (klass == NULL) {
+        error_set(errp, QERR_DEVICE_NOT_FOUND, typename);
+        return NULL;
+    }
+
+    klass = object_class_dynamic_cast(klass, TYPE_DEVICE);
+    if (klass == NULL) {
+        error_set(errp, QERR_INVALID_PARAMETER_VALUE,
+                  "name", TYPE_DEVICE);
+        return NULL;
+    }
+
+    do {
+        for (prop = DEVICE_CLASS(klass)->props; prop && prop->name; prop++) {
+            DevicePropertyInfoList *entry;
+            DevicePropertyInfo *info;
+
+            /*
+             * TODO Properties without a parser are just for dirty hacks.
+             * qdev_prop_ptr is the only such PropertyInfo.  It's marked
+             * for removal.  This conditional should be removed along with
+             * it.
+             */
+            if (!prop->info->set) {
+                continue;           /* no way to set it, don't show */
+            }
+
+            info = g_malloc0(sizeof(*info));
+            info->name = g_strdup(prop->name);
+            info->type = g_strdup(prop->info->legacy_name ?: prop->info->name);
+
+            entry = g_malloc0(sizeof(*entry));
+            entry->value = info;
+            entry->next = prop_list;
+            prop_list = entry;
+        }
+        klass = object_class_get_parent(klass);
+    } while (klass != object_class_by_name(TYPE_DEVICE));
+
+    return prop_list;
+}
-- 
1.7.5.4

[Qemu-devel] [PATCH 2/7] qapi: mark QOM commands stable

[Anthony Liguori]

We've had a cycle to tweak.  It is time to commit to supporting them.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 qapi-schema.json |   19 ++++---------------
 1 files changed, 4 insertions(+), 15 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 015a84a..28e9914 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1360,9 +1360,7 @@
 #        4) A link type in the form 'link<subtype>' where subtype is a qdev
 #           device type name.  Link properties form the device model graph.
 #
-# Since: 1.1
-#
-# Notes: This type is experimental.  Its syntax may change in future releases.
+# Since: 1.2
 ##
 { 'type': 'ObjectPropertyInfo',
   'data': { 'name': 'str', 'type': 'str' } }
@@ -1379,10 +1377,7 @@
 # Returns: a list of @ObjectPropertyInfo that describe the properties of the
 #          object.
 #
-# Since: 1.1
-#
-# Notes: This command is experimental.  It's syntax may change in future
-#        releases.
+# Since: 1.2
 ##
 { 'command': 'qom-list',
   'data': { 'path': 'str' },
@@ -1418,9 +1413,7 @@
 #          returns as #str pathnames.  All integer property types (u8, u16, etc)
 #          are returned as #int.
 #
-# Since: 1.1
-#
-# Notes: This command is experimental and may change syntax in future releases.
+# Since: 1.2
 ##
 { 'command': 'qom-get',
   'data': { 'path': 'str', 'property': 'str' },
@@ -1439,9 +1432,7 @@
 # @value: a value who's type is appropriate for the property type.  See @qom-get
 #         for a description of type mapping.
 #
-# Since: 1.1
-#
-# Notes: This command is experimental and may change syntax in future releases.
+# Since: 1.2
 ##
 { 'command': 'qom-set',
   'data': { 'path': 'str', 'property': 'str', 'value': 'visitor' },
@@ -1719,8 +1710,6 @@
 # Returns: a list of @ObjectTypeInfo or an empty list if no results are found
 #
 # Since: 1.1
-#
-# Notes: This command is experimental and may change syntax in future releases.
 ##
 { 'command': 'qom-list-types',
   'data': { '*implements': 'str', '*abstract': 'bool' },
-- 
1.7.5.4

[Qemu-devel] [PATCH 3/7] qapi: add query-machines command

[Anthony Liguori]

This provides the same output as -M ? but in a structured way.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 qapi-schema.json |   28 ++++++++++++++++++++++++++++
 qmp-commands.hx  |    6 ++++++
 vl.c             |   31 +++++++++++++++++++++++++++++++
 3 files changed, 65 insertions(+), 0 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 28e9914..5b47026 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2200,3 +2200,31 @@
 # Since: 0.14.0
 ##
 { 'command': 'closefd', 'data': {'fdname': 'str'} }
+
+##
+# @MachineInfo:
+#
+# Information describing a machine.
+#
+# @name: the name of the machine
+#
+# @alias: #optional an alias for the machine name
+#
+# @default: #optional whether the machine is default
+#
+# Since: 1.2.0
+##
+{ 'type': 'MachineInfo',
+  'data': { 'name': 'str', '*alias': 'str',
+            '*is-default': 'bool' } }
+
+##
+# @query-machines:
+#
+# Return a list of supported machines
+#
+# Returns: a list of MachineInfo
+#
+# Since: 1.2.0
+##
+{ 'command': 'query-machines', 'returns': ['MachineInfo'] }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 5c55528..a6f82fc 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2222,3 +2222,9 @@ EQMP
         .mhandler.cmd_new = qmp_marshal_input_device_list_properties,
     },
 
+    {
+        .name       = "query-machines",
+        .args_type  = "",
+        .mhandler.cmd_new = qmp_marshal_input_query_machines,
+    },
+
diff --git a/vl.c b/vl.c
index 8904db1..cd900e0 100644
--- a/vl.c
+++ b/vl.c
@@ -1209,6 +1209,37 @@ QEMUMachine *find_default_machine(void)
     return NULL;
 }
 
+MachineInfoList *qmp_query_machines(Error **errp)
+{
+    MachineInfoList *mach_list = NULL;
+    QEMUMachine *m;
+
+    for (m = first_machine; m; m = m->next) {
+        MachineInfoList *entry;
+        MachineInfo *info;
+
+        info = g_malloc0(sizeof(*info));
+        if (m->is_default) {
+            info->has_is_default = true;
+            info->is_default = true;
+        }
+
+        if (m->alias) {
+            info->has_alias = true;
+            info->alias = g_strdup(m->alias);
+        }
+
+        info->name = g_strdup(m->name);
+
+        entry = g_malloc0(sizeof(*entry));
+        entry->value = info;
+        entry->next = mach_list;
+        mach_list = entry;
+    }
+
+    return mach_list;
+}
+
 /***********************************************************/
 /* main execution loop */
 
-- 
1.7.5.4

[Qemu-devel] [PATCH 4/7] compiler: add macro for GCC weak symbols

[Anthony Liguori]

This lets us provide a default implementation of a symbol which targets can
override.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 compiler.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/compiler.h b/compiler.h
index 736e770..f76921e 100644
--- a/compiler.h
+++ b/compiler.h
@@ -45,6 +45,7 @@
 #  define GCC_ATTR __attribute__((__unused__, format(gnu_printf, 1, 2)))
 #  define GCC_FMT_ATTR(n, m) __attribute__((format(gnu_printf, n, m)))
 # endif
+#define GCC_WEAK __attribute__((weak))
 #else
 #define GCC_ATTR /**/
 #define GCC_FMT_ATTR(n, m)
-- 
1.7.5.4

[Qemu-devel] [PATCH 5/7] qapi: add query-cpudefs command

[Anthony Liguori]

This command attempts to map to the behavior of -cpu ?.  Unfortunately, the
output of this command differs wildly across targets.

To accomodate this, we use a weak symbol to implement a default version of the
command that fails with a QERR_NOT_SUPPORTED error code.  Targets can then
override and implement this command if it makes sense for them.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 qapi-schema.json |   23 +++++++++++++++++++++++
 qmp-commands.hx  |    6 ++++++
 qmp.c            |    6 ++++++
 3 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 5b47026..768fb44 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2228,3 +2228,26 @@
 # Since: 1.2.0
 ##
 { 'command': 'query-machines', 'returns': ['MachineInfo'] }
+
+##
+# @CpuDefInfo:
+#
+# Virtual CPU definition.
+#
+# @name: the name of the CPU definition
+#
+# Since: 1.2.0
+##
+{ 'type': 'CpuDefInfo',
+  'data': { 'name': 'str' } }
+
+##
+# @query-cpudefs:
+#
+# Return a list of supported virtual CPU definitions
+#
+# Returns: a list of CpuDefInfo
+#
+# Since: 1.2.0
+##
+{ 'command': 'query-cpudefs', 'returns': ['CpuDefInfo'] }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index a6f82fc..73dfeab 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2228,3 +2228,9 @@ EQMP
         .mhandler.cmd_new = qmp_marshal_input_query_machines,
     },
 
+    {
+        .name       = "query-cpudefs",
+        .args_type  = "",
+        .mhandler.cmd_new = qmp_marshal_input_query_cpudefs,
+    },
+
diff --git a/qmp.c b/qmp.c
index 254a32f..51b4f75 100644
--- a/qmp.c
+++ b/qmp.c
@@ -467,3 +467,9 @@ DevicePropertyInfoList *qmp_device_list_properties(const char *typename,
 
     return prop_list;
 }
+
+CpuDefInfoList GCC_WEAK *qmp_query_cpudefs(Error **errp)
+{
+    error_set(errp, QERR_NOT_SUPPORTED);
+    return NULL;
+}
-- 
1.7.5.4

[Qemu-devel] [PATCH 6/7] target-i386: add implementation of query-cpudefs

[Anthony Liguori]

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 target-i386/cpu.c |   22 ++++++++++++++++++++++
 1 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 6b9659f..b398439 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -28,6 +28,7 @@
 #include "qemu-config.h"
 
 #include "qapi/qapi-visit-core.h"
+#include "qmp-commands.h"
 
 #include "hyperv.h"
 
@@ -1123,6 +1124,27 @@ void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf, const char *optarg)
     }
 }
 
+CpuDefInfoList *qmp_query_cpudefs(Error **errp)
+{
+    CpuDefInfoList *cpu_list = NULL;
+    x86_def_t *def;
+
+    for (def = x86_defs; def; def = def->next) {
+        CpuDefInfoList *entry;
+        CpuDefInfo *info;
+
+        info = g_malloc0(sizeof(*info));
+        info->name = g_strdup(def->name);
+
+        entry = g_malloc0(sizeof(*entry));
+        entry->value = info;
+        entry->next = cpu_list;
+        cpu_list = entry;
+    }
+
+    return cpu_list;
+}
+
 int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
 {
     CPUX86State *env = &cpu->env;
-- 
1.7.5.4

[Qemu-devel] [PATCH 7/7] target-ppc: add implementation of query-cpudefs

[Anthony Liguori]

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 target-ppc/translate_init.c |   25 +++++++++++++++++++++++++
 1 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 5742229..7e025cb 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -10345,6 +10345,31 @@ void ppc_cpu_list (FILE *f, fprintf_function cpu_fprintf)
     }
 }
 
+CpuDefInfoList *qmp_query_cpudefs(Error **errp)
+{
+    CpuDefInfoList *cpu_list = NULL;
+    int i;
+
+    for (i = 0; i < ARRAY_SIZE(ppc_defs); i++) {
+        CpuDefInfoList *entry;
+        CpuDefInfo *info;
+
+        if (!ppc_cpu_usable(&ppc_defs[i])) {
+            continue;
+        }
+
+        info = g_malloc0(sizeof(*info));
+        info->name = g_strdup(pcc_defs[i].name);
+
+        entry = g_malloc0(sizeof(*entry));
+        entry->value = info;
+        entry->next = cpu_list;
+        cpu_list = entry;
+    }
+
+    return cpu_list;
+}
+
 /* CPUClass::reset() */
 static void ppc_cpu_reset(CPUState *s)
 {
-- 
1.7.5.4

Re: [Qemu-devel] [PATCH 4/7] compiler: add macro for GCC weak symbols

[Peter Maydell]

On 27 July 2012 14:37, Anthony Liguori <aliguori@us.ibm.com> wrote:
> --- a/compiler.h
> +++ b/compiler.h
> @@ -45,6 +45,7 @@
>  #  define GCC_ATTR __attribute__((__unused__, format(gnu_printf, 1, 2)))
>  #  define GCC_FMT_ATTR(n, m) __attribute__((format(gnu_printf, n, m)))
>  # endif
> +#define GCC_WEAK __attribute__((weak))
>  #else
>  #define GCC_ATTR /**/
>  #define GCC_FMT_ATTR(n, m)

The GCC manual says "Weak symbols are supported for ELF targets,
and also for a.out targets when using the GNU assembler and linker".
Have you tested this on Windows and MacOSX ?

(Also, no version of the macro in the not-GCC branch of the #if.)

-- PMM

Re: [Qemu-devel] [PATCH 5/7] qapi: add query-cpudefs command

[Peter Maydell]

On 27 July 2012 14:37, Anthony Liguori <aliguori@us.ibm.com> wrote:
> This command attempts to map to the behavior of -cpu ?.  Unfortunately, the
> output of this command differs wildly across targets.

I've never really understood why so much of the cpu selection
logic is deferred to target-*...

> To accomodate this, we use a weak symbol to implement a default version of the
> command that fails with a QERR_NOT_SUPPORTED error code.  Targets can then
> override and implement this command if it makes sense for them.

This is a bit of a weak reason (boom boom!) for requiring a platform
specific thing like weak symbols, though, and it's not how we handle
similar existing cases (eg see the configure/makefile logic for
memory_mapping.c vs memory_mapping-stub.c).

If having separate configure/make stuff for each of these things
sounds a bit heavyweight, we could just have a target-stubs.c which
#includes cpu.h and has a lot of
#ifndef TARGET_QUERY_CPUDEFS
[stub version]
#endif
#ifndef TARGET_GET_MEMORY_MAPPING
[stub version]
#endif

etc.

-- PMM

Re: [Qemu-devel] [PATCH 4/7] compiler: add macro for GCC weak symbols

[Anthony Liguori]

Peter Maydell <peter.maydell@linaro.org> writes:

> On 27 July 2012 14:37, Anthony Liguori <aliguori@us.ibm.com> wrote:
>> --- a/compiler.h
>> +++ b/compiler.h
>> @@ -45,6 +45,7 @@
>>  #  define GCC_ATTR __attribute__((__unused__, format(gnu_printf, 1, 2)))
>>  #  define GCC_FMT_ATTR(n, m) __attribute__((format(gnu_printf, n, m)))
>>  # endif
>> +#define GCC_WEAK __attribute__((weak))
>>  #else
>>  #define GCC_ATTR /**/
>>  #define GCC_FMT_ATTR(n, m)
>
> The GCC manual says "Weak symbols are supported for ELF targets,
> and also for a.out targets when using the GNU assembler and linker".
> Have you tested this on Windows and MacOSX ?

Weak symbols are supposed to be supported by mingw32.

I have no idea about MacOS X.

I have no way to develop or test for MacOS X using free software so I
honestly don't care about it.

Regards,

Anthony Liguori

>
> (Also, no version of the macro in the not-GCC branch of the #if.)
>
> -- PMM

Re: [Qemu-devel] [PATCH 4/7] compiler: add macro for GCC weak symbols

[Peter Maydell]

On 27 July 2012 15:27, Anthony Liguori <aliguori@us.ibm.com> wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
>> The GCC manual says "Weak symbols are supported for ELF targets,
>> and also for a.out targets when using the GNU assembler and linker".
>> Have you tested this on Windows and MacOSX ?
>
> Weak symbols are supposed to be supported by mingw32.
>
> I have no idea about MacOS X.
>
> I have no way to develop or test for MacOS X using free software so I
> honestly don't care about it.

My approach to this is to avoid non-standard things -- if I
write a patch which is pretty much standard C then it's the
platform's problem if it mishandles it. If I write a patch
that uses a compiler-specific or OS-specific thing then I
have to also provide the relevant alternatives...so I try
to avoid doing that :-)

-- PMM

Re: [Qemu-devel] [PATCH 5/7] qapi: add query-cpudefs command

[Anthony Liguori]

Peter Maydell <peter.maydell@linaro.org> writes:

> On 27 July 2012 14:37, Anthony Liguori <aliguori@us.ibm.com> wrote:
>> This command attempts to map to the behavior of -cpu ?.  Unfortunately, the
>> output of this command differs wildly across targets.
>
> I've never really understood why so much of the cpu selection
> logic is deferred to target-*...

It will be fixed as part of the QOM conversion.

>> To accomodate this, we use a weak symbol to implement a default version of the
>> command that fails with a QERR_NOT_SUPPORTED error code.  Targets can then
>> override and implement this command if it makes sense for them.
>
> This is a bit of a weak reason (boom boom!) for requiring a platform
> specific thing like weak symbols, though, and it's not how we handle
> similar existing cases (eg see the configure/makefile logic for
> memory_mapping.c vs memory_mapping-stub.c).

I don't think we have a consistent approach today FWIW.  I think using
weak symbols is sufficiently compelling that it will become consistent.

>
> If having separate configure/make stuff for each of these things
> sounds a bit heavyweight, we could just have a target-stubs.c which
> #includes cpu.h and has a lot of
> #ifndef TARGET_QUERY_CPUDEFS
> [stub version]
> #endif
> #ifndef TARGET_GET_MEMORY_MAPPING
> [stub version]
> #endif

This is pretty hideous.

FWIW, weak symbols are supported on OS X as of 10.2.

Regards,

Anthony Liguori

>
> etc.
>
> -- PMM

Re: [Qemu-devel] [PATCH 4/7] compiler: add macro for GCC weak symbols

[Anthony Liguori]

Peter Maydell <peter.maydell@linaro.org> writes:

> On 27 July 2012 15:27, Anthony Liguori <aliguori@us.ibm.com> wrote:
>> Peter Maydell <peter.maydell@linaro.org> writes:
>>> The GCC manual says "Weak symbols are supported for ELF targets,
>>> and also for a.out targets when using the GNU assembler and linker".
>>> Have you tested this on Windows and MacOSX ?
>>
>> Weak symbols are supposed to be supported by mingw32.
>>
>> I have no idea about MacOS X.
>>
>> I have no way to develop or test for MacOS X using free software so I
>> honestly don't care about it.
>
> My approach to this is to avoid non-standard things

http://en.wikipedia.org/wiki/C99#Implementations

So unless you plan on compiling QEMU with xlc, pgi, or icc, I don't
think relying on "standard things" really helps.

QEMU doesn't support C99, it supports GCC.  There's no point in
debating about whether we should rely on extensions or not.  We already
do--extensively.

Regards,

Anthony Liguori


> -- if I
> write a patch which is pretty much standard C then it's the
> platform's problem if it mishandles it. If I write a patch
> that uses a compiler-specific or OS-specific thing then I
> have to also provide the relevant alternatives...so I try
> to avoid doing that :-)
>
> -- PMM

Re: [Qemu-devel] [PATCH 4/7] compiler: add macro for GCC weak symbols

[Anthony Liguori]

Peter Maydell <peter.maydell@linaro.org> writes:

> On 27 July 2012 14:37, Anthony Liguori <aliguori@us.ibm.com> wrote:
>> --- a/compiler.h
>> +++ b/compiler.h
>> @@ -45,6 +45,7 @@
>>  #  define GCC_ATTR __attribute__((__unused__, format(gnu_printf, 1, 2)))
>>  #  define GCC_FMT_ATTR(n, m) __attribute__((format(gnu_printf, n, m)))
>>  # endif
>> +#define GCC_WEAK __attribute__((weak))
>>  #else
>>  #define GCC_ATTR /**/
>>  #define GCC_FMT_ATTR(n, m)
>
> The GCC manual says "Weak symbols are supported for ELF targets,
> and also for a.out targets when using the GNU assembler and linker".
> Have you tested this on Windows and MacOSX ?
>
> (Also, no version of the macro in the not-GCC branch of the #if.)

(We don't support any compiler other than GCC).

Not really sure why it is even in a branch.

Regards,

Anthony Liguori

>
> -- PMM

Re: [Qemu-devel] [PATCH 1/7] qmp: introduce device-list-properties command

[Luiz Capitulino]

On Fri, 27 Jul 2012 08:37:13 -0500
Anthony Liguori <aliguori@us.ibm.com> wrote:

> This can be used in conjunction with qom-list-types to determine the supported
> set of devices and their parameters.
> 
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> ---
>  qapi-schema.json |   28 ++++++++++++++++++++++++++++
>  qmp-commands.hx  |    7 +++++++
>  qmp.c            |   50 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 85 insertions(+), 0 deletions(-)
> 
> diff --git a/qapi-schema.json b/qapi-schema.json
> index bc55ed2..015a84a 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1727,6 +1727,34 @@
>    'returns': [ 'ObjectTypeInfo' ] }
>  
>  ##
> +# @DevicePropertyInfo:
> +#
> +# Information about device properties.
> +#
> +# @name: the name of the property
> +# @type: the typename of the property
> +#
> +# Since: 1.2
> +##
> +{ 'type': 'DevicePropertyInfo',
> +  'data': { 'name': 'str', 'type': 'str' } }
> +
> +##
> +# @device-list-properties:
> +#
> +# List properties associated with a device.
> +#
> +# @typename: the type name of a device
> +#
> +# Returns: a list of DevicePropertyInfo describing a devices properties
> +#
> +# Since: 1.2
> +##
> +{ 'command': 'device-list-properties',
> +  'data': { 'typename': 'str'},
> +  'returns': [ 'DevicePropertyInfo' ] }

I find it a bit weird that the argument is called typename but everything else
refers to that same argument as a device.

Maybe we should rename this to device-type-list-properties, DeviceTypePropertyInfo
and typename to name.

But that's minor, I won't oppose to it as it's.

> +
> +##
>  # @migrate
>  #
>  # Migrates the current running guest to another Virtual Machine.
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index e3cf3c5..5c55528 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -2215,3 +2215,10 @@ EQMP
>          .args_type  = "implements:s?,abstract:b?",
>          .mhandler.cmd_new = qmp_marshal_input_qom_list_types,
>      },
> +
> +    {
> +        .name       = "device-list-properties",
> +        .args_type  = "typename:s",
> +        .mhandler.cmd_new = qmp_marshal_input_device_list_properties,
> +    },
> +
> diff --git a/qmp.c b/qmp.c
> index fee9fb2..254a32f 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -417,3 +417,53 @@ ObjectTypeInfoList *qmp_qom_list_types(bool has_implements,
>  
>      return ret;
>  }
> +
> +DevicePropertyInfoList *qmp_device_list_properties(const char *typename,
> +                                                   Error **errp)
> +{
> +    ObjectClass *klass;
> +    Property *prop;
> +    DevicePropertyInfoList *prop_list = NULL;
> +
> +    klass = object_class_by_name(typename);
> +    if (klass == NULL) {
> +        error_set(errp, QERR_DEVICE_NOT_FOUND, typename);
> +        return NULL;
> +    }
> +
> +    klass = object_class_dynamic_cast(klass, TYPE_DEVICE);
> +    if (klass == NULL) {
> +        error_set(errp, QERR_INVALID_PARAMETER_VALUE,
> +                  "name", TYPE_DEVICE);
> +        return NULL;
> +    }
> +
> +    do {
> +        for (prop = DEVICE_CLASS(klass)->props; prop && prop->name; prop++) {
> +            DevicePropertyInfoList *entry;
> +            DevicePropertyInfo *info;
> +
> +            /*
> +             * TODO Properties without a parser are just for dirty hacks.
> +             * qdev_prop_ptr is the only such PropertyInfo.  It's marked
> +             * for removal.  This conditional should be removed along with
> +             * it.
> +             */
> +            if (!prop->info->set) {
> +                continue;           /* no way to set it, don't show */
> +            }
> +
> +            info = g_malloc0(sizeof(*info));
> +            info->name = g_strdup(prop->name);
> +            info->type = g_strdup(prop->info->legacy_name ?: prop->info->name);
> +
> +            entry = g_malloc0(sizeof(*entry));
> +            entry->value = info;
> +            entry->next = prop_list;
> +            prop_list = entry;
> +        }
> +        klass = object_class_get_parent(klass);
> +    } while (klass != object_class_by_name(TYPE_DEVICE));
> +
> +    return prop_list;
> +}

Re: [Qemu-devel] [PATCH 2/7] qapi: mark QOM commands stable

[Luiz Capitulino]

On Fri, 27 Jul 2012 08:37:14 -0500
Anthony Liguori <aliguori@us.ibm.com> wrote:

> We've had a cycle to tweak.  It is time to commit to supporting them.

qmp_qom_get() and qpm_qom_set() still use the legacy monitor interface, can't
we convert it to the qapi?

> 
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> ---
>  qapi-schema.json |   19 ++++---------------
>  1 files changed, 4 insertions(+), 15 deletions(-)
> 
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 015a84a..28e9914 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1360,9 +1360,7 @@
>  #        4) A link type in the form 'link<subtype>' where subtype is a qdev
>  #           device type name.  Link properties form the device model graph.
>  #
> -# Since: 1.1
> -#
> -# Notes: This type is experimental.  Its syntax may change in future releases.
> +# Since: 1.2
>  ##
>  { 'type': 'ObjectPropertyInfo',
>    'data': { 'name': 'str', 'type': 'str' } }
> @@ -1379,10 +1377,7 @@
>  # Returns: a list of @ObjectPropertyInfo that describe the properties of the
>  #          object.
>  #
> -# Since: 1.1
> -#
> -# Notes: This command is experimental.  It's syntax may change in future
> -#        releases.
> +# Since: 1.2
>  ##
>  { 'command': 'qom-list',
>    'data': { 'path': 'str' },
> @@ -1418,9 +1413,7 @@
>  #          returns as #str pathnames.  All integer property types (u8, u16, etc)
>  #          are returned as #int.
>  #
> -# Since: 1.1
> -#
> -# Notes: This command is experimental and may change syntax in future releases.
> +# Since: 1.2
>  ##
>  { 'command': 'qom-get',
>    'data': { 'path': 'str', 'property': 'str' },
> @@ -1439,9 +1432,7 @@
>  # @value: a value who's type is appropriate for the property type.  See @qom-get
>  #         for a description of type mapping.
>  #
> -# Since: 1.1
> -#
> -# Notes: This command is experimental and may change syntax in future releases.
> +# Since: 1.2
>  ##
>  { 'command': 'qom-set',
>    'data': { 'path': 'str', 'property': 'str', 'value': 'visitor' },
> @@ -1719,8 +1710,6 @@
>  # Returns: a list of @ObjectTypeInfo or an empty list if no results are found
>  #
>  # Since: 1.1
> -#
> -# Notes: This command is experimental and may change syntax in future releases.
>  ##
>  { 'command': 'qom-list-types',
>    'data': { '*implements': 'str', '*abstract': 'bool' },

Re: [Qemu-devel] [PATCH 3/7] qapi: add query-machines command

[Luiz Capitulino]

On Fri, 27 Jul 2012 08:37:15 -0500
Anthony Liguori <aliguori@us.ibm.com> wrote:

> This provides the same output as -M ? but in a structured way.
> 
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> ---
>  qapi-schema.json |   28 ++++++++++++++++++++++++++++
>  qmp-commands.hx  |    6 ++++++
>  vl.c             |   31 +++++++++++++++++++++++++++++++
>  3 files changed, 65 insertions(+), 0 deletions(-)
> 
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 28e9914..5b47026 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2200,3 +2200,31 @@
>  # Since: 0.14.0
>  ##
>  { 'command': 'closefd', 'data': {'fdname': 'str'} }
> +
> +##
> +# @MachineInfo:
> +#
> +# Information describing a machine.
> +#
> +# @name: the name of the machine
> +#
> +# @alias: #optional an alias for the machine name
> +#
> +# @default: #optional whether the machine is default

Why is default optional?

> +#
> +# Since: 1.2.0
> +##
> +{ 'type': 'MachineInfo',
> +  'data': { 'name': 'str', '*alias': 'str',
> +            '*is-default': 'bool' } }
> +
> +##
> +# @query-machines:
> +#
> +# Return a list of supported machines
> +#
> +# Returns: a list of MachineInfo
> +#
> +# Since: 1.2.0
> +##
> +{ 'command': 'query-machines', 'returns': ['MachineInfo'] }
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 5c55528..a6f82fc 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -2222,3 +2222,9 @@ EQMP
>          .mhandler.cmd_new = qmp_marshal_input_device_list_properties,
>      },
>  
> +    {
> +        .name       = "query-machines",
> +        .args_type  = "",
> +        .mhandler.cmd_new = qmp_marshal_input_query_machines,
> +    },
> +
> diff --git a/vl.c b/vl.c
> index 8904db1..cd900e0 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1209,6 +1209,37 @@ QEMUMachine *find_default_machine(void)
>      return NULL;
>  }
>  
> +MachineInfoList *qmp_query_machines(Error **errp)
> +{
> +    MachineInfoList *mach_list = NULL;
> +    QEMUMachine *m;
> +
> +    for (m = first_machine; m; m = m->next) {
> +        MachineInfoList *entry;
> +        MachineInfo *info;
> +
> +        info = g_malloc0(sizeof(*info));
> +        if (m->is_default) {
> +            info->has_is_default = true;
> +            info->is_default = true;
> +        }
> +
> +        if (m->alias) {
> +            info->has_alias = true;
> +            info->alias = g_strdup(m->alias);
> +        }
> +
> +        info->name = g_strdup(m->name);
> +
> +        entry = g_malloc0(sizeof(*entry));
> +        entry->value = info;
> +        entry->next = mach_list;
> +        mach_list = entry;
> +    }
> +
> +    return mach_list;
> +}
> +
>  /***********************************************************/
>  /* main execution loop */
>  

Re: [Qemu-devel] [PATCH 5/7] qapi: add query-cpudefs command

[Luiz Capitulino]

On Fri, 27 Jul 2012 08:37:17 -0500
Anthony Liguori <aliguori@us.ibm.com> wrote:

> This command attempts to map to the behavior of -cpu ?.  Unfortunately, the
> output of this command differs wildly across targets.
> 
> To accomodate this, we use a weak symbol to implement a default version of the
> command that fails with a QERR_NOT_SUPPORTED error code.  Targets can then
> override and implement this command if it makes sense for them.
> 
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> ---
>  qapi-schema.json |   23 +++++++++++++++++++++++
>  qmp-commands.hx  |    6 ++++++
>  qmp.c            |    6 ++++++
>  3 files changed, 35 insertions(+), 0 deletions(-)
> 
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 5b47026..768fb44 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2228,3 +2228,26 @@
>  # Since: 1.2.0
>  ##
>  { 'command': 'query-machines', 'returns': ['MachineInfo'] }
> +
> +##
> +# @CpuDefInfo:
> +#
> +# Virtual CPU definition.
> +#
> +# @name: the name of the CPU definition
> +#
> +# Since: 1.2.0
> +##
> +{ 'type': 'CpuDefInfo',
> +  'data': { 'name': 'str' } }
> +
> +##
> +# @query-cpudefs:

I'd call this query-cpu-defs or even query-cpu-difinitions. The latter makes
it self-documenting.

> +#
> +# Return a list of supported virtual CPU definitions
> +#
> +# Returns: a list of CpuDefInfo
> +#
> +# Since: 1.2.0
> +##
> +{ 'command': 'query-cpudefs', 'returns': ['CpuDefInfo'] }
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index a6f82fc..73dfeab 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -2228,3 +2228,9 @@ EQMP
>          .mhandler.cmd_new = qmp_marshal_input_query_machines,
>      },
>  
> +    {
> +        .name       = "query-cpudefs",
> +        .args_type  = "",
> +        .mhandler.cmd_new = qmp_marshal_input_query_cpudefs,
> +    },
> +
> diff --git a/qmp.c b/qmp.c
> index 254a32f..51b4f75 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -467,3 +467,9 @@ DevicePropertyInfoList *qmp_device_list_properties(const char *typename,
>  
>      return prop_list;
>  }
> +
> +CpuDefInfoList GCC_WEAK *qmp_query_cpudefs(Error **errp)
> +{
> +    error_set(errp, QERR_NOT_SUPPORTED);
> +    return NULL;
> +}

Re: [Qemu-devel] [PATCH 0/7] qapi: add commands to remove the need to parse -help output

[Luiz Capitulino]

On Fri, 27 Jul 2012 08:37:12 -0500
Anthony Liguori <aliguori@us.ibm.com> wrote:

> This series implements the necessary commands to implements danpb's idea to
> remove -help parsing in libvirt.  We would introduce all of these commands in
> 1.2 and then change the -help output starting in 1.3.

I've reviewed this and apart from small details, it looks good to me.

Would be nice to get an ack from libvirt folks before applying.

> 
> Here is Dan's plan from a previous thread:
> 
> <danpb>
> 
>  Basically I'd sum up my new idea as "just use QMP".
> 
>   * No new command line arguments like -capabilities
> 
>   * libvirt invokes something like
> 
>       $QEMUBINARY -qmp CHARDEV -nodefault -nodefconfig -nographics
> 
>   * libvirt then runs a number of  QMP commands to find out
>     what it needs to know. I'd expect the following existing
>     commands would be used
> 
>       - query-version             - already supported
>       - query-commands            - already supported
>       - query-events              - already supported
>       - query-kvm                 - already supported
>       - qom-{list,list-types,get} - already supported
>       - query-spice/vnc           - already supported
> 
>  And add the following new commands
> 
>       - query-devices             - new, -device ?, and/or -device NAME,? data
>  in QMP
>       - query-machines            - new, -M ? in QMP
>       - query-cpu-types           - new, -cpu ? in QMP
> 
>  The above would take care of probably 50% of the current libvirt
>  capabilities probing, including a portion of the -help stuff. Then
>  there is all the rest of the crap we detect from the -help. We could
>  just take the view, that "as of 1.2", we assume everything we previously
>  detected is just available by default, and thus don't need to probe
>  it.  For stuff that is QOM based, I expect we'll be able to detect new
>  features in the future using the qom-XXX monitor commands. For stuff
>  that is non-qdev, and non-qom, libvirt can just do a plain version
>  number check, unless we decide there is specific info worth exposing
>  via other new 'query-XXX' monitor commands.
>  Basically I'd sum up my new idea as "just use QMP".
> 
>   * No new command line arguments like -capabilities
> 
>   * libvirt invokes something like
> 
>       $QEMUBINARY -qmp CHARDEV -nodefault -nodefconfig -nographics
> 
>   * libvirt then runs a number of  QMP commands to find out
>     what it needs to know. I'd expect the following existing
>     commands would be used
> 
>       - query-version             - already supported
>       - query-commands            - already supported
>       - query-events              - already supported
>       - query-kvm                 - already supported
>       - qom-{list,list-types,get} - already supported
>       - query-spice/vnc           - already supported
> 
>     And add the following new commands
> 
>       - query-devices             - new, -device ?, and/or -device NAME,? data
>  in QMP
>       - query-machines            - new, -M ? in QMP
>       - query-cpu-types           - new, -cpu ? in QMP
> 
>  The above would take care of probably 50% of the current libvirt
>  capabilities probing, including a portion of the -help stuff. Then
>  there is all the rest of the crap we detect from the -help. We could
>  just take the view, that "as of 1.2", we assume everything we previously
>  detected is just available by default, and thus don't need to probe
>  it.  For stuff that is QOM based, I expect we'll be able to detect new
>  features in the future using the qom-XXX monitor commands. For stuff
>  that is non-qdev, and non-qom, libvirt can just do a plain version
>  number check, unless we decide there is specific info worth exposing
>  via other new 'query-XXX' monitor commands.
> 
> </danpb>
> 
> The one thing to note is that I didn't add a query-devices command because you
> can already do:
> 
> qmp query-devices --implements=device --abstract=False
> 
> To get the equivalent output of -device ?.  Instead, I added a command to list
> specific properties of a device which is the equivalent of -device FOO,?
> 
> 

Re: [Qemu-devel] [PATCH 0/7] qapi: add commands to remove the need to parse -help output

[Daniel P. Berrange]

On Fri, Jul 27, 2012 at 01:21:01PM -0300, Luiz Capitulino wrote:
> On Fri, 27 Jul 2012 08:37:12 -0500
> Anthony Liguori <aliguori@us.ibm.com> wrote:
> 
> > This series implements the necessary commands to implements danpb's idea to
> > remove -help parsing in libvirt.  We would introduce all of these commands in
> > 1.2 and then change the -help output starting in 1.3.
> 
> I've reviewed this and apart from small details, it looks good to me.
> 
> Would be nice to get an ack from libvirt folks before applying.

I think what Anthony has posted looks workable for libvirt. The proof
will be in the implementation of course. So we'll aim to get libvirt
proof of concept working with this approach asap, and give feedback
on what, if anything, was missing so we can get it all finalized before
1.2 GA.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 3/7] qapi: add query-machines command

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1157 bytes --]

On 07/27/2012 07:37 AM, Anthony Liguori wrote:
> This provides the same output as -M ? but in a structured way.
> 
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> ---
>  qapi-schema.json |   28 ++++++++++++++++++++++++++++
>  qmp-commands.hx  |    6 ++++++
>  vl.c             |   31 +++++++++++++++++++++++++++++++
>  3 files changed, 65 insertions(+), 0 deletions(-)
> 
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 28e9914..5b47026 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2200,3 +2200,31 @@
>  # Since: 0.14.0
>  ##
>  { 'command': 'closefd', 'data': {'fdname': 'str'} }
> +
> +##
> +# @MachineInfo:
> +#
> +# Information describing a machine.
> +#
> +# @name: the name of the machine
> +#
> +# @alias: #optional an alias for the machine name

Can a machine have more than one alias?  In that case, it might be
better to have:

@name: the name of the machine
@alias-of: #optional if present, name is an alias, and this lists the
canonical form that it resolves to

-- 
Eric Blake   eblake@redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]

Re: [Qemu-devel] [PATCH 3/7] qapi: add query-machines command

[Anthony Liguori]

Eric Blake <eblake@redhat.com> writes:

> On 07/27/2012 07:37 AM, Anthony Liguori wrote:
>> This provides the same output as -M ? but in a structured way.
>> 
>> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>> ---
>>  qapi-schema.json |   28 ++++++++++++++++++++++++++++
>>  qmp-commands.hx  |    6 ++++++
>>  vl.c             |   31 +++++++++++++++++++++++++++++++
>>  3 files changed, 65 insertions(+), 0 deletions(-)
>> 
>> diff --git a/qapi-schema.json b/qapi-schema.json
>> index 28e9914..5b47026 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -2200,3 +2200,31 @@
>>  # Since: 0.14.0
>>  ##
>>  { 'command': 'closefd', 'data': {'fdname': 'str'} }
>> +
>> +##
>> +# @MachineInfo:
>> +#
>> +# Information describing a machine.
>> +#
>> +# @name: the name of the machine
>> +#
>> +# @alias: #optional an alias for the machine name
>
> Can a machine have more than one alias?

No, it can't.  Really, alias is only used to map 'pc' -> 'pc-1.1'.  It's
never more complicated than that.

Regards,

Anthony Liguori

> In that case, it might be better to have:
>
> @name: the name of the machine
> @alias-of: #optional if present, name is an alias, and this lists the
> canonical form that it resolves to

>
> -- 
> Eric Blake   eblake@redhat.com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org

Re: [Qemu-devel] [PATCH 3/7] qapi: add query-machines command

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 710 bytes --]

On 07/27/2012 12:12 PM, Anthony Liguori wrote:
> Eric Blake <eblake@redhat.com> writes:

>>> +# @name: the name of the machine
>>> +#
>>> +# @alias: #optional an alias for the machine name
>>
>> Can a machine have more than one alias?
> 
> No, it can't.  Really, alias is only used to map 'pc' -> 'pc-1.1'.  It's
> never more complicated than that.

Fair enough.  Just making sure that in this case, we would see:

{ "name":"pc-1.1", "alias":"pc" }

and not the other way around, and in a future release, it may change to:

{ "name":"pc-1.1" },
{ "name":"pc-1.2", "alias":"pc" }

-- 
Eric Blake   eblake@redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]

Re: [Qemu-devel] [PATCH 5/7] qapi: add query-cpudefs command

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1090 bytes --]

On 07/27/2012 07:37 AM, Anthony Liguori wrote:
> This command attempts to map to the behavior of -cpu ?.  Unfortunately, the
> output of this command differs wildly across targets.
> 
> To accomodate this, we use a weak symbol to implement a default version of the

s/accomodate/accommodate/

> command that fails with a QERR_NOT_SUPPORTED error code.  Targets can then
> override and implement this command if it makes sense for them.
> 
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

> +##
> +# @CpuDefInfo:
> +#
> +# Virtual CPU definition.
> +#
> +# @name: the name of the CPU definition
> +#
> +# Since: 1.2.0

We are inconsistent in this file whether to use '1.2.0' or just '1.2';
probably worth a followup patch to clean whichever version is deemed
inappropriate.

> +##
> +{ 'type': 'CpuDefInfo',
> +  'data': { 'name': 'str' } }
> +
> +##
> +# @query-cpudefs:

I though QMP favored English words, as in 'query-cpu-definitions'

-- 
Eric Blake   eblake@redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]

Re: [Qemu-devel] [PATCH 4/7] compiler: add macro for GCC weak symbols

[Blue Swirl]

On Fri, Jul 27, 2012 at 3:31 PM, Anthony Liguori <aliguori@us.ibm.com> wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
>
>> On 27 July 2012 15:27, Anthony Liguori <aliguori@us.ibm.com> wrote:
>>> Peter Maydell <peter.maydell@linaro.org> writes:
>>>> The GCC manual says "Weak symbols are supported for ELF targets,
>>>> and also for a.out targets when using the GNU assembler and linker".
>>>> Have you tested this on Windows and MacOSX ?
>>>
>>> Weak symbols are supposed to be supported by mingw32.
>>>
>>> I have no idea about MacOS X.
>>>
>>> I have no way to develop or test for MacOS X using free software so I
>>> honestly don't care about it.
>>
>> My approach to this is to avoid non-standard things
>
> http://en.wikipedia.org/wiki/C99#Implementations
>
> So unless you plan on compiling QEMU with xlc, pgi, or icc, I don't
> think relying on "standard things" really helps.

LLVM/Clang should definitely be in the plan.

>
> QEMU doesn't support C99, it supports GCC.  There's no point in
> debating about whether we should rely on extensions or not.  We already
> do--extensively.

Not so extensively. There are a few extensions for which there is no
simple alternative (like QEMU_PACKED) but other compilers likely need
similar extensions. Then there are other extensions (like :? without
middle expression) which can be easily avoided. We should avoid to use
the non-standard extensions whenever possible.

>
> Regards,
>
> Anthony Liguori
>
>
>> -- if I
>> write a patch which is pretty much standard C then it's the
>> platform's problem if it mishandles it. If I write a patch
>> that uses a compiler-specific or OS-specific thing then I
>> have to also provide the relevant alternatives...so I try
>> to avoid doing that :-)
>>
>> -- PMM
>
>

Re: [Qemu-devel] [PATCH 4/7] compiler: add macro for GCC weak symbols

[Anthony Liguori]

Blue Swirl <blauwirbel@gmail.com> writes:

> On Fri, Jul 27, 2012 at 3:31 PM, Anthony Liguori <aliguori@us.ibm.com> wrote:
>> Peter Maydell <peter.maydell@linaro.org> writes:
>>
>>> On 27 July 2012 15:27, Anthony Liguori <aliguori@us.ibm.com> wrote:
>>>> Peter Maydell <peter.maydell@linaro.org> writes:
>>>>> The GCC manual says "Weak symbols are supported for ELF targets,
>>>>> and also for a.out targets when using the GNU assembler and linker".
>>>>> Have you tested this on Windows and MacOSX ?
>>>>
>>>> Weak symbols are supposed to be supported by mingw32.
>>>>
>>>> I have no idea about MacOS X.
>>>>
>>>> I have no way to develop or test for MacOS X using free software so I
>>>> honestly don't care about it.
>>>
>>> My approach to this is to avoid non-standard things
>>
>> http://en.wikipedia.org/wiki/C99#Implementations
>>
>> So unless you plan on compiling QEMU with xlc, pgi, or icc, I don't
>> think relying on "standard things" really helps.
>
> LLVM/Clang should definitely be in the plan.

weak symbols are supported by clang.

>> QEMU doesn't support C99, it supports GCC.  There's no point in
>> debating about whether we should rely on extensions or not.  We already
>> do--extensively.
>
> Not so extensively. There are a few extensions for which there is no
> simple alternative (like QEMU_PACKED) but other compilers likely need
> similar extensions. Then there are other extensions (like :? without
> middle expression) which can be easily avoided. We should avoid to use
> the non-standard extensions whenever possible.

I disagree.  I don't see a point to it.  QEMU has never been routinely
built on anything other than GCC.  Why go to a lot of trouble to support
a user base that doesn't exist?

If someone comes along and actively maintains support for another
compiler, we can revisit.  But otherwise, there's no practical reason to
avoid extensions.

Regards,

Anthony Liguori

>
>>
>> Regards,
>>
>> Anthony Liguori
>>
>>
>>> -- if I
>>> write a patch which is pretty much standard C then it's the
>>> platform's problem if it mishandles it. If I write a patch
>>> that uses a compiler-specific or OS-specific thing then I
>>> have to also provide the relevant alternatives...so I try
>>> to avoid doing that :-)
>>>
>>> -- PMM
>>
>>

Re: [Qemu-devel] [PATCH 4/7] compiler: add macro for GCC weak symbols

[Blue Swirl]

On Fri, Jul 27, 2012 at 8:51 PM, Anthony Liguori <aliguori@us.ibm.com> wrote:
> Blue Swirl <blauwirbel@gmail.com> writes:
>
>> On Fri, Jul 27, 2012 at 3:31 PM, Anthony Liguori <aliguori@us.ibm.com> wrote:
>>> Peter Maydell <peter.maydell@linaro.org> writes:
>>>
>>>> On 27 July 2012 15:27, Anthony Liguori <aliguori@us.ibm.com> wrote:
>>>>> Peter Maydell <peter.maydell@linaro.org> writes:
>>>>>> The GCC manual says "Weak symbols are supported for ELF targets,
>>>>>> and also for a.out targets when using the GNU assembler and linker".
>>>>>> Have you tested this on Windows and MacOSX ?
>>>>>
>>>>> Weak symbols are supposed to be supported by mingw32.
>>>>>
>>>>> I have no idea about MacOS X.
>>>>>
>>>>> I have no way to develop or test for MacOS X using free software so I
>>>>> honestly don't care about it.
>>>>
>>>> My approach to this is to avoid non-standard things
>>>
>>> http://en.wikipedia.org/wiki/C99#Implementations
>>>
>>> So unless you plan on compiling QEMU with xlc, pgi, or icc, I don't
>>> think relying on "standard things" really helps.
>>
>> LLVM/Clang should definitely be in the plan.
>
> weak symbols are supported by clang.
>
>>> QEMU doesn't support C99, it supports GCC.  There's no point in
>>> debating about whether we should rely on extensions or not.  We already
>>> do--extensively.
>>
>> Not so extensively. There are a few extensions for which there is no
>> simple alternative (like QEMU_PACKED) but other compilers likely need
>> similar extensions. Then there are other extensions (like :? without
>> middle expression) which can be easily avoided. We should avoid to use
>> the non-standard extensions whenever possible.
>
> I disagree.  I don't see a point to it.  QEMU has never been routinely
> built on anything other than GCC.  Why go to a lot of trouble to support
> a user base that doesn't exist?
>
> If someone comes along and actively maintains support for another
> compiler, we can revisit.  But otherwise, there's no practical reason to
> avoid extensions.

Because it's more compliant to standards. There's also very little
benefit from using the nonessential extensions.

>
> Regards,
>
> Anthony Liguori
>
>>
>>>
>>> Regards,
>>>
>>> Anthony Liguori
>>>
>>>
>>>> -- if I
>>>> write a patch which is pretty much standard C then it's the
>>>> platform's problem if it mishandles it. If I write a patch
>>>> that uses a compiler-specific or OS-specific thing then I
>>>> have to also provide the relevant alternatives...so I try
>>>> to avoid doing that :-)
>>>>
>>>> -- PMM
>>>
>>>
>

Re: [Qemu-devel] [PATCH 4/7] compiler: add macro for GCC weak symbols

[Anthony Liguori]

Blue Swirl <blauwirbel@gmail.com> writes:

> On Fri, Jul 27, 2012 at 8:51 PM, Anthony Liguori <aliguori@us.ibm.com> wrote:
>> If someone comes along and actively maintains support for another
>> compiler, we can revisit.  But otherwise, there's no practical reason to
>> avoid extensions.
>
> Because it's more compliant to standards.

That is not a benefit.  There is no practical advantage to sticking to
only C99.

If we stuck to C99 only, QEMU would look very different.  We wouldn't
have type_init() so we wouldn't have the module system we use today.
It literally took me months to get those patches merged because Paul
made exactly the same argument you're making now.

And it's really been one of the better cleanups in QEMU that we've ever
done (using modules to define devices).

> There's also very little benefit from using the nonessential
> extensions.

Using weak symbols eliminates #ifdefs and makes the code base a lot
cleaner.  It makes it easier to introduce architecture specific hooks in
a clean way.

Considering how messy a lot of our target-specific code is, I think
there could be quite a significant benefit down the road.

Regards,

Anthony Liguori

>
>>
>> Regards,
>>
>> Anthony Liguori
>>
>>>
>>>>
>>>> Regards,
>>>>
>>>> Anthony Liguori
>>>>
>>>>
>>>>> -- if I
>>>>> write a patch which is pretty much standard C then it's the
>>>>> platform's problem if it mishandles it. If I write a patch
>>>>> that uses a compiler-specific or OS-specific thing then I
>>>>> have to also provide the relevant alternatives...so I try
>>>>> to avoid doing that :-)
>>>>>
>>>>> -- PMM
>>>>
>>>>
>>

Re: [Qemu-devel] [PATCH 4/7] compiler: add macro for GCC weak symbols

[Markus Armbruster]

Anthony Liguori <aliguori@us.ibm.com> writes:

> Blue Swirl <blauwirbel@gmail.com> writes:
>
>> On Fri, Jul 27, 2012 at 8:51 PM, Anthony Liguori <aliguori@us.ibm.com> wrote:
>>> If someone comes along and actively maintains support for another
>>> compiler, we can revisit.  But otherwise, there's no practical reason to
>>> avoid extensions.

Exactly.

>> Because it's more compliant to standards.
>
> That is not a benefit.  There is no practical advantage to sticking to
> only C99.

Agree.  "Compliance to X" is means, not ends.  Specifically, compliance
to C99 is not a benefit by itself.  Portability to desirable targets is
one.

Right now, the code is portable enough for our current needs.  If you
find a target you wish to support where it doesn't work, patches are
welcome.

> If we stuck to C99 only, QEMU would look very different.  We wouldn't
> have type_init() so we wouldn't have the module system we use today.
> It literally took me months to get those patches merged because Paul
> made exactly the same argument you're making now.
>
> And it's really been one of the better cleanups in QEMU that we've ever
> done (using modules to define devices).

Developing is hard enough without tying ourselves into knots to avoid
useful tools just because they haven't been blessed by the right
standards committee.  Some pragmatism is in order.

>> There's also very little benefit from using the nonessential
>> extensions.

Blanket statement, needs evidence.

> Using weak symbols eliminates #ifdefs and makes the code base a lot
> cleaner.  It makes it easier to introduce architecture specific hooks in
> a clean way.
>
> Considering how messy a lot of our target-specific code is, I think
> there could be quite a significant benefit down the road.

I don't have an informed opinion on this particular case.  All I can say
here is patches are evidence.

Re: [Qemu-devel] [PATCH 4/7] compiler: add macro for GCC weak symbols

[Peter Maydell]

On 27 July 2012 16:31, Anthony Liguori <aliguori@us.ibm.com> wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
>> My approach to this is to avoid non-standard things
>
> http://en.wikipedia.org/wiki/C99#Implementations
>
> So unless you plan on compiling QEMU with xlc, pgi, or icc, I don't
> think relying on "standard things" really helps.
>
> QEMU doesn't support C99, it supports GCC.

OK, you could perhaps rephrase that as 'mainstream' rather than
'standards-compliant'. I don't think we need to be strict C99;
I do think we have more than one working host OS and that patches
that use functionality that's documented not to work on all gcc
targets ought to come attached to a statement that they've been
tested. (MacOSX isn't actually in MAINTAINERS as a host so is
a bit of a red herring. Windows is listed.)

So if you really like weak symbols, go ahead. I'm just saying
you're imposing a bigger testing burden on yourself than if
you handled this some other way.

(I do think it would be nice to care about building with CLANG,
because there are some static analysis tools that we would
then be able to run. That doesn't mean dropping all GCC
extensions, though, because CLANG does support a lot of them.)

-- PMM

Re: [Qemu-devel] [PATCH 4/7] compiler: add macro for GCC weak symbols

[Blue Swirl]

On Fri, Jul 27, 2012 at 10:40 PM, Anthony Liguori <aliguori@us.ibm.com> wrote:
> Blue Swirl <blauwirbel@gmail.com> writes:
>
>> On Fri, Jul 27, 2012 at 8:51 PM, Anthony Liguori <aliguori@us.ibm.com> wrote:
>>> If someone comes along and actively maintains support for another
>>> compiler, we can revisit.  But otherwise, there's no practical reason to
>>> avoid extensions.
>>
>> Because it's more compliant to standards.
>
> That is not a benefit.  There is no practical advantage to sticking to
> only C99.
>
> If we stuck to C99 only, QEMU would look very different.  We wouldn't
> have type_init() so we wouldn't have the module system we use today.
> It literally took me months to get those patches merged because Paul
> made exactly the same argument you're making now.
>
> And it's really been one of the better cleanups in QEMU that we've ever
> done (using modules to define devices).

Yes, it's a very nice cleanup. But I'm not saying things like that
would have to be changed much or at all. As an example for
type_init(), a script could grep and process those declarations (like
the recently posted IDL stuff) and generate type tables in a .c file
for the non-GCC compiler. Then an initialization function would just
call the functions in the table in sequence. That way GCC can use
initializers but with clever hacks, non-GCC compilers can be used too.

>
>> There's also very little benefit from using the nonessential
>> extensions.
>
> Using weak symbols eliminates #ifdefs and makes the code base a lot
> cleaner.  It makes it easier to introduce architecture specific hooks in
> a clean way.
>
> Considering how messy a lot of our target-specific code is, I think
> there could be quite a significant benefit down the road.

I don't have anything specifically against weak symbols, probably with
macros like QEMU_WEAK() they can also be accommodated later if ever
needed. Just offhand, the macro could also take a symbol prefix which
is ignored for GCC, but used in non-GCC case to create a table which a
symbol resolver (not compiled in for GCC) would use.

>
> Regards,
>
> Anthony Liguori
>
>>
>>>
>>> Regards,
>>>
>>> Anthony Liguori
>>>
>>>>
>>>>>
>>>>> Regards,
>>>>>
>>>>> Anthony Liguori
>>>>>
>>>>>
>>>>>> -- if I
>>>>>> write a patch which is pretty much standard C then it's the
>>>>>> platform's problem if it mishandles it. If I write a patch
>>>>>> that uses a compiler-specific or OS-specific thing then I
>>>>>> have to also provide the relevant alternatives...so I try
>>>>>> to avoid doing that :-)
>>>>>>
>>>>>> -- PMM
>>>>>
>>>>>
>>>
>

Re: [Qemu-devel] [PATCH 4/7] compiler: add macro for GCC weak symbols

[Blue Swirl]

On Sat, Jul 28, 2012 at 6:25 AM, Markus Armbruster <armbru@redhat.com> wrote:
> Anthony Liguori <aliguori@us.ibm.com> writes:
>
>> Blue Swirl <blauwirbel@gmail.com> writes:
>>
>>> On Fri, Jul 27, 2012 at 8:51 PM, Anthony Liguori <aliguori@us.ibm.com> wrote:
>>>> If someone comes along and actively maintains support for another
>>>> compiler, we can revisit.  But otherwise, there's no practical reason to
>>>> avoid extensions.
>
> Exactly.
>
>>> Because it's more compliant to standards.
>>
>> That is not a benefit.  There is no practical advantage to sticking to
>> only C99.
>
> Agree.  "Compliance to X" is means, not ends.  Specifically, compliance
> to C99 is not a benefit by itself.  Portability to desirable targets is
> one.
>
> Right now, the code is portable enough for our current needs.  If you
> find a target you wish to support where it doesn't work, patches are
> welcome.
>
>> If we stuck to C99 only, QEMU would look very different.  We wouldn't
>> have type_init() so we wouldn't have the module system we use today.
>> It literally took me months to get those patches merged because Paul
>> made exactly the same argument you're making now.
>>
>> And it's really been one of the better cleanups in QEMU that we've ever
>> done (using modules to define devices).
>
> Developing is hard enough without tying ourselves into knots to avoid
> useful tools just because they haven't been blessed by the right
> standards committee.  Some pragmatism is in order.

IMHO it's just laziness to use easily avoidable constructs like ?:
without middle expression. In most cases (as can be seen in my patch),
you save typing and reviewing whopping 10-20 characters. We are in
seconds range of effort.

>
>>> There's also very little benefit from using the nonessential
>>> extensions.
>
> Blanket statement, needs evidence.
>
>> Using weak symbols eliminates #ifdefs and makes the code base a lot
>> cleaner.  It makes it easier to introduce architecture specific hooks in
>> a clean way.
>>
>> Considering how messy a lot of our target-specific code is, I think
>> there could be quite a significant benefit down the road.
>
> I don't have an informed opinion on this particular case.  All I can say
> here is patches are evidence.

Re: [Qemu-devel] [PATCH 4/7] compiler: add macro for GCC weak symbols

[Blue Swirl]

On Sat, Jul 28, 2012 at 6:50 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 27 July 2012 16:31, Anthony Liguori <aliguori@us.ibm.com> wrote:
>> Peter Maydell <peter.maydell@linaro.org> writes:
>>> My approach to this is to avoid non-standard things
>>
>> http://en.wikipedia.org/wiki/C99#Implementations
>>
>> So unless you plan on compiling QEMU with xlc, pgi, or icc, I don't
>> think relying on "standard things" really helps.
>>
>> QEMU doesn't support C99, it supports GCC.
>
> OK, you could perhaps rephrase that as 'mainstream' rather than
> 'standards-compliant'. I don't think we need to be strict C99;
> I do think we have more than one working host OS and that patches
> that use functionality that's documented not to work on all gcc
> targets ought to come attached to a statement that they've been
> tested. (MacOSX isn't actually in MAINTAINERS as a host so is
> a bit of a red herring. Windows is listed.)

I'd also like to avoid a world where everything only targets GCC on
x86_64 on Linux with KVM. "Embrace and extend" may also be seen to
apply to GCC extensions.

>
> So if you really like weak symbols, go ahead. I'm just saying
> you're imposing a bigger testing burden on yourself than if
> you handled this some other way.
>
> (I do think it would be nice to care about building with CLANG,
> because there are some static analysis tools that we would
> then be able to run. That doesn't mean dropping all GCC
> extensions, though, because CLANG does support a lot of them.)
>
> -- PMM
>

Re: [Qemu-devel] [PATCH 6/7] target-i386: add implementation of query-cpudefs

[Eduardo Habkost]

On Fri, Jul 27, 2012 at 08:37:18AM -0500, Anthony Liguori wrote:
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> ---
>  target-i386/cpu.c |   22 ++++++++++++++++++++++
>  1 files changed, 22 insertions(+), 0 deletions(-)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 6b9659f..b398439 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -28,6 +28,7 @@
>  #include "qemu-config.h"
>  
>  #include "qapi/qapi-visit-core.h"
> +#include "qmp-commands.h"
>  
>  #include "hyperv.h"
>  
> @@ -1123,6 +1124,27 @@ void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf, const char *optarg)
>      }
>  }
>  
> +CpuDefInfoList *qmp_query_cpudefs(Error **errp)
> +{
> +    CpuDefInfoList *cpu_list = NULL;
> +    x86_def_t *def;
> +
> +    for (def = x86_defs; def; def = def->next) {
> +        CpuDefInfoList *entry;
> +        CpuDefInfo *info;
> +
> +        info = g_malloc0(sizeof(*info));
> +        info->name = g_strdup(def->name);
> +
> +        entry = g_malloc0(sizeof(*entry));
> +        entry->value = info;
> +        entry->next = cpu_list;
> +        cpu_list = entry;
> +    }
> +
> +    return cpu_list;
> +}

How would the interface look like once we:
- let libvirt know which features are available on each CPU model
  (libvirt needs that information[1]); and
- add machine-type-specific cpudef compatibility changes?

Would the command report different results depending on -machine?

Would the command return the latest cpudef without any machine-type
hacks, and libvirt would have to query for the cpudef compatibility data
for each machine-type and combine both pieces of information itself?

[1] Note that it doesn't have to be low-level leaf-by-leaf
    register-by-register CPUID bits (I prefer a more high-level
    interface, myself), but it has to at least say "feature FOO is
    enabled/disabled" for a set of features libvirt cares about.

-- 
Eduardo

Re: [Qemu-devel] [PATCH 1/7] qmp: introduce device-list-properties command

[Anthony Liguori]

Luiz Capitulino <lcapitulino@redhat.com> writes:

> On Fri, 27 Jul 2012 08:37:13 -0500
> Anthony Liguori <aliguori@us.ibm.com> wrote:
>
>> This can be used in conjunction with qom-list-types to determine the supported
>> set of devices and their parameters.
>> 
>> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>> ---
>>  qapi-schema.json |   28 ++++++++++++++++++++++++++++
>>  qmp-commands.hx  |    7 +++++++
>>  qmp.c            |   50 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 85 insertions(+), 0 deletions(-)
>> 
>> diff --git a/qapi-schema.json b/qapi-schema.json
>> index bc55ed2..015a84a 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -1727,6 +1727,34 @@
>>    'returns': [ 'ObjectTypeInfo' ] }
>>  
>>  ##
>> +# @DevicePropertyInfo:
>> +#
>> +# Information about device properties.
>> +#
>> +# @name: the name of the property
>> +# @type: the typename of the property
>> +#
>> +# Since: 1.2
>> +##
>> +{ 'type': 'DevicePropertyInfo',
>> +  'data': { 'name': 'str', 'type': 'str' } }
>> +
>> +##
>> +# @device-list-properties:
>> +#
>> +# List properties associated with a device.
>> +#
>> +# @typename: the type name of a device
>> +#
>> +# Returns: a list of DevicePropertyInfo describing a devices properties
>> +#
>> +# Since: 1.2
>> +##
>> +{ 'command': 'device-list-properties',
>> +  'data': { 'typename': 'str'},
>> +  'returns': [ 'DevicePropertyInfo' ] }
>
> I find it a bit weird that the argument is called typename but everything else
> refers to that same argument as a device.
>
> Maybe we should rename this to device-type-list-properties, DeviceTypePropertyInfo
> and typename to name.

I see where you're coming from but I think I'd prefer to leave it
as-is.  'DeviceType' isn't really a concept in QEMU.

Regards,

Anthony Liguori

>
> But that's minor, I won't oppose to it as it's.
>
>> +
>> +##
>>  # @migrate
>>  #
>>  # Migrates the current running guest to another Virtual Machine.
>> diff --git a/qmp-commands.hx b/qmp-commands.hx
>> index e3cf3c5..5c55528 100644
>> --- a/qmp-commands.hx
>> +++ b/qmp-commands.hx
>> @@ -2215,3 +2215,10 @@ EQMP
>>          .args_type  = "implements:s?,abstract:b?",
>>          .mhandler.cmd_new = qmp_marshal_input_qom_list_types,
>>      },
>> +
>> +    {
>> +        .name       = "device-list-properties",
>> +        .args_type  = "typename:s",
>> +        .mhandler.cmd_new = qmp_marshal_input_device_list_properties,
>> +    },
>> +
>> diff --git a/qmp.c b/qmp.c
>> index fee9fb2..254a32f 100644
>> --- a/qmp.c
>> +++ b/qmp.c
>> @@ -417,3 +417,53 @@ ObjectTypeInfoList *qmp_qom_list_types(bool has_implements,
>>  
>>      return ret;
>>  }
>> +
>> +DevicePropertyInfoList *qmp_device_list_properties(const char *typename,
>> +                                                   Error **errp)
>> +{
>> +    ObjectClass *klass;
>> +    Property *prop;
>> +    DevicePropertyInfoList *prop_list = NULL;
>> +
>> +    klass = object_class_by_name(typename);
>> +    if (klass == NULL) {
>> +        error_set(errp, QERR_DEVICE_NOT_FOUND, typename);
>> +        return NULL;
>> +    }
>> +
>> +    klass = object_class_dynamic_cast(klass, TYPE_DEVICE);
>> +    if (klass == NULL) {
>> +        error_set(errp, QERR_INVALID_PARAMETER_VALUE,
>> +                  "name", TYPE_DEVICE);
>> +        return NULL;
>> +    }
>> +
>> +    do {
>> +        for (prop = DEVICE_CLASS(klass)->props; prop && prop->name; prop++) {
>> +            DevicePropertyInfoList *entry;
>> +            DevicePropertyInfo *info;
>> +
>> +            /*
>> +             * TODO Properties without a parser are just for dirty hacks.
>> +             * qdev_prop_ptr is the only such PropertyInfo.  It's marked
>> +             * for removal.  This conditional should be removed along with
>> +             * it.
>> +             */
>> +            if (!prop->info->set) {
>> +                continue;           /* no way to set it, don't show */
>> +            }
>> +
>> +            info = g_malloc0(sizeof(*info));
>> +            info->name = g_strdup(prop->name);
>> +            info->type = g_strdup(prop->info->legacy_name ?: prop->info->name);
>> +
>> +            entry = g_malloc0(sizeof(*entry));
>> +            entry->value = info;
>> +            entry->next = prop_list;
>> +            prop_list = entry;
>> +        }
>> +        klass = object_class_get_parent(klass);
>> +    } while (klass != object_class_by_name(TYPE_DEVICE));
>> +
>> +    return prop_list;
>> +}

Re: [Qemu-devel] [PATCH 2/7] qapi: mark QOM commands stable

[Anthony Liguori]

Luiz Capitulino <lcapitulino@redhat.com> writes:

> On Fri, 27 Jul 2012 08:37:14 -0500
> Anthony Liguori <aliguori@us.ibm.com> wrote:
>
>> We've had a cycle to tweak.  It is time to commit to supporting them.
>
> qmp_qom_get() and qpm_qom_set() still use the legacy monitor interface, can't
> we convert it to the qapi?

qapi doesn't have a concept of type passthrough yet.  If it was added,
it could be converted.

Regards,

Anthony Liguori

>
>> 
>> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>> ---
>>  qapi-schema.json |   19 ++++---------------
>>  1 files changed, 4 insertions(+), 15 deletions(-)
>> 
>> diff --git a/qapi-schema.json b/qapi-schema.json
>> index 015a84a..28e9914 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -1360,9 +1360,7 @@
>>  #        4) A link type in the form 'link<subtype>' where subtype is a qdev
>>  #           device type name.  Link properties form the device model graph.
>>  #
>> -# Since: 1.1
>> -#
>> -# Notes: This type is experimental.  Its syntax may change in future releases.
>> +# Since: 1.2
>>  ##
>>  { 'type': 'ObjectPropertyInfo',
>>    'data': { 'name': 'str', 'type': 'str' } }
>> @@ -1379,10 +1377,7 @@
>>  # Returns: a list of @ObjectPropertyInfo that describe the properties of the
>>  #          object.
>>  #
>> -# Since: 1.1
>> -#
>> -# Notes: This command is experimental.  It's syntax may change in future
>> -#        releases.
>> +# Since: 1.2
>>  ##
>>  { 'command': 'qom-list',
>>    'data': { 'path': 'str' },
>> @@ -1418,9 +1413,7 @@
>>  #          returns as #str pathnames.  All integer property types (u8, u16, etc)
>>  #          are returned as #int.
>>  #
>> -# Since: 1.1
>> -#
>> -# Notes: This command is experimental and may change syntax in future releases.
>> +# Since: 1.2
>>  ##
>>  { 'command': 'qom-get',
>>    'data': { 'path': 'str', 'property': 'str' },
>> @@ -1439,9 +1432,7 @@
>>  # @value: a value who's type is appropriate for the property type.  See @qom-get
>>  #         for a description of type mapping.
>>  #
>> -# Since: 1.1
>> -#
>> -# Notes: This command is experimental and may change syntax in future releases.
>> +# Since: 1.2
>>  ##
>>  { 'command': 'qom-set',
>>    'data': { 'path': 'str', 'property': 'str', 'value': 'visitor' },
>> @@ -1719,8 +1710,6 @@
>>  # Returns: a list of @ObjectTypeInfo or an empty list if no results are found
>>  #
>>  # Since: 1.1
>> -#
>> -# Notes: This command is experimental and may change syntax in future releases.
>>  ##
>>  { 'command': 'qom-list-types',
>>    'data': { '*implements': 'str', '*abstract': 'bool' },

Re: [Qemu-devel] [PATCH 3/7] qapi: add query-machines command

[Anthony Liguori]

Luiz Capitulino <lcapitulino@redhat.com> writes:

> On Fri, 27 Jul 2012 08:37:15 -0500
> Anthony Liguori <aliguori@us.ibm.com> wrote:
>
>> This provides the same output as -M ? but in a structured way.
>> 
>> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>> ---
>>  qapi-schema.json |   28 ++++++++++++++++++++++++++++
>>  qmp-commands.hx  |    6 ++++++
>>  vl.c             |   31 +++++++++++++++++++++++++++++++
>>  3 files changed, 65 insertions(+), 0 deletions(-)
>> 
>> diff --git a/qapi-schema.json b/qapi-schema.json
>> index 28e9914..5b47026 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -2200,3 +2200,31 @@
>>  # Since: 0.14.0
>>  ##
>>  { 'command': 'closefd', 'data': {'fdname': 'str'} }
>> +
>> +##
>> +# @MachineInfo:
>> +#
>> +# Information describing a machine.
>> +#
>> +# @name: the name of the machine
>> +#
>> +# @alias: #optional an alias for the machine name
>> +#
>> +# @default: #optional whether the machine is default
>
> Why is default optional?

Brievity.

Regards,

Anthony Liguori

>
>> +#
>> +# Since: 1.2.0
>> +##
>> +{ 'type': 'MachineInfo',
>> +  'data': { 'name': 'str', '*alias': 'str',
>> +            '*is-default': 'bool' } }
>> +
>> +##
>> +# @query-machines:
>> +#
>> +# Return a list of supported machines
>> +#
>> +# Returns: a list of MachineInfo
>> +#
>> +# Since: 1.2.0
>> +##
>> +{ 'command': 'query-machines', 'returns': ['MachineInfo'] }
>> diff --git a/qmp-commands.hx b/qmp-commands.hx
>> index 5c55528..a6f82fc 100644
>> --- a/qmp-commands.hx
>> +++ b/qmp-commands.hx
>> @@ -2222,3 +2222,9 @@ EQMP
>>          .mhandler.cmd_new = qmp_marshal_input_device_list_properties,
>>      },
>>  
>> +    {
>> +        .name       = "query-machines",
>> +        .args_type  = "",
>> +        .mhandler.cmd_new = qmp_marshal_input_query_machines,
>> +    },
>> +
>> diff --git a/vl.c b/vl.c
>> index 8904db1..cd900e0 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -1209,6 +1209,37 @@ QEMUMachine *find_default_machine(void)
>>      return NULL;
>>  }
>>  
>> +MachineInfoList *qmp_query_machines(Error **errp)
>> +{
>> +    MachineInfoList *mach_list = NULL;
>> +    QEMUMachine *m;
>> +
>> +    for (m = first_machine; m; m = m->next) {
>> +        MachineInfoList *entry;
>> +        MachineInfo *info;
>> +
>> +        info = g_malloc0(sizeof(*info));
>> +        if (m->is_default) {
>> +            info->has_is_default = true;
>> +            info->is_default = true;
>> +        }
>> +
>> +        if (m->alias) {
>> +            info->has_alias = true;
>> +            info->alias = g_strdup(m->alias);
>> +        }
>> +
>> +        info->name = g_strdup(m->name);
>> +
>> +        entry = g_malloc0(sizeof(*entry));
>> +        entry->value = info;
>> +        entry->next = mach_list;
>> +        mach_list = entry;
>> +    }
>> +
>> +    return mach_list;
>> +}
>> +
>>  /***********************************************************/
>>  /* main execution loop */
>>  

Re: [Qemu-devel] [PATCH 6/7] target-i386: add implementation of query-cpudefs

[Anthony Liguori]

Eduardo Habkost <ehabkost@redhat.com> writes:

> On Fri, Jul 27, 2012 at 08:37:18AM -0500, Anthony Liguori wrote:
>> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>> ---
>>  target-i386/cpu.c |   22 ++++++++++++++++++++++
>>  1 files changed, 22 insertions(+), 0 deletions(-)
>> 
>> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
>> index 6b9659f..b398439 100644
>> --- a/target-i386/cpu.c
>> +++ b/target-i386/cpu.c
>> @@ -28,6 +28,7 @@
>>  #include "qemu-config.h"
>>  
>>  #include "qapi/qapi-visit-core.h"
>> +#include "qmp-commands.h"
>>  
>>  #include "hyperv.h"
>>  
>> @@ -1123,6 +1124,27 @@ void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf, const char *optarg)
>>      }
>>  }
>>  
>> +CpuDefInfoList *qmp_query_cpudefs(Error **errp)
>> +{
>> +    CpuDefInfoList *cpu_list = NULL;
>> +    x86_def_t *def;
>> +
>> +    for (def = x86_defs; def; def = def->next) {
>> +        CpuDefInfoList *entry;
>> +        CpuDefInfo *info;
>> +
>> +        info = g_malloc0(sizeof(*info));
>> +        info->name = g_strdup(def->name);
>> +
>> +        entry = g_malloc0(sizeof(*entry));
>> +        entry->value = info;
>> +        entry->next = cpu_list;
>> +        cpu_list = entry;
>> +    }
>> +
>> +    return cpu_list;
>> +}
>
> How would the interface look like once we:
> - let libvirt know which features are available on each CPU model
>   (libvirt needs that information[1]); and

I'm not sure I understand why libvirt needs this information.  Can you elaborate?

> - add machine-type-specific cpudef compatibility changes?

I think we've discussed this in IRC.  I don't think we need to worry
about this.

> Would the command report different results depending on -machine?

No.

>
> Would the command return the latest cpudef without any machine-type
> hacks, and libvirt would have to query for the cpudef compatibility data
> for each machine-type and combine both pieces of information itself?

I'm not sure what you mean by compatibility data.

Regards,

Anthony Liguori

>
> [1] Note that it doesn't have to be low-level leaf-by-leaf
>     register-by-register CPUID bits (I prefer a more high-level
>     interface, myself), but it has to at least say "feature FOO is
>     enabled/disabled" for a set of features libvirt cares about.
>
> -- 
> Eduardo

Re: [Qemu-devel] [PATCH 3/7] qapi: add query-machines command

[Luiz Capitulino]

On Fri, 10 Aug 2012 09:41:20 -0500
Anthony Liguori <aliguori@us.ibm.com> wrote:

> Luiz Capitulino <lcapitulino@redhat.com> writes:
> 
> > On Fri, 27 Jul 2012 08:37:15 -0500
> > Anthony Liguori <aliguori@us.ibm.com> wrote:
> >
> >> This provides the same output as -M ? but in a structured way.
> >> 
> >> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> >> ---
> >>  qapi-schema.json |   28 ++++++++++++++++++++++++++++
> >>  qmp-commands.hx  |    6 ++++++
> >>  vl.c             |   31 +++++++++++++++++++++++++++++++
> >>  3 files changed, 65 insertions(+), 0 deletions(-)
> >> 
> >> diff --git a/qapi-schema.json b/qapi-schema.json
> >> index 28e9914..5b47026 100644
> >> --- a/qapi-schema.json
> >> +++ b/qapi-schema.json
> >> @@ -2200,3 +2200,31 @@
> >>  # Since: 0.14.0
> >>  ##
> >>  { 'command': 'closefd', 'data': {'fdname': 'str'} }
> >> +
> >> +##
> >> +# @MachineInfo:
> >> +#
> >> +# Information describing a machine.
> >> +#
> >> +# @name: the name of the machine
> >> +#
> >> +# @alias: #optional an alias for the machine name
> >> +#
> >> +# @default: #optional whether the machine is default
> >
> > Why is default optional?
> 
> Brievity.

Can you elaborate, please?

> 
> Regards,
> 
> Anthony Liguori
> 
> >
> >> +#
> >> +# Since: 1.2.0
> >> +##
> >> +{ 'type': 'MachineInfo',
> >> +  'data': { 'name': 'str', '*alias': 'str',
> >> +            '*is-default': 'bool' } }
> >> +
> >> +##
> >> +# @query-machines:
> >> +#
> >> +# Return a list of supported machines
> >> +#
> >> +# Returns: a list of MachineInfo
> >> +#
> >> +# Since: 1.2.0
> >> +##
> >> +{ 'command': 'query-machines', 'returns': ['MachineInfo'] }
> >> diff --git a/qmp-commands.hx b/qmp-commands.hx
> >> index 5c55528..a6f82fc 100644
> >> --- a/qmp-commands.hx
> >> +++ b/qmp-commands.hx
> >> @@ -2222,3 +2222,9 @@ EQMP
> >>          .mhandler.cmd_new = qmp_marshal_input_device_list_properties,
> >>      },
> >>  
> >> +    {
> >> +        .name       = "query-machines",
> >> +        .args_type  = "",
> >> +        .mhandler.cmd_new = qmp_marshal_input_query_machines,
> >> +    },
> >> +
> >> diff --git a/vl.c b/vl.c
> >> index 8904db1..cd900e0 100644
> >> --- a/vl.c
> >> +++ b/vl.c
> >> @@ -1209,6 +1209,37 @@ QEMUMachine *find_default_machine(void)
> >>      return NULL;
> >>  }
> >>  
> >> +MachineInfoList *qmp_query_machines(Error **errp)
> >> +{
> >> +    MachineInfoList *mach_list = NULL;
> >> +    QEMUMachine *m;
> >> +
> >> +    for (m = first_machine; m; m = m->next) {
> >> +        MachineInfoList *entry;
> >> +        MachineInfo *info;
> >> +
> >> +        info = g_malloc0(sizeof(*info));
> >> +        if (m->is_default) {
> >> +            info->has_is_default = true;
> >> +            info->is_default = true;
> >> +        }
> >> +
> >> +        if (m->alias) {
> >> +            info->has_alias = true;
> >> +            info->alias = g_strdup(m->alias);
> >> +        }
> >> +
> >> +        info->name = g_strdup(m->name);
> >> +
> >> +        entry = g_malloc0(sizeof(*entry));
> >> +        entry->value = info;
> >> +        entry->next = mach_list;
> >> +        mach_list = entry;
> >> +    }
> >> +
> >> +    return mach_list;
> >> +}
> >> +
> >>  /***********************************************************/
> >>  /* main execution loop */
> >>  
> 

Re: [Qemu-devel] [PATCH 6/7] target-i386: add implementation of query-cpudefs

[Eduardo Habkost]

On Fri, Aug 10, 2012 at 09:43:21AM -0500, Anthony Liguori wrote:
> Eduardo Habkost <ehabkost@redhat.com> writes:
> 
> > On Fri, Jul 27, 2012 at 08:37:18AM -0500, Anthony Liguori wrote:
> >> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> >> ---
> >>  target-i386/cpu.c |   22 ++++++++++++++++++++++
> >>  1 files changed, 22 insertions(+), 0 deletions(-)
> >> 
> >> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> >> index 6b9659f..b398439 100644
> >> --- a/target-i386/cpu.c
> >> +++ b/target-i386/cpu.c
> >> @@ -28,6 +28,7 @@
> >>  #include "qemu-config.h"
> >>  
> >>  #include "qapi/qapi-visit-core.h"
> >> +#include "qmp-commands.h"
> >>  
> >>  #include "hyperv.h"
> >>  
> >> @@ -1123,6 +1124,27 @@ void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf, const char *optarg)
> >>      }
> >>  }
> >>  
> >> +CpuDefInfoList *qmp_query_cpudefs(Error **errp)
> >> +{
> >> +    CpuDefInfoList *cpu_list = NULL;
> >> +    x86_def_t *def;
> >> +
> >> +    for (def = x86_defs; def; def = def->next) {
> >> +        CpuDefInfoList *entry;
> >> +        CpuDefInfo *info;
> >> +
> >> +        info = g_malloc0(sizeof(*info));
> >> +        info->name = g_strdup(def->name);
> >> +
> >> +        entry = g_malloc0(sizeof(*entry));
> >> +        entry->value = info;
> >> +        entry->next = cpu_list;
> >> +        cpu_list = entry;
> >> +    }
> >> +
> >> +    return cpu_list;
> >> +}
> >
> > How would the interface look like once we:
> > - let libvirt know which features are available on each CPU model
> >   (libvirt needs that information[1]); and
> 
> I'm not sure I understand why libvirt needs this information.  Can you elaborate?

I see two reasons:

- The libvirt API has functions to tell the user which features are
  going to be enabled for each CPU model, so it needs to know which
  features are enabled or not, for each machine-type + cpu-model
  combination, so this information can be reported proeprly.
  - Also, if libvirt can enable/disable specific CPU features in the
    command-line, it just makes sens to know which ones are already
    enabled in each built-in CPU model.

- Probing for migration: libvirt needs to know if a given CPU model on a
  host can be migrated to another host. To know that, two pieces of
  information are needed:
  A) Which CPU features are visible to the guest for a specific
     configuration;
  B) Which of those features are really supported by the host
     hardware+kernel+QEMU, on the destination host, so it can
     know if migration is really possible.
  I am assuming that libvirt will query for A and B, and then combine it
  itself. But QEMU could also simply calculate (A&B) itself, and just
  have a boolean function that tells if a given model can be run on a
  specific host or not. But then it wouldn't solve the first item above
  (knowing which features are already enabled on a CPU model).
  - The problem is: even if QEMU does the check itself, the result of
    that "can this host run this VM?" probing function depends on the
    machine-type, too (see explanation below).


> 
> > - add machine-type-specific cpudef compatibility changes?
> 
> I think we've discussed this in IRC.  I don't think we need to worry
> about this.

I remember discussing a lot about the mechanism we will use to add the
compatibility changes, but I don t know how the query API will look
like, after we implement this mechanism.


> 
> > Would the command report different results depending on -machine?
> 
> No.

The problem is:

1) We need to introduce fixes on a CPU model that changes the set of
   guest-visible features (add or remove a feature)[1];
2) The fix has to keep compatibility, so older machine-types will
   keep exposing the old set of gues-visible features;
   - That means different machine-types will have different CPU
     features being exposed.
3) libvirt needs to control/know which guest-visible CPU features are
   available to the guest (see above);
4) Because of (2), the querying system used by libvirt need to depend on
   the CPU model and machine-type.


[1] Example:
    The SandyBridge model today has the "tsc-deadline" bit set, but
    QEMU-1.1 did not expose the tsc-deadline feature properly because of
    incorrect expectations about the GET_SUPPORTED_CPUID ioctl. This was
    fixed on qemu-1.2.
    
    That means "qemu-1.1 -machine pc-1.1 -cpu SandyBridge" does _not_
    expose tsc-deadline to the guest, and we need to make "qemu-1.2
    -machine pc-1.1 -cpu SandyBridge" _not_ expose it, too (otherwise
    migration from qemu-1.1 to qemu-1.2 will be broken).

> 
> >
> > Would the command return the latest cpudef without any machine-type
> > hacks, and libvirt would have to query for the cpudef compatibility data
> > for each machine-type and combine both pieces of information itself?
> 
> I'm not sure what you mean by compatibility data.

I mean any guest-visible compatibility bit that we will need to
introduce on older machine-types, when making changes on CPU models (see
the SandyBridge + tsc-deadline example above).

I see two options:
- Libvirt queries for a [f(machine_type, cpu_model) -> cpu_features]
  function, that will take into account the machine-type-specific
  compatibility bits.
- Libvirt queries for a [f(cpu_model) -> cpu_features] function and a
  [f(machine_type) -> compatibility_changes] function, and combine both.
  - I don't like this approach, I am just including it as a possible
    alternative.

> 
> Regards,
> 
> Anthony Liguori
> 
> >
> > [1] Note that it doesn't have to be low-level leaf-by-leaf
> >     register-by-register CPUID bits (I prefer a more high-level
> >     interface, myself), but it has to at least say "feature FOO is
> >     enabled/disabled" for a set of features libvirt cares about.
> >
> > -- 
> > Eduardo
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH 3/7] qapi: add query-machines command

[Anthony Liguori]

Luiz Capitulino <lcapitulino@redhat.com> writes:

> On Fri, 10 Aug 2012 09:41:20 -0500
> Anthony Liguori <aliguori@us.ibm.com> wrote:
>
>> Luiz Capitulino <lcapitulino@redhat.com> writes:
>> 
>> > On Fri, 27 Jul 2012 08:37:15 -0500
>> > Anthony Liguori <aliguori@us.ibm.com> wrote:
>> >
>> >> This provides the same output as -M ? but in a structured way.
>> >> 
>> >> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>> >> ---
>> >>  qapi-schema.json |   28 ++++++++++++++++++++++++++++
>> >>  qmp-commands.hx  |    6 ++++++
>> >>  vl.c             |   31 +++++++++++++++++++++++++++++++
>> >>  3 files changed, 65 insertions(+), 0 deletions(-)
>> >> 
>> >> diff --git a/qapi-schema.json b/qapi-schema.json
>> >> index 28e9914..5b47026 100644
>> >> --- a/qapi-schema.json
>> >> +++ b/qapi-schema.json
>> >> @@ -2200,3 +2200,31 @@
>> >>  # Since: 0.14.0
>> >>  ##
>> >>  { 'command': 'closefd', 'data': {'fdname': 'str'} }
>> >> +
>> >> +##
>> >> +# @MachineInfo:
>> >> +#
>> >> +# Information describing a machine.
>> >> +#
>> >> +# @name: the name of the machine
>> >> +#
>> >> +# @alias: #optional an alias for the machine name
>> >> +#
>> >> +# @default: #optional whether the machine is default
>> >
>> > Why is default optional?
>> 
>> Brievity.
>
> Can you elaborate, please?

There is only one machine that is default.  Having default=false for all
of the rest just adds a lot of unnecessary information in the response.

Regards,

Anthony Liguori

>
>> 
>> Regards,
>> 
>> Anthony Liguori
>> 
>> >
>> >> +#
>> >> +# Since: 1.2.0
>> >> +##
>> >> +{ 'type': 'MachineInfo',
>> >> +  'data': { 'name': 'str', '*alias': 'str',
>> >> +            '*is-default': 'bool' } }
>> >> +
>> >> +##
>> >> +# @query-machines:
>> >> +#
>> >> +# Return a list of supported machines
>> >> +#
>> >> +# Returns: a list of MachineInfo
>> >> +#
>> >> +# Since: 1.2.0
>> >> +##
>> >> +{ 'command': 'query-machines', 'returns': ['MachineInfo'] }
>> >> diff --git a/qmp-commands.hx b/qmp-commands.hx
>> >> index 5c55528..a6f82fc 100644
>> >> --- a/qmp-commands.hx
>> >> +++ b/qmp-commands.hx
>> >> @@ -2222,3 +2222,9 @@ EQMP
>> >>          .mhandler.cmd_new = qmp_marshal_input_device_list_properties,
>> >>      },
>> >>  
>> >> +    {
>> >> +        .name       = "query-machines",
>> >> +        .args_type  = "",
>> >> +        .mhandler.cmd_new = qmp_marshal_input_query_machines,
>> >> +    },
>> >> +
>> >> diff --git a/vl.c b/vl.c
>> >> index 8904db1..cd900e0 100644
>> >> --- a/vl.c
>> >> +++ b/vl.c
>> >> @@ -1209,6 +1209,37 @@ QEMUMachine *find_default_machine(void)
>> >>      return NULL;
>> >>  }
>> >>  
>> >> +MachineInfoList *qmp_query_machines(Error **errp)
>> >> +{
>> >> +    MachineInfoList *mach_list = NULL;
>> >> +    QEMUMachine *m;
>> >> +
>> >> +    for (m = first_machine; m; m = m->next) {
>> >> +        MachineInfoList *entry;
>> >> +        MachineInfo *info;
>> >> +
>> >> +        info = g_malloc0(sizeof(*info));
>> >> +        if (m->is_default) {
>> >> +            info->has_is_default = true;
>> >> +            info->is_default = true;
>> >> +        }
>> >> +
>> >> +        if (m->alias) {
>> >> +            info->has_alias = true;
>> >> +            info->alias = g_strdup(m->alias);
>> >> +        }
>> >> +
>> >> +        info->name = g_strdup(m->name);
>> >> +
>> >> +        entry = g_malloc0(sizeof(*entry));
>> >> +        entry->value = info;
>> >> +        entry->next = mach_list;
>> >> +        mach_list = entry;
>> >> +    }
>> >> +
>> >> +    return mach_list;
>> >> +}
>> >> +
>> >>  /***********************************************************/
>> >>  /* main execution loop */
>> >>  
>> 

Re: [Qemu-devel] [PATCH 3/7] qapi: add query-machines command

[Luiz Capitulino]

On Fri, 10 Aug 2012 11:06:14 -0500
Anthony Liguori <aliguori@us.ibm.com> wrote:

> Luiz Capitulino <lcapitulino@redhat.com> writes:
> 
> > On Fri, 10 Aug 2012 09:41:20 -0500
> > Anthony Liguori <aliguori@us.ibm.com> wrote:
> >
> >> Luiz Capitulino <lcapitulino@redhat.com> writes:
> >> 
> >> > On Fri, 27 Jul 2012 08:37:15 -0500
> >> > Anthony Liguori <aliguori@us.ibm.com> wrote:
> >> >
> >> >> This provides the same output as -M ? but in a structured way.
> >> >> 
> >> >> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> >> >> ---
> >> >>  qapi-schema.json |   28 ++++++++++++++++++++++++++++
> >> >>  qmp-commands.hx  |    6 ++++++
> >> >>  vl.c             |   31 +++++++++++++++++++++++++++++++
> >> >>  3 files changed, 65 insertions(+), 0 deletions(-)
> >> >> 
> >> >> diff --git a/qapi-schema.json b/qapi-schema.json
> >> >> index 28e9914..5b47026 100644
> >> >> --- a/qapi-schema.json
> >> >> +++ b/qapi-schema.json
> >> >> @@ -2200,3 +2200,31 @@
> >> >>  # Since: 0.14.0
> >> >>  ##
> >> >>  { 'command': 'closefd', 'data': {'fdname': 'str'} }
> >> >> +
> >> >> +##
> >> >> +# @MachineInfo:
> >> >> +#
> >> >> +# Information describing a machine.
> >> >> +#
> >> >> +# @name: the name of the machine
> >> >> +#
> >> >> +# @alias: #optional an alias for the machine name
> >> >> +#
> >> >> +# @default: #optional whether the machine is default
> >> >
> >> > Why is default optional?
> >> 
> >> Brievity.
> >
> > Can you elaborate, please?
> 
> There is only one machine that is default.  Having default=false for all
> of the rest just adds a lot of unnecessary information in the response.

I think it's more consistent to have the key (also, there are better ways
to save bytes on the wire if this is an issue), but I don't mind much though.

Re: [Qemu-devel] [PATCH 6/7] target-i386: add implementation of query-cpudefs

[Anthony Liguori]

Eduardo Habkost <ehabkost@redhat.com> writes:

> On Fri, Aug 10, 2012 at 09:43:21AM -0500, Anthony Liguori wrote:
>> Eduardo Habkost <ehabkost@redhat.com> writes:
>> 
>> > On Fri, Jul 27, 2012 at 08:37:18AM -0500, Anthony Liguori wrote:
>> >> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>> >> ---
>> >>  target-i386/cpu.c |   22 ++++++++++++++++++++++
>> >>  1 files changed, 22 insertions(+), 0 deletions(-)
>> >> 
>> >> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
>> >> index 6b9659f..b398439 100644
>> >> --- a/target-i386/cpu.c
>> >> +++ b/target-i386/cpu.c
>> >> @@ -28,6 +28,7 @@
>> >>  #include "qemu-config.h"
>> >>  
>> >>  #include "qapi/qapi-visit-core.h"
>> >> +#include "qmp-commands.h"
>> >>  
>> >>  #include "hyperv.h"
>> >>  
>> >> @@ -1123,6 +1124,27 @@ void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf, const char *optarg)
>> >>      }
>> >>  }
>> >>  
>> >> +CpuDefInfoList *qmp_query_cpudefs(Error **errp)
>> >> +{
>> >> +    CpuDefInfoList *cpu_list = NULL;
>> >> +    x86_def_t *def;
>> >> +
>> >> +    for (def = x86_defs; def; def = def->next) {
>> >> +        CpuDefInfoList *entry;
>> >> +        CpuDefInfo *info;
>> >> +
>> >> +        info = g_malloc0(sizeof(*info));
>> >> +        info->name = g_strdup(def->name);
>> >> +
>> >> +        entry = g_malloc0(sizeof(*entry));
>> >> +        entry->value = info;
>> >> +        entry->next = cpu_list;
>> >> +        cpu_list = entry;
>> >> +    }
>> >> +
>> >> +    return cpu_list;
>> >> +}
>> >
>> > How would the interface look like once we:
>> > - let libvirt know which features are available on each CPU model
>> >   (libvirt needs that information[1]); and
>> 
>> I'm not sure I understand why libvirt needs this information.  Can you elaborate?
>
> I see two reasons:
>
> - The libvirt API has functions to tell the user which features are
>   going to be enabled for each CPU model, so it needs to know which
>   features are enabled or not, for each machine-type + cpu-model
>   combination, so this information can be reported proeprly.

Ok, step number one is that CPU 'features' need to be defined more
formally.  By formally, I mean via qapi-schema.json.

Then we can extend this command to return the set of features supported
by each CPU type.

The first step will need to sort out how this maps across architectures.

>   - Also, if libvirt can enable/disable specific CPU features in the
>     command-line, it just makes sens to know which ones are already
>     enabled in each built-in CPU model.
>
> - Probing for migration: libvirt needs to know if a given CPU model on a
>   host can be migrated to another host. To know that, two pieces of
>   information are needed:
>   A) Which CPU features are visible to the guest for a specific
>      configuration;
>   B) Which of those features are really supported by the host
>      hardware+kernel+QEMU, on the destination host, so it can
>      know if migration is really possible.

Note that what QEMU thinks it exposes is not necessarily what gets
exposed.  KVM may mask additional features.  How is this handled today?

>> > - add machine-type-specific cpudef compatibility changes?
>> 
>> I think we've discussed this in IRC.  I don't think we need to worry
>> about this.
>
> I remember discussing a lot about the mechanism we will use to add the
> compatibility changes, but I don t know how the query API will look
> like, after we implement this mechanism.

0) User-defined CPU definitions go away
   - We already made a big step in this direction

1) CPU becomes a DeviceState

2) Features are expressed as properties

3) Same global mechanism used for everything else is used for CPUs

Regards,

Anthony Liguori

>> > Would the command report different results depending on -machine?
>> 
>> No.
>
> The problem is:
>
> 1) We need to introduce fixes on a CPU model that changes the set of
>    guest-visible features (add or remove a feature)[1];
> 2) The fix has to keep compatibility, so older machine-types will
>    keep exposing the old set of gues-visible features;
>    - That means different machine-types will have different CPU
>      features being exposed.
> 3) libvirt needs to control/know which guest-visible CPU features are
>    available to the guest (see above);
> 4) Because of (2), the querying system used by libvirt need to depend on
>    the CPU model and machine-type.
>
>
> [1] Example:
>     The SandyBridge model today has the "tsc-deadline" bit set, but
>     QEMU-1.1 did not expose the tsc-deadline feature properly because of
>     incorrect expectations about the GET_SUPPORTED_CPUID ioctl. This was
>     fixed on qemu-1.2.
>     
>     That means "qemu-1.1 -machine pc-1.1 -cpu SandyBridge" does _not_
>     expose tsc-deadline to the guest, and we need to make "qemu-1.2
>     -machine pc-1.1 -cpu SandyBridge" _not_ expose it, too (otherwise
>     migration from qemu-1.1 to qemu-1.2 will be broken).
>
>> 
>> >
>> > Would the command return the latest cpudef without any machine-type
>> > hacks, and libvirt would have to query for the cpudef compatibility data
>> > for each machine-type and combine both pieces of information itself?
>> 
>> I'm not sure what you mean by compatibility data.
>
> I mean any guest-visible compatibility bit that we will need to
> introduce on older machine-types, when making changes on CPU models (see
> the SandyBridge + tsc-deadline example above).
>
> I see two options:
> - Libvirt queries for a [f(machine_type, cpu_model) -> cpu_features]
>   function, that will take into account the machine-type-specific
>   compatibility bits.
> - Libvirt queries for a [f(cpu_model) -> cpu_features] function and a
>   [f(machine_type) -> compatibility_changes] function, and combine both.
>   - I don't like this approach, I am just including it as a possible
>     alternative.
>
>> 
>> Regards,
>> 
>> Anthony Liguori
>> 
>> >
>> > [1] Note that it doesn't have to be low-level leaf-by-leaf
>> >     register-by-register CPUID bits (I prefer a more high-level
>> >     interface, myself), but it has to at least say "feature FOO is
>> >     enabled/disabled" for a set of features libvirt cares about.
>> >
>> > -- 
>> > Eduardo
>> 
>
> -- 
> Eduardo

Re: [Qemu-devel] [PATCH 6/7] target-i386: add implementation of query-cpudefs

[Eduardo Habkost]

On Fri, Aug 10, 2012 at 11:37:30AM -0500, Anthony Liguori wrote:
> Eduardo Habkost <ehabkost@redhat.com> writes:
> 
> > On Fri, Aug 10, 2012 at 09:43:21AM -0500, Anthony Liguori wrote:
> >> Eduardo Habkost <ehabkost@redhat.com> writes:
> >> 
> >> > On Fri, Jul 27, 2012 at 08:37:18AM -0500, Anthony Liguori wrote:
> >> >> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> >> >> ---
> >> >>  target-i386/cpu.c |   22 ++++++++++++++++++++++
> >> >>  1 files changed, 22 insertions(+), 0 deletions(-)
> >> >> 
> >> >> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> >> >> index 6b9659f..b398439 100644
> >> >> --- a/target-i386/cpu.c
> >> >> +++ b/target-i386/cpu.c
> >> >> @@ -28,6 +28,7 @@
> >> >>  #include "qemu-config.h"
> >> >>  
> >> >>  #include "qapi/qapi-visit-core.h"
> >> >> +#include "qmp-commands.h"
> >> >>  
> >> >>  #include "hyperv.h"
> >> >>  
> >> >> @@ -1123,6 +1124,27 @@ void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf, const char *optarg)
> >> >>      }
> >> >>  }
> >> >>  
> >> >> +CpuDefInfoList *qmp_query_cpudefs(Error **errp)
> >> >> +{
> >> >> +    CpuDefInfoList *cpu_list = NULL;
> >> >> +    x86_def_t *def;
> >> >> +
> >> >> +    for (def = x86_defs; def; def = def->next) {
> >> >> +        CpuDefInfoList *entry;
> >> >> +        CpuDefInfo *info;
> >> >> +
> >> >> +        info = g_malloc0(sizeof(*info));
> >> >> +        info->name = g_strdup(def->name);
> >> >> +
> >> >> +        entry = g_malloc0(sizeof(*entry));
> >> >> +        entry->value = info;
> >> >> +        entry->next = cpu_list;
> >> >> +        cpu_list = entry;
> >> >> +    }
> >> >> +
> >> >> +    return cpu_list;
> >> >> +}
> >> >
> >> > How would the interface look like once we:
> >> > - let libvirt know which features are available on each CPU model
> >> >   (libvirt needs that information[1]); and
> >> 
> >> I'm not sure I understand why libvirt needs this information.  Can you elaborate?
> >
> > I see two reasons:
> >
> > - The libvirt API has functions to tell the user which features are
> >   going to be enabled for each CPU model, so it needs to know which
> >   features are enabled or not, for each machine-type + cpu-model
> >   combination, so this information can be reported proeprly.
> 
> Ok, step number one is that CPU 'features' need to be defined more
> formally.  By formally, I mean via qapi-schema.json.
> 
> Then we can extend this command to return the set of features supported
> by each CPU type.
> 
> The first step will need to sort out how this maps across architectures.
> 
> >   - Also, if libvirt can enable/disable specific CPU features in the
> >     command-line, it just makes sens to know which ones are already
> >     enabled in each built-in CPU model.
> >
> > - Probing for migration: libvirt needs to know if a given CPU model on a
> >   host can be migrated to another host. To know that, two pieces of
> >   information are needed:
> >   A) Which CPU features are visible to the guest for a specific
> >      configuration;
> >   B) Which of those features are really supported by the host
> >      hardware+kernel+QEMU, on the destination host, so it can
> >      know if migration is really possible.
> 
> Note that what QEMU thinks it exposes is not necessarily what gets
> exposed.  KVM may mask additional features.  How is this handled today?

No, what QEMU thinks it exposes actually is what gets exposed (and if it
is not, it's a bug we have to fix). This is handled using the KVM
GET_SUPPORTED_CPUID ioctl().


> 
> >> > - add machine-type-specific cpudef compatibility changes?
> >> 
> >> I think we've discussed this in IRC.  I don't think we need to worry
> >> about this.
> >
> > I remember discussing a lot about the mechanism we will use to add the
> > compatibility changes, but I don t know how the query API will look
> > like, after we implement this mechanism.
> 
> 0) User-defined CPU definitions go away
>    - We already made a big step in this direction
> 
> 1) CPU becomes a DeviceState

1.1) CPU models become classes

> 
> 2) Features are expressed as properties
> 
> 3) Same global mechanism used for everything else is used for CPUs

This is basically the compatibility mechanism we agreed upon, yes, but
what about the probing mechanism to allow libvirt to know what will be
the result of "-machine M -cpu C"[1] before actually starting a VM?

[1] By "result" I mean:
   - Whether that combination can be run properly on that host;
   - Which CPU features will be visible to the guest in case it runs.
   Both items depend on CPU model _and_ machine-type, that's why we need
   some probing mechanism that depends on the machine-type or use the
   machine-type as input.


> 
> Regards,
> 
> Anthony Liguori
> 
> >> > Would the command report different results depending on -machine?
> >> 
> >> No.
> >
> > The problem is:
> >
> > 1) We need to introduce fixes on a CPU model that changes the set of
> >    guest-visible features (add or remove a feature)[1];
> > 2) The fix has to keep compatibility, so older machine-types will
> >    keep exposing the old set of gues-visible features;
> >    - That means different machine-types will have different CPU
> >      features being exposed.
> > 3) libvirt needs to control/know which guest-visible CPU features are
> >    available to the guest (see above);
> > 4) Because of (2), the querying system used by libvirt need to depend on
> >    the CPU model and machine-type.
> >
> >
> > [1] Example:
> >     The SandyBridge model today has the "tsc-deadline" bit set, but
> >     QEMU-1.1 did not expose the tsc-deadline feature properly because of
> >     incorrect expectations about the GET_SUPPORTED_CPUID ioctl. This was
> >     fixed on qemu-1.2.
> >     
> >     That means "qemu-1.1 -machine pc-1.1 -cpu SandyBridge" does _not_
> >     expose tsc-deadline to the guest, and we need to make "qemu-1.2
> >     -machine pc-1.1 -cpu SandyBridge" _not_ expose it, too (otherwise
> >     migration from qemu-1.1 to qemu-1.2 will be broken).
> >
> >> 
> >> >
> >> > Would the command return the latest cpudef without any machine-type
> >> > hacks, and libvirt would have to query for the cpudef compatibility data
> >> > for each machine-type and combine both pieces of information itself?
> >> 
> >> I'm not sure what you mean by compatibility data.
> >
> > I mean any guest-visible compatibility bit that we will need to
> > introduce on older machine-types, when making changes on CPU models (see
> > the SandyBridge + tsc-deadline example above).
> >
> > I see two options:
> > - Libvirt queries for a [f(machine_type, cpu_model) -> cpu_features]
> >   function, that will take into account the machine-type-specific
> >   compatibility bits.
> > - Libvirt queries for a [f(cpu_model) -> cpu_features] function and a
> >   [f(machine_type) -> compatibility_changes] function, and combine both.
> >   - I don't like this approach, I am just including it as a possible
> >     alternative.
> >
> >> 
> >> Regards,
> >> 
> >> Anthony Liguori
> >> 
> >> >
> >> > [1] Note that it doesn't have to be low-level leaf-by-leaf
> >> >     register-by-register CPUID bits (I prefer a more high-level
> >> >     interface, myself), but it has to at least say "feature FOO is
> >> >     enabled/disabled" for a set of features libvirt cares about.
> >> >
> >> > -- 
> >> > Eduardo
> >> 
> >
> > -- 
> > Eduardo
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH 6/7] target-i386: add implementation of query-cpudefs

[Anthony Liguori]

Eduardo Habkost <ehabkost@redhat.com> writes:

> On Fri, Aug 10, 2012 at 11:37:30AM -0500, Anthony Liguori wrote:
>> Eduardo Habkost <ehabkost@redhat.com> writes:
>> >> > - add machine-type-specific cpudef compatibility changes?
>> >> 
>> >> I think we've discussed this in IRC.  I don't think we need to worry
>> >> about this.
>> >
>> > I remember discussing a lot about the mechanism we will use to add the
>> > compatibility changes, but I don t know how the query API will look
>> > like, after we implement this mechanism.
>> 
>> 0) User-defined CPU definitions go away
>>    - We already made a big step in this direction
>> 
>> 1) CPU becomes a DeviceState
>
> 1.1) CPU models become classes
>
>> 
>> 2) Features are expressed as properties
>> 
>> 3) Same global mechanism used for everything else is used for CPUs
>
> This is basically the compatibility mechanism we agreed upon, yes, but
> what about the probing mechanism to allow libvirt to know what will be
> the result of "-machine M -cpu C"[1] before actually starting a VM?

I think that the requirement of "before actually starting a VM" is
unreasonable.

Presumably migration compatibility checking would happen after launching
a guest so libvirt could surely delay querying the CPUID info until
after the guest has started.

There's a lot of logic involved in deciding what gets exposed to the
guest.  We don't really fully know until we've created the VCPU.  It's a
whole lot easier and saner to just create the VCPU.

Regards,

Anthony Liguori

>
> [1] By "result" I mean:
>    - Whether that combination can be run properly on that host;
>    - Which CPU features will be visible to the guest in case it runs.
>    Both items depend on CPU model _and_ machine-type, that's why we need
>    some probing mechanism that depends on the machine-type or use the
>    machine-type as input.
>
>
>> 
>> Regards,
>> 
>> Anthony Liguori
>> 
>> >> > Would the command report different results depending on -machine?
>> >> 
>> >> No.
>> >
>> > The problem is:
>> >
>> > 1) We need to introduce fixes on a CPU model that changes the set of
>> >    guest-visible features (add or remove a feature)[1];
>> > 2) The fix has to keep compatibility, so older machine-types will
>> >    keep exposing the old set of gues-visible features;
>> >    - That means different machine-types will have different CPU
>> >      features being exposed.
>> > 3) libvirt needs to control/know which guest-visible CPU features are
>> >    available to the guest (see above);
>> > 4) Because of (2), the querying system used by libvirt need to depend on
>> >    the CPU model and machine-type.
>> >
>> >
>> > [1] Example:
>> >     The SandyBridge model today has the "tsc-deadline" bit set, but
>> >     QEMU-1.1 did not expose the tsc-deadline feature properly because of
>> >     incorrect expectations about the GET_SUPPORTED_CPUID ioctl. This was
>> >     fixed on qemu-1.2.
>> >     
>> >     That means "qemu-1.1 -machine pc-1.1 -cpu SandyBridge" does _not_
>> >     expose tsc-deadline to the guest, and we need to make "qemu-1.2
>> >     -machine pc-1.1 -cpu SandyBridge" _not_ expose it, too (otherwise
>> >     migration from qemu-1.1 to qemu-1.2 will be broken).
>> >
>> >> 
>> >> >
>> >> > Would the command return the latest cpudef without any machine-type
>> >> > hacks, and libvirt would have to query for the cpudef compatibility data
>> >> > for each machine-type and combine both pieces of information itself?
>> >> 
>> >> I'm not sure what you mean by compatibility data.
>> >
>> > I mean any guest-visible compatibility bit that we will need to
>> > introduce on older machine-types, when making changes on CPU models (see
>> > the SandyBridge + tsc-deadline example above).
>> >
>> > I see two options:
>> > - Libvirt queries for a [f(machine_type, cpu_model) -> cpu_features]
>> >   function, that will take into account the machine-type-specific
>> >   compatibility bits.
>> > - Libvirt queries for a [f(cpu_model) -> cpu_features] function and a
>> >   [f(machine_type) -> compatibility_changes] function, and combine both.
>> >   - I don't like this approach, I am just including it as a possible
>> >     alternative.
>> >
>> >> 
>> >> Regards,
>> >> 
>> >> Anthony Liguori
>> >> 
>> >> >
>> >> > [1] Note that it doesn't have to be low-level leaf-by-leaf
>> >> >     register-by-register CPUID bits (I prefer a more high-level
>> >> >     interface, myself), but it has to at least say "feature FOO is
>> >> >     enabled/disabled" for a set of features libvirt cares about.
>> >> >
>> >> > -- 
>> >> > Eduardo
>> >> 
>> >
>> > -- 
>> > Eduardo
>> 
>
> -- 
> Eduardo

Re: [Qemu-devel] [PATCH 6/7] target-i386: add implementation of query-cpudefs

[Eduardo Habkost]

On Fri, Aug 10, 2012 at 12:09:44PM -0500, Anthony Liguori wrote:
> Eduardo Habkost <ehabkost@redhat.com> writes:
> 
> > On Fri, Aug 10, 2012 at 11:37:30AM -0500, Anthony Liguori wrote:
> >> Eduardo Habkost <ehabkost@redhat.com> writes:
> >> >> > - add machine-type-specific cpudef compatibility changes?
> >> >> 
> >> >> I think we've discussed this in IRC.  I don't think we need to worry
> >> >> about this.
> >> >
> >> > I remember discussing a lot about the mechanism we will use to add the
> >> > compatibility changes, but I don t know how the query API will look
> >> > like, after we implement this mechanism.
> >> 
> >> 0) User-defined CPU definitions go away
> >>    - We already made a big step in this direction
> >> 
> >> 1) CPU becomes a DeviceState
> >
> > 1.1) CPU models become classes
> >
> >> 
> >> 2) Features are expressed as properties
> >> 
> >> 3) Same global mechanism used for everything else is used for CPUs
> >
> > This is basically the compatibility mechanism we agreed upon, yes, but
> > what about the probing mechanism to allow libvirt to know what will be
> > the result of "-machine M -cpu C"[1] before actually starting a VM?
> 
> I think that the requirement of "before actually starting a VM" is
> unreasonable.

How is it unreasonable to expect an API where you can know what will be
the results of an operation before actually running it? Maybe it doesn't
fit in the beautiful and elegant model you are trying to push, but that
doesn't make it unreasonable.


> 
> Presumably migration compatibility checking would happen after launching
> a guest so libvirt could surely delay querying the CPUID info until
> after the guest has started.

This is what I call unreasonable. A management layer needs to know if a
host can run a VM before trying to migrate it, so the software or the
user can take better decisions about migration before asking the VMs to
be actually migrated.

Note that I don't argue for every single CPUID bit to be available and
queriable, but the (un)availability of some features need to be
predictable.


> 
> There's a lot of logic involved in deciding what gets exposed to the
> guest.  We don't really fully know until we've created the VCPU.  It's a
> whole lot easier and saner to just create the VCPU.

If the logic is too complex and unpredictable, we have to make it
clearer and more predictable. It's important to do so even if libvirt
didn't need a probing interface, otherwise we would never be sure if the
code is migration-safe.

> 
> Regards,
> 
> Anthony Liguori
> 
> >
> > [1] By "result" I mean:
> >    - Whether that combination can be run properly on that host;
> >    - Which CPU features will be visible to the guest in case it runs.
> >    Both items depend on CPU model _and_ machine-type, that's why we need
> >    some probing mechanism that depends on the machine-type or use the
> >    machine-type as input.
> >
> >
> >> 
> >> Regards,
> >> 
> >> Anthony Liguori
> >> 
> >> >> > Would the command report different results depending on -machine?
> >> >> 
> >> >> No.
> >> >
> >> > The problem is:
> >> >
> >> > 1) We need to introduce fixes on a CPU model that changes the set of
> >> >    guest-visible features (add or remove a feature)[1];
> >> > 2) The fix has to keep compatibility, so older machine-types will
> >> >    keep exposing the old set of gues-visible features;
> >> >    - That means different machine-types will have different CPU
> >> >      features being exposed.
> >> > 3) libvirt needs to control/know which guest-visible CPU features are
> >> >    available to the guest (see above);
> >> > 4) Because of (2), the querying system used by libvirt need to depend on
> >> >    the CPU model and machine-type.
> >> >
> >> >
> >> > [1] Example:
> >> >     The SandyBridge model today has the "tsc-deadline" bit set, but
> >> >     QEMU-1.1 did not expose the tsc-deadline feature properly because of
> >> >     incorrect expectations about the GET_SUPPORTED_CPUID ioctl. This was
> >> >     fixed on qemu-1.2.
> >> >     
> >> >     That means "qemu-1.1 -machine pc-1.1 -cpu SandyBridge" does _not_
> >> >     expose tsc-deadline to the guest, and we need to make "qemu-1.2
> >> >     -machine pc-1.1 -cpu SandyBridge" _not_ expose it, too (otherwise
> >> >     migration from qemu-1.1 to qemu-1.2 will be broken).
> >> >
> >> >> 
> >> >> >
> >> >> > Would the command return the latest cpudef without any machine-type
> >> >> > hacks, and libvirt would have to query for the cpudef compatibility data
> >> >> > for each machine-type and combine both pieces of information itself?
> >> >> 
> >> >> I'm not sure what you mean by compatibility data.
> >> >
> >> > I mean any guest-visible compatibility bit that we will need to
> >> > introduce on older machine-types, when making changes on CPU models (see
> >> > the SandyBridge + tsc-deadline example above).
> >> >
> >> > I see two options:
> >> > - Libvirt queries for a [f(machine_type, cpu_model) -> cpu_features]
> >> >   function, that will take into account the machine-type-specific
> >> >   compatibility bits.
> >> > - Libvirt queries for a [f(cpu_model) -> cpu_features] function and a
> >> >   [f(machine_type) -> compatibility_changes] function, and combine both.
> >> >   - I don't like this approach, I am just including it as a possible
> >> >     alternative.
> >> >
> >> >> 
> >> >> Regards,
> >> >> 
> >> >> Anthony Liguori
> >> >> 
> >> >> >
> >> >> > [1] Note that it doesn't have to be low-level leaf-by-leaf
> >> >> >     register-by-register CPUID bits (I prefer a more high-level
> >> >> >     interface, myself), but it has to at least say "feature FOO is
> >> >> >     enabled/disabled" for a set of features libvirt cares about.
> >> >> >
> >> >> > -- 
> >> >> > Eduardo
> >> >> 
> >> >
> >> > -- 
> >> > Eduardo
> >> 
> >
> > -- 
> > Eduardo
> 

-- 
Eduardo

[Qemu-devel] [PATCH 3/7] qapi: add query-machines command

[Anthony Liguori]

This provides the same output as -M ? but in a structured way.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 qapi-schema.json |   28 ++++++++++++++++++++++++++++
 qmp-commands.hx  |    6 ++++++
 vl.c             |   31 +++++++++++++++++++++++++++++++
 3 files changed, 65 insertions(+), 0 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index a938c8d..1eb0b0f 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2216,3 +2216,31 @@
 # Since: 0.14.0
 ##
 { 'command': 'closefd', 'data': {'fdname': 'str'} }
+
+##
+# @MachineInfo:
+#
+# Information describing a machine.
+#
+# @name: the name of the machine
+#
+# @alias: #optional an alias for the machine name
+#
+# @default: #optional whether the machine is default
+#
+# Since: 1.2.0
+##
+{ 'type': 'MachineInfo',
+  'data': { 'name': 'str', '*alias': 'str',
+            '*is-default': 'bool' } }
+
+##
+# @query-machines:
+#
+# Return a list of supported machines
+#
+# Returns: a list of MachineInfo
+#
+# Since: 1.2.0
+##
+{ 'command': 'query-machines', 'returns': ['MachineInfo'] }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 52127a9..f343772 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2224,3 +2224,9 @@ EQMP
         .mhandler.cmd_new = qmp_marshal_input_device_list_properties,
     },
 
+    {
+        .name       = "query-machines",
+        .args_type  = "",
+        .mhandler.cmd_new = qmp_marshal_input_query_machines,
+    },
+
diff --git a/vl.c b/vl.c
index ad9b036..084d671 100644
--- a/vl.c
+++ b/vl.c
@@ -1213,6 +1213,37 @@ QEMUMachine *find_default_machine(void)
     return NULL;
 }
 
+MachineInfoList *qmp_query_machines(Error **errp)
+{
+    MachineInfoList *mach_list = NULL;
+    QEMUMachine *m;
+
+    for (m = first_machine; m; m = m->next) {
+        MachineInfoList *entry;
+        MachineInfo *info;
+
+        info = g_malloc0(sizeof(*info));
+        if (m->is_default) {
+            info->has_is_default = true;
+            info->is_default = true;
+        }
+
+        if (m->alias) {
+            info->has_alias = true;
+            info->alias = g_strdup(m->alias);
+        }
+
+        info->name = g_strdup(m->name);
+
+        entry = g_malloc0(sizeof(*entry));
+        entry->value = info;
+        entry->next = mach_list;
+        mach_list = entry;
+    }
+
+    return mach_list;
+}
+
 /***********************************************************/
 /* main execution loop */
 
-- 
1.7.5.4