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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux