From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:59866) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Rbb92-00065z-Cm for qemu-devel@nongnu.org; Fri, 16 Dec 2011 12:00:29 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Rbb8u-0006vI-1z for qemu-devel@nongnu.org; Fri, 16 Dec 2011 12:00:28 -0500 Received: from mail-yx0-f173.google.com ([209.85.213.173]:33147) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Rbb8t-0006v9-Uk for qemu-devel@nongnu.org; Fri, 16 Dec 2011 12:00:20 -0500 Received: by yenm6 with SMTP id m6so2750212yen.4 for ; Fri, 16 Dec 2011 09:00:18 -0800 (PST) Message-ID: <4EEB7916.5010000@codemonkey.ws> Date: Fri, 16 Dec 2011 11:00:06 -0600 From: Anthony Liguori MIME-Version: 1.0 References: <1324054053-20484-1-git-send-email-pbonzini@redhat.com> <1324054053-20484-5-git-send-email-pbonzini@redhat.com> In-Reply-To: <1324054053-20484-5-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 4/8] qom: interpret the return value when setting legacy properties List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: qemu-devel@nongnu.org On 12/16/2011 10:47 AM, Paolo Bonzini wrote: > Signed-off-by: Paolo Bonzini > --- > hw/qdev-properties.c | 39 ++++++++++++++++++++++++--------------- > hw/qdev.c | 3 +-- > hw/qdev.h | 2 ++ > 3 files changed, 27 insertions(+), 17 deletions(-) > > diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c > index f0b811c..76ecb38 100644 > --- a/hw/qdev-properties.c > +++ b/hw/qdev-properties.c > @@ -614,6 +614,26 @@ int qdev_prop_exists(DeviceState *dev, const char *name) > return qdev_prop_find(dev, name) ? true : false; > } > > +void qdev_prop_error(Error **errp, int ret, > + DeviceState *dev, Property *prop, const char *value) > +{ > + switch (ret) { > + case -EEXIST: > + error_set(errp, QERR_PROPERTY_VALUE_IN_USE, > + dev->info->name, prop->name, value); > + break; > + default: > + case -EINVAL: > + error_set(errp, QERR_PROPERTY_VALUE_BAD, > + dev->info->name, prop->name, value); > + break; > + case -ENOENT: > + error_set(errp, QERR_PROPERTY_VALUE_NOT_FOUND, > + dev->info->name, prop->name, value); > + break; > + } > +} > + > int qdev_prop_parse(DeviceState *dev, const char *name, const char *value) > { > Property *prop; > @@ -632,21 +652,10 @@ int qdev_prop_parse(DeviceState *dev, const char *name, const char *value) > } > ret = prop->info->parse(dev, prop, value); > if (ret< 0) { > - switch (ret) { > - case -EEXIST: > - qerror_report(QERR_PROPERTY_VALUE_IN_USE, > - dev->info->name, name, value); > - break; > - default: > - case -EINVAL: > - qerror_report(QERR_PROPERTY_VALUE_BAD, > - dev->info->name, name, value); > - break; > - case -ENOENT: > - qerror_report(QERR_PROPERTY_VALUE_NOT_FOUND, > - dev->info->name, name, value); > - break; > - } > + Error *err; > + qdev_prop_error(&err, ret, dev, prop, value); > + qerror_report_err(err); > + error_free(err); > return -1; > } > return 0; > diff --git a/hw/qdev.c b/hw/qdev.c > index c020a6f..c8ab7b7 100644 > --- a/hw/qdev.c > +++ b/hw/qdev.c > @@ -1163,8 +1163,7 @@ static void qdev_set_legacy_property(DeviceState *dev, Visitor *v, void *opaque, > > ret = prop->info->parse(dev, prop, ptr); > if (ret != 0) { > - error_set(errp, QERR_INVALID_PARAMETER_VALUE, > - name, prop->info->name); > + qdev_prop_error(errp, ret, dev, prop, ptr); > } > g_free(ptr); > } > diff --git a/hw/qdev.h b/hw/qdev.h > index 6e18427..828d811 100644 > --- a/hw/qdev.h > +++ b/hw/qdev.h > @@ -370,6 +370,8 @@ void qdev_prop_set_defaults(DeviceState *dev, Property *props); > > void qdev_prop_register_global_list(GlobalProperty *props); > void qdev_prop_set_globals(DeviceState *dev); > +void qdev_prop_error(Error **errp, int ret, DeviceState *name, > + Property *prop, const char *value); s/name/dev And perhaps it would make more sense to return Error * and make the function name be a constructor: Error *error_from_qdev_prop_err(int ret, DeviceState *dev, Property *prop, const char *value); I was fairly confused about what was going on here at first. Reards, Anthony Liguori > > static inline const char *qdev_fw_name(DeviceState *dev) > {