Re: [libvirt-users] Descriptions of mdev types?

2019-11-20 Thread Daniel P . Berrangé
On Wed, Nov 20, 2019 at 09:52:15AM +0100, Milan Zamazal wrote:
> Erik Skultety  writes:
> 
> > On Tue, Nov 19, 2019 at 05:38:54PM +0100, Milan Zamazal wrote:
> >> Hi, when retrieving an mdev device info using `virsh nodedev-dumpxml' or
> >> the libvirt API, something like the following is returned:
> >
> >>
> >> 
> >>   
> >> GRID M60-2B4
> >> vfio-pci
> >> 4
> >>   
> >>   ...
> >> 
> >>
> >> Besides device_api, available_instances and (optional) `name',
> >> `description' of the given mdev type may be optionally provided in
> >> /sys/.../mdev_supported_types/... for each of the available mdev types.
> >> I can see in the sources that libvirt doesn't try to retrieve it -- is
> >> it intentionally or is it just an omission?  If the latter, could it be
> >> added, please?  It looks like a useful piece of information for the user
> >> to get an idea what the given mdev type means.
> >
> > The reasoning we had about not including the "description" attribute when we
> > introduced mdevs to libvirt's nodedev driver was that there was no way for
> > NVIDIA and Intel to agree on the values to be exposed by the attribute -
> > especially the data NVIDIA puts in there (like you already said) is useful, 
> > but
> > there was also no agreement on extracting the data into a different 
> > attribute
> > (a set of attributes) and make them structured within the XML.
> >
> > Which brings me to the actual content of the "description" attribute, it
> > contains unstructured free-form text and we didn't want to expose that kind 
> > of
> > thing in the XML even though it just so happens that NVIDIA put some
> > interesting data in it - since the attribute is optional and free-form, one
> > day, you find the useful data you're interested in now, but tomorrow that 
> > may
> > not be the case anymore and can easily change. I've got no problem with the
> > idea of exposing some kind of description as part of the XML per se, the
> > problem I see is that someone will try and start parsing the description 
> > field
> > because of the potentially useful data and if it changes and I'm afraid
> > complaints will head our way even though we cannot guarantee anything wrt to
> > that specific field (I'm still open to a discussion though).
> 
> I agree that it would not be a good idea to rely on the content of the
> description data or trying to parse it.  But if we can accept the
> description is basically a free-form, less or more informative, text
> then it is still useful for some purposes.  I think your worries about
> the users relying on the content of the "data" can be handled in
> documentation, by clearly stating the item may contain anything, which
> may change any time.

The challenge is that in the spec / design of mdev it is explicitly
suggested that 'description' should be a way to report features, and
this leads to drivers exposing feature info in a way that is structured
& explicitly intended to be parsable.

I think this is a reflection of a mistake in the initial mdev design.

IMHO there needs to be a way for drivers to expose feature information
in sysfs explicitly, so that "description" can fullfill the purpose of
providing a simple user friendly identifier for the device type.

This was discussed, but it got ratholed in a debate about whether we
should standardize various feature names and values for different types
of devices. People were wary of trying to standardize something before
there were many real world examples of usage to learn from, which is
understandable. It also meant someone had to do the work to look at
different device types & identify common features & no one volunteered
to try to do that.

So we're left in a situation where description is the only place for
drivers to officially expose feature info which is very undesirable.


As a way forward to move forward, I think we should accept that we're
in an imperfect reality where we are unlikely to ever standardize on
features to expose from drivers. Even if we do standardize on some
features, it is likely to only be a subset of what the driver can
actually do.

IOW, we must enhance the mdev spec to document an explicit way to
expose vendor specific feature info that people can use today, while
still allowing us scope to standardize on some features  in the
future.

I'm CC'ing alex in case he has thoughts on this...

As an example of what I'm suggesting though consider

diff --git a/Documentation/driver-api/vfio-mediated-device.rst 
b/Documentation/driver-api/vfio-mediated-device.rst
index 25eb7d5b834b..0abdb99e1d4d 100644
--- a/Documentation/driver-api/vfio-mediated-device.rst
+++ b/Documentation/driver-api/vfio-mediated-device.rst
@@ -202,6 +202,12 @@ Directories and files under the sysfs for Each Physical 
Device
   | |   |--- available_instances
   | |   |--- device_api
   | |   |--- description
+  | |   |--- features
+  | |   | |--- feature-A
+  | |   | |--- 

Re: [libvirt-users] Descriptions of mdev types?

2019-11-20 Thread Milan Zamazal
Erik Skultety  writes:

> On Tue, Nov 19, 2019 at 05:38:54PM +0100, Milan Zamazal wrote:
>> Hi, when retrieving an mdev device info using `virsh nodedev-dumpxml' or
>> the libvirt API, something like the following is returned:
>
>>
>> 
>>   
>> GRID M60-2B4
>> vfio-pci
>> 4
>>   
>>   ...
>> 
>>
>> Besides device_api, available_instances and (optional) `name',
>> `description' of the given mdev type may be optionally provided in
>> /sys/.../mdev_supported_types/... for each of the available mdev types.
>> I can see in the sources that libvirt doesn't try to retrieve it -- is
>> it intentionally or is it just an omission?  If the latter, could it be
>> added, please?  It looks like a useful piece of information for the user
>> to get an idea what the given mdev type means.
>
> The reasoning we had about not including the "description" attribute when we
> introduced mdevs to libvirt's nodedev driver was that there was no way for
> NVIDIA and Intel to agree on the values to be exposed by the attribute -
> especially the data NVIDIA puts in there (like you already said) is useful, 
> but
> there was also no agreement on extracting the data into a different attribute
> (a set of attributes) and make them structured within the XML.
>
> Which brings me to the actual content of the "description" attribute, it
> contains unstructured free-form text and we didn't want to expose that kind of
> thing in the XML even though it just so happens that NVIDIA put some
> interesting data in it - since the attribute is optional and free-form, one
> day, you find the useful data you're interested in now, but tomorrow that may
> not be the case anymore and can easily change. I've got no problem with the
> idea of exposing some kind of description as part of the XML per se, the
> problem I see is that someone will try and start parsing the description field
> because of the potentially useful data and if it changes and I'm afraid
> complaints will head our way even though we cannot guarantee anything wrt to
> that specific field (I'm still open to a discussion though).

I agree that it would not be a good idea to rely on the content of the
description data or trying to parse it.  But if we can accept the
description is basically a free-form, less or more informative, text
then it is still useful for some purposes.  I think your worries about
the users relying on the content of the "data" can be handled in
documentation, by clearly stating the item may contain anything, which
may change any time.

Our use case in oVirt is to display some accompanied information when a
user selects one of the many mdev types in the UI.  If we provided
description (as a normal text) next to each of the mdev types, it would
be definitely helpful and a significant improvement over just providing
the cryptic mdev type numbers + available instances.  Currently, we have
no better choice than to read `description' files from /sys, which makes
little sense when all the other info is already available from libvirt.
This is why I think having  element with a text content
copied 1:1 from `description' file would be useful.

Thanks,
Milan


___
libvirt-users mailing list
libvirt-users@redhat.com
https://www.redhat.com/mailman/listinfo/libvirt-users



Re: [libvirt-users] Descriptions of mdev types?

2019-11-19 Thread Erik Skultety
On Tue, Nov 19, 2019 at 05:38:54PM +0100, Milan Zamazal wrote:
> Hi, when retrieving an mdev device info using `virsh nodedev-dumpxml' or
> the libvirt API, something like the following is returned:
>
> 
>   
> GRID M60-2B4
> vfio-pci
> 4
>   
>   ...
> 
>
> Besides device_api, available_instances and (optional) `name',
> `description' of the given mdev type may be optionally provided in
> /sys/.../mdev_supported_types/... for each of the available mdev types.
> I can see in the sources that libvirt doesn't try to retrieve it -- is
> it intentionally or is it just an omission?  If the latter, could it be
> added, please?  It looks like a useful piece of information for the user
> to get an idea what the given mdev type means.

The reasoning we had about not including the "description" attribute when we
introduced mdevs to libvirt's nodedev driver was that there was no way for
NVIDIA and Intel to agree on the values to be exposed by the attribute -
especially the data NVIDIA puts in there (like you already said) is useful, but
there was also no agreement on extracting the data into a different attribute
(a set of attributes) and make them structured within the XML.

Which brings me to the actual content of the "description" attribute, it
contains unstructured free-form text and we didn't want to expose that kind of
thing in the XML even though it just so happens that NVIDIA put some
interesting data in it - since the attribute is optional and free-form, one
day, you find the useful data you're interested in now, but tomorrow that may
not be the case anymore and can easily change. I've got no problem with the
idea of exposing some kind of description as part of the XML per se, the
problem I see is that someone will try and start parsing the description field
because of the potentially useful data and if it changes and I'm afraid
complaints will head our way even though we cannot guarantee anything wrt to
that specific field (I'm still open to a discussion though).

Regards,
Erik

___
libvirt-users mailing list
libvirt-users@redhat.com
https://www.redhat.com/mailman/listinfo/libvirt-users



[libvirt-users] Descriptions of mdev types?

2019-11-19 Thread Milan Zamazal
Hi, when retrieving an mdev device info using `virsh nodedev-dumpxml' or
the libvirt API, something like the following is returned:


  
GRID M60-2B4
vfio-pci
4
  
  ...


Besides device_api, available_instances and (optional) `name',
`description' of the given mdev type may be optionally provided in
/sys/.../mdev_supported_types/... for each of the available mdev types.
I can see in the sources that libvirt doesn't try to retrieve it -- is
it intentionally or is it just an omission?  If the latter, could it be
added, please?  It looks like a useful piece of information for the user
to get an idea what the given mdev type means.

Thanks,
Milan


___
libvirt-users mailing list
libvirt-users@redhat.com
https://www.redhat.com/mailman/listinfo/libvirt-users