Re: [Qemu-devel] [PATCH v2 2/6] s390x/vfio: ap: Use the APdevice as a child of the APBus

[Pierre Morel]

On 30/11/2018 16:58, Tony Krowiak wrote:
> On 11/30/18 4:31 AM, Pierre Morel wrote:
> > On 29/11/2018 21:42, Tony Krowiak wrote:
> > > On 11/22/18 11:35 AM, Pierre Morel wrote:
> > > > Two good reasons to use the base device as a child of the
> > > > AP BUS:
> > > > - We can easily find the device without traversing the qtree.
> > > > - In case we have different APdevice instantiation, VFIO with
> > > >    interception or emulation, we will need the APDevice as
> > > >    a parent device.
> > > >
> > > > Signed-off-by: Pierre Morel <address@hidden>
> > > > ---
> > > >   hw/s390x/ap-device.c         | 22 ++++++++++++++++++++++
> > > >   hw/vfio/ap.c                 | 16 ++++++----------
> > > >   include/hw/s390x/ap-device.h |  2 ++
> > > >   3 files changed, 30 insertions(+), 10 deletions(-)
> > > >
> > > > diff --git a/hw/s390x/ap-device.c b/hw/s390x/ap-device.c
> > > > index f5ac8db..554d5aa 100644
> > > > --- a/hw/s390x/ap-device.c
> > > > +++ b/hw/s390x/ap-device.c
> > > > @@ -11,13 +11,35 @@
> > > >   #include "qemu/module.h"
> > > >   #include "qapi/error.h"
> > > >   #include "hw/qdev.h"
> > > > +#include "hw/s390x/ap-bridge.h"
> > > >   #include "hw/s390x/ap-device.h"
> > > > +APDevice *s390_get_ap(void)
> > >
> > > How about ap_get_device(void)?
> >
> > Yes, keep same conventions.
> >
> > > > +{
> > > > +    static DeviceState *apb;
> > >
> > > Why static if you call s390_get_ap_bridge()
> > > to retrieve it without first checking for NULL?
> >
> > static are initialized as NULL.
>
> Yes, but down below, you call s390_get_ap_bridge() to set apb
> regardless of whether apb is NULL or not. It makes no sense to
> declare is as static here. It is also redundant because the 
> s390_get_ap_bridge() function already caches it in a static
> variable.


OK thanks, yes we do not need the static for apb here.



--
Pierre Morel
Linux/KVM/QEMU in Böblingen - Germany