8:55 a.m.
On Tue, 16 Feb 2021 15:20:28 -0700
Alex Williamson <alex.williamson(a)redhat.com> wrote:
On Tue, 16 Feb 2021 16:00:40 -0600
Jonathon Jongsma <jjongsma(a)redhat.com> wrote:
> On Mon, 15 Feb 2021 18:22:08 +0100
> Erik Skultety <eskultet(a)redhat.com> wrote:
>
> > On Wed, Feb 03, 2021 at 11:38:49AM -0600, Jonathon Jongsma wrote:
> >
> > > diff --git a/tests/nodedevmdevctldata/mdevctl-list-multiple.json
> > > b/tests/nodedevmdevctldata/mdevctl-list-multiple.json new file
> > > mode 100644 index 0000000000..eefcd90c62
> > > --- /dev/null
> > > +++ b/tests/nodedevmdevctldata/mdevctl-list-multiple.json
> > > @@ -0,0 +1,59 @@
> > > +[
> > > + {
> > > + "0000:00:02.0": [
> > > + {
> > > + "200f228a-c80a-4d50-bfb7-f5a0e4e34045": {
> > > + "mdev_type": "i915-GVTg_V5_4",
> > > + "start": "manual"
> > > + }
> > > + },
> > > + {
> > > + "de807ffc-1923-4d5f-b6c9-b20ecebc6d4b": {
> > > + "mdev_type": "i915-GVTg_V5_4",
> > > + "start": "auto"
> > > + }
> > > + },
> > > + {
> > > + "435722ea-5f43-468a-874f-da34f1217f13": {
> > > + "mdev_type": "i915-GVTg_V5_8",
> > > + "start": "manual",
> > > + "attrs": [
> > > + {
> > > + "testattr": "42"
> > > + }
> > > + ]
> > > + }
> > > + }
> > > + ]
> > > + },
> > > + {
> > > + "matrix": [
> > > + { "783e6dbb-ea0e-411f-94e2-717eaad438bf": {
> > > + "mdev_type": "vfio_ap-passthrough",
> > > + "start": "manual",
> > > + "attrs": [
> > > + {
> > > + "assign_adapter": "5"
> > > + },
> > > + {
> > > + "assign_adapter": "6"
> > > + },
> >
> > I'd still like to know why there are 2 assign_adapter and 2
> > assign_domain attributes here, I'm simply confused what the
> > outcome should be.
>
> As far as I can recall, i was just trying to use some real-world-ish
> mdevctl output to test the parsing and handling of mdev attributes.
> In this case, I believe that I simply copied the example output
> from the mdevctl documentation. See:
>
> https://github.com/mdevctl/mdevctl#advanced-usage-attributes-and-json
>
>
> >
> > > + {
> > > + "assign_domain": "0xab"
> > > + },
> > > + {
> > > + "assign_control_domain": "0xab"
> > > + },
> > > + {
> > > + "assign_domain": "4"
> > > + },
> > > + {
> > > + "assign_control_domain": "4"
> > > + }
> > > + ]
> > > + }
> > > + }
> > > + ]
> > > + }
> > > +]
> > > +
> >
> >
> > ...
> >
> >
> > > + <name>mdev_783e6dbb_ea0e_411f_94e2_717eaad438bf</name>
> > > + <parent>matrix</parent>
> > > + <capability type='mdev'>
> > > + <type id='vfio_ap-passthrough'/>
> > > + <iommuGroup number='0'/>
> > > + <attr name='assign_adapter' value='5'/>
> > > + <attr name='assign_adapter' value='6'/>
> > > + <attr name='assign_domain' value='0xab'/>
> > > + <attr name='assign_control_domain'
value='0xab'/>
> > > + <attr name='assign_domain' value='4'/>
> > > + <attr name='assign_control_domain' value='4'/>
> >
> > Here too I'd like to hear an opinion (since v3) on naming the
> > attributes in such way that they correspond to the respective
> > elements: ap-adapter, ap-domain, etc. This naming is very
> > unintuitive if not documented properly; unless there's an internal
> > reason why they have to be named "assign_control", etc.
>
> These are the names of the attributes that are used to configure
> these devices in sysfs:
>
https://www.ibm.com/support/knowledgecenter/linuxonibm/com.ibm.linux.z.lj...
>
> The idea here is that <attr> is a direct mapping to and from the
> mdev sysfs attributes, since that is how these devices are
> configured. An attribute name means nothing to libvirt, it is just
> an opaque name that we pass to mdevctl. If we were to deviate from
> this strict mapping and add "friendlier" names in libvirt, we would
> need to maintain a custom per-device mapping from mdev sysfs
> attribute name to libvirt friendly-name. That seems unmaintainable.
>
Yep, and the names don't mean anything to mdevctl either, as you say
they're just sysfs attributes, mdevctl looks for the named attribute
and writes the value to it. Note that ordering of the attributes
might be important, which is why mdevctl stores them in an array and
provides some utility to re-index attributes. I can't tell if the
above example necessarily imposes that ordering from the xml. Thanks,
Yes, the order of xml attributes is significant, they should be passed
to mdevctl in the same order as they are defined in the xml. Though I
don't think I've documented that anywhere...
Jonathon