[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]