Re: [libvirt] [PATCH 2/3] reattach pci device when pciBindDeviceToStub() failed

On 03/28/2011 01:01 AM, Wen Congyang wrote: > We should bind pci device to original driver when pciBindDeviceToStub() failed. > If the pci device is not bound to any driver before calling pciBindDeviceToStub(), > we should only unbind it from pci-stub. If it is bound to pci-stub, we should not > unbid it from pci-stub. > > --- > src/util/pci.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++++------ > 1 files changed, 80 insertions(+), 10 deletions(-) > > diff --git a/src/util/pci.c b/src/util/pci.c > index 8d2dbb0..e30f5cf 100644 > --- a/src/util/pci.c > +++ b/src/util/pci.c > @@ -65,6 +65,11 @@ struct _pciDevice { > unsigned has_flr : 1; > unsigned has_pm_reset : 1; > unsigned managed : 1; > + > + /* used by reattach function */ > + unsigned unbind_from_stub : 1; > + unsigned remove_slot : 1; > + unsigned reprobe : 1; > }; > > struct _pciDeviceList { > @@ -834,11 +839,25 @@ recheck: > } > > > +static int pciUnBindDeviceFromStub(pciDevice *dev, const char *driver); I would have used Unbind rather than UnBind, but that's cosmetic. Is it possible to float that function up, instead of having to have a forward declaration? I tend to topologically sort my static functions when possible (again, cosmetic).

> static int > pciBindDeviceToStub(pciDevice *dev, const char *driver) > { > char drvdir[PATH_MAX]; > char path[PATH_MAX]; > + int reprobe = 0; > + int ret = 0; > + > + /* check whether the device is already bound to a driver */ > + pciDriverDir(drvdir, sizeof(drvdir), driver); > + pciDeviceFile(path, sizeof(path), dev->name, "driver"); Ouch - conflict with Matthias's patches to avoid PATH_MAX. If that goes in first, your rebase might not be trivial, so it would be worth a v2.

But overall, I think the idea of this patch is sane, and nothing obvious jumped out at me as needing fixing other than rebase issues.