From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([209.51.188.92]:33354) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ghY6J-0005si-Kn for qemu-devel@nongnu.org; Thu, 10 Jan 2019 06:02:16 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ghXwc-0004Gy-1x for qemu-devel@nongnu.org; Thu, 10 Jan 2019 05:52:14 -0500 Received: from mx1.redhat.com ([209.132.183.28]:59264) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1ghXwb-0004Go-S7 for qemu-devel@nongnu.org; Thu, 10 Jan 2019 05:52:13 -0500 Date: Thu, 10 Jan 2019 11:52:03 +0100 From: Cornelia Huck Message-ID: <20190110115203.47ec729f.cohuck@redhat.com> In-Reply-To: <20190110020259.8492-3-ehabkost@redhat.com> References: <20190110020259.8492-1-ehabkost@redhat.com> <20190110020259.8492-3-ehabkost@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 2/3] globals: Allow global properties to be optional List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: qemu-devel@nongnu.org, Thomas Huth , Marcel Apfelbaum , "Michael S. Tsirkin" , =?UTF-8?B?TWFyYy1BbmRyw6k=?= Lureau , "Dr. David Alan Gilbert" On Thu, 10 Jan 2019 00:02:58 -0200 Eduardo Habkost wrote: > Making some global properties optional will let us simplify > compat code when a given property works on most (but not all) > subclasses of a given type. > > Device types will be able to opt out from optional compat > properties by simply not registering those properties, or by > making the property setter report an error. I'm not sure whether we want to conflate "property does not exist" and "setter returns an error". I can see the value in "apply a setting if the property exists; if it doesn't exist, the setting is not applicable". But if you also allow the setter to return an error, you will not notice if you e.g. have been providing junk in the value and it errors out everywhere. > > Signed-off-by: Eduardo Habkost > --- > Note: the "An error is fatal for non-hotplugged devices [...]" > comment that appears in the diff scope is inaccurate, but I will > fix that in a separate patch because I don't want this to get > into the way of fixing the crash. > --- > include/hw/qdev-core.h | 3 +++ > qom/object.c | 4 +++- > 2 files changed, 6 insertions(+), 1 deletion(-) > > diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h > index bc014c1c9f..463e1ddb1e 100644 > --- a/include/hw/qdev-core.h > +++ b/include/hw/qdev-core.h > @@ -250,6 +250,8 @@ struct PropertyInfo { > /** > * GlobalProperty: > * @used: Set to true if property was used when initializing a device. > + * @optional: If set to true, errors when applying the property won't be > + * reported by object_apply_global_props(). > * > * An error is fatal for non-hotplugged devices, when the global is applied. > */ > @@ -258,6 +260,7 @@ typedef struct GlobalProperty { > const char *property; > const char *value; > bool used; > + bool optional; > } GlobalProperty; > > static inline void > diff --git a/qom/object.c b/qom/object.c > index 4e5226ca12..226ddf0189 100644 > --- a/qom/object.c > +++ b/qom/object.c > @@ -387,7 +387,9 @@ void object_apply_global_props(Object *obj, const GPtrArray *props, Error **errp > } > p->used = true; > object_property_parse(obj, p->value, p->property, &err); > - if (err != NULL) { > + if (p->optional) { > + error_free(err); > + } else if (err != NULL) { > error_prepend(&err, "can't apply global %s.%s=%s: ", > p->driver, p->property, p->value); > /*