[03/20] qdev-properties: add PROP_TYPE_ENUM

Example usage:

EnumTable foo_enum_table[] = {
    {"bar", 1},
    {"buz", 2},
    {NULL, 0},
};

DEFINE_PROP_ENUM("foo", State, foo, 1, foo_enum_table)

When using qemu -device foodev,? it will appear as:
 foodev.foo=bar/buz
---
 hw/qdev-properties.c |   60 ++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/qdev.h            |   15 ++++++++++++
 2 files changed, 75 insertions(+), 0 deletions(-)

On 02/02/2011 02:28 PM, Alon Levy wrote:
> Example usage:
>
> EnumTable foo_enum_table[] = {
>      {"bar", 1},
>      {"buz", 2},
>      {NULL, 0},
> };
>
> DEFINE_PROP_ENUM("foo", State, foo, 1, foo_enum_table)
>
> When using qemu -device foodev,? it will appear as:
>   foodev.foo=bar/buz
> ---
>   hw/qdev-properties.c |   60 ++++++++++++++++++++++++++++++++++++++++++++++++++
>   hw/qdev.h            |   15 ++++++++++++
>   2 files changed, 75 insertions(+), 0 deletions(-)
>    

Missing Signed-off-by.

Is it really necessary to introduce this in this series?  There's a lot 
missing here like the ability to determine which values are valid for an 
enum through QMP.

I think this will also unexpected break the device_add QMP interface 
since there's some special logic to handle types.  It's unclear to me 
whether we should be sending the symbolic name or the value through QMP.

Regards,

Anthony Liguori

> diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
> index a493087..3157721 100644
> --- a/hw/qdev-properties.c
> +++ b/hw/qdev-properties.c
> @@ -63,6 +63,66 @@ PropertyInfo qdev_prop_bit = {
>       .print = print_bit,
>   };
>
> +/* --- Enumeration --- */
> +/* Example usage:
> +EnumTable foo_enum_table[] = {
> +    {"bar", 1},
> +    {"buz", 2},
> +    {NULL, 0},
> +};
> +DEFINE_PROP_ENUM("foo", State, foo, 1, foo_enum_table),
> + */
> +static int parse_enum(DeviceState *dev, Property *prop, const char *str)
> +{
> +    uint8_t *ptr = qdev_get_prop_ptr(dev, prop);
> +    EnumTable *option = (EnumTable*)prop->data;
> +
> +    while (option->name != NULL) {
> +        if (!strncmp(str, option->name, strlen(option->name))) {
> +            *ptr = option->value;
> +            return 0;
> +        }
> +        option++;
> +    }
> +    return -EINVAL;
> +}
> +
> +static int print_enum(DeviceState *dev, Property *prop, char *dest, size_t len)
> +{
> +    uint32_t *p = qdev_get_prop_ptr(dev, prop);
> +    EnumTable *option = (EnumTable*)prop->data;
> +    while (option->name != NULL) {
> +        if (*p == option->value) {
> +            return snprintf(dest, len, "%s", option->name);
> +        }
> +        option++;
> +    }
> +    return 0;
> +}
> +
> +static int print_enum_options(DeviceInfo *info, Property *prop, char *dest, size_t len)
> +{
> +    int ret = 0;
> +    EnumTable *option = (EnumTable*)prop->data;
> +    while (option->name != NULL) {
> +        ret += snprintf(dest + ret, len - ret, "%s", option->name);
> +        if (option[1].name != NULL) {
> +            ret += snprintf(dest + ret, len - ret, "/");
> +        }
> +        option++;
> +    }
> +    return ret;
> +}
> +
> +PropertyInfo qdev_prop_enum = {
> +    .name  = "enum",
> +    .type  = PROP_TYPE_ENUM,
> +    .size  = sizeof(uint32_t),
> +    .parse = parse_enum,
> +    .print = print_enum,
> +    .print_options = print_enum_options,
> +};
> +
>   /* --- 8bit integer --- */
>
>   static int parse_uint8(DeviceState *dev, Property *prop, const char *str)
> diff --git a/hw/qdev.h b/hw/qdev.h
> index 3d9acd7..3701d83 100644
> --- a/hw/qdev.h
> +++ b/hw/qdev.h
> @@ -102,6 +102,7 @@ enum PropertyType {
>       PROP_TYPE_VLAN,
>       PROP_TYPE_PTR,
>       PROP_TYPE_BIT,
> +    PROP_TYPE_ENUM,
>   };
>
>   struct PropertyInfo {
> @@ -121,6 +122,11 @@ typedef struct GlobalProperty {
>       QTAILQ_ENTRY(GlobalProperty) next;
>   } GlobalProperty;
>
> +typedef struct EnumTable {
> +    const char *name;
> +    uint32_t    value;
> +} EnumTable;
> +
>   /*** Board API.  This should go away once we have a machine config file.  ***/
>
>   DeviceState *qdev_create(BusState *bus, const char *name);
> @@ -235,6 +241,7 @@ extern PropertyInfo qdev_prop_drive;
>   extern PropertyInfo qdev_prop_netdev;
>   extern PropertyInfo qdev_prop_vlan;
>   extern PropertyInfo qdev_prop_pci_devfn;
> +extern PropertyInfo qdev_prop_enum;
>
>   #define DEFINE_PROP(_name, _state, _field, _prop, _type) { \
>           .name      = (_name),                                    \
> @@ -257,6 +264,14 @@ extern PropertyInfo qdev_prop_pci_devfn;
>               + type_check(uint32_t,typeof_field(_state, _field)), \
>           .defval    = (bool[]) { (_defval) },                     \
>           }
> +#define DEFINE_PROP_ENUM(_name, _state, _field, _defval, _options) {    \
> +        .name      = (_name),                                           \
> +        .info      =&(qdev_prop_enum),                                 \
> +        .offset    = offsetof(_state, _field)                           \
> +            + type_check(uint32_t,typeof_field(_state, _field)),        \
> +        .defval    = (uint32_t[]) { (_defval) },                        \
> +        .data      = (void*)(_options),                                 \
> +        }
>
>   #define DEFINE_PROP_UINT8(_n, _s, _f, _d)                       \
>       DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_uint8, uint8_t)
>

