Re: [Qemu-devel] [PATCH 03/20] qdev-properties: add PROP_TYPE_ENUM

[Alon Levy <alevy@redhat.com>]

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