Re: [Qemu-devel] [RFC 02/19] s390: Add FIXME for unexplained user_creatable=false line

[Cornelia Huck]

On Mon, 3 Apr 2017 16:20:11 -0300
Eduardo Habkost <address@hidden> wrote:

> On Mon, Apr 03, 2017 at 10:55:38AM +0200, Cornelia Huck wrote:
> > On Fri, 31 Mar 2017 21:46:07 -0300
> > Eduardo Habkost <address@hidden> wrote:
> > 
> > > TYPE_S390_PCI_HOST_BRIDGE has user_creatable=false but has
> > > no comment explaining why. Add a FIXME to document that.
> > > 
> > > Cc: Frank Blaschka <address@hidden>
> > > Cc: Cornelia Huck <address@hidden>
> > > Cc: Christian Borntraeger <address@hidden>
> > > Cc: Alexander Graf <address@hidden>
> > > Cc: Richard Henderson <address@hidden>
> > > Signed-off-by: Eduardo Habkost <address@hidden>
> > > ---
> > >  hw/s390x/s390-pci-bus.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> > > index 1ec30c45ce..2c3b960bdf 100644
> > > --- a/hw/s390x/s390-pci-bus.c
> > > +++ b/hw/s390x/s390-pci-bus.c
> > > @@ -867,7 +867,7 @@ static void s390_pcihost_class_init(ObjectClass 
> > > *klass, void *data)
> > >      DeviceClass *dc = DEVICE_CLASS(klass);
> > >      HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass);
> > >  
> > > -    dc->user_creatable = false;
> > > +    dc->user_creatable = false; /*FIXME: explain why */
> > >      dc->reset = s390_pcihost_reset;
> > >      k->init = s390_pcihost_init;
> > >      hc->plug = s390_pcihost_hot_plug;
> > 
> > (adding some more possibly interested parties)
> > 
> > We currently have one master s390 phb (and it's been that way since
> > s390 pci was introduced). Recently, there has been some remodelling
> > going on to make this more similar to what sPAPR does. I think we could
> > make this even more similar to sPAPR and have this user createable; but
> > I'm currently not sure it's worth the effort. Opinions?
> 
> It looks -device s390-pcihost was never possible, anyway, because
> no s390x machine has has_dynamic_sysbus=1, and TYPE_PCI_HOST_BRIDGE
> is a sys-bus-device.

Yes.

> 
> Also, patch 03/19 on this series would make the explicit
> user_creatable=false assignment in s390_pcihost_class_init()
> unnecessary.

If we switch to "sysbus devices are never user creatable, except for a
select few" anyway, I think we can just get rid of this.

> 
> I don't think it is worth the effort to change that, unless you
> have a specific use case that would benefit from it.

Not really. We're building a highly artificical "topology" that does
not exist on real hardware (and is not seen by the guest) here, so we
can basically do whatever works best. We _might_ want to be more
similar to sPAPR, so that we're not the complete oddball, but it seems
that we can make everything work with the current setup already.