From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:36938) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RuPaB-0003Z4-JK for qemu-devel@nongnu.org; Mon, 06 Feb 2012 09:30:20 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RuPa5-0002dZ-CX for qemu-devel@nongnu.org; Mon, 06 Feb 2012 09:30:15 -0500 Received: from mail-pw0-f45.google.com ([209.85.160.45]:33824) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RuPa5-0002d0-0s for qemu-devel@nongnu.org; Mon, 06 Feb 2012 09:30:09 -0500 Received: by pbaa11 with SMTP id a11so6791008pba.4 for ; Mon, 06 Feb 2012 06:30:07 -0800 (PST) Message-ID: <4F2FE3EB.5070606@codemonkey.ws> Date: Mon, 06 Feb 2012 08:30:03 -0600 From: Anthony Liguori MIME-Version: 1.0 References: <1328342577-25732-1-git-send-email-pbonzini@redhat.com> <1328342577-25732-14-git-send-email-pbonzini@redhat.com> In-Reply-To: <1328342577-25732-14-git-send-email-pbonzini@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 13/27] qdev: remove direct calls to print/parse List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: qemu-devel@nongnu.org On 02/04/2012 02:02 AM, Paolo Bonzini wrote: > There's no need to call into ->parse and ->print manually. The > QOM legacy properties do that for us. > > Furthermore, in some cases legacy and static properties have exactly > the same behavior, and we could drop the legacy properties right away. > Add an appropriate fallback to prepare for this. > > Signed-off-by: Paolo Bonzini Nice cleanup. Reviewed-by: Anthony Liguori I wonder how far we are from moving most of qdev-monitor.c to hmp.c and qmp.c respectively. I think that given this patch, we can actually implement info qtree/qdm via qom QMP operations. That would be a fairly nice win. Regards, Anthony Liguori > --- > hw/qdev-monitor.c | 30 +++++++++++++++++------------- > hw/qdev-properties.c | 26 ++++++++++---------------- > hw/qdev.c | 9 +++++++++ > 3 files changed, 36 insertions(+), 29 deletions(-) > > diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c > index 135c2bf..49f13ca 100644 > --- a/hw/qdev-monitor.c > +++ b/hw/qdev-monitor.c > @@ -485,22 +485,26 @@ static void qbus_print(Monitor *mon, BusState *bus, int indent); > static void qdev_print_props(Monitor *mon, DeviceState *dev, Property *props, > const char *prefix, int indent) > { > - char buf[64]; > - > if (!props) > return; > - while (props->name) { > - /* > - * TODO Properties without a print method are just for dirty > - * hacks. qdev_prop_ptr is the only such PropertyInfo. It's > - * marked for removal. The test props->info->print should be > - * removed along with it. > - */ > - if (props->info->print) { > - props->info->print(dev, props, buf, sizeof(buf)); > - qdev_printf("%s-prop: %s = %s\n", prefix, props->name, buf); > + for (; props->name; props++) { > + Error *err = NULL; > + char *value; > + char *legacy_name = g_strdup_printf("legacy-%s", props->name); > + if (object_property_get_type(OBJECT(dev), legacy_name, NULL)) { > + value = object_property_get_str(OBJECT(dev), legacy_name,&err); > + } else { > + value = object_property_get_str(OBJECT(dev), props->name,&err); > + } > + g_free(legacy_name); > + > + if (err) { > + error_free(err); > + continue; > } > - props++; > + qdev_printf("%s-prop: %s = %s\n", prefix, props->name, > + value&& *value ? value : ""); > + g_free(value); > } > } > > diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c > index c4583a1..5e19ec8 100644 > --- a/hw/qdev-properties.c > +++ b/hw/qdev-properties.c > @@ -1073,24 +1073,18 @@ void error_set_from_qdev_prop_error(Error **errp, int ret, DeviceState *dev, > > int qdev_prop_parse(DeviceState *dev, const char *name, const char *value) > { > - Property *prop; > - int ret; > + char *legacy_name; > + Error *err = NULL; > > - prop = qdev_prop_find(dev, name); > - /* > - * TODO Properties without a parse method are just for dirty > - * hacks. qdev_prop_ptr is the only such PropertyInfo. It's > - * marked for removal. The test !prop->info->parse should be > - * removed along with it. > - */ > - if (!prop || !prop->info->parse) { > - qerror_report(QERR_PROPERTY_NOT_FOUND, object_get_typename(OBJECT(dev)), name); > - return -1; > + legacy_name = g_strdup_printf("legacy-%s", name); > + if (object_property_get_type(OBJECT(dev), legacy_name, NULL)) { > + object_property_set_str(OBJECT(dev), value, legacy_name,&err); > + } else { > + object_property_set_str(OBJECT(dev), value, name,&err); > } > - ret = prop->info->parse(dev, prop, value); > - if (ret< 0) { > - Error *err; > - error_set_from_qdev_prop_error(&err, ret, dev, prop, value); > + g_free(legacy_name); > + > + if (err) { > qerror_report_err(err); > error_free(err); > return -1; > diff --git a/hw/qdev.c b/hw/qdev.c > index e3b53b7..a731e41 100644 > --- a/hw/qdev.c > +++ b/hw/qdev.c > @@ -581,6 +581,15 @@ void qdev_property_add_legacy(DeviceState *dev, Property *prop, > void qdev_property_add_static(DeviceState *dev, Property *prop, > Error **errp) > { > + /* > + * TODO qdev_prop_ptr does not have getters or setters. It must > + * go now that it can be replaced with links. The test should be > + * removed along with it: all static properties are read/write. > + */ > + if (!prop->info->get&& !prop->info->set) { > + return; > + } > + > object_property_add(OBJECT(dev), prop->name, prop->info->name, > prop->info->get, prop->info->set, > NULL,