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