On Thu, Feb 03, 2011 at 10:44:13AM -0600, Anthony Liguori wrote:
> On 02/02/2011 02:28 PM, Alon Levy wrote:
> >Example usage:
> >
> >EnumTable foo_enum_table[] = {
> >     {"bar", 1},
> >     {"buz", 2},
> >     {NULL, 0},
> >};
> >
> >DEFINE_PROP_ENUM("foo", State, foo, 1, foo_enum_table)
> >
> >When using qemu -device foodev,? it will appear as:
> >  foodev.foo=bar/buz
> >---
> >  hw/qdev-properties.c |   60 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >  hw/qdev.h            |   15 ++++++++++++
> >  2 files changed, 75 insertions(+), 0 deletions(-)
> 
> Missing Signed-off-by.
> 
> Is it really necessary to introduce this in this series?  There's a
> lot missing here like the ability to determine which values are
> valid for an enum through QMP.
> 

I currently added this for command line querying of the backends supplied by
the ccid-card-emulated for libvirt. I can add QMP support. Are you talking
about a single "query-options device property" jsonified?

> I think this will also unexpected break the device_add QMP interface
> since there's some special logic to handle types.  It's unclear to
> me whether we should be sending the symbolic name or the value
> through QMP.

I don't understand where this could happen. QMP won't break if it keeps
sending the value like it always did. I could add support for sending the
name in a later patch (they are easily distinguished as json would be
1 and "something" with the quotation marks for integer and string respectively).

