Re: [libvirt] [PATCH] fix the failure of nodedev-reattach caused by the variables from nodedev-attach

At 2011-7-2 13:27, Guannan Ren write: > On 07/01/2011 11:30 PM, ghostwcy wrote: >> At 2011-7-1 18:24, Guannan Ren write: >>> the "virsh nodedev-reattch" command could return successfully, but >>> the pci device is still >>> bound to pci-stub driver. The reason is noddev-reattach trys to use >>> the variables set by >>> nodedev-dettach commands. Becuase these variables is not persistent, >>> this is not we expected. >>> the patch try to fix it. >> >> I do not agree with this patch. You should read this mail: >> https://www.redhat.com/archives/libvir-list/2011-April/msg00315.html >> >> This patch will cause regression about hotpluging host pci device. >> >> I think the problem is that: the init value of these variables is >> wrong. >> We can fix this bug by modifing pciGetDevice() or its caller. >> >>> > > I read the patch, it introduced the mechanism to check whether the > device is bound to > pci-stub when we write dev-id to new_id if a new device with this ID is > hotpluggged, or if > a probe is triggered for such a device, I think my patch kept it > working. I think my explanation is not detailed. For example: The user runs 'virsh attach-device' to pass through host pci device to guest os. We will call the function qemuPrepareHostdevPCIDevices() to bind the pci device to pci-stub, and reset it. If the pci device does not support reset function, we will call pciReAttachDevice() to rollback the things we have done. If the device is alread bound to pci-stub before the user runs 'virsh attach-device', we should do nothing. If the device is not bound to any driver, we should unbound it from pci-stub. If the device is bound to other driver, we should unbound it from pci-stub, and reprobe it. When we call pciReAttachDevice(), the device is bound to pci-stub, but we do the different thing, because the origin state of the pci device is different. So I add these three variables to remember what we do in pciDettachDevice().

> The overall idea is as follows: > If the device is already bound to pci-stub, then do nothing. > If not, try to write dev-id to "new_id", then check the state of its > driver(hotplug issue) > Unbind from its original driver , then bind to pci-stub driver > Anyway, we should write dev-id to "remove_id" to protect the device from > causing > problems when reattaching it. > About fixing the problem from pciGetDevice(), unless we feedback the > state of pci device to > libvirtd but I don't think it is easier. We already have pciDeviceSetManaged() and pciDeviceGetManaged(). I think we should add some similar APIs in util/pci.c to modify and read these three variables.

And we call these new APIs in qemudNodeDeviceDettach() to set these three variables to correct value(I think these three variables should be 1).