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

[Tian, Kevin]

> From: Alex Williamson [mailto:address@hidden
> Sent: Thursday, September 22, 2016 11:02 AM
> 
> 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,
> 

Yes, it's a valid concern. Alex, curious about your opinion of supporting
some device-agnostic optional attributes. One example is to set QoS
parameters (priority, weight, etc.), which may be enabled by vendor
driver to allow dynamic adjustment even after a mdev device is created.
Do we still put a QoS class under type-id, meaning must-be-configured 
before creation (not a bad limitation since from SLA p.o.v all QoS 
attributes should be negotiated down in advance)? Would there be any 
concern if a type-id contains multiple classes (e.g. include both 'gpu' 
class and 'qos' class)? Or not need to generalize a QoS class at all... 
just define those attributes under every possible device-specific class,
regardless of whether they are device-agnostic?

Thanks
Kevin