[Qemu-devel] [PATCH 00/12] qdev: cleanup legacy properties

[Qemu-devel] [PATCH 00/12] qdev: cleanup legacy properties

[Paolo Bonzini]

The conversion of qdev to QOM brought with it legacy properties.
Legacy properties are always have a string type (the accessors always
call visit_type_str), and were used to support -device syntax while
keeping QOM properties strongly typed.  For example, an hex8 property
is registered twice, once as an integer-typed property and once as a
legacy property that enforces base 16 for its input.

However, when introducing legacy properties, the hex8/32/64 had a small
change applied: the previously-optional "0x" prefix became mandatory,
and an error was raised if you omitted it.  This was in preparation
for making the legacy properties read-only, and changing the hex8/32/64
properties to uint8/32/64.  This series does exactly this in patches 1-6.

On the printing side, legacy properties are used by "info qtree" to
tweak its presentation: strings are quoted, hex8/32/64 properties are
printed in hexadecimal, and so on.  In this series, patches 7-10 add a
"human" mode to StringOutputVisitors.  This mode employs a slightly
different presentation, more suitable for human consumption, but its
output cannot be sent back to a StringInputVisitor.  The main change
is that numbers are printed in both decimal and 0x-prefixed hexadecimal.
This lets us drop hex8/32/64 property types.

Finally, patches 11-12 clean up the type names used for properties.
These are always QAPI names, so that in the future QOM introspection
can piggyback on QAPI introspection for describing property types.

Paolo Bonzini (12):
  qapi: add size parser to StringInputVisitor
  qdev: sizes are now parsed by StringInputVisitor
  qdev: remove legacy parsers for hex8/32/64
  qdev: legacy properties are now read-only
  qdev: legacy properties are just strings
  qdev: inline qdev_prop_parse
  qapi: add human mode to StringOutputVisitor
  qdev: use human mode in "info qtree"
  qdev: remove most legacy printers
  qdev: remove hex8/32/64 property types
  qdev: add enum property types to QAPI schema
  qdev: use QAPI type names for properties

 hw/audio/adlib.c                     |   2 +-
 hw/audio/cs4231a.c                   |   2 +-
 hw/audio/gus.c                       |   2 +-
 hw/audio/pcspk.c                     |   2 +-
 hw/audio/sb16.c                      |   4 +-
 hw/block/fdc.c                       |   2 +-
 hw/char/debugcon.c                   |   4 +-
 hw/char/parallel.c                   |   2 +-
 hw/char/serial-isa.c                 |   2 +-
 hw/core/qdev-properties-system.c     |  12 ++-
 hw/core/qdev-properties.c            | 204 +++--------------------------------
 hw/core/qdev.c                       |  38 +------
 hw/display/g364fb.c                  |   2 +-
 hw/display/tcx.c                     |   4 +-
 hw/dma/i82374.c                      |   2 +-
 hw/dma/sun4m_iommu.c                 |   2 +-
 hw/i386/kvm/i8254.c                  |   8 +-
 hw/ide/isa.c                         |   4 +-
 hw/ide/qdev.c                        |   2 +-
 hw/intc/i8259_common.c               |   6 +-
 hw/isa/pc87312.c                     |   2 +-
 hw/misc/applesmc.c                   |   2 +-
 hw/misc/debugexit.c                  |   4 +-
 hw/misc/eccmemctl.c                  |   2 +-
 hw/net/ne2000-isa.c                  |   2 +-
 hw/nvram/fw_cfg.c                    |   4 +-
 hw/ppc/spapr_pci.c                   |  16 +--
 hw/scsi/megasas.c                    |   2 +-
 hw/scsi/scsi-disk.c                  |   6 +-
 hw/sd/sdhci.c                        |   4 +-
 hw/timer/i8254.c                     |   2 +-
 hw/timer/m48t59.c                    |   4 +-
 hw/timer/mc146818rtc.c               |  14 +--
 hw/usb/host-libusb.c                 |   4 +-
 hw/virtio/virtio-pci.c               |   6 +-
 include/hw/block/block.h             |   6 --
 include/hw/qdev-core.h               |   1 -
 include/hw/qdev-dma.h                |   2 +-
 include/hw/qdev-properties.h         |  11 --
 include/qapi/string-output-visitor.h |   2 +-
 include/qemu-common.h                |   8 --
 include/qom/object.h                 |   3 +-
 qapi-schema.json                     |  58 ++++++++++
 qapi/string-input-visitor.c          |  24 +++++
 qapi/string-output-visitor.c         |  55 +++++++++-
 qdev-monitor.c                       |   6 +-
 qom/object.c                         |   4 +-
 tests/test-string-output-visitor.c   |   2 +-
 tests/test-visitor-serialization.c   |   2 +-
 49 files changed, 235 insertions(+), 329 deletions(-)

-- 
1.8.4.2

[Qemu-devel] [PATCH 01/12] qapi: add size parser to StringInputVisitor

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qapi/string-input-visitor.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
index 8f1bc41..793548a 100644
--- a/qapi/string-input-visitor.c
+++ b/qapi/string-input-visitor.c
@@ -14,6 +14,7 @@
 #include "qapi/string-input-visitor.h"
 #include "qapi/visitor-impl.h"
 #include "qapi/qmp/qerror.h"
+#include "qemu/option.h"
 
 struct StringInputVisitor
 {
@@ -41,6 +42,28 @@ static void parse_type_int(Visitor *v, int64_t *obj, const char *name,
     *obj = val;
 }
 
+static void parse_type_size(Visitor *v, uint64_t *obj, const char *name,
+                            Error **errp)
+{
+    StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v);
+    Error *err = NULL;
+    uint64_t val;
+
+    if (siv->string) {
+        parse_option_size(name, siv->string, &val, &err);
+    } else {
+        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
+                  "size");
+        return;
+    }
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+
+    *obj = val;
+}
+
 static void parse_type_bool(Visitor *v, bool *obj, const char *name,
                             Error **errp)
 {
@@ -128,6 +151,7 @@ StringInputVisitor *string_input_visitor_new(const char *str)
 
     v->visitor.type_enum = input_type_enum;
     v->visitor.type_int = parse_type_int;
+    v->visitor.type_size = parse_type_size;
     v->visitor.type_bool = parse_type_bool;
     v->visitor.type_str = parse_type_str;
     v->visitor.type_number = parse_type_number;
-- 
1.8.4.2

Re: [Qemu-devel] [PATCH 01/12] qapi: add size parser to StringInputVisitor

[Eric Blake]

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

On 01/30/2014 06:09 AM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  qapi/string-input-visitor.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
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: 604 bytes --]

Re: [Qemu-devel] [PATCH 01/12] qapi: add size parser to StringInputVisitor

[Andreas Färber]

Am 30.01.2014 14:09, schrieb Paolo Bonzini:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  qapi/string-input-visitor.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
> index 8f1bc41..793548a 100644
> --- a/qapi/string-input-visitor.c
> +++ b/qapi/string-input-visitor.c
> @@ -14,6 +14,7 @@
>  #include "qapi/string-input-visitor.h"
>  #include "qapi/visitor-impl.h"
>  #include "qapi/qmp/qerror.h"
> +#include "qemu/option.h"
>  
>  struct StringInputVisitor
>  {
> @@ -41,6 +42,28 @@ static void parse_type_int(Visitor *v, int64_t *obj, const char *name,
>      *obj = val;
>  }
>  
> +static void parse_type_size(Visitor *v, uint64_t *obj, const char *name,
> +                            Error **errp)
> +{
> +    StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v);
> +    Error *err = NULL;
> +    uint64_t val;
> +
> +    if (siv->string) {
> +        parse_option_size(name, siv->string, &val, &err);
> +    } else {
> +        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
> +                  "size");

error_setg()?

Otherwise looks fine. CC'ing Luiz.

Andreas

> +        return;
> +    }
> +    if (err) {
> +        error_propagate(errp, err);
> +        return;
> +    }
> +
> +    *obj = val;
> +}
> +
>  static void parse_type_bool(Visitor *v, bool *obj, const char *name,
>                              Error **errp)
>  {
> @@ -128,6 +151,7 @@ StringInputVisitor *string_input_visitor_new(const char *str)
>  
>      v->visitor.type_enum = input_type_enum;
>      v->visitor.type_int = parse_type_int;
> +    v->visitor.type_size = parse_type_size;
>      v->visitor.type_bool = parse_type_bool;
>      v->visitor.type_str = parse_type_str;
>      v->visitor.type_number = parse_type_number;
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 01/12] qapi: add size parser to StringInputVisitor

[Paolo Bonzini]

Il 05/02/2014 18:13, Andreas Färber ha scritto:
> > +        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
> > +                  "size");
>
> error_setg()?

We're still using error_set for existing errors.  In this case, the code 
is taken from qdev-properties.c.

Paolo

> Otherwise looks fine. CC'ing Luiz.

Re: [Qemu-devel] [PATCH 01/12] qapi: add size parser to StringInputVisitor

[Eric Blake]

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

On 02/05/2014 10:18 AM, Paolo Bonzini wrote:
> Il 05/02/2014 18:13, Andreas Färber ha scritto:
>> > +        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name :
>> "null",
>> > +                  "size");
>>
>> error_setg()?
> 
> We're still using error_set for existing errors.  In this case, the code
> is taken from qdev-properties.c.

error_setg uses the generic category, but here, we specifically want the
INVALID_PARAMETER_TYPE category.

-- 
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: 604 bytes --]

[Qemu-devel] [PATCH 02/12] qdev: sizes are now parsed by StringInputVisitor

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/core/qdev-properties.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index b949f0e..da37710 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -1140,16 +1140,6 @@ static void set_size(Object *obj, Visitor *v, void *opaque,
     visit_type_size(v, ptr, name, errp);
 }
 
