On Fri, Jul 22, 2016 at 09:16:57AM +0200, Greg Kurz wrote: > On Fri, 22 Jul 2016 11:28:48 +1000 > David Gibson <da...@gibson.dropbear.id.au> wrote: > > > On Fri, Jul 22, 2016 at 01:01:26AM +0200, Greg Kurz wrote: > > > This patch ensures QEMU won't terminate while hotplugging a device if the > > > global property cannot be set and errp points to error_fatal or > > > error_abort. > > > > > > While here, it also fixes indentation of the typename argument. > > > > > > Suggested-by: Eduardo Habkost <ehabk...@redhat.com> > > > Signed-off-by: Greg Kurz <gr...@kaod.org> > > > > This seems kind of bogus to me - we have this whole infrastructure for > > handling errors, and here we throw it away. > > > > It seems like the right solution would be to make the caller in the > > hotplug case *not* use error_abort or error_fatal, and instead get the > > error propagated back to the monitor which will display it. > > The caller is QOM initialization here. Are you asking to add an errp argument > to object_initialize() and friends ?
Ugh. I guess I am. I can see why you'd want to avoid that. On the
other hand, it really does seem like the right approach.
>
> > > ---
> > > hw/core/qdev-properties.c | 4 ++--
> > > include/hw/qdev-core.h | 4 +++-
> > > 2 files changed, 5 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> > > index 14e544ab17d2..311af6da7684 100644
> > > --- a/hw/core/qdev-properties.c
> > > +++ b/hw/core/qdev-properties.c
> > > @@ -1084,7 +1084,7 @@ int qdev_prop_check_globals(void)
> > > }
> > >
> > > static void qdev_prop_set_globals_for_type(DeviceState *dev,
> > > - const char *typename)
> > > + const char *typename)
> > > {
> > > GList *l;
> > >
> > > @@ -1100,7 +1100,7 @@ static void
> > > qdev_prop_set_globals_for_type(DeviceState *dev,
> > > if (err != NULL) {
> > > error_prepend(&err, "can't apply global %s.%s=%s: ",
> > > prop->driver, prop->property, prop->value);
> > > - if (prop->errp) {
> > > + if (!dev->hotplugged && prop->errp) {
> > > error_propagate(prop->errp, err);
> > > } else {
> > > assert(prop->user_provided);
> > > diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> > > index 1d1f8612a9b8..4b4b33bec885 100644
> > > --- a/include/hw/qdev-core.h
> > > +++ b/include/hw/qdev-core.h
> > > @@ -261,7 +261,9 @@ struct PropertyInfo {
> > > * @used: Set to true if property was used when initializing a device.
> > > * @errp: Error destination, used like first argument of error_setg()
> > > * in case property setting fails later. If @errp is NULL, we
> > > - * print warnings instead of ignoring errors silently.
> > > + * print warnings instead of ignoring errors silently. For
> > > + * hotplugged devices, errp is always ignored and warnings are
> > > + * printed instead.
> > > */
> > > typedef struct GlobalProperty {
> > > const char *driver;
> > >
> >
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
