From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:50415) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RWPbH-0006N4-Vg for qemu-devel@nongnu.org; Fri, 02 Dec 2011 04:40:12 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RWPbG-0003QT-Tn for qemu-devel@nongnu.org; Fri, 02 Dec 2011 04:40:11 -0500 Received: from mx1.redhat.com ([209.132.183.28]:12598) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RWPbG-0003QH-M4 for qemu-devel@nongnu.org; Fri, 02 Dec 2011 04:40:10 -0500 Message-ID: <4ED89DB4.6040007@redhat.com> Date: Fri, 02 Dec 2011 10:43:16 +0100 From: Kevin Wolf MIME-Version: 1.0 References: <1322687028-29714-1-git-send-email-aliguori@us.ibm.com> <1322687028-29714-2-git-send-email-aliguori@us.ibm.com> <4ED7A2A1.80002@redhat.com> <4ED824F0.6010506@codemonkey.ws> In-Reply-To: <4ED824F0.6010506@codemonkey.ws> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 01/18] qom: add new dynamic property infrastructure based on Visitors List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: Peter Maydell , Stefan Hajnoczi , Markus Armbruster , qemu-devel@nongnu.org, Luiz Capitulino , Jan Kiszka Am 02.12.2011 02:08, schrieb Anthony Liguori: > On 12/01/2011 09:52 AM, Kevin Wolf wrote: >> Am 30.11.2011 22:03, schrieb Anthony Liguori: >>> qdev properties are settable only during construction and static to classes. >>> This isn't flexible enough for QOM. >>> >>> This patch introduces a property interface for qdev that provides dynamic >>> properties that are tied to objects, instead of classes. These properties are >>> Visitor based instead of string based too. >>> >>> Signed-off-by: Anthony Liguori >>> --- >>> hw/qdev.c | 99 +++++++++++++++++++++++++++++++++++++++++++++++++++ >>> hw/qdev.h | 118 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ >>> qerror.c | 4 ++ >>> qerror.h | 3 ++ >>> 4 files changed, 224 insertions(+), 0 deletions(-) >>> >>> diff --git a/hw/qdev.c b/hw/qdev.c >>> index 106407f..ad2d44f 100644 >>> --- a/hw/qdev.c >>> +++ b/hw/qdev.c >>> @@ -390,12 +390,33 @@ void qdev_init_nofail(DeviceState *dev) >>> } >>> } >>> >>> +static void qdev_property_del_all(DeviceState *dev) >>> +{ >>> + while (dev->properties) { >>> + GSList *i = dev->properties; >>> + DeviceProperty *prop = i->data; >>> + >>> + dev->properties = i->next; >>> + >>> + if (prop->release) { >>> + prop->release(dev, prop->name, prop->opaque); >>> + } >>> + >>> + g_free(prop->name); >>> + g_free(prop->type); >>> + g_free(prop); >>> + g_free(i); >>> + } >>> +} >>> + >>> /* Unlink device from bus and free the structure. */ >>> void qdev_free(DeviceState *dev) >>> { >>> BusState *bus; >>> Property *prop; >>> >>> + qdev_property_del_all(dev); >>> + >>> if (dev->state == DEV_STATE_INITIALIZED) { >>> while (dev->num_child_bus) { >>> bus = QLIST_FIRST(&dev->child_bus); >>> @@ -962,3 +983,81 @@ char* qdev_get_fw_dev_path(DeviceState *dev) >>> >>> return strdup(path); >>> } >>> + >>> +void qdev_property_add(DeviceState *dev, const char *name, const char *type, >>> + DevicePropertyEtter *get, DevicePropertyEtter *set, >>> + DevicePropertyRelease *release, void *opaque, >>> + Error **errp) >> >> How about letting the caller pass in a DeviceProperty for improved >> readability and usability? Instead of memorizing the order of currently >> eight parameters (could probably become more in the future) you can use >> proper C99 initializers then. > > Yeah, instead of taking a void *opaque, it could then just take the > DeviceProperty and use container_of adding a good bit more type safety. I like > it, thanks for the suggestion. > >> >>> @@ -45,6 +82,7 @@ struct DeviceState { >>> QTAILQ_ENTRY(DeviceState) sibling; >>> int instance_id_alias; >>> int alias_required_for_version; >>> + GSList *properties; >>> }; >> >> Why GSList instead of qemu-queue.h macros that would provide type safety? > > You're clearly thwarting my attempts at slowly introducing GSList as a > replacement for qemu-queue ;-) > > I really dislike qemu-queue. I think it's a whole lot more difficult to use in > practice. The glib data structures are much more rich than qemu-queue. qemu-queue.h is type safe, GSList is not. IMO that's a show stopper and I can't understand why we even need to talk about it. If you want to convince me of the opposite, it certainly needs more than vague "easier to use" hand waving. Kevin