Re: [libvirt] [RFC PATCH 00/11] Add mdev reporting capability to the nodedev driver
On Tue, Apr 04, 2017 at 04:23:18PM +0100, Daniel P. Berrange wrote:
On Wed, Mar 29, 2017 at 02:51:10PM +0200, Erik Skultety wrote:
> This series enables the node device driver to report information about the
> existing mediated devices on the host. There is no device creation involved
> yet. The information reported by the node device driver is split into two
> parts, one that is reported within the physical parent's capabilities
> (the generic stuff that comes from the mdev types' sysfs attributes, note the
> 'description' attribute which is verbatim - raw,unstructured string) and
the
> other that is reported within the mdev child device and merely contains the
> mdev type id, which the device was instantiated from, and the iommu group
> number.
>
> Basically, the format of the XML I went for is as follows:
>
> PCI parent:
> <device>
> <name>pci_0000_06_00_0</name>
> <path>/sys/devices/.../0000:06:00.0</path>
> <parent>pci_0000_05_08_0</parent>
> ...
> <capability type='pci'>
> ...
> <capability type='mdev'>
> <type id='nvidia-11'>
> <name>GRID M60-0B</name>
> <description>num_heads=2, frl_config=45, framebuffer=512M,
max_resolution=2560x1600, max_instance=16</description>
This 'description' field is pretty horrific.
We were quite clear that we were not going to expose arbitrary attributes
in the XML without modelling them explicitly as XML elements. Using the
description field in this way is just doing arbitrary attribute passthrough
via the backdoor - it is clear that applications are doing to end up parsing
this 'description' data and will them complain if we later change it.
I remember us stating that, but this is the case with NVIDIA (who at least
reports some useful information in it) and Intel - what if other vendor comes
and creates a valid, human readable description of a device without specifying
any attributes like in the case above? Just because some vendors "abused" the
attribute doesn't mean we should stop reporting it completely. IMHO the name
"description" should mean that it's something raw, possibly unstructured,
and
thus the applications should treat it that way. Now, this is my bad and I need
to revisit the last patch with documentation and add a paragraph for the
<description> element as an optional element with raw data.
Until all the attributes are properly unified/standardized under sysfs ABI and
respected by vendors, I think we should report everything we're able to gather
about a device, description included. If properly documented, nobody can
complain about us breaking anything, since how do you guarantee format-less raw
string anyway? After all, applications will end up parsing it anyway, be it from
our XML or not.
Erik
So, NACK to including a description element with this kind of
content.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|