From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=37707 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Pl2I7-0006if-FS for qemu-devel@nongnu.org; Thu, 03 Feb 2011 11:44:20 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Pl2I5-0003gn-Nh for qemu-devel@nongnu.org; Thu, 03 Feb 2011 11:44:19 -0500 Received: from mail-iy0-f173.google.com ([209.85.210.173]:47649) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Pl2I5-0003gI-Iw for qemu-devel@nongnu.org; Thu, 03 Feb 2011 11:44:17 -0500 Received: by iye19 with SMTP id 19so1321122iye.4 for ; Thu, 03 Feb 2011 08:44:16 -0800 (PST) Message-ID: <4D4ADB5D.1010005@codemonkey.ws> Date: Thu, 03 Feb 2011 10:44:13 -0600 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH 03/20] qdev-properties: add PROP_TYPE_ENUM References: <1296678500-19497-1-git-send-email-alevy@redhat.com> <1296678500-19497-4-git-send-email-alevy@redhat.com> In-Reply-To: <1296678500-19497-4-git-send-email-alevy@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alon Levy Cc: qemu-devel@nongnu.org, Gerd Hoffmann 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) >