Re: mdevctl: A shoestring mediated device management and persistence utility

[Matthew Rosato <mjrosato@linux.ibm.com>]

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.

> Similar for deleting the "assign_adapter" key. We have an
> "unassign_adapter" attribute, but this is not something we can infer
> automatically; we need to know that we're dealing with an vfio-ap
> matrix device...>
> 
>>
>> We should think about whether ordering is important and maybe
>> incorporate that into key naming conventions or come up with some
>> syntax for specifying startup blocks.  Thanks,
>>
>> Alex
> 
> Hm...
> 
> {
>   "foo": "1",
>   "bar": "42",
>   "baz": {
>     "depends": ["foo", "bar"],
>     "value": "plahh"
>   }
> }
> 
> Something like that?
>