On Tue, Jan 19, 2016 at 04:36:05PM +0100, Andrea Bolognani wrote: > This ensures the behavior for reattach is consistent, no matter how it > was triggered (eg. 'virsh nodedev-reattach', 'virsh detach-device' or > shutdown of a domain that is configured to use hostdevs). > --- > src/util/virhostdev.c | 45 +++++++++++++++++++++++++++++++++++++++------ > 1 file changed, 39 insertions(+), 6 deletions(-) > > diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c > index 267aa55..74c43f2 100644 > --- a/src/util/virhostdev.c > +++ b/src/util/virhostdev.c > @@ -1625,10 +1625,24 @@ virHostdevPCINodeDeviceDetach(virHostdevManagerPtr hostdev_mgr, > return ret; > } > > +/** > + * virHostdevPCINodeDeviceReAttach: > + * @hostdev_mgr: hostdev manager > + * @pci: PCI device > + * > + * Reattach a specific PCI device to the host. > + * > + * This function makes sure the device is not in use before reattaching it > + * to the host; if the device has already been reattached to the host, the > + * operation is considered successful. > + * > + * Returns: 0 on success, <0 on failure > + */ > int > virHostdevPCINodeDeviceReAttach(virHostdevManagerPtr hostdev_mgr, > virPCIDevicePtr pci) > { > + virPCIDeviceListPtr pcidevs = NULL; > struct virHostdevIsPCINodeDeviceUsedData data = {hostdev_mgr, NULL, > false}; > int ret = -1; > @@ -1637,18 +1651,37 @@ virHostdevPCINodeDeviceReAttach(virHostdevManagerPtr hostdev_mgr, > virObjectLock(hostdev_mgr->inactivePCIHostdevs); > > if (virHostdevIsPCINodeDeviceUsed(virPCIDeviceGetAddress(pci), &data)) > - goto out; > + goto cleanup; > > - virPCIDeviceReattachInit(pci); > + /* We want this function to be idempotent, so if the device has already > + * been removed from the inactive list (and is not active either, as > + * per the check above) just return right away */ > + if (!virPCIDeviceListFind(hostdev_mgr->inactivePCIHostdevs, pci)) { > + VIR_DEBUG("PCI device %s is already attached to the host", > + virPCIDeviceGetName(pci)); > + ret = 0; > + goto cleanup; > + } > > - if (virPCIDeviceReattach(pci, hostdev_mgr->activePCIHostdevs, > - hostdev_mgr->inactivePCIHostdevs) < 0) > - goto out; > + /* Create a temporary device list */ > + if (!(pcidevs = virPCIDeviceListNew())) > + goto cleanup; > + if (virPCIDeviceListAddCopy(pcidevs, pci) < 0) > + goto cleanup; This is rather crazy - if we need to reattach single PCI devices we should have an API for doing that, and have the list API call the individual API multiple times. > + > + /* Reattach the device. We don't want to skip unmanaged devices in > + * this case, because the user explicitly asked for the device to > + * be reattached to the host driver */ > + if (reattachPCIDevices(hostdev_mgr, pcidevs, false) < 0) > + goto cleanup; Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list