On 6/28/19 5:06 AM, Cornelia Huck wrote: > On Thu, 27 Jun 2019 19:57:04 -0600 > Alex Williamson <alex.william...@redhat.com> wrote: > >> On Thu, 27 Jun 2019 15:15:02 -0600 >> Alex Williamson <alex.william...@redhat.com> wrote: >> >>> On Thu, 27 Jun 2019 09:38:32 -0600 >>> Alex Williamson <alex.william...@redhat.com> wrote: >>>>> On 6/27/19 8:26 AM, Cornelia Huck wrote: >>>>>> >>>>>> { >>>>>> "foo": "1", >>>>>> "bar": "42", >>>>>> "baz": { >>>>>> "depends": ["foo", "bar"], >>>>>> "value": "plahh" >>>>>> } >>>>>> } >>>>>> >>>>>> Something like that? >>>> >>>> I'm not sure yet. I think we need to look at what's feasible (and >>>> easy) with jq. Thanks, >>> >>> I think it's not too much trouble to remove and insert into arrays, so >>> what if we were to define the config as: >>> >>> { >>> "mdev_type":"vendor-type", >>> "start":"auto", >>> "attrs": [ >>> {"attrX":["Xvalue1","Xvalue2"]}, >>> {"dir/attrY": "Yvalue1"}, >>> {"attrX": "Xvalue3"} >>> ] >>> } >>> >>> "attr" here would define sysfs attributes under the device. The array >>> would be processed in order, so in the above example we'd do the >>> following: >>> >>> 1. echo Xvalue1 > attrX >>> 2. echo Xvalue2 > attrX >>> 3. echo Yvalue1 > dir/attrY >>> 4. echo Xvalue3 > attrX >>> >>> When starting the device mdevctl would simply walk the array, if the >>> attribute key exists write the value(s). If a write fails or the >>> attribute doesn't exist, remove the device and report error. > > Yes, I think it makes sense to fail the startup of a device where we > cannot set all attributes to the requested values. > >>> >>> I think it's easiest with jq to manipulate arrays by removing and >>> inserting by index. Also if we end up with something like above, it's >>> ambiguous if we reference the "attrX" key. So perhaps we add the >>> following options to the modify command: >>> >>> --addattr=ATTRIBUTE --delattr --index=INDEX --value=VALUE1[,VALUE2] >>> >>> We could handle it like a stack, so if --index is not supplied, add to >>> the end or remove from the end. If --index is provided, delete that >>> index or add the attribute at that index. So if you had the above and >>> wanted to remove Xvalue1 but keep the ordering, you'd do: >>> >>> --delattr --index=0 >>> --addattr --index=0 --value=Xvalue2 >>> >>> Which should results in: >>> >>> "attrs": [ >>> {"attrX": "Xvalue2"}, >>> {"dir/attrY": "Yvalue1"}, >>> {"attrX": "Xvalue3"} >>> ] > > Modifying by index looks reasonable; I just sent a pull request to > print the index of an attribute out as well, so it is easier to specify > the right attribute to modify. > >>> >>> If we want to modify a running device, I'm thinking we probably want a >>> new command and options --attr=ATTRIBUTE --value=VALUE might suffice. >>> >>> Do we need to support something like this for the 'start' command or >>> should we leave that for simple devices and require a sequence of: >>> >>> # mdevctl define ... >>> # mdevctl modify --addattr... >>> ... >>> # mdevctl start >>> # mdevctl undefine >>> >>> This is effectively the long way to get a transient device. Otherwise >>> we'd need to figure out how to have --attr --value appear multiple >>> times on the start command line. Thanks, > > What do you think of a way to specify JSON for the attributes directly > on the command line? Or would it be better to just edit the config > files directly? > >> >> This is now implemented, and yes you can specify '--addattr remove >> --value 1' and mdevctl will immediately remove the device after it's >> created (more power to the admin). Listing defined devices also lists > > Fun ;) > >> any attributes defined for easy inspection. It is also possible to >> override the conversion of comma separated values into an array by >> encoding and escaping the comma. It's a little cumbersome, but >> possible in case a driver isn't fully on board with the one attribute, >> one value rule of sysfs. Does this work for vfio-ap? I also still > > I do not have ap devices to actually test this with; but defining a > device and adding attributes seems to work. >
I pulled and did a quick test with vfio-ap, it's working. I was able to define, modify with the appropriate attributes and start, resulting in a correctly-configured device. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list