[Qemu-devel] [PATCH v8] qdev: Add support for property type bool

[Qemu-devel] [PATCH v8] qdev: Add support for property type bool

[Andreas Färber]

From: Andreas Färber <andreas.faerber@web.de>

VMState supports the type bool but qdev instead supports bit, backed by
uint32_t. Therefore let's add PROP_TYPE_BOOL and qdev_prop_set_bool().

With non-programmers in mind, instead of universal true/false provide
two different property types:
* on/off for DEFINE_PROP_SWITCH() (requested by Jan) and
* yes/no for DEFINE_PROP_BOOL() (also accepting true/false as input).

Signed-off-by: Andreas Färber <andreas.faerber@web.de>
Cc: Juan Quintela <quintela@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>
Cc: Jan Kiszka <jan.kiszka@siemens.com>
Cc: Vasilis Liaskovitis <vasilis.liaskovitis@profitbricks.com>
---
 v7 -> v8:
 * Add missing extern qdev_prop_bool_switch for DEFINE_PROP_SWITCH().

 v6 -> v7:
 * Rename existing "boolean" type to "bit".
 * Re-introduce qdev_prop_set_bool().
 * Split "bool" into "switch" (on/off) and "boolean" (yes/no; true/false input).

 v5 -> v6:
 * Rebased onto QOM properties.
 * Parse and print true/false for bool, leave bit untouched.
 Please review, v6 untested.
 [* Accidentally dropped qdev_prop_set_bool().]

 v4 -> v5 (40P):
 * Parse on/off in addition to yes/no for both bit and bool, print yes/no for bool.

 v4 (ISA):
 * Introduced.
 hw/qdev-properties.c |  102 +++++++++++++++++++++++++++++++++++++++++++++++++-
 hw/qdev.h            |    8 ++++
 2 files changed, 109 insertions(+), 1 deletions(-)

diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 02f0dae..0b1e9ea 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -86,7 +86,7 @@ static void set_bit(DeviceState *dev, Visitor *v, void *opaque,
 }
 
 PropertyInfo qdev_prop_bit = {
-    .name  = "boolean",
+    .name  = "bit",
     .legacy_name  = "on/off",
     .type  = PROP_TYPE_BIT,
     .size  = sizeof(uint32_t),
@@ -96,6 +96,101 @@ PropertyInfo qdev_prop_bit = {
     .set   = set_bit,
 };
 
+/* --- bool --- */
+
+static int parse_bool(DeviceState *dev, Property *prop, const char *str)
+{
+    bool *ptr = qdev_get_prop_ptr(dev, prop);
+    if (strcmp(str, "true") == 0 || strcmp(str, "yes") == 0) {
+        *ptr = true;
+    } else if (strcmp(str, "false") == 0 || strcmp(str, "no") == 0) {
+        *ptr = false;
+    } else {
+        return -EINVAL;
+    }
+
+    return 0;
+}
+
+static int parse_bool_switch(DeviceState *dev, Property *prop,
+                             const char *str)
+{
+    bool *ptr = qdev_get_prop_ptr(dev, prop);
+    if (strcmp(str, "on") == 0) {
+        *ptr = true;
+    } else if (strcmp(str, "off") == 0) {
+        *ptr = false;
+    } else {
+        return -EINVAL;
+    }
+
+    return 0;
+}
+
+static int print_bool(DeviceState *dev, Property *prop,
+                      char *dest, size_t len)
+{
+    bool *ptr = qdev_get_prop_ptr(dev, prop);
+    return snprintf(dest, len, *ptr ? "yes" : "no");
+}
+
+static int print_bool_switch(DeviceState *dev, Property *prop,
+                             char *dest, size_t len)
+{
+    bool *ptr = qdev_get_prop_ptr(dev, prop);
+    return snprintf(dest, len, *ptr ? "on" : "off");
+}
+
+static void get_bool(DeviceState *dev, Visitor *v, void *opaque,
+                     const char *name, Error **errp)
+{
+    Property *prop = opaque;
+    bool *ptr = qdev_get_prop_ptr(dev, prop);
+
+    visit_type_bool(v, ptr, name, errp);
+}
+
+static void set_bool(DeviceState *dev, Visitor *v, void *opaque,
+                     const char *name, Error **errp)
+{
+    Property *prop = opaque;
+    bool *ptr = qdev_get_prop_ptr(dev, prop);
+    Error *local_err = NULL;
+    bool value;
+
+    if (dev->state != DEV_STATE_CREATED) {
+        error_set(errp, QERR_PERMISSION_DENIED);
+        return;
+    }
+
+    visit_type_bool(v, &value, name, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+    *ptr = value;
+}
+
+PropertyInfo qdev_prop_bool = {
+    .name = "boolean",
+    .type = PROP_TYPE_BOOL,
+    .size = sizeof(bool),
+    .parse = parse_bool,
+    .print = print_bool,
+    .get   = get_bool,
+    .set   = set_bool,
+};
+
+PropertyInfo qdev_prop_bool_switch = {
+    .name = "switch",
+    .type = PROP_TYPE_BOOL,
+    .size = sizeof(bool),
+    .parse = parse_bool_switch,
+    .print = print_bool_switch,
+    .get   = get_bool,
+    .set   = set_bool,
+};
+
 /* --- 8bit integer --- */
 
 static int parse_uint8(DeviceState *dev, Property *prop, const char *str)
@@ -1055,6 +1150,11 @@ void qdev_prop_set_bit(DeviceState *dev, const char *name, bool value)
     qdev_prop_set(dev, name, &value, PROP_TYPE_BIT);
 }
 
+void qdev_prop_set_bool(DeviceState *dev, const char *name, bool value)
+{
+    qdev_prop_set(dev, name, &value, PROP_TYPE_BOOL);
+}
+
 void qdev_prop_set_uint8(DeviceState *dev, const char *name, uint8_t value)
 {
     qdev_prop_set(dev, name, &value, PROP_TYPE_UINT8);
diff --git a/hw/qdev.h b/hw/qdev.h
index 6b58dd8..cef20f6 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -152,6 +152,7 @@ enum PropertyType {
     PROP_TYPE_VLAN,
     PROP_TYPE_PTR,
     PROP_TYPE_BIT,
+    PROP_TYPE_BOOL,
 };
 
 struct PropertyInfo {
@@ -276,6 +277,8 @@ int do_device_del(Monitor *mon, const QDict *qdict, QObject **ret_data);
 /*** qdev-properties.c ***/
 
 extern PropertyInfo qdev_prop_bit;
+extern PropertyInfo qdev_prop_bool;
+extern PropertyInfo qdev_prop_bool_switch;
 extern PropertyInfo qdev_prop_uint8;
 extern PropertyInfo qdev_prop_uint16;
 extern PropertyInfo qdev_prop_uint32;
@@ -315,6 +318,10 @@ extern PropertyInfo qdev_prop_pci_devfn;
         .defval    = (bool[]) { (_defval) },                     \
         }
 
+#define DEFINE_PROP_BOOL(_n, _s, _f, _d)                        \
+    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_bool, bool)
+#define DEFINE_PROP_SWITCH(_n, _s, _f, _d)                      \
+    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_bool_switch, bool)
 #define DEFINE_PROP_UINT8(_n, _s, _f, _d)                       \
     DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_uint8, uint8_t)
 #define DEFINE_PROP_UINT16(_n, _s, _f, _d)                      \
@@ -358,6 +365,7 @@ int qdev_prop_exists(DeviceState *dev, const char *name);
 int qdev_prop_parse(DeviceState *dev, const char *name, const char *value);
 void qdev_prop_set(DeviceState *dev, const char *name, void *src, enum PropertyType type);
 void qdev_prop_set_bit(DeviceState *dev, const char *name, bool value);
+void qdev_prop_set_bool(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);
 void qdev_prop_set_uint32(DeviceState *dev, const char *name, uint32_t value);
-- 
1.7.7

Re: [Qemu-devel] [PATCH v8] qdev: Add support for property type bool

[Andreas Färber]

