Re: [Qemu-devel] [RFC v2] libvirt vGPU QEMU integration

[Alex Williamson]

On Thu, 22 Sep 2016 02:33:01 +0000
"Tian, Kevin" <address@hidden> wrote:

> > From: Alex Williamson [mailto:address@hidden
> > Sent: Wednesday, September 21, 2016 12:37 PM
> > 
> > On Wed, 21 Sep 2016 03:56:21 +0000
> > "Tian, Kevin" <address@hidden> wrote:
> >   
> > > > From: Kirti Wankhede [mailto:address@hidden
> > > > Sent: Tuesday, September 20, 2016 10:22 PM  
> > > > >>
> > > > >> 'max_instance':
> > > > >>     Read-Only file. Mandatory.
> > > > >>     Returns integer.  Returns maximum mdev device could be created
> > > > >> at the moment when this file is read. This count would be updated by
> > > > >> vendor driver. Before creating mdev device of this type, check if
> > > > >> max_instance is > 0.  
> > > > >
> > > > > Didn't we discuss this being being something like 
> > > > > "available_instances"
> > > > > with a "devices" sub-directory linking current devices of that type?
> > > > >  
> > > >
> > > > I'm ok with 'available_instances' as name of this file.
> > > > Yes, mdev device directory will have links to devices of that type. I
> > > > think that is part of mdev core module discussion. I'm trying to list
> > > > sysfs files for libvirt interface here to focus more on libvirt
> > > > interface. Hence didn't add that. I'll make sure to add all such details
> > > > in future.
> > > >
> > > >  
> > >
> > > Definitely you need provide those details since they also contain
> > > libvirt interface, e.g. 'destroy' which we discussed should be in
> > > mdev directory.  
> > 
> > $ find /sys/devices -name remove | wc -l
> > 24
> > $ find /sys/devices -name destroy | wc -l
> > 0
> > 
> > 'remove' is the sysfs deconstructor we should use.  
> 
> make sense.
> 
> >   
> > > Another example is class-specific attributes such
> > > as 'resolution', etc. which you listed under 'display class'. All those
> > > attributes should be moved to mdev directory. Only class ID is
> > > required under each type.  
> > 
> > Can you elaborate on what you're proposing here?  If we don't have
> > attributes like 'resolution' under the type-id then we can't describe
> > to the user the features of the mdev before we create it.  I don't
> > think anybody wants a 'create it and find out' type of interface.
> > Thanks,
> >   
> 
> I think such way would be racy. What about two clients creating mdev 
> devices simultaneously? How to guarantee their configuration of class
> specific attributes not mutual-impacted since before creation any such
> configuration would be global under that type?

A simple mutex in the sysfs store attribute to serialize, return write
error if insufficient resources to create additional devices...  Easy.
Let's not exaggerate the issue.
 
> My feeling is that we'd better keep create simple - just write a UUID
> to "type-id/create". Any class-specific attribute, if we really want to
> support, should be able to be individually configured with required
> resource allocated incrementally on top of basic resources allocated 
> at create stage. Then libvirt can set those attributes between create
> and open a mdev device. If this direction is accepted, then naturally
> such attributes should be put under mdev directory. Possibly we 
> don't need a class description under type-id. libvirt just checks directly
> whether any known class represented in each mdev directory (vendor
> driver will expose it on demand), and then operate attributes under
> that class.

It seems like this just moves the hypothetical race to the point where
we're adjusting individual attributes for the mdev device, but now the
problem is worse because there's no simple locking or serialization
that guarantees we can get all the attributes we want.  We might get
half the attributes for one device and half the attributes for another
and not be able to complete initialization of either.  I think we also
lose a lot of our ability to expose pre-defined configurations to the
user and the vendor's test matrix explodes.  We also can't provide any
sort of reasonable estimate of how many devices can be created because
we don't know the resource requirements of each device.  I don't like
this idea at all, sorry.  Thanks,

Alex