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

On 07/02/2011 06:10 PM, Wen Congyang wrote: > 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(). > Yep, the state of the pci device is different possibly or probably, I made the determination in pciUnbindDeviceFromStub() by examining the name and existence of pci device driver. > >> 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. The "managed" is from the xml file to "virsh attach-device" executed each time, but the values of these three variables are generated in running time , it is hard to keep them to the next command. > > And we call these new APIs in qemudNodeDeviceDettach() to set these > three > variables to correct value(I think these three variables should be 1).

-- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list