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.