Re: [libvirt] [PATCH RFC 1/1] allow to specify additional config data

On Thu, 6 Jun 2019 09:32:24 -0600 Alex Williamson <alex.williamson(a)redhat.com&gt; wrote: > On Thu, 6 Jun 2019 16:44:17 +0200 > Cornelia Huck <cohuck(a)redhat.com&gt; wrote: > > > Add a rough implementation for vfio-ap. > > > > Signed-off-by: Cornelia Huck <cohuck(a)redhat.com&gt; > > --- > > mdevctl.libexec | 25 ++++++++++++++++++++++ > > mdevctl.sbin | 56 ++++++++++++++++++++++++++++++++++++++++++++++++- > > 2 files changed, 80 insertions(+), 1 deletion(-) > > > > diff --git a/mdevctl.libexec b/mdevctl.libexec > > index 804166b5086d..cc0546142924 100755 > > --- a/mdevctl.libexec > > +++ b/mdevctl.libexec > > @@ -54,6 +54,19 @@ wait_for_supported_types () { > > fi > > } > > > > +# configure vfio-ap devices <config entry> <matrix attribute> > > +configure_ap_devices() { > > + list="`echo "${config[$1]}" | sed 's/,/ /'`" > > + [ -z "$list" ] && return > > + for a in $list; do > > + echo "$a" > "$supported_types/${config[mdev_type]}/devices/$uuid/$2" > > + if [ $? -ne 0 ]; then > > + echo "Error writing '$a' to '$uuid/$2'" >&2 > > + exit 1 > > + fi > > + done > > +} > > + > > case ${1} in > > start-mdev|stop-mdev) > > if [ $# -ne 2 ]; then > > @@ -148,6 +161,18 @@ case ${cmd} in > > echo "Error creating mdev type ${config[mdev_type]} on $parent" >&2 > > exit 1 > > fi > > + > > + # some types may specify additional config data > > + case ${config[mdev_type]} in > > + vfio_ap-passthrough) > > I think this could have some application beyond ap too, I know NVIDIA > GRID vGPUs do have some controls under the vendor hierarchy of the > device, ex. setting the frame rate limiter. The implementation here is > a bit rigid, we know a specific protocol for a specific mdev type, but > for supporting arbitrary vendor options we'd really just want to try to > apply whatever options are provided. If we didn't care about ordering, > we could just look for keys for every file in the device's immediate > sysfs hierarchy and apply any value we find, independent of the > mdev_type, ex. intel_vgpu/foo=bar Thanks, For example: for key in find -P $mdev_base/$uuid/ \( -path "$mdev_base/$uuid/power/*" -o -path $mdev_base/$uuid/uevent -o -path $mdev_base/$uuid/remove \) -prune -o -type f -print | sed -e "s|$mdev_base/$uuid/||g"); do [ -z ${config[$key]} ] && continue ... parse value(s) and iteratively apply to key done The find is a little ugly to exclude stuff, maybe we just let people do screwy stuff like specify remove=1 in their config. Also need to think about whether we're imposing a delimiter to apply multiple values to a key that conflicts with the attribute usage. Thanks, Alex

> > + configure_ap_devices ap_adapters assign_adapter > > + configure_ap_devices ap_domains assign_domain > > + configure_ap_devices ap_control_domains assign_control_domain > > + # TODO: is assigning idempotent? Should we unwind on error? > > + ;; > > + *) > > + ;; > > + esac > > ;; > > > > add-mdev)