Re: [libvirt] [PATCHv2 08/12] qemu: don't reset PCI devices being assigned with VFIO
On 06/25/2013 06:44 AM, Daniel P. Berrange wrote:
On Mon, Jun 24, 2013 at 11:05:34PM -0400, Laine Stump wrote:
> I just learned that VFIO resets PCI devices when they are assigned to
> guests / returned to the host, so it is redundant for libvirt to reset
> the devices. This patch inhibits calling virPCIDeviceReset to devices
> that will be/were assigned using VFIO.
> ---
> src/qemu/qemu_hostdev.c | 4 ++++
> src/qemu/qemu_hotplug.c | 5 +++--
> 2 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
> index dfe39c6..d7d54d7 100644
> --- a/src/qemu/qemu_hostdev.c
> +++ b/src/qemu/qemu_hostdev.c
> @@ -548,6 +548,8 @@ int qemuPrepareHostdevPCIDevices(virQEMUDriverPtr driver,
> * can safely reset them */
> for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) {
> virPCIDevicePtr dev = virPCIDeviceListGet(pcidevs, i);
> + if (STREQ_NULLABLE(virPCIDeviceGetStubDriver(dev), "vfio-pci"))
> + continue;
> if (virPCIDeviceReset(dev, driver->activePciHostdevs,
> driver->inactivePciHostdevs) < 0)
> goto reattachdevs;
> @@ -1114,6 +1116,8 @@ void qemuDomainReAttachHostdevDevices(virQEMUDriverPtr driver,
>
> for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) {
> virPCIDevicePtr dev = virPCIDeviceListGet(pcidevs, i);
> + if (STREQ_NULLABLE(virPCIDeviceGetStubDriver(dev), "vfio-pci"))
> + continue;
> if (virPCIDeviceReset(dev, driver->activePciHostdevs,
> driver->inactivePciHostdevs) < 0) {
> virErrorPtr err = virGetLastError();
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 18f5fa5..46875ad 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -2528,8 +2528,9 @@ qemuDomainDetachHostPciDevice(virQEMUDriverPtr driver,
> if (pci) {
> activePci = virPCIDeviceListSteal(driver->activePciHostdevs, pci);
> if (activePci &&
> - virPCIDeviceReset(activePci, driver->activePciHostdevs,
> - driver->inactivePciHostdevs) == 0) {
> + (subsys->u.pci.backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO ||
> + virPCIDeviceReset(activePci, driver->activePciHostdevs,
> + driver->inactivePciHostdevs) == 0)) {
I'm guessing that virPCIDeviceGetStubDriver() isn't returning 'pci-stub'
here, right ? Otherwise you'd would have used the same pattern as earlier
No, the stub driver should be set correctly at this time[*]. It was just
a case of having multiple options for learning the answer in this case,
and choosing the one that's more efficient.
In the other cases where I need to decide whether or not to call
virPCIDeviceReset, I don't have access to the virDomainHostdevDef, only
to a virPCIDevice that was created from the virDomainHostdevDef, so I
use the less efficient method of a strcmp of the stubDriverName.
[*] When the device is initially attached to the guest, It's set from
the hostdev right at the time the virPCIDevice is created based on
config data in virDomainHostdevDef. That virPCIDevice is placed in the
activePciHostdevs list and is what is found in "activePci" in the above
code chunk. In the case that libvirtd is restarted, again the
virPCIDevice that is in the activePciHostdevs list is created based on
the virDomainHostdevDef in the domain's config, so it will have the
correct stubDriverName.