Am 26.01.2012 16:54, schrieb Andreas Färber:
> From: Andreas Färber <andreas.faerber@web.de>
> 
> VMState supports the type bool but qdev instead supports bit, backed by
> uint32_t. Therefore let's add PROP_TYPE_BOOL and qdev_prop_set_bool().
> 
> With non-programmers in mind, instead of universal true/false provide
> two different property types:
> * on/off for DEFINE_PROP_SWITCH() (requested by Jan) and
> * yes/no for DEFINE_PROP_BOOL() (also accepting true/false as input).
> 
> Signed-off-by: Andreas Färber <andreas.faerber@web.de>
> Cc: Juan Quintela <quintela@redhat.com>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Jan Kiszka <jan.kiszka@siemens.com>
> Cc: Vasilis Liaskovitis <vasilis.liaskovitis@profitbricks.com>
> ---
>  v7 -> v8:
>  * Add missing extern qdev_prop_bool_switch for DEFINE_PROP_SWITCH().
> 
>  v6 -> v7:
>  * Rename existing "boolean" type to "bit".
>  * Re-introduce qdev_prop_set_bool().
>  * Split "bool" into "switch" (on/off) and "boolean" (yes/no; true/false input).
> 
>  v5 -> v6:
>  * Rebased onto QOM properties.
>  * Parse and print true/false for bool, leave bit untouched.
>  Please review, v6 untested.
>  [* Accidentally dropped qdev_prop_set_bool().]
> 
>  v4 -> v5 (40P):
>  * Parse on/off in addition to yes/no for both bit and bool, print yes/no for bool.
> 
>  v4 (ISA):
>  * Introduced.
>  hw/qdev-properties.c |  102 +++++++++++++++++++++++++++++++++++++++++++++++++-
>  hw/qdev.h            |    8 ++++
>  2 files changed, 109 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
> index 02f0dae..0b1e9ea 100644
> --- a/hw/qdev-properties.c
> +++ b/hw/qdev-properties.c
> @@ -86,7 +86,7 @@ static void set_bit(DeviceState *dev, Visitor *v, void *opaque,
>  }
>  
>  PropertyInfo qdev_prop_bit = {
> -    .name  = "boolean",
> +    .name  = "bit",
>      .legacy_name  = "on/off",
>      .type  = PROP_TYPE_BIT,
>      .size  = sizeof(uint32_t),
> @@ -96,6 +96,101 @@ PropertyInfo qdev_prop_bit = {
>      .set   = set_bit,
>  };
>  
> +/* --- bool --- */
> +
> +static int parse_bool(DeviceState *dev, Property *prop, const char *str)
> +{
> +    bool *ptr = qdev_get_prop_ptr(dev, prop);
> +    if (strcmp(str, "true") == 0 || strcmp(str, "yes") == 0) {
> +        *ptr = true;
> +    } else if (strcmp(str, "false") == 0 || strcmp(str, "no") == 0) {

I just noticed that Jan's patch does strncasecmp() for bit, so if we
want those semantics here too (foo=TRUE,bar=False,foobar=yES) I can send
a v9. I really hate replying to myself though, so I'll wait a bit for
other review comments to avoid a further surge in patch revisions.

Andreas

> +        *ptr = false;
> +    } else {
> +        return -EINVAL;
> +    }
> +
> +    return 0;
> +}
> +
> +static int parse_bool_switch(DeviceState *dev, Property *prop,
> +                             const char *str)
> +{
> +    bool *ptr = qdev_get_prop_ptr(dev, prop);
> +    if (strcmp(str, "on") == 0) {
> +        *ptr = true;
> +    } else if (strcmp(str, "off") == 0) {
> +        *ptr = false;
> +    } else {
> +        return -EINVAL;
> +    }
> +
> +    return 0;
> +}
> +
> +static int print_bool(DeviceState *dev, Property *prop,
> +                      char *dest, size_t len)
> +{
> +    bool *ptr = qdev_get_prop_ptr(dev, prop);
> +    return snprintf(dest, len, *ptr ? "yes" : "no");
> +}
> +
> +static int print_bool_switch(DeviceState *dev, Property *prop,
> +                             char *dest, size_t len)
> +{
> +    bool *ptr = qdev_get_prop_ptr(dev, prop);
> +    return snprintf(dest, len, *ptr ? "on" : "off");
> +}
> +
> +static void get_bool(DeviceState *dev, Visitor *v, void *opaque,
> +                     const char *name, Error **errp)
> +{
> +    Property *prop = opaque;
> +    bool *ptr = qdev_get_prop_ptr(dev, prop);
> +
> +    visit_type_bool(v, ptr, name, errp);
> +}
> +
> +static void set_bool(DeviceState *dev, Visitor *v, void *opaque,
> +                     const char *name, Error **errp)
> +{
> +    Property *prop = opaque;
> +    bool *ptr = qdev_get_prop_ptr(dev, prop);
> +    Error *local_err = NULL;
> +    bool value;
> +
> +    if (dev->state != DEV_STATE_CREATED) {
> +        error_set(errp, QERR_PERMISSION_DENIED);
> +        return;
> +    }
> +
> +    visit_type_bool(v, &value, name, &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        return;
> +    }
> +    *ptr = value;
> +}
> +
> +PropertyInfo qdev_prop_bool = {
> +    .name = "boolean",
> +    .type = PROP_TYPE_BOOL,
> +    .size = sizeof(bool),
> +    .parse = parse_bool,
> +    .print = print_bool,
> +    .get   = get_bool,
> +    .set   = set_bool,
> +};
> +
> +PropertyInfo qdev_prop_bool_switch = {
> +    .name = "switch",
> +    .type = PROP_TYPE_BOOL,
> +    .size = sizeof(bool),
> +    .parse = parse_bool_switch,
> +    .print = print_bool_switch,
> +    .get   = get_bool,
> +    .set   = set_bool,
> +};
> +
>  /* --- 8bit integer --- */
>  
>  static int parse_uint8(DeviceState *dev, Property *prop, const char *str)
> @@ -1055,6 +1150,11 @@ void qdev_prop_set_bit(DeviceState *dev, const char *name, bool value)
>      qdev_prop_set(dev, name, &value, PROP_TYPE_BIT);
>  }
>  
> +void qdev_prop_set_bool(DeviceState *dev, const char *name, bool value)
> +{
> +    qdev_prop_set(dev, name, &value, PROP_TYPE_BOOL);
> +}
> +
>  void qdev_prop_set_uint8(DeviceState *dev, const char *name, uint8_t value)
>  {
>      qdev_prop_set(dev, name, &value, PROP_TYPE_UINT8);
> diff --git a/hw/qdev.h b/hw/qdev.h
> index 6b58dd8..cef20f6 100644
> --- a/hw/qdev.h
> +++ b/hw/qdev.h
> @@ -152,6 +152,7 @@ enum PropertyType {
>      PROP_TYPE_VLAN,
>      PROP_TYPE_PTR,
>      PROP_TYPE_BIT,
> +    PROP_TYPE_BOOL,
>  };
>  
>  struct PropertyInfo {
> @@ -276,6 +277,8 @@ int do_device_del(Monitor *mon, const QDict *qdict, QObject **ret_data);
>  /*** qdev-properties.c ***/
>  
>  extern PropertyInfo qdev_prop_bit;
> +extern PropertyInfo qdev_prop_bool;
> +extern PropertyInfo qdev_prop_bool_switch;
>  extern PropertyInfo qdev_prop_uint8;
>  extern PropertyInfo qdev_prop_uint16;
>  extern PropertyInfo qdev_prop_uint32;
> @@ -315,6 +318,10 @@ extern PropertyInfo qdev_prop_pci_devfn;
>          .defval    = (bool[]) { (_defval) },                     \
>          }
>  
> +#define DEFINE_PROP_BOOL(_n, _s, _f, _d)                        \
> +    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_bool, bool)
> +#define DEFINE_PROP_SWITCH(_n, _s, _f, _d)                      \
> +    DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_bool_switch, bool)
>  #define DEFINE_PROP_UINT8(_n, _s, _f, _d)                       \
>      DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_uint8, uint8_t)
>  #define DEFINE_PROP_UINT16(_n, _s, _f, _d)                      \
> @@ -358,6 +365,7 @@ int qdev_prop_exists(DeviceState *dev, const char *name);
>  int qdev_prop_parse(DeviceState *dev, const char *name, const char *value);
>  void qdev_prop_set(DeviceState *dev, const char *name, void *src, enum PropertyType type);
>  void qdev_prop_set_bit(DeviceState *dev, const char *name, bool value);
> +void qdev_prop_set_bool(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);
>  void qdev_prop_set_uint32(DeviceState *dev, const char *name, uint32_t value);


