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.

> So, NACK to including a description element with this kind of content.