> 
> Regards,
> 
> Anthony Liguori
> 
> >diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
> >index a493087..3157721 100644
> >--- a/hw/qdev-properties.c
> >+++ b/hw/qdev-properties.c
> >@@ -63,6 +63,66 @@ PropertyInfo qdev_prop_bit = {
> >      .print = print_bit,
> >  };
> >
> >+/* --- Enumeration --- */
> >+/* Example usage:
> >+EnumTable foo_enum_table[] = {
> >+    {"bar", 1},
> >+    {"buz", 2},
> >+    {NULL, 0},
> >+};
> >+DEFINE_PROP_ENUM("foo", State, foo, 1, foo_enum_table),
> >+ */
> >+static int parse_enum(DeviceState *dev, Property *prop, const char *str)
> >+{
> >+    uint8_t *ptr = qdev_get_prop_ptr(dev, prop);
> >+    EnumTable *option = (EnumTable*)prop->data;
> >+
> >+    while (option->name != NULL) {
> >+        if (!strncmp(str, option->name, strlen(option->name))) {
> >+            *ptr = option->value;
> >+            return 0;
> >+        }
> >+        option++;
> >+    }
> >+    return -EINVAL;
> >+}
> >+
> >+static int print_enum(DeviceState *dev, Property *prop, char *dest, size_t len)
> >+{
> >+    uint32_t *p = qdev_get_prop_ptr(dev, prop);
> >+    EnumTable *option = (EnumTable*)prop->data;
> >+    while (option->name != NULL) {
> >+        if (*p == option->value) {
> >+            return snprintf(dest, len, "%s", option->name);
> >+        }
> >+        option++;
> >+    }
> >+    return 0;
> >+}
> >+
> >+static int print_enum_options(DeviceInfo *info, Property *prop, char *dest, size_t len)
> >+{
> >+    int ret = 0;
> >+    EnumTable *option = (EnumTable*)prop->data;
> >+    while (option->name != NULL) {
> >+        ret += snprintf(dest + ret, len - ret, "%s", option->name);
> >+        if (option[1].name != NULL) {
> >+            ret += snprintf(dest + ret, len - ret, "/");
> >+        }
> >+        option++;
> >+    }
> >+    return ret;
> >+}
> >+
> >+PropertyInfo qdev_prop_enum = {
> >+    .name  = "enum",
> >+    .type  = PROP_TYPE_ENUM,
> >+    .size  = sizeof(uint32_t),
> >+    .parse = parse_enum,
> >+    .print = print_enum,
> >+    .print_options = print_enum_options,
> >+};
> >+
> >  /* --- 8bit integer --- */
> >
> >  static int parse_uint8(DeviceState *dev, Property *prop, const char *str)
> >diff --git a/hw/qdev.h b/hw/qdev.h
> >index 3d9acd7..3701d83 100644
> >--- a/hw/qdev.h
> >+++ b/hw/qdev.h
> >@@ -102,6 +102,7 @@ enum PropertyType {
> >      PROP_TYPE_VLAN,
> >      PROP_TYPE_PTR,
> >      PROP_TYPE_BIT,
> >+    PROP_TYPE_ENUM,
> >  };
> >
> >  struct PropertyInfo {
> >@@ -121,6 +122,11 @@ typedef struct GlobalProperty {
> >      QTAILQ_ENTRY(GlobalProperty) next;
> >  } GlobalProperty;
> >
> >+typedef struct EnumTable {
> >+    const char *name;
> >+    uint32_t    value;
> >+} EnumTable;
> >+
> >  /*** Board API.  This should go away once we have a machine config file.  ***/
> >
> >  DeviceState *qdev_create(BusState *bus, const char *name);
> >@@ -235,6 +241,7 @@ extern PropertyInfo qdev_prop_drive;
> >  extern PropertyInfo qdev_prop_netdev;
> >  extern PropertyInfo qdev_prop_vlan;
> >  extern PropertyInfo qdev_prop_pci_devfn;
> >+extern PropertyInfo qdev_prop_enum;
> >
> >  #define DEFINE_PROP(_name, _state, _field, _prop, _type) { \
> >          .name      = (_name),                                    \
> >@@ -257,6 +264,14 @@ extern PropertyInfo qdev_prop_pci_devfn;
> >              + type_check(uint32_t,typeof_field(_state, _field)), \
> >          .defval    = (bool[]) { (_defval) },                     \
> >          }
> >+#define DEFINE_PROP_ENUM(_name, _state, _field, _defval, _options) {    \
> >+        .name      = (_name),                                           \
> >+        .info      =&(qdev_prop_enum),                                 \
> >+        .offset    = offsetof(_state, _field)                           \
> >+            + type_check(uint32_t,typeof_field(_state, _field)),        \
> >+        .defval    = (uint32_t[]) { (_defval) },                        \
> >+        .data      = (void*)(_options),                                 \
> >+        }
> >
> >  #define DEFINE_PROP_UINT8(_n, _s, _f, _d)                       \
> >      DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_uint8, uint8_t)
>