On 6/27/19 11:38 AM, Alex Williamson wrote:
On Thu, 27 Jun 2019 11:00:31 -0400 Matthew Rosato <mjrosato@linux.ibm.com> wrote:
On 6/27/19 8:26 AM, Cornelia Huck wrote:
On Wed, 26 Jun 2019 19:53:50 -0600 Alex Williamson <alex.williamson@redhat.com> wrote:
On Wed, 26 Jun 2019 08:37:20 -0600 Alex Williamson <alex.williamson@redhat.com> wrote:
On Wed, 26 Jun 2019 11:58:06 +0200 Cornelia Huck <cohuck@redhat.com> wrote:
On Tue, 25 Jun 2019 16:52:51 -0600 Alex Williamson <alex.williamson@redhat.com> wrote:
> Hi, > > Based on the discussions we've had, I've rewritten the bulk of > mdevctl. I think it largely does everything we want now, modulo > devices that will need some sort of 1:N values per key for > configuration in the config file versus the 1:1 key:value setup we > currently have (so don't consider the format final just yet).
We might want to factor out that config format handling while we're trying to finalize it.
cc:ing Matt for his awareness. I'm currently not quite sure how to handle those vfio-ap "write several values to an attribute one at a time" requirements. Maybe 1:N key:value is the way to go; maybe we need/want JSON or something like that.
Maybe we should just do JSON for future flexibility. I assume there are lots of helpers that should make it easy even from a bash script. I'll look at that next.
Done. Throw away any old mdev config files, we use JSON now.
The code changes look quite straightforward, thanks.
The per mdev config now looks like this:
{ "mdev_type": "i915-GVTg_V4_8", "start": "auto" }
My expectation, and what I've already pre-enabled support in set_key and get_key functions, is that we'd use arrays for values, so we might have:
"new_key": ["value1", "value2"]
set_key will automatically convert a comma separated list of values into such an array, so I'm thinking this would be specified by the user as:
# mdevctl modify -u UUID --key=new_key --value=value1,value2
Looks sensible.
For vfio-ap, we'd probably end up with something like the following:
{ "mdev_type": "vfio_ap-passthrough", "start": "auto", "assign_adapter": ["5", "6"], "assign_domain": ["4", "0xab"] }
(following the Guest1 example in the kernel documentation)
<As an aside, what should happen if e.g "assign_adapter" is set to ["6", "7"]? Remove 5, add 7? Remove all values, then set the new ones?
IMO remove 5, add 7 would make the most sense. I'm not sure that doing an unassign of all adapters (effectively removing all APQNs) followed by an assign of the new ones would work nicely with Tony's vfio-ap dynamic configuration patches.
Are we conflating operating on the config file versus operating on the device? I was thinking that setting a new key value replaces the existing key, because anything else adds unnecessary complication to the code and command line. So in the above example, if the user specified:
mdevctl modify -u UUID --key=assign_adapter --value=6,7
The new value is simply ["6", "7"]. This would take effect the next time the device is started. We haven't yet considered how to change running devices, but I think the semantics we have since the respin of mdevctl separate saved config vs running devices in order to generalize the support of transient devices.
Yeah, my comment was aimed specifically at changes to a running device. When considering only the config file I agree: the new key value can just replace the existing key value.