On Tue, 16 Feb 2021 15:20:28 -0700
Alex Williamson <alex.williamson@xxxxxxxxxx> wrote:
> On Tue, 16 Feb 2021 16:00:40 -0600
> Jonathon Jongsma <jjongsma@xxxxxxxxxx> wrote:
>
> > On Mon, 15 Feb 2021 18:22:08 +0100
> > Erik Skultety <eskultet@xxxxxxxxxx> 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.ljdd/ljdd_c_vfio_bind_ap.html
> >
> > 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