-- 
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 v8] qdev: Add support for property type bool

[Juan Quintela]

Andreas Färber <afaerber@suse.de> wrote:
> From: Andreas Färber <andreas.faerber@web.de>
>
> VMState supports the type bool but qdev instead supports bit, backed by
> uint32_t. Therefore let's add PROP_TYPE_BOOL and qdev_prop_set_bool().
>  PropertyInfo qdev_prop_bit = {
> -    .name  = "boolean",
> +    .name  = "bit",

my plan for vmstate was to name this "bool32", as it feels more
descriptive (on QDEV, we can change it, but for qemu for compatibility
reasons, it needs to stay as 4 bytes on the wire, but we can have a bool
on the struct).

This are all the uses of DEFINE_PROP_BIT.

./hw/scsi-disk.c:1736:            DEFINE_PROP_BIT("removable", SCSIDiskState, removable, 0, false),

Con be movde to bool

./hw/scsi-disk.c:1780:            DEFINE_PROP_BIT("removable", SCSIDiskState, removable, 0, false),

same

./hw/usb-msd.c:658:        DEFINE_PROP_BIT("removable", MSDState, removable, 0, false),

    s->scsi_dev = scsi_bus_legacy_add_drive(&s->bus, bs, 0, !!s->removable,
                                            s->conf.bootindex);
*should* be moved to bool

./hw/hpet.c:707:        DEFINE_PROP_BIT("msi", HPETState, flags, HPET_MSI_SUPPORT, false),

it is a bitmap with only one bit defined, not sure about this one.


./hw/virtio.h:213:	DEFINE_PROP_BIT("indirect_desc", _state, _field, \
./hw/virtio.h:215:	DEFINE_PROP_BIT("event_idx", _state, _field, \

This is a real bitmap with 2 values.

./hw/virtio-pci.c:800:            DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags,
./hw/virtio-pci.c:819:            DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags,
./hw/virtio-pci.c:843:            DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags,
./hw/9pfs/virtio-9p-device.c:175:        DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags,

Another bitmap with only one defined value.

./hw/virtio-blk.h:103:        DEFINE_PROP_BIT("scsi", _state, _field, VIRTIO_BLK_F_SCSI, true)

It is a bitmap

./hw/ivshmem.c:782:        DEFINE_PROP_BIT("ioeventfd", IVShmemState, features, IVSHMEM_IOEVENTFD, false),
./hw/ivshmem.c:783:        DEFINE_PROP_BIT("msi", IVShmemState,
features, IVSHMEM_MSI, true),

Another bitmap with two values.

./hw/virtio-net.h:172:        DEFINE_PROP_BIT("csum", _state, _field, VIRTIO_NET_F_CSUM, true), \
./hw/virtio-net.h:173:        DEFINE_PROP_BIT("guest_csum", _state, _field, VIRTIO_NET_F_GUEST_CSUM, true), \
./hw/virtio-net.h:174:        DEFINE_PROP_BIT("gso", _state, _field, VIRTIO_NET_F_GSO, true), \
./hw/virtio-net.h:175:        DEFINE_PROP_BIT("guest_tso4", _state, _field, VIRTIO_NET_F_GUEST_TSO4, true), \
./hw/virtio-net.h:176:        DEFINE_PROP_BIT("guest_tso6", _state, _field, VIRTIO_NET_F_GUEST_TSO6, true), \
./hw/virtio-net.h:177:        DEFINE_PROP_BIT("guest_ecn", _state, _field, VIRTIO_NET_F_GUEST_ECN, true), \
./hw/virtio-net.h:178:        DEFINE_PROP_BIT("guest_ufo", _state, _field, VIRTIO_NET_F_GUEST_UFO, true), \
./hw/virtio-net.h:179:        DEFINE_PROP_BIT("host_tso4", _state, _field, VIRTIO_NET_F_HOST_TSO4, true), \
./hw/virtio-net.h:180:        DEFINE_PROP_BIT("host_tso6", _state, _field, VIRTIO_NET_F_HOST_TSO6, true), \
./hw/virtio-net.h:181:        DEFINE_PROP_BIT("host_ecn", _state, _field, VIRTIO_NET_F_HOST_ECN, true), \
./hw/virtio-net.h:182:        DEFINE_PROP_BIT("host_ufo", _state, _field, VIRTIO_NET_F_HOST_UFO, true), \
./hw/virtio-net.h:183:        DEFINE_PROP_BIT("mrg_rxbuf", _state, _field, VIRTIO_NET_F_MRG_RXBUF, true), \
./hw/virtio-net.h:184:        DEFINE_PROP_BIT("status", _state, _field, VIRTIO_NET_F_STATUS, true), \
./hw/virtio-net.h:185:        DEFINE_PROP_BIT("ctrl_vq", _state, _field, VIRTIO_NET_F_CTRL_VQ, true), \
./hw/virtio-net.h:186:        DEFINE_PROP_BIT("ctrl_rx", _state, _field, VIRTIO_NET_F_CTRL_RX, true), \
./hw/virtio-net.h:187:        DEFINE_PROP_BIT("ctrl_vlan", _state, _field, VIRTIO_NET_F_CTRL_VLAN, true), \
./hw/virtio-net.h:188:        DEFINE_PROP_BIT("ctrl_rx_extra", _state, _field, VIRTIO_NET_F_CTRL_RX_EXTRA, true)

Bitmap.

./hw/i8259.c:570:        DEFINE_PROP_BIT("master", PicState, master,  0, false),

BOOL

./hw/pci.c:58:        DEFINE_PROP_BIT("multifunction", PCIDevice, cap_present,
./hw/pci.c:60:        DEFINE_PROP_BIT("command_serr_enable", PCIDevice, cap_present,

bitmap with two values.

./hw/pxa2xx_timer.c:488:        DEFINE_PROP_BIT("tm4", PXA2xxTimerInfo, flags,
./hw/pxa2xx_timer.c:502:        DEFINE_PROP_BIT("tm4", PXA2xxTimerInfo, flags,

Bitmap with only one value.


> +{
> +    bool *ptr = qdev_get_prop_ptr(dev, prop);
> +    if (strcmp(str, "true") == 0 || strcmp(str, "yes") == 0) {
> +        *ptr = true;
> +    } else if (strcmp(str, "false") == 0 || strcmp(str, "no") == 0) {
> +        *ptr = false;
> +    } else {
> +        return -EINVAL;
> +    }
> +
> +    return 0;
> +}
> +
> +static int parse_bool_switch(DeviceState *dev, Property *prop,
> +                             const char *str)
> +{
> +    bool *ptr = qdev_get_prop_ptr(dev, prop);
> +    if (strcmp(str, "on") == 0) {
> +        *ptr = true;
> +    } else if (strcmp(str, "off") == 0) {
> +        *ptr = false;
> +    } else {
> +        return -EINVAL;
> +    }
> +
> +    return 0;
> +}

As I am joining late to this discussion, I am not going to point it very
strong.  But I think that it is an easy to have a single bool type that
accept yes/on/true and no/off/false.  Didn't really see a strong
advantage with the split.

Later, Juan.

Re: [Qemu-devel] [PATCH v8] qdev: Add support for property type bool

[Jan Kiszka]

On 2012-01-27 13:17, Juan Quintela wrote:
>> +{
>> +    bool *ptr = qdev_get_prop_ptr(dev, prop);
>> +    if (strcmp(str, "true") == 0 || strcmp(str, "yes") == 0) {
>> +        *ptr = true;
>> +    } else if (strcmp(str, "false") == 0 || strcmp(str, "no") == 0) {
>> +        *ptr = false;
>> +    } else {
>> +        return -EINVAL;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static int parse_bool_switch(DeviceState *dev, Property *prop,
>> +                             const char *str)
>> +{
>> +    bool *ptr = qdev_get_prop_ptr(dev, prop);
>> +    if (strcmp(str, "on") == 0) {
>> +        *ptr = true;
>> +    } else if (strcmp(str, "off") == 0) {
>> +        *ptr = false;
>> +    } else {
>> +        return -EINVAL;
>> +    }
>> +
>> +    return 0;
>> +}
> 
> As I am joining late to this discussion, I am not going to point it very
> strong.  But I think that it is an easy to have a single bool type that
> accept yes/on/true and no/off/false.  Didn't really see a strong
> advantage with the split.

Accepting all this on input is a non-issue, but true/false as output is
suboptimal for quite a few existing and future switches.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux