Re: [libvirt] [PATCH v3] qemu: Honor the orginal PCI dev properties when reattaching

On 10/13/2011 02:30 AM, Osier Yang wrote: > BZ# https://bugzilla.redhat.com/show_bug.cgi?id=736214 > > The problem is caused by the original info of domain's PCI dev is > maintained by qemu_driver->activePciHostdevs list, (E.g. dev->reprobe, > which stands for whether need to reprobe driver for the dev when do > reattachment). The fields (dev->reprobe, dev->unbind_from_stub, and > dev->remove_slot) are initialized properly when preparing the PCI > device for managed attachment. However, when do reattachment, it > construct a complete new "pciDevice" without honoring the original > dev info, and thus the dev won't get the original driver or can get > other problem. > > This patch is to fix the problem by get the devs from list > driver->activePciHostdevs. > > Tested with following 3 scenarios: > * the PCI was bound to some driver not pci-stub before attaching > > result: the device will be bound to the original driver > > * the PCI was bound to pci-stub before attaching > > result: no driver reprobing, and still bound to pci-stub > > * The PCI was not bound to any driver > > result: no driver reproing, and still not bound to any driver. s/reproing/reprobing/ I like v3 better, even though it has the longer diffstat, because it divides the work into smaller self-contained functions rather than making an already long function longer. However, I got a merge conflict when applying on top of your other pci hostdev fixup patch; but I think I resolved that correctly. ACK and pushed, after I reproduced the issue and tested that the patch fixes it. It didn't help matters that virt-manager has a bug. It manually calls the equivalent of 'nodedev-detach' prior to hot-plugging any hostdev device, _even though it requests managed=1_. That means the device is already bound to pci-stub before the hotplug attempt is started; and on failure to hotplug, libvirt _faithfully_ restores the the device back to its original state (still pci-stub), but virt-manager does not recognize the error condition and does not call nodedev-reattach.