Re: [libvirt PATCH v2 09/15] tests: remove explicit <driver name='vfio'/> from hostdev test cases
On 11/27/23 10:03 AM, Peter Krempa wrote:
On Mon, Nov 06, 2023 at 02:38:54 -0500, Laine Stump wrote:
> The long-deprecated use of <driver name='vfio|xen|kvm'/> in domain xml
> for <hostdev> devices was only ever necessary during the period when
> libvirt (and the Linux kernel) supported both VFIO and "legacy KVM"
> styles of hostdev device assignment for QEMU. This became pointless
> many years ago when legacy KVM device assignment was removed from the
> kernel, and support for that style of device assignment was completely
> disabled in the libvirt source in 2019 (commit
> v5.6.0-316-g2e7225ea8c).
>
> Nevertheless, there were instances of <driver name='vfio'/> in the
> unit test data that were then (unnecessarily) propagated to several
> more tests over the years. This patch cleans out those unnecessary
> explicit settings of driver name='vfio' in all QEMU unit test data,
> proving that the attribute is no longer (externally) needed while also
> making it simpler to properly test when a later patch "re-animates"
> the driver name attribute for a slightly different (but related) use.
>
> Signed-off-by: Laine Stump <laine(a)redhat.com>
> ---
Few issues:
1) There are few bits with domain XML bits that seem to mention it:
tests/qemuhotplugtestdevices/qemuhotplug-hostdev-pci.xml: <driver
name='vfio'/>
tests/qemuhotplugtestdomains/qemuhotplug-base-live+hostdev-pci.xml: <driver
name='vfio'/>
tests/qemuhotplugtestdomains/qemuhotplug-pseries-base-live+hostdev-pci.xml:
<driver name='vfio'/>
I *thought* that I needed to keep those for now (until patch 12/15), but
it turns out I didn't, so I removed them too.
tests/qemumemlockdata/qemumemlock-pc-hardlimit+hostdev.xml:
<driver name='vfio'/>
tests/qemumemlockdata/qemumemlock-pc-hardlimit+locked+hostdev.xml: <driver
name='vfio'/>
tests/qemumemlockdata/qemumemlock-pc-hostdev-nvme.xml: <driver
name='vfio'/>
tests/qemumemlockdata/qemumemlock-pc-hostdev.xml: <driver
name='vfio'/>
tests/qemumemlockdata/qemumemlock-pc-locked+hostdev.xml: <driver
name='vfio'/>
tests/qemumemlockdata/qemumemlock-pseries-hardlimit+hostdev.xml: <driver
name='vfio'/>
tests/qemumemlockdata/qemumemlock-pseries-hardlimit+locked+hostdev.xml: <driver
name='vfio'/>
tests/qemumemlockdata/qemumemlock-pseries-hostdev.xml: <driver
name='vfio'/>
tests/qemumemlockdata/qemumemlock-pseries-locked+hostdev.xml: <driver
name='vfio'/>
I had previously also left these in because the test was failing when I
removed them, then I forgot to go back and investiate why. Now that
you've reminded me to look back at it, I see this failure was due to the
patch 11/15 - "conf: replace virHostdevIsVFIODevice with
virHostdevIsPCIDevice" - not yet being applied. I've switched the order
of the patches and now am able to remove these as well.
tests/qemustatusxml2xmldata/modern-in.xml: <driver
name='vfio'/>
This is status XML generated by
2) There's documentation that seems to mention it:
docs/formatdomain.rst: <driver name='vfio'/>
docs/formatnetwork.rst: <driver name='vfio'/>
docs/formatnetworkport.rst: <driver name='vfio'/>
docs/pci-addresses.rst: <driver name='vfio'/>
removed (I forgot to search in docs)
3) You are not leaving any example with the existing syntax. To prevent
regression, either keep some in or create a specific new test case to
cover this now-corner-case.
One of the test cases added in patch 12/15 covers the "legacy" case of
"driver name='vfio'" along with the new "driver
type='vfio'", and the
new usage of "driver name='vfio-pci-igbvf'".
With at least 2 & 3 addressed:
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
_______________________________________________
Devel mailing list -- devel(a)lists.libvirt.org
To unsubscribe send an email to devel-leave(a)lists.libvirt.org