[Qemu-devel] [PATCH 0/7] qapi: add commands to remove the need (v2)

[Qemu-devel] [PATCH 0/7] qapi: add commands to remove the need (v2)

[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,?
---
v1 -> v2
 - rename query-cpudefs -> query-cpu-definitions

[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 bd9c450..191a889 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1729,6 +1729,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 ac46638..52127a9 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2217,3 +2217,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 191a889..a938c8d 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1363,9 +1363,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' } }
@@ -1382,10 +1380,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' },
@@ -1421,9 +1416,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' },
@@ -1442,9 +1435,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' },
@@ -1721,8 +1712,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

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

[Eric Blake]

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

On 08/10/2012 10:04 AM, Anthony Liguori wrote:
> 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 191a889..a938c8d 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1363,9 +1363,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

Per https://lists.gnu.org/archive/html/qemu-devel/2012-08/msg01416.html,
this should be 1.2.0 (throughout the series).

> @@ -1382,10 +1380,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

Yay, getting rid of bad grammar in the process (s/It's/Its/ if the
comment were to remain).

-- 
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 2/7] qapi: mark QOM commands stable

[Anthony Liguori]

Eric Blake <eblake@redhat.com> writes:

> On 08/10/2012 10:04 AM, Anthony Liguori wrote:
>> 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 191a889..a938c8d 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -1363,9 +1363,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
>
> Per https://lists.gnu.org/archive/html/qemu-devel/2012-08/msg01416.html,
> this should be 1.2.0 (throughout the series).

I'll do a follow up to fix this across the board for the entire file.
That's what I took away from your previous comment.

Regards,

Anthony Liguori

>
>> @@ -1382,10 +1380,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
>
> Yay, getting rid of bad grammar in the process (s/It's/Its/ if the
> comment were to remain).
>
> -- 
> Eric Blake   eblake@redhat.com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org

[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

[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

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

[Peter Maydell]

On 10 August 2012 17:04, Anthony Liguori <aliguori@us.ibm.com> wrote:
> This lets us provide a default implementation of a symbol which targets can
> override.
>
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

I'm sure you'll be thrilled to hear that this doesn't seem to break MacOS
builds :-)

-- PMM

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

[Anthony Liguori]

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

> On 10 August 2012 17:04, Anthony Liguori <aliguori@us.ibm.com> wrote:
>> This lets us provide a default implementation of a symbol which targets can
>> override.
>>
>> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>
> I'm sure you'll be thrilled to hear that this doesn't seem to break MacOS
> builds :-)

Thank you for testing it.  I neglected to mention that I did a fair bit
of investigation before hand and was able to confirm that all platforms
we care about (Windows, Linux/Unix, OS X) and all compilers we care
about (GCC, LLVM) support weak symbols.

There may be different attribute names across compilers--it wasn't very
clear to me, but they definitely all have the feature in some form.

Regards,

Anthony Liguori

>
> -- PMM

[Qemu-devel] [PATCH 5/7] qapi: add query-cpu-definitions command (v2)

[Anthony Liguori]

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

To accommodate 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>
---
v1 -> v2
 - rename query-cpudefs -> query-cpu-definitions
---
 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 1eb0b0f..f1da7ee 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2244,3 +2244,26 @@
 # Since: 1.2.0
 ##
 { 'command': 'query-machines', 'returns': ['MachineInfo'] }
+
+##
+# @CpuDefinitionInfo:
+#
+# Virtual CPU definition.
+#
+# @name: the name of the CPU definition
+#
+# Since: 1.2.0
+##
+{ 'type': 'CpuDefinitionInfo',
+  'data': { 'name': 'str' } }
+
+##
+# @query-cpu-definitions:
+#
+# Return a list of supported virtual CPU definitions
+#
+# Returns: a list of CpuDefInfo
+#
+# Since: 1.2.0
+##
+{ 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'] }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index f343772..4da2b86 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2230,3 +2230,9 @@ EQMP
         .mhandler.cmd_new = qmp_marshal_input_query_machines,
     },
 
+    {
+        .name       = "query-cpu-definitions",
+        .args_type  = "",
+        .mhandler.cmd_new = qmp_marshal_input_query_cpu_definitions,
+    },
+
diff --git a/qmp.c b/qmp.c
index 254a32f..6c1e4e8 100644
--- a/qmp.c
+++ b/qmp.c
@@ -467,3 +467,9 @@ DevicePropertyInfoList *qmp_device_list_properties(const char *typename,
 
     return prop_list;
 }
+
+CpuDefinitionInfoList GCC_WEAK *qmp_query_cpu_definitions(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-cpu-definitions (v2)

[Anthony Liguori]

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
v1 -> v2
 - rename query-cpudefs -> query-cpu-definitions
---
 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 880cfea..6d5d0d6 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"
 
@@ -1125,6 +1126,27 @@ void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf, const char *optarg)
     }
 }
 
+CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
+{
+    CpuDefinitionInfoList *cpu_list = NULL;
+    x86_def_t *def;
+
+    for (def = x86_defs; def; def = def->next) {
+        CpuDefinitionInfoList *entry;
+        CpuDefinitionInfo *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-cpu-definitions (v2)

[Anthony Liguori]

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
v1 -> v2
 - rename query-cpudefs -> query-cpu-definitions
---
 target-ppc/translate_init.c |   26 ++++++++++++++++++++++++++
 1 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 5742229..6fe4168 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -27,6 +27,7 @@
 #include "gdbstub.h"
 #include <kvm.h>
 #include "kvm_ppc.h"
+#include "qmp-commands.h"
 
 //#define PPC_DUMP_CPU
 //#define PPC_DEBUG_SPR
@@ -10345,6 +10346,31 @@ void ppc_cpu_list (FILE *f, fprintf_function cpu_fprintf)
     }
 }
 
+CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
+{
+    CpuDefinitionInfoList *cpu_list = NULL;
+    int i;
+
+    for (i = 0; i < ARRAY_SIZE(ppc_defs); i++) {
+        CpuDefinitionInfoList *entry;
+        CpuDefinitionInfo *info;
+
+        if (!ppc_cpu_usable(&ppc_defs[i])) {
+            continue;
+        }
+
+        info = g_malloc0(sizeof(*info));
+        info->name = g_strdup(ppc_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 0/7] qapi: add commands to remove the need (v2)

[Luiz Capitulino]

On Fri, 10 Aug 2012 11:04:08 -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.

Applied to the qmp branch, thanks.

Re: [Qemu-devel] [PATCH 0/7] qapi: add commands to remove the need (v2)

[Luiz Capitulino]

On Mon, 13 Aug 2012 14:16:58 -0300
Luiz Capitulino <lcapitulino@redhat.com> wrote:

> On Fri, 10 Aug 2012 11:04:08 -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.
> 
> Applied to the qmp branch, thanks.

Hmm, this series broke ppc-softmmu for me:

In file included from /home/lcapitulino/work/src/qmp-unstable/target-ppc/translate_init.c:30:0,
                 from /home/lcapitulino/work/src/qmp-unstable/target-ppc/translate.c:9404:
../qmp-commands.h:23:1: error: unknown type name ‘QDict’
../qmp-commands.h:23:68: error: unknown type name ‘QObject’

But it's not its fault. The problem here is probably a patch in my error
series that is doing header cleanup and qmp-commands.h was probably relying
on qapi-types.h (or some of its include files) including qdict.h.

I'm going to include the following patch in my pull request:

Subject: [PATCH 36/48] scripts: qapi-commands.py: qmp-commands.h: include
 qdict.h

qmp-commands.h declares several functions that have arguments of
type QDict. However, qdict.h is not included. This will cause a
build breakage when a file includes qmp-commands.h but doesn't
include qdict.h.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 scripts/qapi-commands.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 9eed40e..3c4678d 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -342,6 +342,7 @@ def gen_command_decl_prologue(header, guard, prefix=""):
 #define %(guard)s
 
 #include "%(prefix)sqapi-types.h"
+#include "qdict.h"
 #include "error.h"
 
 ''',
-- 
1.7.11.2.249.g31c7954.dirty

Re: [Qemu-devel] [PATCH 0/7] qapi: add commands to remove the need (v2)

[Anthony Liguori]

Luiz Capitulino <lcapitulino@redhat.com> writes:

> On Mon, 13 Aug 2012 14:16:58 -0300
> Luiz Capitulino <lcapitulino@redhat.com> wrote:
>
>> On Fri, 10 Aug 2012 11:04:08 -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.
>> 
>> Applied to the qmp branch, thanks.
>
> Hmm, this series broke ppc-softmmu for me:

Curious, I'll look into that.  Thanks

Regards,

Anthony Liguori

>
> In file included from /home/lcapitulino/work/src/qmp-unstable/target-ppc/translate_init.c:30:0,
>                  from /home/lcapitulino/work/src/qmp-unstable/target-ppc/translate.c:9404:
> ../qmp-commands.h:23:1: error: unknown type name ‘QDict’
> ../qmp-commands.h:23:68: error: unknown type name ‘QObject’
>
> But it's not its fault. The problem here is probably a patch in my error
> series that is doing header cleanup and qmp-commands.h was probably relying
> on qapi-types.h (or some of its include files) including qdict.h.
>
> I'm going to include the following patch in my pull request:
>
> Subject: [PATCH 36/48] scripts: qapi-commands.py: qmp-commands.h: include
>  qdict.h
>
> qmp-commands.h declares several functions that have arguments of
> type QDict. However, qdict.h is not included. This will cause a
> build breakage when a file includes qmp-commands.h but doesn't
> include qdict.h.
>
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
>  scripts/qapi-commands.py | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
> index 9eed40e..3c4678d 100644
> --- a/scripts/qapi-commands.py
> +++ b/scripts/qapi-commands.py
> @@ -342,6 +342,7 @@ def gen_command_decl_prologue(header, guard, prefix=""):
>  #define %(guard)s
>  
>  #include "%(prefix)sqapi-types.h"
> +#include "qdict.h"
>  #include "error.h"
>  
>  ''',
> -- 
> 1.7.11.2.249.g31c7954.dirty

Re: [Qemu-devel] [PATCH 0/7] qapi: add commands to remove the need (v2)

[Stefan Weil]

Am 13.08.2012 19:16, schrieb Luiz Capitulino:
> On Fri, 10 Aug 2012 11:04:08 -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.
>
> Applied to the qmp branch, thanks.
>


The series breaks cross compilation of QEMU for w32 on Debian Linux:

   LINK  arm-softmmu/qemu-system-armw.exe
../qmp-marshal.o: In function `qmp_marshal_input_query_cpu_definitions':
/home/stefan/w32/qmp-marshal.c:2585: undefined reference to 
`_qmp_query_cpu_definitions'

Weak symbols obviously use a different name mangling, therefore 
qmp_query_cpu_definitions
is not found by the linker.

Adding GCC_WEAK to the declaration of qmp_query_cpu_definitions in 
generated file
qmp-commands.h fixes that.

Regards,

Stefan Weil

Re: [Qemu-devel] [PATCH 0/7] qapi: add commands to remove the need (v2)

[Luiz Capitulino]

On Wed, 15 Aug 2012 08:07:57 +0200
Stefan Weil <sw@weilnetz.de> wrote:

> Am 13.08.2012 19:16, schrieb Luiz Capitulino:
> > On Fri, 10 Aug 2012 11:04:08 -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.
> >
> > Applied to the qmp branch, thanks.
> >
> 
> 
> The series breaks cross compilation of QEMU for w32 on Debian Linux:
> 
>    LINK  arm-softmmu/qemu-system-armw.exe
> ../qmp-marshal.o: In function `qmp_marshal_input_query_cpu_definitions':
> /home/stefan/w32/qmp-marshal.c:2585: undefined reference to 
> `_qmp_query_cpu_definitions'

Does this patch fix it?

 http://lists.gnu.org/archive/html/qemu-devel/2012-08/msg02676.html

> 
> Weak symbols obviously use a different name mangling, therefore 
> qmp_query_cpu_definitions
> is not found by the linker.
> 
> Adding GCC_WEAK to the declaration of qmp_query_cpu_definitions in 
> generated file
> qmp-commands.h fixes that.
> 
> Regards,
> 
> Stefan Weil
> 

Re: [Qemu-devel] [PATCH 0/7] qapi: add commands to remove the need (v2)

[Stefan Weil]

Am 15.08.2012 18:53, schrieb Luiz Capitulino:
> On Wed, 15 Aug 2012 08:07:57 +0200
> Stefan Weil <sw@weilnetz.de> wrote:
>
>> Am 13.08.2012 19:16, schrieb Luiz Capitulino:
>>> On Fri, 10 Aug 2012 11:04:08 -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.
>>> Applied to the qmp branch, thanks.
>>>
>>
>> The series breaks cross compilation of QEMU for w32 on Debian Linux:
>>
>>     LINK  arm-softmmu/qemu-system-armw.exe
>> ../qmp-marshal.o: In function `qmp_marshal_input_query_cpu_definitions':
>> /home/stefan/w32/qmp-marshal.c:2585: undefined reference to
>> `_qmp_query_cpu_definitions'
> Does this patch fix it?
>
>   http://lists.gnu.org/archive/html/qemu-devel/2012-08/msg02676.html

Yes, Anthony's patch fixes that. I just noticed that he already
applied it to git master.

The solution which I had suggested below would have been much
simpler. Using GCC_WEAK in the declaration worked for me with
gcc from Debian Lenny, too.

Which gcc requires different handling for w32 and non-w32?


> Weak symbols obviously use a different name mangling, therefore
> qmp_query_cpu_definitions is not found by the linker.
>
> Adding GCC_WEAK to the declaration of qmp_query_cpu_definitions in
> generated file
> qmp-commands.h fixes that.
>
> Regards,
>
> Stefan Weil
>

[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 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

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 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 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

[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

[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 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

[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 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 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