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

[Wen Congyang]

At 04/05/2011 06:53 AM, Eric Blake Write:
> 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).

Yes, 'Unbind' is better than UnBind.
I will send a patch to rename it and float this function up.

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

Yes, this patch conflicts with Matthias's patches. And his patches have been
pushed. I will send 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.
>