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

Guannan Ren gren at redhat.com
Sun Jul 3 09:43:26 UTC 2011

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).

More information about the libvir-list mailing list