-static int parse_size(DeviceState *dev, Property *prop, const char *str)
-{
-    uint64_t *ptr = qdev_get_prop_ptr(dev, prop);
-
-    if (str != NULL) {
-        parse_option_size(prop->name, str, ptr, &error_abort);
-    }
-    return 0;
-}
-
 static int print_size(DeviceState *dev, Property *prop, char *dest, size_t len)
 {
     static const char suffixes[] = { 'B', 'K', 'M', 'G', 'T' };
@@ -1171,7 +1161,6 @@ static int print_size(DeviceState *dev, Property *prop, char *dest, size_t len)
 
 PropertyInfo qdev_prop_size = {
     .name  = "size",
-    .parse = parse_size,
     .print = print_size,
     .get = get_size,
     .set = set_size,
-- 
1.8.4.2

Re: [Qemu-devel] [PATCH 02/12] qdev: sizes are now parsed by StringInputVisitor

[Eric Blake]

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

On 01/30/2014 06:09 AM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/core/qdev-properties.c | 11 -----------
>  1 file changed, 11 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
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: 604 bytes --]

[Qemu-devel] [PATCH 03/12] qdev: remove legacy parsers for hex8/32/64

[Paolo Bonzini]

The hexNN property types have not been accepting values not prefixed
by "0x" since QEMU 1.2.  Parse those values as decimals now.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/core/qdev-properties.c | 54 -----------------------------------------------
 1 file changed, 54 deletions(-)

diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index da37710..e223ce1 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -189,23 +189,6 @@ PropertyInfo qdev_prop_uint8 = {
 
 /* --- 8bit hex value --- */
 
-static int parse_hex8(DeviceState *dev, Property *prop, const char *str)
-{
-    uint8_t *ptr = qdev_get_prop_ptr(dev, prop);
-    char *end;
-
-    if (str[0] != '0' || str[1] != 'x') {
-        return -EINVAL;
-    }
-
-    *ptr = strtoul(str, &end, 16);
-    if ((*end != '\0') || (end == str)) {
-        return -EINVAL;
-    }
-
-    return 0;
-}
-
 static int print_hex8(DeviceState *dev, Property *prop, char *dest, size_t len)
 {
     uint8_t *ptr = qdev_get_prop_ptr(dev, prop);
@@ -215,7 +198,6 @@ static int print_hex8(DeviceState *dev, Property *prop, char *dest, size_t len)
 PropertyInfo qdev_prop_hex8 = {
     .name  = "uint8",
     .legacy_name  = "hex8",
-    .parse = parse_hex8,
     .print = print_hex8,
     .get   = get_uint8,
     .set   = set_uint8,
@@ -320,23 +302,6 @@ PropertyInfo qdev_prop_int32 = {
 
 /* --- 32bit hex value --- */
 
-static int parse_hex32(DeviceState *dev, Property *prop, const char *str)
-{
-    uint32_t *ptr = qdev_get_prop_ptr(dev, prop);
-    char *end;
-
-    if (str[0] != '0' || str[1] != 'x') {
-        return -EINVAL;
-    }
-
-    *ptr = strtoul(str, &end, 16);
-    if ((*end != '\0') || (end == str)) {
-        return -EINVAL;
-    }
-
-    return 0;
-}
-
 static int print_hex32(DeviceState *dev, Property *prop, char *dest, size_t len)
 {
     uint32_t *ptr = qdev_get_prop_ptr(dev, prop);
@@ -346,7 +311,6 @@ static int print_hex32(DeviceState *dev, Property *prop, char *dest, size_t len)
 PropertyInfo qdev_prop_hex32 = {
     .name  = "uint32",
     .legacy_name  = "hex32",
-    .parse = parse_hex32,
     .print = print_hex32,
     .get   = get_uint32,
     .set   = set_uint32,
@@ -387,23 +351,6 @@ PropertyInfo qdev_prop_uint64 = {
 
 /* --- 64bit hex value --- */
 
-static int parse_hex64(DeviceState *dev, Property *prop, const char *str)
-{
-    uint64_t *ptr = qdev_get_prop_ptr(dev, prop);
-    char *end;
-
-    if (str[0] != '0' || str[1] != 'x') {
-        return -EINVAL;
-    }
-
-    *ptr = strtoull(str, &end, 16);
-    if ((*end != '\0') || (end == str)) {
-        return -EINVAL;
-    }
-
-    return 0;
-}
-
 static int print_hex64(DeviceState *dev, Property *prop, char *dest, size_t len)
 {
     uint64_t *ptr = qdev_get_prop_ptr(dev, prop);
@@ -413,7 +360,6 @@ static int print_hex64(DeviceState *dev, Property *prop, char *dest, size_t len)
 PropertyInfo qdev_prop_hex64 = {
     .name  = "uint64",
     .legacy_name  = "hex64",
-    .parse = parse_hex64,
     .print = print_hex64,
     .get   = get_uint64,
     .set   = set_uint64,
-- 
1.8.4.2

Re: [Qemu-devel] [PATCH 03/12] qdev: remove legacy parsers for hex8/32/64

[Eric Blake]

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

On 01/30/2014 06:09 AM, Paolo Bonzini wrote:
> The hexNN property types have not been accepting values not prefixed
> by "0x" since QEMU 1.2.  Parse those values as decimals now.

It also adds parsing of hex prefixed with "0X" - probably also worth
mentioning in the commit message.

> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/core/qdev-properties.c | 54 -----------------------------------------------
>  1 file changed, 54 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
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: 604 bytes --]

[Qemu-devel] [PATCH 04/12] qdev: legacy properties are now read-only

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/core/qdev-properties.c | 10 +---------
 hw/core/qdev.c            | 30 ++----------------------------
 include/hw/qdev-core.h    |  1 -
 3 files changed, 3 insertions(+), 38 deletions(-)

diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index e223ce1..a60a183 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -936,15 +936,7 @@ void error_set_from_qdev_prop_error(Error **errp, int ret, DeviceState *dev,
 void qdev_prop_parse(DeviceState *dev, const char *name, const char *value,
                      Error **errp)
 {
-    char *legacy_name;
-
-    legacy_name = g_strdup_printf("legacy-%s", name);
-    if (object_property_get_type(OBJECT(dev), legacy_name, NULL)) {
-        object_property_parse(OBJECT(dev), value, legacy_name, errp);
-    } else {
-        object_property_parse(OBJECT(dev), value, name, errp);
-    }
-    g_free(legacy_name);
+    object_property_parse(OBJECT(dev), value, name, errp);
 }
 
 void qdev_prop_set_bit(DeviceState *dev, const char *name, bool value)
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 82a9123..7c1b732 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -578,31 +578,6 @@ static void qdev_get_legacy_property(Object *obj, Visitor *v, void *opaque,
     visit_type_str(v, &ptr, name, errp);
 }
 
-static void qdev_set_legacy_property(Object *obj, Visitor *v, void *opaque,
-                                     const char *name, Error **errp)
-{
-    DeviceState *dev = DEVICE(obj);
-    Property *prop = opaque;
-    Error *local_err = NULL;
-    char *ptr = NULL;
-    int ret;
-
-    if (dev->realized) {
-        qdev_prop_set_after_realize(dev, name, errp);
-        return;
-    }
-
-    visit_type_str(v, &ptr, name, &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
-        return;
-    }
-
-    ret = prop->info->parse(dev, prop, ptr);
-    error_set_from_qdev_prop_error(errp, ret, dev, prop, ptr);
-    g_free(ptr);
-}
-
 /**
  * @qdev_add_legacy_property - adds a legacy property
  *
@@ -618,8 +593,7 @@ void qdev_property_add_legacy(DeviceState *dev, Property *prop,
     gchar *name, *type;
 
     /* Register pointer properties as legacy properties */
-    if (!prop->info->print && !prop->info->parse &&
-        (prop->info->set || prop->info->get)) {
+    if (!prop->info->print && prop->info->get) {
         return;
     }
 
@@ -629,7 +603,7 @@ void qdev_property_add_legacy(DeviceState *dev, Property *prop,
 
     object_property_add(OBJECT(dev), name, type,
                         prop->info->print ? qdev_get_legacy_property : prop->info->get,
-                        prop->info->parse ? qdev_set_legacy_property : prop->info->set,
+                        NULL,
                         NULL,
                         prop, errp);
 
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 2c4f140..d0cda38 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -209,7 +209,6 @@ struct PropertyInfo {
     const char *name;
     const char *legacy_name;
     const char **enum_table;
-    int (*parse)(DeviceState *dev, Property *prop, const char *str);
     int (*print)(DeviceState *dev, Property *prop, char *dest, size_t len);
     ObjectPropertyAccessor *get;
     ObjectPropertyAccessor *set;
-- 
1.8.4.2

Re: [Qemu-devel] [PATCH 04/12] qdev: legacy properties are now read-only

[Eric Blake]

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

On 01/30/2014 06:09 AM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/core/qdev-properties.c | 10 +---------
>  hw/core/qdev.c            | 30 ++----------------------------
>  include/hw/qdev-core.h    |  1 -
>  3 files changed, 3 insertions(+), 38 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
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: 604 bytes --]

[Qemu-devel] [PATCH 05/12] qdev: legacy properties are just strings

[Paolo Bonzini]

prop->info->legacy_name is still used by "-device foo,?".

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/core/qdev.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 7c1b732..482a978 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -590,7 +590,7 @@ static void qdev_get_legacy_property(Object *obj, Visitor *v, void *opaque,
 void qdev_property_add_legacy(DeviceState *dev, Property *prop,
                               Error **errp)
 {
-    gchar *name, *type;
+    gchar *name;
 
     /* Register pointer properties as legacy properties */
     if (!prop->info->print && prop->info->get) {
@@ -598,16 +598,12 @@ void qdev_property_add_legacy(DeviceState *dev, Property *prop,
     }
 
     name = g_strdup_printf("legacy-%s", prop->name);
-    type = g_strdup_printf("legacy<%s>",
-                           prop->info->legacy_name ?: prop->info->name);
-
-    object_property_add(OBJECT(dev), name, type,
+    object_property_add(OBJECT(dev), name, "str",
                         prop->info->print ? qdev_get_legacy_property : prop->info->get,
                         NULL,
                         NULL,
                         prop, errp);
 
-    g_free(type);
     g_free(name);
 }
 
-- 
1.8.4.2

Re: [Qemu-devel] [PATCH 05/12] qdev: legacy properties are just strings

[Eric Blake]

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

On 01/30/2014 06:09 AM, Paolo Bonzini wrote:
> prop->info->legacy_name is still used by "-device foo,?".
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/core/qdev.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
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: 604 bytes --]

[Qemu-devel] [PATCH 06/12] qdev: inline qdev_prop_parse

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/core/qdev-properties.c    | 8 +-------
 include/hw/qdev-properties.h | 2 --
 qdev-monitor.c               | 4 ++--
 3 files changed, 3 insertions(+), 11 deletions(-)

diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index a60a183..22ddebf 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -933,12 +933,6 @@ void error_set_from_qdev_prop_error(Error **errp, int ret, DeviceState *dev,
     }
 }
 
-void qdev_prop_parse(DeviceState *dev, const char *name, const char *value,
-                     Error **errp)
-{
-    object_property_parse(OBJECT(dev), value, name, errp);
-}
-
 void qdev_prop_set_bit(DeviceState *dev, const char *name, bool value)
 {
     object_property_set_bool(OBJECT(dev), value, name, &error_abort);
@@ -1031,7 +1025,7 @@ void qdev_prop_set_globals_for_type(DeviceState *dev, const char *typename,
         if (strcmp(typename, prop->driver) != 0) {
             continue;
         }
-        qdev_prop_parse(dev, prop->property, prop->value, &err);
+        object_property_parse(OBJECT(dev), prop->value, prop->property, &err);
         if (err != NULL) {
             error_propagate(errp, err);
             return;
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 77c6f7c..4651459 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -168,8 +168,6 @@ extern PropertyInfo qdev_prop_arraylen;
 
 /* Set properties between creation and init.  */
 void *qdev_get_prop_ptr(DeviceState *dev, Property *prop);
-void qdev_prop_parse(DeviceState *dev, const char *name, const char *value,
-                     Error **errp);
 void qdev_prop_set_bit(DeviceState *dev, const char *name, bool value);
 void qdev_prop_set_uint8(DeviceState *dev, const char *name, uint8_t value);
 void qdev_prop_set_uint16(DeviceState *dev, const char *name, uint16_t value);
diff --git a/qdev-monitor.c b/qdev-monitor.c
index 1d3b68d..4d1634c 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -145,7 +145,7 @@ static void qdev_print_devinfos(bool show_no_user)
 
 static int set_property(const char *name, const char *value, void *opaque)
 {
-    DeviceState *dev = opaque;
+    Object *obj = opaque;
     Error *err = NULL;
 
     if (strcmp(name, "driver") == 0)
@@ -153,7 +153,7 @@ static int set_property(const char *name, const char *value, void *opaque)
     if (strcmp(name, "bus") == 0)
         return 0;
 
-    qdev_prop_parse(dev, name, value, &err);
+    object_property_parse(obj, value, name, &err);
     if (err != NULL) {
         qerror_report_err(err);
         error_free(err);
-- 
1.8.4.2

Re: [Qemu-devel] [PATCH 06/12] qdev: inline qdev_prop_parse

[Eric Blake]

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

On 01/30/2014 06:09 AM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/core/qdev-properties.c    | 8 +-------
>  include/hw/qdev-properties.h | 2 --
>  qdev-monitor.c               | 4 ++--
>  3 files changed, 3 insertions(+), 11 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
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: 604 bytes --]

[Qemu-devel] [PATCH 07/12] qapi: add human mode to StringOutputVisitor

[Paolo Bonzini]

This will be used by "info qtree".  For numbers it prints both the
decimal and hex values.  For sizes it rounds to the nearest power
of 2^10.  For strings, it puts quotes around the string and separates
NULL and empty string.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/qapi/string-output-visitor.h |  2 +-
 include/qom/object.h                 |  3 +-
 qapi/string-output-visitor.c         | 55 ++++++++++++++++++++++++++++++++++--
 qdev-monitor.c                       |  2 +-
 qom/object.c                         |  4 +--
 tests/test-string-output-visitor.c   |  2 +-
 tests/test-visitor-serialization.c   |  2 +-
 7 files changed, 60 insertions(+), 10 deletions(-)

diff --git a/include/qapi/string-output-visitor.h b/include/qapi/string-output-visitor.h
index ec81e42..d99717f 100644
--- a/include/qapi/string-output-visitor.h
+++ b/include/qapi/string-output-visitor.h
@@ -17,7 +17,7 @@
 
 typedef struct StringOutputVisitor StringOutputVisitor;
 
-StringOutputVisitor *string_output_visitor_new(void);
+StringOutputVisitor *string_output_visitor_new(bool human);
 void string_output_visitor_cleanup(StringOutputVisitor *v);
 
 char *string_output_get_string(StringOutputVisitor *v);
diff --git a/include/qom/object.h b/include/qom/object.h
index e0ff212..9c7c361 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -946,12 +946,13 @@ void object_property_parse(Object *obj, const char *string,
  * object_property_print:
  * @obj: the object
  * @name: the name of the property
+ * @human: if true, print for human consumption
  * @errp: returns an error if this function fails
  *
  * Returns a string representation of the value of the property.  The
  * caller shall free the string.
  */
-char *object_property_print(Object *obj, const char *name,
+char *object_property_print(Object *obj, const char *name, bool human,
                             Error **errp);
 
 /**
diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c
index 921653d..67a8798 100644
--- a/qapi/string-output-visitor.c
+++ b/qapi/string-output-visitor.c
@@ -14,10 +14,12 @@
 #include "qapi/string-output-visitor.h"
 #include "qapi/visitor-impl.h"
 #include "qapi/qmp/qerror.h"
+#include "qemu/host-utils.h"
 
 struct StringOutputVisitor
 {
     Visitor visitor;
+    bool human;
     char *string;
 };
 
@@ -31,7 +33,45 @@ static void print_type_int(Visitor *v, int64_t *obj, const char *name,
                            Error **errp)
 {
     StringOutputVisitor *sov = DO_UPCAST(StringOutputVisitor, visitor, v);
-    string_output_set(sov, g_strdup_printf("%lld", (long long) *obj));
+    char *out;
+
+    if (sov->human) {
+        out = g_strdup_printf("%lld (%#llx)", (long long) *obj, (long long) *obj);
+    } else {
+        out = g_strdup_printf("%lld", (long long) *obj);
+    }
+    string_output_set(sov, out);
+}
+
+static void print_type_size(Visitor *v, uint64_t *obj, const char *name,
+                           Error **errp)
+{
+    StringOutputVisitor *sov = DO_UPCAST(StringOutputVisitor, visitor, v);
+    static const char suffixes[] = { 'B', 'K', 'M', 'G', 'T' };
+    uint64_t div, val;
+    char *out;
+    int i;
+
+    if (!sov->human) {
+        out = g_strdup_printf("%llu", (long long) *obj);
+        string_output_set(sov, out);
+        return;
+    }
+
+    val = *obj;
+
+    /* Compute floor(log2(val)).  */
+    i = 64 - clz64(val);
+
+    /* Find the power of 1024 that we'll display as the units.  */
+    i /= 10;
+    if (i >= ARRAY_SIZE(suffixes)) {
+        i = ARRAY_SIZE(suffixes) - 1;
+    }
+    div = 1ULL << (i * 10);
+
+    out = g_strdup_printf("%0.03f%c", (double)val/div, suffixes[i]);
+    string_output_set(sov, out);
 }
 
 static void print_type_bool(Visitor *v, bool *obj, const char *name,
@@ -45,7 +85,14 @@ static void print_type_str(Visitor *v, char **obj, const char *name,
                            Error **errp)
 {
     StringOutputVisitor *sov = DO_UPCAST(StringOutputVisitor, visitor, v);
-    string_output_set(sov, g_strdup(*obj ? *obj : ""));
+    char *out;
+
+    if (sov->human) {
+        out = *obj ? g_strdup_printf("\"%s\"", *obj) : g_strdup("<null>");
+    } else {
+        out = g_strdup(*obj ? *obj : "");
+    }
+    string_output_set(sov, out);
 }
 
 static void print_type_number(Visitor *v, double *obj, const char *name,
@@ -73,14 +120,16 @@ void string_output_visitor_cleanup(StringOutputVisitor *sov)
     g_free(sov);
 }
 
-StringOutputVisitor *string_output_visitor_new(void)
+StringOutputVisitor *string_output_visitor_new(bool human)
 {
     StringOutputVisitor *v;
 
     v = g_malloc0(sizeof(*v));
 
+    v->human = human;
     v->visitor.type_enum = output_type_enum;
     v->visitor.type_int = print_type_int;
+    v->visitor.type_size = print_type_size;
     v->visitor.type_bool = print_type_bool;
     v->visitor.type_str = print_type_str;
     v->visitor.type_number = print_type_number;
diff --git a/qdev-monitor.c b/qdev-monitor.c
index 4d1634c..f385fb3 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -577,7 +577,7 @@ static void qdev_print_props(Monitor *mon, DeviceState *dev, Property *props,
         if (object_property_get_type(OBJECT(dev), legacy_name, NULL)) {
             value = object_property_get_str(OBJECT(dev), legacy_name, &err);
         } else {
-            value = object_property_print(OBJECT(dev), props->name, &err);
+            value = object_property_print(OBJECT(dev), props->name, false, &err);
         }
         g_free(legacy_name);
 
diff --git a/qom/object.c b/qom/object.c
index 62e7e41..660859c 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -948,13 +948,13 @@ void object_property_parse(Object *obj, const char *string,
     string_input_visitor_cleanup(mi);
 }
 
-char *object_property_print(Object *obj, const char *name,
+char *object_property_print(Object *obj, const char *name, bool human,
                             Error **errp)
 {
     StringOutputVisitor *mo;
     char *string;
 
-    mo = string_output_visitor_new();
+    mo = string_output_visitor_new(human);
     object_property_get(obj, string_output_get_visitor(mo), name, errp);
     string = string_output_get_string(mo);
     string_output_visitor_cleanup(mo);
diff --git a/tests/test-string-output-visitor.c b/tests/test-string-output-visitor.c
index 79d815f..56cc21d 100644
--- a/tests/test-string-output-visitor.c
+++ b/tests/test-string-output-visitor.c
@@ -26,7 +26,7 @@ typedef struct TestOutputVisitorData {
 static void visitor_output_setup(TestOutputVisitorData *data,
                                  const void *unused)
 {
-    data->sov = string_output_visitor_new();
+    data->sov = string_output_visitor_new(false);
     g_assert(data->sov != NULL);
 
     data->ov = string_output_get_visitor(data->sov);
diff --git a/tests/test-visitor-serialization.c b/tests/test-visitor-serialization.c
index 9aaa587..6bff950 100644
--- a/tests/test-visitor-serialization.c
+++ b/tests/test-visitor-serialization.c
@@ -1083,7 +1083,7 @@ static void string_serialize(void *native_in, void **datap,
 {
     StringSerializeData *d = g_malloc0(sizeof(*d));
 
-    d->sov = string_output_visitor_new();
+    d->sov = string_output_visitor_new(false);
     visit(string_output_get_visitor(d->sov), &native_in, errp);
     *datap = d;
 }
-- 
1.8.4.2

Re: [Qemu-devel] [PATCH 07/12] qapi: add human mode to StringOutputVisitor

[Eric Blake]

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

On 01/30/2014 06:09 AM, Paolo Bonzini wrote:
> This will be used by "info qtree".  For numbers it prints both the
> decimal and hex values.  For sizes it rounds to the nearest power
> of 2^10.  For strings, it puts quotes around the string and separates
> NULL and empty string.

Nice idea!  But needs a v2.

> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  include/qapi/string-output-visitor.h |  2 +-
>  include/qom/object.h                 |  3 +-
>  qapi/string-output-visitor.c         | 55 ++++++++++++++++++++++++++++++++++--
>  qdev-monitor.c                       |  2 +-
>  qom/object.c                         |  4 +--
>  tests/test-string-output-visitor.c   |  2 +-
>  tests/test-visitor-serialization.c   |  2 +-
>  7 files changed, 60 insertions(+), 10 deletions(-)
> 

> +static void print_type_size(Visitor *v, uint64_t *obj, const char *name,
> +                           Error **errp)
> +{
> +    StringOutputVisitor *sov = DO_UPCAST(StringOutputVisitor, visitor, v);
> +    static const char suffixes[] = { 'B', 'K', 'M', 'G', 'T' };

Any reason you don't include 'P' and 'E' at the end of this array?  We
parse them just fine (at least in some contexts, going by the fact that
qemu-common.h has a STRTOSZ_DEFSUFFIX_ value for them), so we also ought
to output them.

> +    uint64_t div, val;
> +    char *out;
> +    int i;
> +
> +    if (!sov->human) {
> +        out = g_strdup_printf("%llu", (long long) *obj);

(unsigned long long) would look better for the cast.

> +        string_output_set(sov, out);
> +        return;
> +    }
> +
> +    val = *obj;
> +
> +    /* Compute floor(log2(val)).  */
> +    i = 64 - clz64(val);
> +
> +    /* Find the power of 1024 that we'll display as the units.  */
> +    i /= 10;
> +    if (i >= ARRAY_SIZE(suffixes)) {
> +        i = ARRAY_SIZE(suffixes) - 1;
> +    }
> +    div = 1ULL << (i * 10);
> +
> +    out = g_strdup_printf("%0.03f%c", (double)val/div, suffixes[i]);

If val < 1024, this gives dumb output: 1.000B.  Should you special case
bytes?

Also, I like how your int printout was both decimal and hex; but here
you are throwing away information (and the bigger the number, the more
we lose as to how much got rounded away).  I'd rather see this as:

"%0.03f%c (%llu)"

so that we also have access to the unrounded exact amount.

-- 
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: 604 bytes --]

Re: [Qemu-devel] [PATCH 07/12] qapi: add human mode to StringOutputVisitor

[Paolo Bonzini]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Il 30/01/2014 15:09, Eric Blake ha scritto:
| Also, I like how your int printout was both decimal and hex; but
| here you are throwing away information (and the bigger the number,
| the more we lose as to how much got rounded away).  I'd rather see
| this as:
|
| "%0.03f%c (%llu)"
|
| so that we also have access to the unrounded exact amount.

Perhaps the other way round (since hex is in parentheses)?

This patch is just moving the code from qdev-properties.c.  I agree
with all your suggestion, but I'd prefer to tackle it as follow-ups or
as patch 13/12.  There are other problems, like 512MB printed as
0.500G (which is correct but looks weird).

Paolo

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJS6l3QAAoJEBvWZb6bTYbyiSAQAIUXn6u0GTdSI6KbWxF1hOpC
nXno3OekuKomBMPZxUYzXKztwz3LhKKrPpuFHPpcIXvdIENZ7tmzTAoJFaUWCvIl
R4wCPpNRiSAI5wrF8CnqPzirsc+9SlKrFCqP+mjwYQ56vGhmKcvzT0uAlLdHMOyB
dn1g3PH2z8RNKSf1WxCViRwgHrSBa+w/ZjWrVQ8yEJHfBPTlQNA/x+cKKBcIQA5v
SmYGUSCWj84cgEs9o9psLefxkSHyzWN+OoH1zU8VL5qegwFeQYGbHC8Q2zlM6hZO
G1EtSwEknhPPTOP7+muRgNZaVaZtBi4irkZ677OtdXgrudeGaSlHnVsuYJQ72+tv
B2A6DuFq4kdLB3eV/+FPnpZk1ky896nDguFuj2A8Yo7SJr88JszcI/OAosW0mF6d
cML2NEt2AbwMUccbpr43qmkiLDKN0G1MIN4BVAukSGCdQfU5X6DRWWZIuicKzBbf
VPYjK7puI57wC8L2Xjj8fTW6PbBG09R6IJuDvnYLRubY48qTQupOLlxx16sw61EV
450s+QkZmRTPP3GmwfsNiz+/rDlpqNB/iWnVFrek3K9Lsz0eM/CNYqiyG+LUVBCB
e9D2ktLZ2mmKahiwQY4lUP4UYFjHLwhYly+OefznhYq/gKthnQD3jNaHtf3ZkNaW
HO4JZAjvTsFddSr7cCTD
=LRTe
-----END PGP SIGNATURE-----

Re: [Qemu-devel] [PATCH 07/12] qapi: add human mode to StringOutputVisitor

[Eric Blake]

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

On 01/30/2014 07:12 AM, Paolo Bonzini wrote:
> Il 30/01/2014 15:09, Eric Blake ha scritto:
> | Also, I like how your int printout was both decimal and hex; but
> | here you are throwing away information (and the bigger the number,
> | the more we lose as to how much got rounded away).  I'd rather see
> | this as:
> |
> | "%0.03f%c (%llu)"
> |
> | so that we also have access to the unrounded exact amount.
> 
> Perhaps the other way round (since hex is in parentheses)?

Indeed, exact(human) is better than human(exact).

> 
> This patch is just moving the code from qdev-properties.c.  I agree
> with all your suggestion, but I'd prefer to tackle it as follow-ups or
> as patch 13/12.  There are other problems, like 512MB printed as
> 0.500G (which is correct but looks weird).

Sure - if you post 13/12, then I'm fine with this one having:
Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
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: 604 bytes --]

Re: [Qemu-devel] [PATCH 07/12] qapi: add human mode to StringOutputVisitor

[Andreas Färber]

Am 30.01.2014 14:09, schrieb Paolo Bonzini:
> This will be used by "info qtree".  For numbers it prints both the
> decimal and hex values.  For sizes it rounds to the nearest power
> of 2^10.  For strings, it puts quotes around the string and separates
> NULL and empty string.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  include/qapi/string-output-visitor.h |  2 +-
>  include/qom/object.h                 |  3 +-
>  qapi/string-output-visitor.c         | 55 ++++++++++++++++++++++++++++++++++--
>  qdev-monitor.c                       |  2 +-
>  qom/object.c                         |  4 +--
>  tests/test-string-output-visitor.c   |  2 +-
>  tests/test-visitor-serialization.c   |  2 +-
>  7 files changed, 60 insertions(+), 10 deletions(-)
> 
> diff --git a/include/qapi/string-output-visitor.h b/include/qapi/string-output-visitor.h
> index ec81e42..d99717f 100644
> --- a/include/qapi/string-output-visitor.h
> +++ b/include/qapi/string-output-visitor.h
> @@ -17,7 +17,7 @@
>  
>  typedef struct StringOutputVisitor StringOutputVisitor;
>  
> -StringOutputVisitor *string_output_visitor_new(void);
> +StringOutputVisitor *string_output_visitor_new(bool human);
>  void string_output_visitor_cleanup(StringOutputVisitor *v);
>  
>  char *string_output_get_string(StringOutputVisitor *v);
> diff --git a/include/qom/object.h b/include/qom/object.h
> index e0ff212..9c7c361 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -946,12 +946,13 @@ void object_property_parse(Object *obj, const char *string,
>   * object_property_print:
>   * @obj: the object
>   * @name: the name of the property
> + * @human: if true, print for human consumption

This would ideally be %true, but I better not fiddle with that to not
create merge conflicts, so hoping it can be fixed in the follow-up.

Andreas

>   * @errp: returns an error if this function fails
>   *
>   * Returns a string representation of the value of the property.  The
>   * caller shall free the string.
>   */
> -char *object_property_print(Object *obj, const char *name,
> +char *object_property_print(Object *obj, const char *name, bool human,
>                              Error **errp);
>  
>  /**
[snip]

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

[Qemu-devel] [PATCH 08/12] qdev: use human mode in "info qtree"

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qdev-monitor.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qdev-monitor.c b/qdev-monitor.c
index f385fb3..b37778f 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -577,7 +577,7 @@ static void qdev_print_props(Monitor *mon, DeviceState *dev, Property *props,
         if (object_property_get_type(OBJECT(dev), legacy_name, NULL)) {
             value = object_property_get_str(OBJECT(dev), legacy_name, &err);
         } else {
-            value = object_property_print(OBJECT(dev), props->name, false, &err);
+            value = object_property_print(OBJECT(dev), props->name, true, &err);
         }
         g_free(legacy_name);
 
-- 
1.8.4.2

Re: [Qemu-devel] [PATCH 08/12] qdev: use human mode in "info qtree"

[Eric Blake]

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

On 01/30/2014 06:09 AM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  qdev-monitor.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
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: 604 bytes --]

[Qemu-devel] [PATCH 09/12] qdev: remove most legacy printers

[Paolo Bonzini]

Their functionality is either aesthetic only (e.g. on/off vs. true/false)
or obtained by the "human mode" of StringOutputVisitor.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/core/qdev-properties.c | 60 -----------------------------------------------
 1 file changed, 60 deletions(-)

diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 22ddebf..a4f1f78 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -74,13 +74,6 @@ static void bit_prop_set(DeviceState *dev, Property *props, bool val)
     }
 }
 
-static int prop_print_bit(DeviceState *dev, Property *prop, char *dest,
-                          size_t len)
-{
-    uint32_t *p = qdev_get_prop_ptr(dev, prop);
-    return snprintf(dest, len, (*p & qdev_get_prop_mask(prop)) ? "on" : "off");
-}
-
 static void prop_get_bit(Object *obj, Visitor *v, void *opaque,
                     const char *name, Error **errp)
 {
@@ -116,7 +109,6 @@ static void prop_set_bit(Object *obj, Visitor *v, void *opaque,
 PropertyInfo qdev_prop_bit = {
     .name  = "boolean",
     .legacy_name  = "on/off",
-    .print = prop_print_bit,
     .get   = prop_get_bit,
     .set   = prop_set_bit,
 };
@@ -189,16 +181,9 @@ PropertyInfo qdev_prop_uint8 = {
 
 /* --- 8bit hex value --- */
 
-static int print_hex8(DeviceState *dev, Property *prop, char *dest, size_t len)
-{
-    uint8_t *ptr = qdev_get_prop_ptr(dev, prop);
-    return snprintf(dest, len, "0x%" PRIx8, *ptr);
-}
-
 PropertyInfo qdev_prop_hex8 = {
     .name  = "uint8",
     .legacy_name  = "hex8",
-    .print = print_hex8,
     .get   = get_uint8,
     .set   = set_uint8,
 };
@@ -302,16 +287,9 @@ PropertyInfo qdev_prop_int32 = {
 
 /* --- 32bit hex value --- */
 
-static int print_hex32(DeviceState *dev, Property *prop, char *dest, size_t len)
-{
-    uint32_t *ptr = qdev_get_prop_ptr(dev, prop);
-    return snprintf(dest, len, "0x%" PRIx32, *ptr);
-}
-
 PropertyInfo qdev_prop_hex32 = {
     .name  = "uint32",
     .legacy_name  = "hex32",
-    .print = print_hex32,
     .get   = get_uint32,
     .set   = set_uint32,
 };
@@ -351,16 +329,9 @@ PropertyInfo qdev_prop_uint64 = {
 
 /* --- 64bit hex value --- */
 
-static int print_hex64(DeviceState *dev, Property *prop, char *dest, size_t len)
-{
-    uint64_t *ptr = qdev_get_prop_ptr(dev, prop);
-    return snprintf(dest, len, "0x%" PRIx64, *ptr);
-}
-
 PropertyInfo qdev_prop_hex64 = {
     .name  = "uint64",
     .legacy_name  = "hex64",
-    .print = print_hex64,
     .get   = get_uint64,
     .set   = set_uint64,
 };
@@ -373,16 +344,6 @@ static void release_string(Object *obj, const char *name, void *opaque)
     g_free(*(char **)qdev_get_prop_ptr(DEVICE(obj), prop));
 }
 
-static int print_string(DeviceState *dev, Property *prop, char *dest,
-                        size_t len)
-{
-    char **ptr = qdev_get_prop_ptr(dev, prop);
-    if (!*ptr) {
-        return snprintf(dest, len, "<null>");
-    }
-    return snprintf(dest, len, "\"%s\"", *ptr);
-}
-
 static void get_string(Object *obj, Visitor *v, void *opaque,
                        const char *name, Error **errp)
 {
@@ -425,7 +386,6 @@ static void set_string(Object *obj, Visitor *v, void *opaque,
 
 PropertyInfo qdev_prop_string = {
     .name  = "string",
-    .print = print_string,
     .release = release_string,
     .get   = get_string,
     .set   = set_string,
@@ -1072,28 +1032,8 @@ static void set_size(Object *obj, Visitor *v, void *opaque,
     visit_type_size(v, ptr, name, errp);
 }
 
-static int print_size(DeviceState *dev, Property *prop, char *dest, size_t len)
-{
-    static const char suffixes[] = { 'B', 'K', 'M', 'G', 'T' };
-    uint64_t div, val = *(uint64_t *)qdev_get_prop_ptr(dev, prop);
-    int i;
-
-    /* Compute floor(log2(val)).  */
-    i = 64 - clz64(val);
-
-    /* Find the power of 1024 that we'll display as the units.  */
-    i /= 10;
-    if (i >= ARRAY_SIZE(suffixes)) {
-        i = ARRAY_SIZE(suffixes) - 1;
-    }
-    div = 1ULL << (i * 10);
-
-    return snprintf(dest, len, "%0.03f%c", (double)val/div, suffixes[i]);
-}
-
 PropertyInfo qdev_prop_size = {
     .name  = "size",
-    .print = print_size,
     .get = get_size,
     .set = set_size,
 };
-- 
1.8.4.2

Re: [Qemu-devel] [PATCH 09/12] qdev: remove most legacy printers

[Eric Blake]

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

On 01/30/2014 06:09 AM, Paolo Bonzini wrote:
> Their functionality is either aesthetic only (e.g. on/off vs. true/false)
> or obtained by the "human mode" of StringOutputVisitor.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/core/qdev-properties.c | 60 -----------------------------------------------
>  1 file changed, 60 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
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: 604 bytes --]

[Qemu-devel] [PATCH 10/12] qdev: remove hex8/32/64 property types

[Paolo Bonzini]

Replace them with uint8/32/64.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/audio/adlib.c             |  2 +-
 hw/audio/cs4231a.c           |  2 +-
 hw/audio/gus.c               |  2 +-
 hw/audio/pcspk.c             |  2 +-
 hw/audio/sb16.c              |  4 ++--
 hw/block/fdc.c               |  2 +-
 hw/char/debugcon.c           |  4 ++--
 hw/char/parallel.c           |  2 +-
 hw/char/serial-isa.c         |  2 +-
 hw/core/qdev-properties.c    | 27 ---------------------------
 hw/display/g364fb.c          |  2 +-
 hw/display/tcx.c             |  4 ++--
 hw/dma/i82374.c              |  2 +-
 hw/dma/sun4m_iommu.c         |  2 +-
 hw/i386/kvm/i8254.c          |  2 +-
 hw/ide/isa.c                 |  4 ++--
 hw/ide/qdev.c                |  2 +-
 hw/intc/i8259_common.c       |  6 +++---
 hw/isa/pc87312.c             |  2 +-
 hw/misc/applesmc.c           |  2 +-
 hw/misc/debugexit.c          |  4 ++--
 hw/misc/eccmemctl.c          |  2 +-
 hw/net/ne2000-isa.c          |  2 +-
 hw/nvram/fw_cfg.c            |  4 ++--
 hw/ppc/spapr_pci.c           | 16 ++++++++--------
 hw/scsi/megasas.c            |  2 +-
 hw/scsi/scsi-disk.c          |  6 +++---
 hw/sd/sdhci.c                |  4 ++--
 hw/timer/i8254.c             |  2 +-
 hw/timer/m48t59.c            |  4 ++--
 hw/usb/host-libusb.c         |  4 ++--
 hw/virtio/virtio-pci.c       |  6 +++---
 include/hw/qdev-dma.h        |  2 +-
 include/hw/qdev-properties.h |  9 ---------
 34 files changed, 54 insertions(+), 90 deletions(-)

diff --git a/hw/audio/adlib.c b/hw/audio/adlib.c
index e88d2dd..28eed81 100644
--- a/hw/audio/adlib.c
+++ b/hw/audio/adlib.c
@@ -354,7 +354,7 @@ static void adlib_realizefn (DeviceState *dev, Error **errp)
 }
 
 static Property adlib_properties[] = {
-    DEFINE_PROP_HEX32  ("iobase",  AdlibState, port, 0x220),
+    DEFINE_PROP_UINT32 ("iobase",  AdlibState, port, 0x220),
     DEFINE_PROP_UINT32 ("freq",    AdlibState, freq,  44100),
     DEFINE_PROP_END_OF_LIST (),
 };
diff --git a/hw/audio/cs4231a.c b/hw/audio/cs4231a.c
index 666096b..a0ec17a 100644
--- a/hw/audio/cs4231a.c
+++ b/hw/audio/cs4231a.c
@@ -673,7 +673,7 @@ static int cs4231a_init (ISABus *bus)
 }
 
 static Property cs4231a_properties[] = {
-    DEFINE_PROP_HEX32  ("iobase",  CSState, port, 0x534),
+    DEFINE_PROP_UINT32 ("iobase",  CSState, port, 0x534),
     DEFINE_PROP_UINT32 ("irq",     CSState, irq,  9),
     DEFINE_PROP_UINT32 ("dma",     CSState, dma,  3),
     DEFINE_PROP_END_OF_LIST (),
diff --git a/hw/audio/gus.c b/hw/audio/gus.c
index 71be3c6..e29a571 100644
--- a/hw/audio/gus.c
+++ b/hw/audio/gus.c
@@ -304,7 +304,7 @@ static int GUS_init (ISABus *bus)
 
 static Property gus_properties[] = {
     DEFINE_PROP_UINT32 ("freq",    GUSState, freq,        44100),
-    DEFINE_PROP_HEX32  ("iobase",  GUSState, port,        0x240),
+    DEFINE_PROP_UINT32 ("iobase",  GUSState, port,        0x240),
     DEFINE_PROP_UINT32 ("irq",     GUSState, emu.gusirq,  7),
     DEFINE_PROP_UINT32 ("dma",     GUSState, emu.gusdma,  3),
     DEFINE_PROP_END_OF_LIST (),
diff --git a/hw/audio/pcspk.c b/hw/audio/pcspk.c
index f980d66..1d81bbe 100644
--- a/hw/audio/pcspk.c
+++ b/hw/audio/pcspk.c
@@ -181,7 +181,7 @@ static void pcspk_realizefn(DeviceState *dev, Error **errp)
 }
 
 static Property pcspk_properties[] = {
-    DEFINE_PROP_HEX32("iobase", PCSpkState, iobase,  -1),
+    DEFINE_PROP_UINT32("iobase", PCSpkState, iobase,  -1),
     DEFINE_PROP_PTR("pit", PCSpkState, pit),
     DEFINE_PROP_END_OF_LIST(),
 };
diff --git a/hw/audio/sb16.c b/hw/audio/sb16.c
index db79131..bb24e00 100644
--- a/hw/audio/sb16.c
+++ b/hw/audio/sb16.c
@@ -1399,8 +1399,8 @@ static int SB16_init (ISABus *bus)
 }
 
 static Property sb16_properties[] = {
-    DEFINE_PROP_HEX32  ("version", SB16State, ver,  0x0405), /* 4.5 */
-    DEFINE_PROP_HEX32  ("iobase",  SB16State, port, 0x220),
+    DEFINE_PROP_UINT32 ("version", SB16State, ver,  0x0405), /* 4.5 */
+    DEFINE_PROP_UINT32 ("iobase",  SB16State, port, 0x220),
     DEFINE_PROP_UINT32 ("irq",     SB16State, irq,  5),
     DEFINE_PROP_UINT32 ("dma",     SB16State, dma,  1),
     DEFINE_PROP_UINT32 ("dma16",   SB16State, hdma, 5),
diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 592b58f..1651007 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -2216,7 +2216,7 @@ static const VMStateDescription vmstate_isa_fdc ={
 };
 
 static Property isa_fdc_properties[] = {
-    DEFINE_PROP_HEX32("iobase", FDCtrlISABus, iobase, 0x3f0),
+    DEFINE_PROP_UINT32("iobase", FDCtrlISABus, iobase, 0x3f0),
     DEFINE_PROP_UINT32("irq", FDCtrlISABus, irq, 6),
     DEFINE_PROP_UINT32("dma", FDCtrlISABus, dma, 2),
     DEFINE_PROP_DRIVE("driveA", FDCtrlISABus, state.drives[0].bs),
diff --git a/hw/char/debugcon.c b/hw/char/debugcon.c
index 02d0d57..36f1c4a 100644
--- a/hw/char/debugcon.c
+++ b/hw/char/debugcon.c
@@ -110,9 +110,9 @@ static void debugcon_isa_realizefn(DeviceState *dev, Error **errp)
 }
 
 static Property debugcon_isa_properties[] = {
-    DEFINE_PROP_HEX32("iobase", ISADebugconState, iobase, 0xe9),
+    DEFINE_PROP_UINT32("iobase", ISADebugconState, iobase, 0xe9),
     DEFINE_PROP_CHR("chardev",  ISADebugconState, state.chr),
-    DEFINE_PROP_HEX32("readback", ISADebugconState, state.readback, 0xe9),
+    DEFINE_PROP_UINT32("readback", ISADebugconState, state.readback, 0xe9),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/char/parallel.c b/hw/char/parallel.c
index 7a3b264..7ac90a5 100644
--- a/hw/char/parallel.c
+++ b/hw/char/parallel.c
@@ -595,7 +595,7 @@ bool parallel_mm_init(MemoryRegion *address_space,
 
 static Property parallel_isa_properties[] = {
     DEFINE_PROP_UINT32("index", ISAParallelState, index,   -1),
-    DEFINE_PROP_HEX32("iobase", ISAParallelState, iobase,  -1),
+    DEFINE_PROP_UINT32("iobase", ISAParallelState, iobase,  -1),
     DEFINE_PROP_UINT32("irq",   ISAParallelState, isairq,  7),
     DEFINE_PROP_CHR("chardev",  ISAParallelState, state.chr),
     DEFINE_PROP_END_OF_LIST(),
diff --git a/hw/char/serial-isa.c b/hw/char/serial-isa.c
index 5cb77b3..c9fcb27 100644
--- a/hw/char/serial-isa.c
+++ b/hw/char/serial-isa.c
@@ -88,7 +88,7 @@ static const VMStateDescription vmstate_isa_serial = {
 
 static Property serial_isa_properties[] = {
     DEFINE_PROP_UINT32("index",  ISASerialState, index,   -1),
-    DEFINE_PROP_HEX32("iobase",  ISASerialState, iobase,  -1),
+    DEFINE_PROP_UINT32("iobase",  ISASerialState, iobase,  -1),
     DEFINE_PROP_UINT32("irq",    ISASerialState, isairq,  -1),
     DEFINE_PROP_CHR("chardev",   ISASerialState, state.chr),
     DEFINE_PROP_UINT32("wakeup", ISASerialState, state.wakeup, 0),
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index a4f1f78..2c3a756 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -179,15 +179,6 @@ PropertyInfo qdev_prop_uint8 = {
     .set   = set_uint8,
 };
 
-/* --- 8bit hex value --- */
-
-PropertyInfo qdev_prop_hex8 = {
-    .name  = "uint8",
-    .legacy_name  = "hex8",
-    .get   = get_uint8,
-    .set   = set_uint8,
-};
-
 /* --- 16bit integer --- */
 
 static void get_uint16(Object *obj, Visitor *v, void *opaque,
@@ -285,15 +276,6 @@ PropertyInfo qdev_prop_int32 = {
     .set   = set_int32,
 };
 
-/* --- 32bit hex value --- */
-
-PropertyInfo qdev_prop_hex32 = {
-    .name  = "uint32",
-    .legacy_name  = "hex32",
-    .get   = get_uint32,
-    .set   = set_uint32,
-};
-
 /* --- 64bit integer --- */
 
 static void get_uint64(Object *obj, Visitor *v, void *opaque,
@@ -327,15 +309,6 @@ PropertyInfo qdev_prop_uint64 = {
     .set   = set_uint64,
 };
 
-/* --- 64bit hex value --- */
-
-PropertyInfo qdev_prop_hex64 = {
-    .name  = "uint64",
-    .legacy_name  = "hex64",
-    .get   = get_uint64,
-    .set   = set_uint64,
-};
-
 /* --- string --- */
 
 static void release_string(Object *obj, const char *name, void *opaque)
diff --git a/hw/display/g364fb.c b/hw/display/g364fb.c
index 7082171..bc909bb 100644
--- a/hw/display/g364fb.c
+++ b/hw/display/g364fb.c
@@ -524,7 +524,7 @@ static void g364fb_sysbus_reset(DeviceState *d)
 }
 
 static Property g364fb_sysbus_properties[] = {
-    DEFINE_PROP_HEX32("vram_size", G364SysBusState, g364.vram_size,
+    DEFINE_PROP_UINT32("vram_size", G364SysBusState, g364.vram_size,
     8 * 1024 * 1024),
     DEFINE_PROP_END_OF_LIST(),
 };
diff --git a/hw/display/tcx.c b/hw/display/tcx.c
index 873b82c..e60769c 100644
--- a/hw/display/tcx.c
+++ b/hw/display/tcx.c
@@ -617,11 +617,11 @@ static int tcx_init1(SysBusDevice *dev)
 }
 
 static Property tcx_properties[] = {
-    DEFINE_PROP_HEX32("vram_size", TCXState, vram_size, -1),
+    DEFINE_PROP_UINT32("vram_size", TCXState, vram_size, -1),
     DEFINE_PROP_UINT16("width",    TCXState, width,     -1),
     DEFINE_PROP_UINT16("height",   TCXState, height,    -1),
     DEFINE_PROP_UINT16("depth",    TCXState, depth,     -1),
-    DEFINE_PROP_HEX64("prom_addr", TCXState, prom_addr, -1),
+    DEFINE_PROP_UINT64("prom_addr", TCXState, prom_addr, -1),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/dma/i82374.c b/hw/dma/i82374.c
index a5b891f..dc7a767 100644
--- a/hw/dma/i82374.c
+++ b/hw/dma/i82374.c
@@ -149,7 +149,7 @@ static void i82374_isa_realize(DeviceState *dev, Error **errp)
 }
 
 static Property i82374_properties[] = {
-    DEFINE_PROP_HEX32("iobase", ISAi82374State, iobase, 0x400),
+    DEFINE_PROP_UINT32("iobase", ISAi82374State, iobase, 0x400),
     DEFINE_PROP_END_OF_LIST()
 };
 
diff --git a/hw/dma/sun4m_iommu.c b/hw/dma/sun4m_iommu.c
index a04409a..2c7bd48 100644
--- a/hw/dma/sun4m_iommu.c
+++ b/hw/dma/sun4m_iommu.c
@@ -361,7 +361,7 @@ static int iommu_init1(SysBusDevice *dev)
 }
 
 static Property iommu_properties[] = {
-    DEFINE_PROP_HEX32("version", IOMMUState, version, 0),
+    DEFINE_PROP_UINT32("version", IOMMUState, version, 0),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/i386/kvm/i8254.c b/hw/i386/kvm/i8254.c
index 20b6457..f8f3021 100644
--- a/hw/i386/kvm/i8254.c
+++ b/hw/i386/kvm/i8254.c
@@ -298,7 +298,7 @@ static void kvm_pit_realizefn(DeviceState *dev, Error **errp)
 }
 
 static Property kvm_pit_properties[] = {
-    DEFINE_PROP_HEX32("iobase", PITCommonState, iobase,  -1),
+    DEFINE_PROP_UINT32("iobase", PITCommonState, iobase,  -1),
     DEFINE_PROP_LOSTTICKPOLICY("lost_tick_policy", KVMPITState,
                                lost_tick_policy, LOST_TICK_DELAY),
     DEFINE_PROP_END_OF_LIST(),
diff --git a/hw/ide/isa.c b/hw/ide/isa.c
index afc24d4..d2cabc1 100644
--- a/hw/ide/isa.c
+++ b/hw/ide/isa.c
@@ -104,8 +104,8 @@ ISADevice *isa_ide_init(ISABus *bus, int iobase, int iobase2, int isairq,
 }
 
 static Property isa_ide_properties[] = {
-    DEFINE_PROP_HEX32("iobase",  ISAIDEState, iobase,  0x1f0),
-    DEFINE_PROP_HEX32("iobase2", ISAIDEState, iobase2, 0x3f6),
+    DEFINE_PROP_UINT32("iobase",  ISAIDEState, iobase,  0x1f0),
+    DEFINE_PROP_UINT32("iobase2", ISAIDEState, iobase2, 0x3f6),
     DEFINE_PROP_UINT32("irq",    ISAIDEState, isairq,  14),
     DEFINE_PROP_END_OF_LIST(),
 };
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 18c4b7e..6e475e6 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -206,7 +206,7 @@ static int ide_drive_initfn(IDEDevice *dev)
 #define DEFINE_IDE_DEV_PROPERTIES()                     \
     DEFINE_BLOCK_PROPERTIES(IDEDrive, dev.conf),        \
     DEFINE_PROP_STRING("ver",  IDEDrive, dev.version),  \
-    DEFINE_PROP_HEX64("wwn",  IDEDrive, dev.wwn, 0),    \
+    DEFINE_PROP_UINT64("wwn",  IDEDrive, dev.wwn, 0),    \
     DEFINE_PROP_STRING("serial",  IDEDrive, dev.serial),\
     DEFINE_PROP_STRING("model", IDEDrive, dev.model)
 
diff --git a/hw/intc/i8259_common.c b/hw/intc/i8259_common.c
index 9d29399..61381c4 100644
--- a/hw/intc/i8259_common.c
+++ b/hw/intc/i8259_common.c
@@ -123,9 +123,9 @@ static const VMStateDescription vmstate_pic_common = {
 };
 
 static Property pic_properties_common[] = {
-    DEFINE_PROP_HEX32("iobase", PICCommonState, iobase,  -1),
-    DEFINE_PROP_HEX32("elcr_addr", PICCommonState, elcr_addr,  -1),
-    DEFINE_PROP_HEX8("elcr_mask", PICCommonState, elcr_mask,  -1),
+    DEFINE_PROP_UINT32("iobase", PICCommonState, iobase,  -1),
+    DEFINE_PROP_UINT32("elcr_addr", PICCommonState, elcr_addr,  -1),
+    DEFINE_PROP_UINT8("elcr_mask", PICCommonState, elcr_mask,  -1),
     DEFINE_PROP_BIT("master", PICCommonState, master,  0, false),
     DEFINE_PROP_END_OF_LIST(),
 };
diff --git a/hw/isa/pc87312.c b/hw/isa/pc87312.c
index 46a23fb..b352b49 100644
--- a/hw/isa/pc87312.c
+++ b/hw/isa/pc87312.c
@@ -369,7 +369,7 @@ static const VMStateDescription vmstate_pc87312 = {
 };
 
 static Property pc87312_properties[] = {
-    DEFINE_PROP_HEX32("iobase", PC87312State, iobase, 0x398),
+    DEFINE_PROP_UINT32("iobase", PC87312State, iobase, 0x398),
     DEFINE_PROP_UINT8("config", PC87312State, config, 1),
     DEFINE_PROP_END_OF_LIST()
 };
diff --git a/hw/misc/applesmc.c b/hw/misc/applesmc.c
index 1e8d183..8a642de 100644
--- a/hw/misc/applesmc.c
+++ b/hw/misc/applesmc.c
@@ -250,7 +250,7 @@ static void applesmc_isa_realize(DeviceState *dev, Error **errp)
 }
 
 static Property applesmc_isa_properties[] = {
-    DEFINE_PROP_HEX32("iobase", AppleSMCState, iobase,
+    DEFINE_PROP_UINT32("iobase", AppleSMCState, iobase,
                       APPLESMC_DEFAULT_IOBASE),
     DEFINE_PROP_STRING("osk", AppleSMCState, osk),
     DEFINE_PROP_END_OF_LIST(),
diff --git a/hw/misc/debugexit.c b/hw/misc/debugexit.c
index 9db5680..69a1b00 100644
--- a/hw/misc/debugexit.c
+++ b/hw/misc/debugexit.c
@@ -47,8 +47,8 @@ static void debug_exit_realizefn(DeviceState *d, Error **errp)
 }
 
 static Property debug_exit_properties[] = {
-    DEFINE_PROP_HEX32("iobase", ISADebugExitState, iobase, 0x501),
-    DEFINE_PROP_HEX32("iosize", ISADebugExitState, iosize, 0x02),
+    DEFINE_PROP_UINT32("iobase", ISADebugExitState, iobase, 0x501),
+    DEFINE_PROP_UINT32("iosize", ISADebugExitState, iosize, 0x02),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/misc/eccmemctl.c b/hw/misc/eccmemctl.c
index 96a69d4..549431c 100644
--- a/hw/misc/eccmemctl.c
+++ b/hw/misc/eccmemctl.c
@@ -314,7 +314,7 @@ static int ecc_init1(SysBusDevice *dev)
 }
 
 static Property ecc_properties[] = {
-    DEFINE_PROP_HEX32("version", ECCState, version, -1),
+    DEFINE_PROP_UINT32("version", ECCState, version, -1),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/net/ne2000-isa.c b/hw/net/ne2000-isa.c
index 26b83ce..c660e58 100644
--- a/hw/net/ne2000-isa.c
+++ b/hw/net/ne2000-isa.c
@@ -86,7 +86,7 @@ static void isa_ne2000_realizefn(DeviceState *dev, Error **errp)
 }
 
 static Property ne2000_isa_properties[] = {
-    DEFINE_PROP_HEX32("iobase", ISANE2000State, iobase, 0x300),
+    DEFINE_PROP_UINT32("iobase", ISANE2000State, iobase, 0x300),
     DEFINE_PROP_UINT32("irq",   ISANE2000State, isairq, 9),
     DEFINE_NIC_PROPERTIES(ISANE2000State, ne2000.c),
     DEFINE_PROP_END_OF_LIST(),
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index ee96c16..cb36dc2 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -584,8 +584,8 @@ static void fw_cfg_realize(DeviceState *dev, Error **errp)
 }
 
 static Property fw_cfg_properties[] = {
-    DEFINE_PROP_HEX32("ctl_iobase", FWCfgState, ctl_iobase, -1),
-    DEFINE_PROP_HEX32("data_iobase", FWCfgState, data_iobase, -1),
+    DEFINE_PROP_UINT32("ctl_iobase", FWCfgState, ctl_iobase, -1),
+    DEFINE_PROP_UINT32("data_iobase", FWCfgState, data_iobase, -1),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index ec00300..4c7c3ae 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -651,14 +651,14 @@ static void spapr_phb_reset(DeviceState *qdev)
 
 static Property spapr_phb_properties[] = {
     DEFINE_PROP_INT32("index", sPAPRPHBState, index, -1),
-    DEFINE_PROP_HEX64("buid", sPAPRPHBState, buid, -1),
-    DEFINE_PROP_HEX32("liobn", sPAPRPHBState, dma_liobn, -1),
-    DEFINE_PROP_HEX64("mem_win_addr", sPAPRPHBState, mem_win_addr, -1),
-    DEFINE_PROP_HEX64("mem_win_size", sPAPRPHBState, mem_win_size,
-                      SPAPR_PCI_MMIO_WIN_SIZE),
-    DEFINE_PROP_HEX64("io_win_addr", sPAPRPHBState, io_win_addr, -1),
-    DEFINE_PROP_HEX64("io_win_size", sPAPRPHBState, io_win_size,
-                      SPAPR_PCI_IO_WIN_SIZE),
+    DEFINE_PROP_UINT64("buid", sPAPRPHBState, buid, -1),
+    DEFINE_PROP_UINT32("liobn", sPAPRPHBState, dma_liobn, -1),
+    DEFINE_PROP_UINT64("mem_win_addr", sPAPRPHBState, mem_win_addr, -1),
+    DEFINE_PROP_UINT64("mem_win_size", sPAPRPHBState, mem_win_size,
+                       SPAPR_PCI_MMIO_WIN_SIZE),
+    DEFINE_PROP_UINT64("io_win_addr", sPAPRPHBState, io_win_addr, -1),
+    DEFINE_PROP_UINT64("io_win_size", sPAPRPHBState, io_win_size,
+                       SPAPR_PCI_IO_WIN_SIZE),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index 7c5a1a2..8e1cc1d 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -2189,7 +2189,7 @@ static Property megasas_properties[] = {
     DEFINE_PROP_UINT32("max_cmds", MegasasState, fw_cmds,
                        MEGASAS_DEFAULT_FRAMES),
     DEFINE_PROP_STRING("hba_serial", MegasasState, hba_serial),
-    DEFINE_PROP_HEX64("sas_address", MegasasState, sas_addr, 0),
+    DEFINE_PROP_UINT64("sas_address", MegasasState, sas_addr, 0),
 #ifdef USE_MSIX
     DEFINE_PROP_BIT("use_msix", MegasasState, flags,
                     MEGASAS_FLAG_USE_MSIX, false),
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index a8d0f15..b4fadd2 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2535,7 +2535,7 @@ static Property scsi_hd_properties[] = {
                     SCSI_DISK_F_REMOVABLE, false),
     DEFINE_PROP_BIT("dpofua", SCSIDiskState, features,
                     SCSI_DISK_F_DPOFUA, false),
-    DEFINE_PROP_HEX64("wwn", SCSIDiskState, wwn, 0),
+    DEFINE_PROP_UINT64("wwn", SCSIDiskState, wwn, 0),
     DEFINE_PROP_UINT64("max_unmap_size", SCSIDiskState, max_unmap_size,
                        DEFAULT_MAX_UNMAP_SIZE),
     DEFINE_BLOCK_CHS_PROPERTIES(SCSIDiskState, qdev.conf),
@@ -2583,7 +2583,7 @@ static const TypeInfo scsi_hd_info = {
 
 static Property scsi_cd_properties[] = {
     DEFINE_SCSI_DISK_PROPERTIES(),
-    DEFINE_PROP_HEX64("wwn", SCSIDiskState, wwn, 0),
+    DEFINE_PROP_UINT64("wwn", SCSIDiskState, wwn, 0),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -2646,7 +2646,7 @@ static Property scsi_disk_properties[] = {
                     SCSI_DISK_F_REMOVABLE, false),
     DEFINE_PROP_BIT("dpofua", SCSIDiskState, features,
                     SCSI_DISK_F_DPOFUA, false),
-    DEFINE_PROP_HEX64("wwn", SCSIDiskState, wwn, 0),
+    DEFINE_PROP_UINT64("wwn", SCSIDiskState, wwn, 0),
     DEFINE_PROP_UINT64("max_unmap_size", SCSIDiskState, max_unmap_size,
                        DEFAULT_MAX_UNMAP_SIZE),
     DEFINE_PROP_END_OF_LIST(),
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 0906a1d..027a6fa 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1234,9 +1234,9 @@ const VMStateDescription sdhci_vmstate = {
 /* Capabilities registers provide information on supported features of this
  * specific host controller implementation */
 static Property sdhci_properties[] = {
-    DEFINE_PROP_HEX32("capareg", SDHCIState, capareg,
+    DEFINE_PROP_UINT32("capareg", SDHCIState, capareg,
             SDHC_CAPAB_REG_DEFAULT),
-    DEFINE_PROP_HEX32("maxcurr", SDHCIState, maxcurr, 0),
+    DEFINE_PROP_UINT32("maxcurr", SDHCIState, maxcurr, 0),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/timer/i8254.c b/hw/timer/i8254.c
index cdbf481..28152d8 100644
--- a/hw/timer/i8254.c
+++ b/hw/timer/i8254.c
@@ -342,7 +342,7 @@ static void pit_realizefn(DeviceState *dev, Error **err)
 }
 
 static Property pit_properties[] = {
-    DEFINE_PROP_HEX32("iobase", PITCommonState, iobase,  -1),
+    DEFINE_PROP_UINT32("iobase", PITCommonState, iobase,  -1),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/timer/m48t59.c b/hw/timer/m48t59.c
index 3cfb18a..7cf8684 100644
--- a/hw/timer/m48t59.c
+++ b/hw/timer/m48t59.c
@@ -741,7 +741,7 @@ static int m48t59_init1(SysBusDevice *dev)
 static Property m48t59_isa_properties[] = {
     DEFINE_PROP_UINT32("size",    M48t59ISAState, state.size,    -1),
     DEFINE_PROP_UINT32("model",   M48t59ISAState, state.model,   -1),
-    DEFINE_PROP_HEX32( "io_base", M48t59ISAState, state.io_base,  0),
+    DEFINE_PROP_UINT32("io_base", M48t59ISAState, state.io_base,  0),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -766,7 +766,7 @@ static const TypeInfo m48t59_isa_info = {
 static Property m48t59_properties[] = {
     DEFINE_PROP_UINT32("size",    M48t59SysBusState, state.size,    -1),
     DEFINE_PROP_UINT32("model",   M48t59SysBusState, state.model,   -1),
-    DEFINE_PROP_HEX32( "io_base", M48t59SysBusState, state.io_base,  0),
+    DEFINE_PROP_UINT32("io_base", M48t59SysBusState, state.io_base,  0),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
index fd320cd..57bed09 100644
--- a/hw/usb/host-libusb.c
+++ b/hw/usb/host-libusb.c
@@ -1324,8 +1324,8 @@ static Property usb_host_dev_properties[] = {
     DEFINE_PROP_UINT32("hostbus",  USBHostDevice, match.bus_num,    0),
     DEFINE_PROP_UINT32("hostaddr", USBHostDevice, match.addr,       0),
     DEFINE_PROP_STRING("hostport", USBHostDevice, match.port),
-    DEFINE_PROP_HEX32("vendorid",  USBHostDevice, match.vendor_id,  0),
-    DEFINE_PROP_HEX32("productid", USBHostDevice, match.product_id, 0),
+    DEFINE_PROP_UINT32("vendorid",  USBHostDevice, match.vendor_id,  0),
+    DEFINE_PROP_UINT32("productid", USBHostDevice, match.product_id, 0),
     DEFINE_PROP_UINT32("isobufs",  USBHostDevice, iso_urb_count,    4),
     DEFINE_PROP_UINT32("isobsize", USBHostDevice, iso_urb_frames,   32),
     DEFINE_PROP_INT32("bootindex", USBHostDevice, bootindex,        -1),
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 30c9f2b..7b91841 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1063,7 +1063,7 @@ static const TypeInfo virtio_pci_info = {
 /* virtio-blk-pci */
 
 static Property virtio_blk_pci_properties[] = {
-    DEFINE_PROP_HEX32("class", VirtIOPCIProxy, class_code, 0),
+    DEFINE_PROP_UINT32("class", VirtIOPCIProxy, class_code, 0),
     DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags,
                     VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
     DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2),
@@ -1275,7 +1275,7 @@ static void balloon_pci_stats_set_poll_interval(Object *obj, struct Visitor *v,
 
 static Property virtio_balloon_pci_properties[] = {
     DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features),
-    DEFINE_PROP_HEX32("class", VirtIOPCIProxy, class_code, 0),
+    DEFINE_PROP_UINT32("class", VirtIOPCIProxy, class_code, 0),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -1376,7 +1376,7 @@ static Property virtio_serial_pci_properties[] = {
     DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags,
                     VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
     DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2),
-    DEFINE_PROP_HEX32("class", VirtIOPCIProxy, class_code, 0),
+    DEFINE_PROP_UINT32("class", VirtIOPCIProxy, class_code, 0),
     DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features),
     DEFINE_VIRTIO_SERIAL_PROPERTIES(VirtIOSerialPCI, vdev.serial),
     DEFINE_PROP_END_OF_LIST(),
diff --git a/include/hw/qdev-dma.h b/include/hw/qdev-dma.h
index 6812735..8cfb0f3 100644
--- a/include/hw/qdev-dma.h
+++ b/include/hw/qdev-dma.h
@@ -7,4 +7,4 @@
  * See the COPYING file in the top-level directory.
  */
 #define DEFINE_PROP_DMAADDR(_n, _s, _f, _d)                               \
-    DEFINE_PROP_HEX64(_n, _s, _f, _d)
+    DEFINE_PROP_UINT64(_n, _s, _f, _d)
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 4651459..0c0babf 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -12,9 +12,6 @@ extern PropertyInfo qdev_prop_uint16;
 extern PropertyInfo qdev_prop_uint32;
 extern PropertyInfo qdev_prop_int32;
 extern PropertyInfo qdev_prop_uint64;
-extern PropertyInfo qdev_prop_hex8;
-extern PropertyInfo qdev_prop_hex32;
-extern PropertyInfo qdev_prop_hex64;
 extern PropertyInfo qdev_prop_size;
 extern PropertyInfo qdev_prop_string;
 extern PropertyInfo qdev_prop_chr;
@@ -111,12 +108,6 @@ extern PropertyInfo qdev_prop_arraylen;
     DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_int32, int32_t)
 #define DEFINE_PROP_UINT64(_n, _s, _f, _d)                      \
     DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_uint64, uint64_t)
-#define DEFINE_PROP_HEX8(_n, _s, _f, _d)                       \
-    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_hex8, uint8_t)
-#define DEFINE_PROP_HEX32(_n, _s, _f, _d)                       \
-    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_hex32, uint32_t)
-#define DEFINE_PROP_HEX64(_n, _s, _f, _d)                       \
-    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_hex64, uint64_t)
 #define DEFINE_PROP_SIZE(_n, _s, _f, _d)                       \
     DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_size, uint64_t)
 #define DEFINE_PROP_PCI_DEVFN(_n, _s, _f, _d)                   \
-- 
1.8.4.2

Re: [Qemu-devel] [PATCH 10/12] qdev: remove hex8/32/64 property types

[Eric Blake]

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

On 01/30/2014 06:09 AM, Paolo Bonzini wrote:
> Replace them with uint8/32/64.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---

Reviewed-by: Eric Blake <eblake@redhat.com>

> +++ b/hw/audio/pcspk.c
> @@ -181,7 +181,7 @@ static void pcspk_realizefn(DeviceState *dev, Error **errp)
>  }
>  
>  static Property pcspk_properties[] = {
> -    DEFINE_PROP_HEX32("iobase", PCSpkState, iobase,  -1),
> +    DEFINE_PROP_UINT32("iobase", PCSpkState, iobase,  -1),

When there's multiple properties, I can see the value of alignment.  But
for this instance, the two spaces before -1 seem spurious; you could fix
them in this patch.


> +++ b/hw/char/parallel.c
> @@ -595,7 +595,7 @@ bool parallel_mm_init(MemoryRegion *address_space,
>  
>  static Property parallel_isa_properties[] = {
>      DEFINE_PROP_UINT32("index", ISAParallelState, index,   -1),
> -    DEFINE_PROP_HEX32("iobase", ISAParallelState, iobase,  -1),
> +    DEFINE_PROP_UINT32("iobase", ISAParallelState, iobase,  -1),
>      DEFINE_PROP_UINT32("irq",   ISAParallelState, isairq,  7),
>      DEFINE_PROP_CHR("chardev",  ISAParallelState, state.chr),

And alignment looks all messed up on this one.

>      DEFINE_PROP_END_OF_LIST(),
> diff --git a/hw/char/serial-isa.c b/hw/char/serial-isa.c
> index 5cb77b3..c9fcb27 100644
> --- a/hw/char/serial-isa.c
> +++ b/hw/char/serial-isa.c
> @@ -88,7 +88,7 @@ static const VMStateDescription vmstate_isa_serial = {
>  
>  static Property serial_isa_properties[] = {
>      DEFINE_PROP_UINT32("index",  ISASerialState, index,   -1),
> -    DEFINE_PROP_HEX32("iobase",  ISASerialState, iobase,  -1),
> +    DEFINE_PROP_UINT32("iobase",  ISASerialState, iobase,  -1),

Drop a space before ISASerialState to make this one look nice.


> +++ b/hw/display/tcx.c
> @@ -617,11 +617,11 @@ static int tcx_init1(SysBusDevice *dev)
>  }
>  
>  static Property tcx_properties[] = {
> -    DEFINE_PROP_HEX32("vram_size", TCXState, vram_size, -1),
> +    DEFINE_PROP_UINT32("vram_size", TCXState, vram_size, -1),
>      DEFINE_PROP_UINT16("width",    TCXState, width,     -1),
>      DEFINE_PROP_UINT16("height",   TCXState, height,    -1),
>      DEFINE_PROP_UINT16("depth",    TCXState, depth,     -1),
> -    DEFINE_PROP_HEX64("prom_addr", TCXState, prom_addr, -1),
> +    DEFINE_PROP_UINT64("prom_addr", TCXState, prom_addr, -1),

Another one where alignment is now funky.


> +++ b/hw/ide/isa.c
> @@ -104,8 +104,8 @@ ISADevice *isa_ide_init(ISABus *bus, int iobase, int iobase2, int isairq,
>  }
>  
>  static Property isa_ide_properties[] = {
> -    DEFINE_PROP_HEX32("iobase",  ISAIDEState, iobase,  0x1f0),
> -    DEFINE_PROP_HEX32("iobase2", ISAIDEState, iobase2, 0x3f6),
> +    DEFINE_PROP_UINT32("iobase",  ISAIDEState, iobase,  0x1f0),
> +    DEFINE_PROP_UINT32("iobase2", ISAIDEState, iobase2, 0x3f6),
>      DEFINE_PROP_UINT32("irq",    ISAIDEState, isairq,  14),

And another one.

>      DEFINE_PROP_END_OF_LIST(),
>  };
> diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
> index 18c4b7e..6e475e6 100644
> --- a/hw/ide/qdev.c
> +++ b/hw/ide/qdev.c
> @@ -206,7 +206,7 @@ static int ide_drive_initfn(IDEDevice *dev)
>  #define DEFINE_IDE_DEV_PROPERTIES()                     \
>      DEFINE_BLOCK_PROPERTIES(IDEDrive, dev.conf),        \
>      DEFINE_PROP_STRING("ver",  IDEDrive, dev.version),  \
> -    DEFINE_PROP_HEX64("wwn",  IDEDrive, dev.wwn, 0),    \
> +    DEFINE_PROP_UINT64("wwn",  IDEDrive, dev.wwn, 0),    \

Here, the trailing \ lost alignment.

> +++ b/hw/intc/i8259_common.c
> @@ -123,9 +123,9 @@ static const VMStateDescription vmstate_pic_common = {
>  };
>  
>  static Property pic_properties_common[] = {
> -    DEFINE_PROP_HEX32("iobase", PICCommonState, iobase,  -1),
> -    DEFINE_PROP_HEX32("elcr_addr", PICCommonState, elcr_addr,  -1),
> -    DEFINE_PROP_HEX8("elcr_mask", PICCommonState, elcr_mask,  -1),
> +    DEFINE_PROP_UINT32("iobase", PICCommonState, iobase,  -1),
> +    DEFINE_PROP_UINT32("elcr_addr", PICCommonState, elcr_addr,  -1),
> +    DEFINE_PROP_UINT8("elcr_mask", PICCommonState, elcr_mask,  -1),
>      DEFINE_PROP_BIT("master", PICCommonState, master,  0, false),

Here there's no alignment, but lots of doubled spaces.

> +++ b/hw/misc/applesmc.c
> @@ -250,7 +250,7 @@ static void applesmc_isa_realize(DeviceState *dev, Error **errp)
>  }
>  
>  static Property applesmc_isa_properties[] = {
> -    DEFINE_PROP_HEX32("iobase", AppleSMCState, iobase,
> +    DEFINE_PROP_UINT32("iobase", AppleSMCState, iobase,
>                        APPLESMC_DEFAULT_IOBASE),

Indentation is now off.

> +++ b/hw/net/ne2000-isa.c
> @@ -86,7 +86,7 @@ static void isa_ne2000_realizefn(DeviceState *dev, Error **errp)
>  }
>  
>  static Property ne2000_isa_properties[] = {
> -    DEFINE_PROP_HEX32("iobase", ISANE2000State, iobase, 0x300),
> +    DEFINE_PROP_UINT32("iobase", ISANE2000State, iobase, 0x300),
>      DEFINE_PROP_UINT32("irq",   ISANE2000State, isairq, 9),
>      DEFINE_NIC_PROPERTIES(ISANE2000State, ne2000.c),

Here, the line you didn't touch looks awkward.

> +++ b/hw/sd/sdhci.c
> @@ -1234,9 +1234,9 @@ const VMStateDescription sdhci_vmstate = {
>  /* Capabilities registers provide information on supported features of this
>   * specific host controller implementation */
>  static Property sdhci_properties[] = {
> -    DEFINE_PROP_HEX32("capareg", SDHCIState, capareg,
> +    DEFINE_PROP_UINT32("capareg", SDHCIState, capareg,
>              SDHC_CAPAB_REG_DEFAULT),

Might as well fix indentation while touching this.


> +++ b/hw/timer/i8254.c
> @@ -342,7 +342,7 @@ static void pit_realizefn(DeviceState *dev, Error **err)
>  }
>  
>  static Property pit_properties[] = {
> -    DEFINE_PROP_HEX32("iobase", PITCommonState, iobase,  -1),
> +    DEFINE_PROP_UINT32("iobase", PITCommonState, iobase,  -1),
>      DEFINE_PROP_END_OF_LIST(),

Another instance of unneeded double space.


> +++ b/hw/usb/host-libusb.c
> @@ -1324,8 +1324,8 @@ static Property usb_host_dev_properties[] = {
>      DEFINE_PROP_UINT32("hostbus",  USBHostDevice, match.bus_num,    0),
>      DEFINE_PROP_UINT32("hostaddr", USBHostDevice, match.addr,       0),
>      DEFINE_PROP_STRING("hostport", USBHostDevice, match.port),
> -    DEFINE_PROP_HEX32("vendorid",  USBHostDevice, match.vendor_id,  0),
> -    DEFINE_PROP_HEX32("productid", USBHostDevice, match.product_id, 0),
> +    DEFINE_PROP_UINT32("vendorid",  USBHostDevice, match.vendor_id,  0),
> +    DEFINE_PROP_UINT32("productid", USBHostDevice, match.product_id, 0),
>      DEFINE_PROP_UINT32("isobufs",  USBHostDevice, iso_urb_count,    4),

Alignment looks off.


> +++ b/include/hw/qdev-dma.h
> @@ -7,4 +7,4 @@
>   * See the COPYING file in the top-level directory.
>   */
>  #define DEFINE_PROP_DMAADDR(_n, _s, _f, _d)                               \
> -    DEFINE_PROP_HEX64(_n, _s, _f, _d)
> +    DEFINE_PROP_UINT64(_n, _s, _f, _d)

Do we even need this #define, or would it be smarter to just inline the
use of DEFINE_PROP_UINT64 in the rest of the file?

All my comments are cosmetic, so my review still stands; I'm fine
whether you apply this as-is or touch it up per the suggestions.

-- 
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: 604 bytes --]

[Qemu-devel] [PATCH 11/12] qdev: add enum property types to QAPI schema

[Paolo Bonzini]

Prepare for when QOM introspection will be able to piggyback on the QAPI
schema for describing property types.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/core/qdev-properties.c | 18 +++------------
 hw/i386/kvm/i8254.c       |  6 ++---
 hw/timer/mc146818rtc.c    | 14 ++++++------
 include/hw/block/block.h  |  6 -----
 include/qemu-common.h     |  8 -------
 qapi-schema.json          | 58 +++++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 71 insertions(+), 39 deletions(-)

diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 2c3a756..0a2ca05 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -449,34 +449,22 @@ PropertyInfo qdev_prop_macaddr = {
 
 /* --- lost tick policy --- */
 
-static const char *lost_tick_policy_table[LOST_TICK_MAX+1] = {
-    [LOST_TICK_DISCARD] = "discard",
-    [LOST_TICK_DELAY] = "delay",
-    [LOST_TICK_MERGE] = "merge",
-    [LOST_TICK_SLEW] = "slew",
-    [LOST_TICK_MAX] = NULL,
-};
-
 QEMU_BUILD_BUG_ON(sizeof(LostTickPolicy) != sizeof(int));
 
 PropertyInfo qdev_prop_losttickpolicy = {
     .name  = "LostTickPolicy",
-    .enum_table  = lost_tick_policy_table,
+    .enum_table  = LostTickPolicy_lookup,
     .get   = get_enum,
     .set   = set_enum,
 };
 
 /* --- BIOS CHS translation */
 
-static const char *bios_chs_trans_table[] = {
-    [BIOS_ATA_TRANSLATION_AUTO] = "auto",
-    [BIOS_ATA_TRANSLATION_NONE] = "none",
-    [BIOS_ATA_TRANSLATION_LBA]  = "lba",
-};
+QEMU_BUILD_BUG_ON(sizeof(BiosAtaTranslation) != sizeof(int));
 
 PropertyInfo qdev_prop_bios_chs_trans = {
     .name = "bios-chs-trans",
-    .enum_table = bios_chs_trans_table,
+    .enum_table = BiosAtaTranslation_lookup,
     .get = get_enum,
     .set = set_enum,
 };
diff --git a/hw/i386/kvm/i8254.c b/hw/i386/kvm/i8254.c
index f8f3021..59373aa 100644
--- a/hw/i386/kvm/i8254.c
+++ b/hw/i386/kvm/i8254.c
@@ -268,9 +268,9 @@ static void kvm_pit_realizefn(DeviceState *dev, Error **errp)
         return;
     }
     switch (s->lost_tick_policy) {
-    case LOST_TICK_DELAY:
+    case LOST_TICK_POLICY_DELAY:
         break; /* enabled by default */
-    case LOST_TICK_DISCARD:
+    case LOST_TICK_POLICY_DISCARD:
         if (kvm_check_extension(kvm_state, KVM_CAP_REINJECT_CONTROL)) {
             struct kvm_reinject_control control = { .pit_reinject = 0 };
 
@@ -300,7 +300,7 @@ static void kvm_pit_realizefn(DeviceState *dev, Error **errp)
 static Property kvm_pit_properties[] = {
     DEFINE_PROP_UINT32("iobase", PITCommonState, iobase,  -1),
     DEFINE_PROP_LOSTTICKPOLICY("lost_tick_policy", KVMPITState,
-                               lost_tick_policy, LOST_TICK_DELAY),
+                               lost_tick_policy, LOST_TICK_POLICY_DELAY),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c
index 6fb124f..8509309 100644
--- a/hw/timer/mc146818rtc.c
+++ b/hw/timer/mc146818rtc.c
@@ -185,7 +185,7 @@ static void rtc_periodic_timer(void *opaque)
     if (s->cmos_data[RTC_REG_B] & REG_B_PIE) {
         s->cmos_data[RTC_REG_C] |= REG_C_IRQF;
 #ifdef TARGET_I386
-        if (s->lost_tick_policy == LOST_TICK_SLEW) {
+        if (s->lost_tick_policy == LOST_TICK_POLICY_SLEW) {
             if (s->irq_reinject_on_ack_count >= RTC_REINJECT_ON_ACK_COUNT)
                 s->irq_reinject_on_ack_count = 0;		
             apic_reset_irq_delivered();
@@ -708,7 +708,7 @@ static int rtc_post_load(void *opaque, int version_id)
 
 #ifdef TARGET_I386
     if (version_id >= 2) {
-        if (s->lost_tick_policy == LOST_TICK_SLEW) {
+        if (s->lost_tick_policy == LOST_TICK_POLICY_SLEW) {
             rtc_coalesced_timer_update(s);
         }
     }
@@ -749,7 +749,7 @@ static void rtc_notify_clock_reset(Notifier *notifier, void *data)
     periodic_timer_update(s, now);
     check_update_timer(s);
 #ifdef TARGET_I386
-    if (s->lost_tick_policy == LOST_TICK_SLEW) {
+    if (s->lost_tick_policy == LOST_TICK_POLICY_SLEW) {
         rtc_coalesced_timer_update(s);
     }
 #endif
@@ -774,7 +774,7 @@ static void rtc_reset(void *opaque)
     qemu_irq_lower(s->irq);
 
 #ifdef TARGET_I386
-    if (s->lost_tick_policy == LOST_TICK_SLEW) {
+    if (s->lost_tick_policy == LOST_TICK_POLICY_SLEW) {
         s->irq_coalesced = 0;
     }
 #endif
@@ -835,11 +835,11 @@ static void rtc_realizefn(DeviceState *dev, Error **errp)
 
 #ifdef TARGET_I386
     switch (s->lost_tick_policy) {
-    case LOST_TICK_SLEW:
+    case LOST_TICK_POLICY_SLEW:
         s->coalesced_timer =
             timer_new_ns(rtc_clock, rtc_coalesced_timer, s);
         break;
-    case LOST_TICK_DISCARD:
+    case LOST_TICK_POLICY_DISCARD:
         break;
     default:
         error_setg(errp, "Invalid lost tick policy.");
@@ -890,7 +890,7 @@ ISADevice *rtc_init(ISABus *bus, int base_year, qemu_irq intercept_irq)
 static Property mc146818rtc_properties[] = {
     DEFINE_PROP_INT32("base_year", RTCState, base_year, 1980),
     DEFINE_PROP_LOSTTICKPOLICY("lost_tick_policy", RTCState,
-                               lost_tick_policy, LOST_TICK_DISCARD),
+                               lost_tick_policy, LOST_TICK_POLICY_DISCARD),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/include/hw/block/block.h b/include/hw/block/block.h
index dd11532..7c3d6c8 100644
--- a/include/hw/block/block.h
+++ b/include/hw/block/block.h
@@ -65,12 +65,6 @@ int blkconf_geometry(BlockConf *conf, int *trans,
 
 /* Hard disk geometry */
 
-#define BIOS_ATA_TRANSLATION_AUTO   0
-#define BIOS_ATA_TRANSLATION_NONE   1
-#define BIOS_ATA_TRANSLATION_LBA    2
-#define BIOS_ATA_TRANSLATION_LARGE  3
-#define BIOS_ATA_TRANSLATION_RECHS  4
-
 void hd_geometry_guess(BlockDriverState *bs,
                        uint32_t *pcyls, uint32_t *pheads, uint32_t *psecs,
                        int *ptrans);
diff --git a/include/qemu-common.h b/include/qemu-common.h
index 5054836..b0e34b2 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -261,14 +261,6 @@ typedef int (*DMA_transfer_handler) (void *opaque, int nchan, int pos, int size)
 
 typedef uint64_t pcibus_t;
 
-typedef enum LostTickPolicy {
-    LOST_TICK_DISCARD,
-    LOST_TICK_DELAY,
-    LOST_TICK_MERGE,
-    LOST_TICK_SLEW,
-    LOST_TICK_MAX
-} LostTickPolicy;
-
 typedef struct PCIHostDeviceAddress {
     unsigned int domain;
     unsigned int bus;
diff --git a/qapi-schema.json b/qapi-schema.json
index 05ced9d..ed72938 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -28,6 +28,65 @@
   'data': [ 'GenericError', 'CommandNotFound', 'DeviceEncrypted',
             'DeviceNotActive', 'DeviceNotFound', 'KVMMissingCap' ] }
 
+
+##
+# LostTickPolicy:
+#
+# Policy for handling lost ticks in timer devices.
+#
+# @discard: throw away the missed tick(s) and continue with future injection
+#           normally.  Guest time may be delayed, unless the OS has explicit
+#           handling of lost ticks
+#
+# @delay: continue to deliver ticks at the normal rate.  Guest time will be
+#         delayed due to the late tick
+#
+# @merge: merge the missed tick(s) into one tick and inject.  Guest time
+#         may be delayed, depending on how the OS reacts to the merging
+#         of ticks
+#
+# @slew: deliver ticks at a higher rate to catch up with the missed tick. The
+#        guest time should not be delayed once catchup is complete.
+#
+# Since: 2.0
+##
+{ 'enum': 'LostTickPolicy',
+  'data': ['discard', 'delay', 'merge', 'slew' ] }
+
+##
+# BiosAtaTranslation:
+#
+# Policy that BIOS should use to interpret cylinder/head/sector
+# addresses.  Note that Bochs BIOS and SeaBIOS will not actually
+# translate logical CHS to physical; instead, they will use logical
+# block addressing.
+#
+# @auto: If cylinder/heads/sizes are passed, choose between none and LBA
+#        depending on the size of the disk.  If they are not passed,
+#        choose none if QEMU can guess that the disk had 16 or fewer
+#        heads, large if QEMU can guess that the disk had 131072 or
+#        fewer tracks across all heads (i.e. cylinders*heads<131072),
+#        otherwise LBA.
+#
+# @none: The physical disk geometry is equal to the logical geometry.
+#
+# @lba: Assume 63 sectors per track and one of 16, 32, 64, 128 or 255
+#       heads (if fewer than 255 are enough to cover the whole disk
+#       with 1024 cylinders/head).  The number of cylinders/head is
+#       then computed based on the number of sectors and heads.
+#
+# @large: The number of cylinders per head is scaled down to 1024
+#         by correspondingly scaling up the number of heads.
+#
+# @rechs: Same as @large, but first convert a 16-head geometry to
+#         15-head, by proportionally scaling up the number of
+#         cylinders/head.
+#
+# Since: 2.0
+##
+{ 'enum': 'BiosAtaTranslation',
+  'data': ['auto', 'none', 'lba', 'large', 'rechs']}
+
 ##
 # @add_client
 #
-- 
1.8.4.2

Re: [Qemu-devel] [PATCH 11/12] qdev: add enum property types to QAPI schema

[Eric Blake]

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

On 01/30/2014 06:09 AM, Paolo Bonzini wrote:
> Prepare for when QOM introspection will be able to piggyback on the QAPI
> schema for describing property types.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/core/qdev-properties.c | 18 +++------------
>  hw/i386/kvm/i8254.c       |  6 ++---
>  hw/timer/mc146818rtc.c    | 14 ++++++------
>  include/hw/block/block.h  |  6 -----
>  include/qemu-common.h     |  8 -------
>  qapi-schema.json          | 58 +++++++++++++++++++++++++++++++++++++++++++++++
>  6 files changed, 71 insertions(+), 39 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
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: 604 bytes --]

Re: [Qemu-devel] [PATCH 11/12] qdev: add enum property types to QAPI schema

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> Prepare for when QOM introspection will be able to piggyback on the QAPI
> schema for describing property types.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/core/qdev-properties.c | 18 +++------------
>  hw/i386/kvm/i8254.c       |  6 ++---
>  hw/timer/mc146818rtc.c    | 14 ++++++------
>  include/hw/block/block.h  |  6 -----
>  include/qemu-common.h     |  8 -------
>  qapi-schema.json          | 58 +++++++++++++++++++++++++++++++++++++++++++++++
>  6 files changed, 71 insertions(+), 39 deletions(-)
>
> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> index 2c3a756..0a2ca05 100644
> --- a/hw/core/qdev-properties.c
> +++ b/hw/core/qdev-properties.c
> @@ -449,34 +449,22 @@ PropertyInfo qdev_prop_macaddr = {
>  
>  /* --- lost tick policy --- */
>  
> -static const char *lost_tick_policy_table[LOST_TICK_MAX+1] = {
> -    [LOST_TICK_DISCARD] = "discard",
> -    [LOST_TICK_DELAY] = "delay",
> -    [LOST_TICK_MERGE] = "merge",
> -    [LOST_TICK_SLEW] = "slew",
> -    [LOST_TICK_MAX] = NULL,
> -};
> -
>  QEMU_BUILD_BUG_ON(sizeof(LostTickPolicy) != sizeof(int));
>  
>  PropertyInfo qdev_prop_losttickpolicy = {
>      .name  = "LostTickPolicy",
> -    .enum_table  = lost_tick_policy_table,
> +    .enum_table  = LostTickPolicy_lookup,
>      .get   = get_enum,
>      .set   = set_enum,
>  };
>  
>  /* --- BIOS CHS translation */
>  
> -static const char *bios_chs_trans_table[] = {
> -    [BIOS_ATA_TRANSLATION_AUTO] = "auto",
> -    [BIOS_ATA_TRANSLATION_NONE] = "none",
> -    [BIOS_ATA_TRANSLATION_LBA]  = "lba",
> -};
> +QEMU_BUILD_BUG_ON(sizeof(BiosAtaTranslation) != sizeof(int));

Recognized values before your patch: "auto", "none", "lba".

>  
>  PropertyInfo qdev_prop_bios_chs_trans = {
>      .name = "bios-chs-trans",
> -    .enum_table = bios_chs_trans_table,
> +    .enum_table = BiosAtaTranslation_lookup,
>      .get = get_enum,
>      .set = set_enum,
>  };
> diff --git a/hw/i386/kvm/i8254.c b/hw/i386/kvm/i8254.c
> index f8f3021..59373aa 100644
> --- a/hw/i386/kvm/i8254.c
> +++ b/hw/i386/kvm/i8254.c
> @@ -268,9 +268,9 @@ static void kvm_pit_realizefn(DeviceState *dev, Error **errp)
>          return;
>      }
>      switch (s->lost_tick_policy) {
> -    case LOST_TICK_DELAY:
> +    case LOST_TICK_POLICY_DELAY:
>          break; /* enabled by default */
> -    case LOST_TICK_DISCARD:
> +    case LOST_TICK_POLICY_DISCARD:
>          if (kvm_check_extension(kvm_state, KVM_CAP_REINJECT_CONTROL)) {
>              struct kvm_reinject_control control = { .pit_reinject = 0 };
>  
> @@ -300,7 +300,7 @@ static void kvm_pit_realizefn(DeviceState *dev, Error **errp)
>  static Property kvm_pit_properties[] = {
>      DEFINE_PROP_UINT32("iobase", PITCommonState, iobase,  -1),
>      DEFINE_PROP_LOSTTICKPOLICY("lost_tick_policy", KVMPITState,
> -                               lost_tick_policy, LOST_TICK_DELAY),
> +                               lost_tick_policy, LOST_TICK_POLICY_DELAY),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c
> index 6fb124f..8509309 100644
> --- a/hw/timer/mc146818rtc.c
> +++ b/hw/timer/mc146818rtc.c
> @@ -185,7 +185,7 @@ static void rtc_periodic_timer(void *opaque)
>      if (s->cmos_data[RTC_REG_B] & REG_B_PIE) {
>          s->cmos_data[RTC_REG_C] |= REG_C_IRQF;
>  #ifdef TARGET_I386
> -        if (s->lost_tick_policy == LOST_TICK_SLEW) {
> +        if (s->lost_tick_policy == LOST_TICK_POLICY_SLEW) {
>              if (s->irq_reinject_on_ack_count >= RTC_REINJECT_ON_ACK_COUNT)
>                  s->irq_reinject_on_ack_count = 0;		
>              apic_reset_irq_delivered();
> @@ -708,7 +708,7 @@ static int rtc_post_load(void *opaque, int version_id)
>  
>  #ifdef TARGET_I386
>      if (version_id >= 2) {
> -        if (s->lost_tick_policy == LOST_TICK_SLEW) {
> +        if (s->lost_tick_policy == LOST_TICK_POLICY_SLEW) {
>              rtc_coalesced_timer_update(s);
>          }
>      }
> @@ -749,7 +749,7 @@ static void rtc_notify_clock_reset(Notifier *notifier, void *data)
>      periodic_timer_update(s, now);
>      check_update_timer(s);
>  #ifdef TARGET_I386
> -    if (s->lost_tick_policy == LOST_TICK_SLEW) {
> +    if (s->lost_tick_policy == LOST_TICK_POLICY_SLEW) {
>          rtc_coalesced_timer_update(s);
>      }
>  #endif
> @@ -774,7 +774,7 @@ static void rtc_reset(void *opaque)
>      qemu_irq_lower(s->irq);
>  
>  #ifdef TARGET_I386
> -    if (s->lost_tick_policy == LOST_TICK_SLEW) {
> +    if (s->lost_tick_policy == LOST_TICK_POLICY_SLEW) {
>          s->irq_coalesced = 0;
>      }
>  #endif
> @@ -835,11 +835,11 @@ static void rtc_realizefn(DeviceState *dev, Error **errp)
>  
>  #ifdef TARGET_I386
>      switch (s->lost_tick_policy) {
> -    case LOST_TICK_SLEW:
> +    case LOST_TICK_POLICY_SLEW:
>          s->coalesced_timer =
>              timer_new_ns(rtc_clock, rtc_coalesced_timer, s);
>          break;
> -    case LOST_TICK_DISCARD:
> +    case LOST_TICK_POLICY_DISCARD:
>          break;
>      default:
>          error_setg(errp, "Invalid lost tick policy.");
> @@ -890,7 +890,7 @@ ISADevice *rtc_init(ISABus *bus, int base_year, qemu_irq intercept_irq)
>  static Property mc146818rtc_properties[] = {
>      DEFINE_PROP_INT32("base_year", RTCState, base_year, 1980),
>      DEFINE_PROP_LOSTTICKPOLICY("lost_tick_policy", RTCState,
> -                               lost_tick_policy, LOST_TICK_DISCARD),
> +                               lost_tick_policy, LOST_TICK_POLICY_DISCARD),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> diff --git a/include/hw/block/block.h b/include/hw/block/block.h
> index dd11532..7c3d6c8 100644
> --- a/include/hw/block/block.h
> +++ b/include/hw/block/block.h
> @@ -65,12 +65,6 @@ int blkconf_geometry(BlockConf *conf, int *trans,
>  
>  /* Hard disk geometry */
>  
> -#define BIOS_ATA_TRANSLATION_AUTO   0
> -#define BIOS_ATA_TRANSLATION_NONE   1
> -#define BIOS_ATA_TRANSLATION_LBA    2
> -#define BIOS_ATA_TRANSLATION_LARGE  3
> -#define BIOS_ATA_TRANSLATION_RECHS  4
> -
>  void hd_geometry_guess(BlockDriverState *bs,
>                         uint32_t *pcyls, uint32_t *pheads, uint32_t *psecs,
>                         int *ptrans);
> diff --git a/include/qemu-common.h b/include/qemu-common.h
> index 5054836..b0e34b2 100644
> --- a/include/qemu-common.h
> +++ b/include/qemu-common.h
> @@ -261,14 +261,6 @@ typedef int (*DMA_transfer_handler) (void *opaque, int nchan, int pos, int size)
>  
>  typedef uint64_t pcibus_t;
>  
> -typedef enum LostTickPolicy {
> -    LOST_TICK_DISCARD,
> -    LOST_TICK_DELAY,
> -    LOST_TICK_MERGE,
> -    LOST_TICK_SLEW,
> -    LOST_TICK_MAX
> -} LostTickPolicy;
> -
>  typedef struct PCIHostDeviceAddress {
>      unsigned int domain;
>      unsigned int bus;
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 05ced9d..ed72938 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -28,6 +28,65 @@
>    'data': [ 'GenericError', 'CommandNotFound', 'DeviceEncrypted',
>              'DeviceNotActive', 'DeviceNotFound', 'KVMMissingCap' ] }
>  
> +
> +##
> +# LostTickPolicy:
> +#
> +# Policy for handling lost ticks in timer devices.
> +#
> +# @discard: throw away the missed tick(s) and continue with future injection
> +#           normally.  Guest time may be delayed, unless the OS has explicit
> +#           handling of lost ticks
> +#
> +# @delay: continue to deliver ticks at the normal rate.  Guest time will be
> +#         delayed due to the late tick
> +#
> +# @merge: merge the missed tick(s) into one tick and inject.  Guest time
> +#         may be delayed, depending on how the OS reacts to the merging
> +#         of ticks
> +#
> +# @slew: deliver ticks at a higher rate to catch up with the missed tick. The
> +#        guest time should not be delayed once catchup is complete.
> +#
> +# Since: 2.0
> +##
> +{ 'enum': 'LostTickPolicy',
> +  'data': ['discard', 'delay', 'merge', 'slew' ] }
> +
> +##
> +# BiosAtaTranslation:
> +#
> +# Policy that BIOS should use to interpret cylinder/head/sector
> +# addresses.  Note that Bochs BIOS and SeaBIOS will not actually
> +# translate logical CHS to physical; instead, they will use logical
> +# block addressing.
> +#
> +# @auto: If cylinder/heads/sizes are passed, choose between none and LBA
> +#        depending on the size of the disk.  If they are not passed,
> +#        choose none if QEMU can guess that the disk had 16 or fewer
> +#        heads, large if QEMU can guess that the disk had 131072 or
> +#        fewer tracks across all heads (i.e. cylinders*heads<131072),
> +#        otherwise LBA.
> +#
> +# @none: The physical disk geometry is equal to the logical geometry.
> +#
> +# @lba: Assume 63 sectors per track and one of 16, 32, 64, 128 or 255
> +#       heads (if fewer than 255 are enough to cover the whole disk
> +#       with 1024 cylinders/head).  The number of cylinders/head is
> +#       then computed based on the number of sectors and heads.
> +#
> +# @large: The number of cylinders per head is scaled down to 1024
> +#         by correspondingly scaling up the number of heads.
> +#
> +# @rechs: Same as @large, but first convert a 16-head geometry to
> +#         15-head, by proportionally scaling up the number of
> +#         cylinders/head.
> +#
> +# Since: 2.0
> +##
> +{ 'enum': 'BiosAtaTranslation',
> +  'data': ['auto', 'none', 'lba', 'large', 'rechs']}
> +
>  ##
>  # @add_client
>  #

Recognized values after your patch: same as before, plus "large",
"rechs".  Silent change!

Please keep distinct things separate: have one patch introducing new
values "large" and "rechs", and another patch to QAPIfy.

If you want the obsolete ways to specify translation accept the new
values, too, then the patch to do that (currently "[PATCH v2] block:
handle "rechs" and "large" translation options") should immediately
follow the first one, or be squashed into it.

Re: [Qemu-devel] [PATCH 11/12] qdev: add enum property types to QAPI schema

[Paolo Bonzini]

Il 31/01/2014 09:05, Markus Armbruster ha scritto:
>
> If you want the obsolete ways to specify translation accept the new
> values, too, then the patch to do that (currently "[PATCH v2] block:
> handle "rechs" and "large" translation options") should immediately
> follow the first one, or be squashed into it.

I can squash it and put it in this series with an ack from the block 
maintainers.

Paolo

[Qemu-devel] [PATCH 12/12] qdev: use QAPI type names for properties

[Paolo Bonzini]

Use "drive", "chr", etc. only for legacy_name (which shows up
in -device foo,? output).  From the point of view of their type,
these are just strings.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/core/qdev-properties-system.c | 12 ++++++++----
 hw/core/qdev-properties.c        | 18 +++++++++++-------
 2 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 3f29b49..5f5957e 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -109,7 +109,8 @@ static void set_drive(Object *obj, Visitor *v, void *opaque,
 }
 
 PropertyInfo qdev_prop_drive = {
-    .name  = "drive",
+    .name  = "str",
+    .legacy_name  = "drive",
     .get   = get_drive,
     .set   = set_drive,
     .release = release_drive,
@@ -164,7 +165,8 @@ static void set_chr(Object *obj, Visitor *v, void *opaque,
 }
 
 PropertyInfo qdev_prop_chr = {
-    .name  = "chr",
+    .name  = "str",
+    .legacy_name  = "chr",
     .get   = get_chr,
     .set   = set_chr,
     .release = release_chr,
@@ -242,7 +244,8 @@ static void set_netdev(Object *obj, Visitor *v, void *opaque,
 }
 
 PropertyInfo qdev_prop_netdev = {
-    .name  = "netdev",
+    .name  = "str",
+    .legacy_name  = "netdev",
     .get   = get_netdev,
     .set   = set_netdev,
 };
@@ -321,7 +324,8 @@ static void set_vlan(Object *obj, Visitor *v, void *opaque,
 }
 
 PropertyInfo qdev_prop_vlan = {
-    .name  = "vlan",
+    .name  = "int32",
+    .legacy_name  = "vlan",
     .print = print_vlan,
     .get   = get_vlan,
     .set   = set_vlan,
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 0a2ca05..77d0c66 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -107,7 +107,7 @@ static void prop_set_bit(Object *obj, Visitor *v, void *opaque,
 }
 
 PropertyInfo qdev_prop_bit = {
-    .name  = "boolean",
+    .name  = "bool",
     .legacy_name  = "on/off",
     .get   = prop_get_bit,
     .set   = prop_set_bit,
@@ -141,7 +141,8 @@ static void set_bool(Object *obj, Visitor *v, void *opaque,
 }
 
 PropertyInfo qdev_prop_bool = {
-    .name  = "boolean",
+    .name  = "bool",
+    .legacy_name  = "boolean",
     .get   = get_bool,
     .set   = set_bool,
 };
@@ -358,7 +358,7 @@ static void set_string(Object *obj, Visitor *v, void *opaque,
 }
 
 PropertyInfo qdev_prop_string = {
-    .name  = "string",
+    .name  = "str",
     .release = release_string,
     .get   = get_string,
     .set   = set_string,
@@ -442,7 +442,8 @@ inval:
 }
 
 PropertyInfo qdev_prop_macaddr = {
-    .name  = "macaddr",
+    .name  = "str",
+    .legacy_name  = "macaddr",
     .get   = get_mac,
     .set   = set_mac,
 };
@@ -463,7 +464,8 @@ PropertyInfo qdev_prop_losttickpolicy = {
 QEMU_BUILD_BUG_ON(sizeof(BiosAtaTranslation) != sizeof(int));
 
 PropertyInfo qdev_prop_bios_chs_trans = {
-    .name = "bios-chs-trans",
+    .name = "BiosAtaTranslation",
+    .legacy_name = "bios-chs-trans",
     .enum_table = BiosAtaTranslation_lookup,
     .get = get_enum,
     .set = set_enum,
@@ -582,7 +584,8 @@ static void set_blocksize(Object *obj, Visitor *v, void *opaque,
 }
 
 PropertyInfo qdev_prop_blocksize = {
-    .name  = "blocksize",
+    .name  = "uint16",
+    .legacy_name  = "blocksize",
     .get   = get_uint16,
     .set   = set_blocksize,
 };
@@ -689,7 +692,8 @@ inval:
 }
 
 PropertyInfo qdev_prop_pci_host_devaddr = {
-    .name = "pci-host-devaddr",
+    .name = "str",
+    .legacy_name = "pci-host-devaddr",
     .get = get_pci_host_devaddr,
     .set = set_pci_host_devaddr,
 };
-- 
1.8.4.2

Re: [Qemu-devel] [PATCH 12/12] qdev: use QAPI type names for properties

[Eric Blake]

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

On 01/30/2014 06:09 AM, Paolo Bonzini wrote:
> Use "drive", "chr", etc. only for legacy_name (which shows up
> in -device foo,? output).  From the point of view of their type,
> these are just strings.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/core/qdev-properties-system.c | 12 ++++++++----
>  hw/core/qdev-properties.c        | 18 +++++++++++-------
>  2 files changed, 19 insertions(+), 11 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
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: 604 bytes --]

[Qemu-devel] [PATCH 13/12] qapi: refine human printing of sizes

[Paolo Bonzini]

This fixes several bugs or shortcomings of the previous pretty-printer.
In particular:

* use PRIu64 instead of casting to long long

* the exact value is included too

* the correct unit of measure (MiB, GiB, etc.) is used.  PiB and EiB
are added too.

* due to an off-by-one error, 512*2^30 was printed as 0.500MiB rather than
512MiB.  floor(log2(val)) is equal to 63 - clz(val), while the code used 64.

* The desired specification is %g rather than %f, which always uses three
decimals in the current code.  However %g would switch to scientific
notation when the integer part is >= 1000 (e.g. 1000*2^30).  To keep the
code simple, switch to the higher power when the integer part is >= 1000;
overflow is avoided by using frexp instead of clz.

Suggested-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qapi/string-output-visitor.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c
index 67a8798..95dd8fa 100644
--- a/qapi/string-output-visitor.c
+++ b/qapi/string-output-visitor.c
@@ -15,6 +15,7 @@
 #include "qapi/visitor-impl.h"
 #include "qapi/qmp/qerror.h"
 #include "qemu/host-utils.h"
+#include <math.h>
 
 struct StringOutputVisitor
 {
@@ -47,30 +48,30 @@ static void print_type_size(Visitor *v, uint64_t *obj, const char *name,
                            Error **errp)
 {
     StringOutputVisitor *sov = DO_UPCAST(StringOutputVisitor, visitor, v);
-    static const char suffixes[] = { 'B', 'K', 'M', 'G', 'T' };
+    static const char suffixes[] = { 'B', 'K', 'M', 'G', 'T', 'P', 'E' };
     uint64_t div, val;
     char *out;
     int i;
 
     if (!sov->human) {
-        out = g_strdup_printf("%llu", (long long) *obj);
+        out = g_strdup_printf("%"PRIu64, *obj);
         string_output_set(sov, out);
         return;
     }
 
     val = *obj;
 
-    /* Compute floor(log2(val)).  */
-    i = 64 - clz64(val);
-
-    /* Find the power of 1024 that we'll display as the units.  */
-    i /= 10;
-    if (i >= ARRAY_SIZE(suffixes)) {
-        i = ARRAY_SIZE(suffixes) - 1;
-    }
+    /* The exponent (returned in i) minus one gives us
+     * floor(log2(val * 1024 / 1000).  The correction makes us
+     * switch to the higher power when the integer part is >= 1000.
+     */
+    frexp(val / (1000.0 / 1024.0), &i);
+    i = (i - 1) / 10;
+    assert(i < ARRAY_SIZE(suffixes));
     div = 1ULL << (i * 10);
 
-    out = g_strdup_printf("%0.03f%c", (double)val/div, suffixes[i]);
+    out = g_strdup_printf("%"PRIu64" (%0.3g %c%s)", val,
+			  (double)val/div, suffixes[i], i ? "iB" : "");
     string_output_set(sov, out);
 }
 
-- 
1.8.4.2

Re: [Qemu-devel] [PATCH 13/12] qapi: refine human printing of sizes

[Eric Blake]

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

On 01/30/2014 09:42 AM, Paolo Bonzini wrote:
> This fixes several bugs or shortcomings of the previous pretty-printer.
> In particular:
> 
> * use PRIu64 instead of casting to long long
> 
> * the exact value is included too
> 
> * the correct unit of measure (MiB, GiB, etc.) is used.  PiB and EiB
> are added too.
> 
> * due to an off-by-one error, 512*2^30 was printed as 0.500MiB rather than
> 512MiB.  floor(log2(val)) is equal to 63 - clz(val), while the code used 64.
> 
> * The desired specification is %g rather than %f, which always uses three
> decimals in the current code.  However %g would switch to scientific
> notation when the integer part is >= 1000 (e.g. 1000*2^30).  To keep the
> code simple, switch to the higher power when the integer part is >= 1000;
> overflow is avoided by using frexp instead of clz.
> 
> Suggested-by: Eric Blake <eblake@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  qapi/string-output-visitor.c | 23 ++++++++++++-----------
>  1 file changed, 12 insertions(+), 11 deletions(-)
> 

>  
> -    out = g_strdup_printf("%0.03f%c", (double)val/div, suffixes[i]);
> +    out = g_strdup_printf("%"PRIu64" (%0.3g %c%s)", val,

For 1152, the old code would print 1.125k, while the new code prints
only 1.12k.  But that's acceptable in my eyes (Remember, %0.3g is how
many significant digits, where both 0.999 and 1.01 have 3 digits).

I also checked that overflow is not possible, and checked several border
cases.  Much nicer!

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
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: 604 bytes --]

Re: [Qemu-devel] [PATCH 13/12] qapi: refine human printing of sizes

[Igor Mammedov]

On Thu, 30 Jan 2014 17:42:25 +0100
Paolo Bonzini <pbonzini@redhat.com> wrote:

> This fixes several bugs or shortcomings of the previous pretty-printer.
> In particular:
> 
> * use PRIu64 instead of casting to long long
> 
> * the exact value is included too
> 
> * the correct unit of measure (MiB, GiB, etc.) is used.  PiB and EiB
> are added too.
> 
> * due to an off-by-one error, 512*2^30 was printed as 0.500MiB rather than
> 512MiB.  floor(log2(val)) is equal to 63 - clz(val), while the code used 64.
> 
> * The desired specification is %g rather than %f, which always uses three
> decimals in the current code.  However %g would switch to scientific
> notation when the integer part is >= 1000 (e.g. 1000*2^30).  To keep the
> code simple, switch to the higher power when the integer part is >= 1000;
> overflow is avoided by using frexp instead of clz.
> 
> Suggested-by: Eric Blake <eblake@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  qapi/string-output-visitor.c | 23 ++++++++++++-----------
>  1 file changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c
> index 67a8798..95dd8fa 100644
> --- a/qapi/string-output-visitor.c
> +++ b/qapi/string-output-visitor.c
> @@ -15,6 +15,7 @@
>  #include "qapi/visitor-impl.h"
>  #include "qapi/qmp/qerror.h"
>  #include "qemu/host-utils.h"
> +#include <math.h>
>  
>  struct StringOutputVisitor
>  {
> @@ -47,30 +48,30 @@ static void print_type_size(Visitor *v, uint64_t *obj, const char *name,
>                             Error **errp)
>  {
>      StringOutputVisitor *sov = DO_UPCAST(StringOutputVisitor, visitor, v);
> -    static const char suffixes[] = { 'B', 'K', 'M', 'G', 'T' };
> +    static const char suffixes[] = { 'B', 'K', 'M', 'G', 'T', 'P', 'E' };
>      uint64_t div, val;
>      char *out;
>      int i;
>  
>      if (!sov->human) {
> -        out = g_strdup_printf("%llu", (long long) *obj);
> +        out = g_strdup_printf("%"PRIu64, *obj);
>          string_output_set(sov, out);
>          return;
>      }
>  
>      val = *obj;
>  
> -    /* Compute floor(log2(val)).  */
> -    i = 64 - clz64(val);
> -
> -    /* Find the power of 1024 that we'll display as the units.  */
> -    i /= 10;
> -    if (i >= ARRAY_SIZE(suffixes)) {
> -        i = ARRAY_SIZE(suffixes) - 1;
> -    }
> +    /* The exponent (returned in i) minus one gives us
> +     * floor(log2(val * 1024 / 1000).  The correction makes us
> +     * switch to the higher power when the integer part is >= 1000.
> +     */
> +    frexp(val / (1000.0 / 1024.0), &i);
> +    i = (i - 1) / 10;
> +    assert(i < ARRAY_SIZE(suffixes));
>      div = 1ULL << (i * 10);
>  
> -    out = g_strdup_printf("%0.03f%c", (double)val/div, suffixes[i]);
> +    out = g_strdup_printf("%"PRIu64" (%0.3g %c%s)", val,
> +			  (double)val/div, suffixes[i], i ? "iB" : "");
ERROR: code indent should never use tabs
#81: FILE: qapi/string-output-visitor.c:74:
+^I^I^I  (double)val/div, suffixes[i], i ? "iB" : "");$

>      string_output_set(sov, out);
>  }
>  

Re: [Qemu-devel] [PATCH 00/12] qdev: cleanup legacy properties

[Igor Mammedov]

On Thu, 30 Jan 2014 14:09:42 +0100
Paolo Bonzini <pbonzini@redhat.com> wrote:

> The conversion of qdev to QOM brought with it legacy properties.
> Legacy properties are always have a string type (the accessors always
> call visit_type_str), and were used to support -device syntax while
> keeping QOM properties strongly typed.  For example, an hex8 property
> is registered twice, once as an integer-typed property and once as a
> legacy property that enforces base 16 for its input.
> 
> However, when introducing legacy properties, the hex8/32/64 had a small
> change applied: the previously-optional "0x" prefix became mandatory,
> and an error was raised if you omitted it.  This was in preparation
> for making the legacy properties read-only, and changing the hex8/32/64
> properties to uint8/32/64.  This series does exactly this in patches 1-6.
> 
> On the printing side, legacy properties are used by "info qtree" to
> tweak its presentation: strings are quoted, hex8/32/64 properties are
> printed in hexadecimal, and so on.  In this series, patches 7-10 add a
> "human" mode to StringOutputVisitors.  This mode employs a slightly
> different presentation, more suitable for human consumption, but its
> output cannot be sent back to a StringInputVisitor.  The main change
> is that numbers are printed in both decimal and 0x-prefixed hexadecimal.
> This lets us drop hex8/32/64 property types.
> 
> Finally, patches 11-12 clean up the type names used for properties.
> These are always QAPI names, so that in the future QOM introspection
> can piggyback on QAPI introspection for describing property types.
> 
> Paolo Bonzini (12):
>   qapi: add size parser to StringInputVisitor
>   qdev: sizes are now parsed by StringInputVisitor
>   qdev: remove legacy parsers for hex8/32/64
>   qdev: legacy properties are now read-only
>   qdev: legacy properties are just strings
>   qdev: inline qdev_prop_parse
>   qapi: add human mode to StringOutputVisitor
>   qdev: use human mode in "info qtree"
>   qdev: remove most legacy printers
>   qdev: remove hex8/32/64 property types
>   qdev: add enum property types to QAPI schema
>   qdev: use QAPI type names for properties
> 
>  hw/audio/adlib.c                     |   2 +-
>  hw/audio/cs4231a.c                   |   2 +-
>  hw/audio/gus.c                       |   2 +-
>  hw/audio/pcspk.c                     |   2 +-
>  hw/audio/sb16.c                      |   4 +-
>  hw/block/fdc.c                       |   2 +-
>  hw/char/debugcon.c                   |   4 +-
>  hw/char/parallel.c                   |   2 +-
>  hw/char/serial-isa.c                 |   2 +-
>  hw/core/qdev-properties-system.c     |  12 ++-
>  hw/core/qdev-properties.c            | 204 +++--------------------------------
>  hw/core/qdev.c                       |  38 +------
>  hw/display/g364fb.c                  |   2 +-
>  hw/display/tcx.c                     |   4 +-
>  hw/dma/i82374.c                      |   2 +-
>  hw/dma/sun4m_iommu.c                 |   2 +-
>  hw/i386/kvm/i8254.c                  |   8 +-
>  hw/ide/isa.c                         |   4 +-
>  hw/ide/qdev.c                        |   2 +-
>  hw/intc/i8259_common.c               |   6 +-
>  hw/isa/pc87312.c                     |   2 +-
>  hw/misc/applesmc.c                   |   2 +-
>  hw/misc/debugexit.c                  |   4 +-
>  hw/misc/eccmemctl.c                  |   2 +-
>  hw/net/ne2000-isa.c                  |   2 +-
>  hw/nvram/fw_cfg.c                    |   4 +-
>  hw/ppc/spapr_pci.c                   |  16 +--
>  hw/scsi/megasas.c                    |   2 +-
>  hw/scsi/scsi-disk.c                  |   6 +-
>  hw/sd/sdhci.c                        |   4 +-
>  hw/timer/i8254.c                     |   2 +-
>  hw/timer/m48t59.c                    |   4 +-
>  hw/timer/mc146818rtc.c               |  14 +--
>  hw/usb/host-libusb.c                 |   4 +-
>  hw/virtio/virtio-pci.c               |   6 +-
>  include/hw/block/block.h             |   6 --
>  include/hw/qdev-core.h               |   1 -
>  include/hw/qdev-dma.h                |   2 +-
>  include/hw/qdev-properties.h         |  11 --
>  include/qapi/string-output-visitor.h |   2 +-
>  include/qemu-common.h                |   8 --
>  include/qom/object.h                 |   3 +-
>  qapi-schema.json                     |  58 ++++++++++
>  qapi/string-input-visitor.c          |  24 +++++
>  qapi/string-output-visitor.c         |  55 +++++++++-
>  qdev-monitor.c                       |   6 +-
>  qom/object.c                         |   4 +-
>  tests/test-string-output-visitor.c   |   2 +-
>  tests/test-visitor-serialization.c   |   2 +-
>  49 files changed, 235 insertions(+), 329 deletions(-)
> 

with fixed checkpatch error in 13/12
Reviewed-By: Igor Mammedov <imammedo@redhat.com>

Re: [Qemu-devel] [PATCH 00/12] qdev: cleanup legacy properties

[Paolo Bonzini]

Il 05/02/2014 12:12, Igor Mammedov ha scritto:
> with fixed checkpatch error in 13/12
> Reviewed-By: Igor Mammedov <imammedo@redhat.com>

Andreas, can you ack it so that I can send a pull request myself?

Thanks,

Paolo

Re: [Qemu-devel] [PATCH 00/12] qdev: cleanup legacy properties

[Andreas Färber]

Am 05.02.2014 17:36, schrieb Paolo Bonzini:
> Il 05/02/2014 12:12, Igor Mammedov ha scritto:
>> with fixed checkpatch error in 13/12
>> Reviewed-By: Igor Mammedov <imammedo@redhat.com>
> 
> Andreas, can you ack it so that I can send a pull request myself?

Since I just raised a question I cannot ack it yet. ;) But I can queue
them when we're through (or did I miss something non-qdev?).

On the other hand I'd appreciate if you could review Edgar's address
space patches since that series starts with memory API refactorings.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg