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

[Guannan Ren]

On 07/03/2011 05:43 PM, Guannan Ren wrote:
> 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).
          sorry I missed above words, I will try to add a function to 
set these three values to 1 by default.
> -- 
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list