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