Message ID | 1296678500-19497-4-git-send-email-alevy@redhat.com |
---|---|
State | New |
Headers | show |
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) >
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)