qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v4 2/4] qdev: using NULL instead of local_err fo


From: Gonglei (Arei)
Subject: Re: [Qemu-devel] [PATCH v4 2/4] qdev: using NULL instead of local_err for qbus_child unrealize
Date: Tue, 2 Sep 2014 08:50:11 +0000

> From: address@hidden
> >> >
> >> > Forcefully unrealize all children regardless of early-iteration
> >> > errors(if happened). We should keep going with cleanup operation
> >> > rather than report a error immediately.
> >> >
> >> > Signed-off-by: Gonglei <address@hidden>
> >> > ---
> >> >  hw/core/qdev.c | 9 +++------
> >> >  1 file changed, 3 insertions(+), 6 deletions(-)
> >> >
> >> > diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> >> > index 4a1ac5b..d3ff526 100644
> >> > --- a/hw/core/qdev.c
> >> > +++ b/hw/core/qdev.c
> >> > @@ -873,15 +873,12 @@ static void device_set_realized(Object *obj,
> bool
> >> value, Error **errp)
> >> >      } else if (!value && dev->realized) {
> >> >          QLIST_FOREACH(bus, &dev->child_bus, sibling) {
> >> >              object_property_set_bool(OBJECT(bus), false, "realized",
> >> > -                                     &local_err);
> >> > -            if (local_err != NULL) {
> >> > -                break;
> >> > -            }
> >> > +                                     NULL);
> >>
> >> Local err should still be set if at least one child fails to
> >> unrealize. Otherwise the calls gets a silent failure.
> >
> > Yes.
> >
> >> Couple of ways of doing this.
> >>
> >> 1. Propagate only the first failure.
> >> 2. Raise a generic error to indicate "at least one child failed to 
> >> unrealize".
> >> 3. Concatenate all the child unrealize errors together.
> >>
> >> I like option 1 out of simplicity.
> >>
> > OK. Agreed.
> >
> >> >          }
> >> > -        if (qdev_get_vmsd(dev) && local_err == NULL) {
> >> > +        if (qdev_get_vmsd(dev)) {
> >>
> >> You are also forcefully un-vmsding and unrealizing the actual object
> >> too. You can generalise the command message.
> >>
> > Peter, Do you mean change the commit message to cover this change?
> > Or don't un-vmsding and unrealizing actual object when one child
> > unrealized failed? Thanks.
> >
> 
> Just commit msg. Typo by me, s/command/commit.
> 
OK. Thanks. Will do.

Best regards,
-Gonglei

reply via email to

[Prev in Thread] Current Thread [Next in Thread]