[libvirt] [PATCH v4 06/14] security: Enable labeling of vfio mediated devices

[Erik Skultety]

On Sun, Mar 26, 2017 at 02:25:02PM -0400, Laine Stump wrote:
> On 03/22/2017 11:27 AM, Erik Skultety wrote:
> > Label the VFIO IOMMU devices under /dev/vfio/ referenced by the symlinks
> > in the sysfs (e.g. /sys/class/mdev_bus/<uuid>/iommu_group) which what
> > qemu actually gets formatted on the command line. 
> 
> The sentence above is confused (i.e. I don't understand it), but I won't
> know how to unconfuse it until I've gone through the patch.

Now that I'm reading it again, of course I know what I meant, but that's only
because I wrote it, but from the native speaker's perspective, I guess the
reaction must have been something like "wut?!". So I simplified it to the bare
minimum in terms of the patch just adding labeling for mdevs as well.

[...]

> >
> > -    case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV:
> > +    case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV: {
> > +        char *vfiodev = NULL;
> > +        virMediatedDevicePtr mdev = virMediatedDeviceNew(mdevsrc->uuidstr,
> > +                                                         mdevsrc->model);
> > +
> > +        if (!mdev)
> > +            goto done;
> > +
> > +        if (!(vfiodev = virMediatedDeviceGetIOMMUGroupDev(mdev))) {
> > +            virMediatedDeviceFree(mdev);
> > +            goto done;
> > +        }
> 
> Going through the various patches and seeing this (or similar) sequences
> so often makes me think it might be cleaner to have APIs that take a
> uuidstr and model instead (or maybe define
> virDomainHostdevSubsysMediatedDevPtr in util instead of conf, then pass
> the mdevsrc directly - that would make it continue to work if/once we
> add different ways to specify the device.
> 
> But things currently work exactly the same way for PCI devices, so no
> sense rewriting just for that. These are all internal APIs, so we can
> tweak them to our hearts' content in the future.
> 

Yeah, there's definitely some room for 'refurbishment' of the hostdev code.
I'll put that on my TODO list.

> > -    case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV:
> > +    case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV: {
> > +        char *vfiodev = NULL;
> > +        virMediatedDevicePtr mdev = virMediatedDeviceNew(mdevsrc->uuidstr,
> > +                                                         mdevsrc->model);
> > +
> > +        if (!mdev)
> > +            goto done;
> > +
> > +        if (!(vfiodev = virMediatedDeviceGetIOMMUGroupDev(mdev))) {
> > +            virMediatedDeviceFree(mdev);
> > +            goto done;
> > +        }
> 
> (see what I mean?)

Yep.