qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH 3/3] s390x: Replace unchecked qdev_init() by qde


From: Cornelia Huck
Subject: Re: [Qemu-devel] [PATCH 3/3] s390x: Replace unchecked qdev_init() by qdev_init_nofail()
Date: Wed, 18 Feb 2015 16:31:28 +0100

On Wed, 18 Feb 2015 15:45:01 +0100
Markus Armbruster <address@hidden> wrote:

> Connie, your commit 7b35d0c suggests you're highly qualified to review.
> No good deed shall go unpunished ;)

Yeah, it has been on my todo list...

> 
> Markus Armbruster <address@hidden> writes:
> 
> > s390_flic_init() is a helper to create and realize either
> > "s390-flic-kvm" or "s390-flic-qemu".  When qdev_init() fails, it
> > complains to stderr and succeeds.
> >
> > Except it can't actually fail, because the "s390-flic-qemu" is a dummy
> > without a realize method, and "s390-flic-kvm"'s realize can't fail,
> > even when the kernel device is really unavailable.  Odd.

Yes, this _is_ odd. I'd say we'd want to die if we can't instatiate the
kvm-flic, but at least for injecting interrupts we have a fallback to
the old method (but looking at the code, this does not quite work out
as I thought it would). The qemu-flic is really just a dummy.

> >
> > Replace qdev_init() by qdev_init_nofail() to make "can't fail" locally
> > obvious, and get rid of the unreachable error reporting.
> >
> > Cc: Christian Borntraeger <address@hidden>
> > Cc: Cornelia Huck <address@hidden>
> > Cc: Alexander Graf <address@hidden>
> > Signed-off-by: Markus Armbruster <address@hidden>
> > ---
> >  hw/intc/s390_flic.c | 6 +-----
> >  1 file changed, 1 insertion(+), 5 deletions(-)
> >
> > diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c
> > index 03c5e89..02e10b7 100644
> > --- a/hw/intc/s390_flic.c
> > +++ b/hw/intc/s390_flic.c
> > @@ -30,7 +30,6 @@ S390FLICState *s390_get_flic(void)
> >  void s390_flic_init(void)
> >  {
> >      DeviceState *dev;
> > -    int r;
> >  
> >      dev = s390_flic_kvm_create();
> >      if (!dev) {
> > @@ -38,10 +37,7 @@ void s390_flic_init(void)
> >          object_property_add_child(qdev_get_machine(), TYPE_QEMU_S390_FLIC,
> >                                    OBJECT(dev), NULL);
> >      }
> > -    r = qdev_init(dev);
> > -    if (r) {
> > -        error_report("flic: couldn't create qdev");
> > -    }
> > +    qdev_init_nofail(dev);
> >  }
> >  
> >  static int qemu_s390_register_io_adapter(S390FLICState *fs, uint32_t id,

I think this is OK for the time being. We need to look into the various
weirdnesses for the kvm-flic, though.

Acked-by: Cornelia Huck <address@hidden>




reply via email to

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