From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41369) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bEuMF-0003wC-I4 for qemu-devel@nongnu.org; Mon, 20 Jun 2016 04:15:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bEuME-00019r-8b for qemu-devel@nongnu.org; Mon, 20 Jun 2016 04:14:59 -0400 Received: from mx1.redhat.com ([209.132.183.28]:44942) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bEuME-00019V-0j for qemu-devel@nongnu.org; Mon, 20 Jun 2016 04:14:58 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 84DF8C062C93 for ; Mon, 20 Jun 2016 08:14:57 +0000 (UTC) From: Markus Armbruster References: <1466022773-8965-1-git-send-email-ehabkost@redhat.com> <1466022773-8965-6-git-send-email-ehabkost@redhat.com> Date: Mon, 20 Jun 2016 10:14:55 +0200 In-Reply-To: <1466022773-8965-6-git-send-email-ehabkost@redhat.com> (Eduardo Habkost's message of "Wed, 15 Jun 2016 17:32:48 -0300") Message-ID: <87y460b7k0.fsf@dusky.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH 05/10] qdev: GlobalProperty.errp field List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: qemu-devel@nongnu.org, Marcel Apfelbaum , Paolo Bonzini , Igor Mammedov Eduardo Habkost writes: > The new field will allow error handling to be configured by > qdev_prop_register_global() callers: &error_fatal and > &error_abort can be used to make QEMU exit or abort if any errors > are reported when applying the properties. > > Suggested-by: Paolo Bonzini > Signed-off-by: Eduardo Habkost > --- > hw/core/qdev-properties.c | 8 ++++++-- > include/hw/qdev-core.h | 4 ++++ > 2 files changed, 10 insertions(+), 2 deletions(-) > > diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c > index cd19603..0fe7214 100644 > --- a/hw/core/qdev-properties.c > +++ b/hw/core/qdev-properties.c > @@ -1078,10 +1078,14 @@ static void qdev_prop_set_globals_for_type(DeviceState *dev, > prop->used = true; > object_property_parse(OBJECT(dev), prop->value, prop->property, &err); > if (err != NULL) { > - assert(prop->user_provided); > error_prepend(&err, "can't apply global %s.%s=%s: ", > prop->driver, prop->property, prop->value); > - error_reportf_err(err, "Warning: "); > + if (prop->errp) { > + error_propagate(prop->errp, err); > + } else { > + assert(prop->user_provided); > + error_reportf_err(err, "Warning: "); > + } > } > } > } Now I understand. Suggest to squash the previous patch into this one. > diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h > index 24aa0a7..16e7cc0 100644 > --- a/include/hw/qdev-core.h > +++ b/include/hw/qdev-core.h > @@ -256,6 +256,9 @@ struct PropertyInfo { > > /** > * GlobalProperty: > + * @errp: If set, error_propagate() will be called on errors when applying > + * the property. &error_abort or &error_fatal may be used to make > + * errors automatically abort or exit QEMU. "If set" is awkward, because it's not what we usually mean when we talk about an error being set. Suggest "If non-null". But what makes null special isn't that errors won't be propagated. In fact, the code behaves as if they were (propagating to null frees the error, which is exactly what the code does), *except* for one thing that isn't mentioned here, but should be: we print a warning. What about: * @errp: Error destination, used like a first argument of error_setg(), * except with null @errp, we print warnings instead of ignoring errors * silently. > * @user_provided: Set to true if property comes from user-provided config > * (command-line or config file). > * @used: Set to true if property was used when initializing a device. > @@ -264,6 +267,7 @@ typedef struct GlobalProperty { > const char *driver; > const char *property; > const char *value; > + Error **errp; > bool user_provided; > bool used; > } GlobalProperty;