Re: [qemu-s390x] [Qemu-devel] [PATCH RFC 0/2] s390x: cut down on unattached devices

[Cornelia Huck]

On Thu, 7 Dec 2017 18:01:46 +0100
Halil Pasic <address@hidden> wrote:

> On 12/07/2017 05:34 PM, Cornelia Huck wrote:
> > On Tue, 5 Dec 2017 09:59:06 +0100
> > Bjoern Walk <address@hidden> wrote:
> >   
> >> Cornelia Huck <address@hidden> [2017-11-28, 02:46PM +0100]:  
> >>> info qom-tree shows several devices under unattached that probably
> >>> should go somewhere.
> >>>
> >>> The css bridge should attach to the machine, as it has a similar
> >>> purpose as e.g. a pci host bridge.
> >>>
> >>> The autogenerated network devices should be in the same bucket as any
> >>> other device; I'm just not sure about the way I went about it.
> >>>
> >>> The zpci devices are still problematic: I don't have a good idea where
> >>> they should show up.
> >>>
> >>> Remaining in the unattached container are the sysbus, memory regions
> >>> and cpus.
> >>>
> >>> Cornelia Huck (2):
> >>>   s390x/css: attach css bridge
> >>>   s390x: attach autogenerated nics
> >>>
> >>>  hw/s390x/css-bridge.c      | 2 ++
> >>>  hw/s390x/s390-virtio-ccw.c | 2 ++
> >>>  2 files changed, 4 insertions(+)
> >>>
> >>> -- 
> >>> 2.13.6
> >>>
> >>>     
> >>
> >> Regarding the discussion about whether the QOM tree is API and what
> >> exploiters like libvirt should do, Halil asked me to chip in.
> >>
> >> This patch is fine from libvirt perspective. I did a quick smoke test
> >> and you can have a
> >>
> >>     Tested-by: Bjoern Walk <address@hidden>
> >>
> >> for what it's worth.  
> > 
> > Thanks for checking.
> >   
> >>
> >> In general, I kind of agree with Halil. Unless somewhere in QEMU it is
> >> documented that the QOM tree is not guaranteed to be stable for
> >> exploiters, I'd consider is part of the API. libvirt does use at least
> >> some hardcoded paths, most of the time for CPUs in /machine/unattached,
> >> so if that relation would change, things break. However, there is also
> >> code to traverse the QOM tree recursively and find a path for a given
> >> type(?) name. If this is the preferred way, we probably should change
> >> this in libvirt to be safe.  
> > 
> > OK, with that in mind and as we're now adding a property to check on
> > the css bridge, I vote for including patch 1 now (having a fixed
> > location under /machine looks saner that having to
> > check /machine/unattached/device[<n>], which might not be stable).
> > 
> > Patch 2 needs more discussion, as I'm not sure whether what I'm doing
> > is the correct way to go about this (and other machines are in the same
> > situation). Not sure whether it is worth trying to attach the zpci
> > devices somewhere.
> >   
> 
> I think, if it's kind of API, then fixing sooner is better than fixing
> later.
> 
> I also agree that patch 1 should be higher priority.
> 
> Before we do patch 1 I would like having agreed and documented whether
> this is API or not.
> 
> If we decide it's an API, I think we should consider deprecating
> the current interface, but keep it working for two releases or
> so. I think nothing speaks against introducing a link form unattached
> in patch 1 (but I have not tried yet).

No, just no. That's completely overengineered.