Re: [qemu-s390x] [PATCH RFC 2/2] s390x: attach autogenerated nics

[Cornelia Huck]

On Mon, 4 Dec 2017 18:33:24 +0100
Halil Pasic <address@hidden> wrote:

> On 12/04/2017 05:40 PM, Cornelia Huck wrote:
> > On Mon, 4 Dec 2017 12:17:06 +0100
> > Christian Borntraeger <address@hidden> wrote:
> >   
> >> On 11/28/2017 02:46 PM, Cornelia Huck wrote:  
> >>> The autogenerated nics should be treated as any other device; use
> >>> qdev_set_id() to have them show up under peripheral-anon.
> >>>     
> >> I think this is fine, but then I ask myself how x86 does this. So I tried 
> >> to 
> >> find out how the pc-q35 machine does this but I somehow failed to 
> >> understand
> >> how they do it. Do you have any clue?  
> > 
> > It seems they don't. If you start up a machine with only autogenerated
> > devices, you won't find anything under peripheral{-anon}, but several
> > devices under unattached.
> > 
> > So, maybe we should change this for everything? Or just leave it alone?
> > 
> > (The css-bridge change is a different thing IMO, it clearly should be
> > attached to the machine.)
> >   
> 
> IMHO (try to) change everywhere. The devices are attached to the machine,
> and them showing up as unattached is misleading. IMHO we still to have the
> 'is it API or not' question/problem so we need to be careful.
> 
> Another think I was wondering about is ids: there are QMP commands which
> designate devices by path and there are commands which designate by id
> (and we even have either-or via the same parameter in case of device_del).
> Since the paths do not seem to be directly assigned/controlled by the user
> ([1]] but id's are I would argue that ids are easier to understand and
> use. Would generating an id for each auto-generated device be a good idea?
> 
> I'm trying to figure out, how the QAPI is supposed to be used, and feel like.
> So take my comments with a grain of salt.
> 
> [1] One can, but does not have to specify the bus. Libvirt does not seem
> to for virtio-ccw devices.  And if one were to, the other patch in the
> series could break that code.

I'm inclined to rather just drop this patch and put it into the backlog
for idle times, before this escalates into a wholesale rewrite of core
infrastructure.