[libvirt] [PATCH 6/8] Pass activeDevs and inactiveDevs to virPCIDeviceUnbindFromStub and virPCIDeviceBindToStub

[Shivaprasad bhat]

On Fri, Oct 30, 2015 at 9:13 PM, Andrea Bolognani <abologna at redhat.com> wrote:
> On Fri, 2015-10-30 at 05:00 +0530, Shivaprasad G Bhat wrote:
>> The inactiveDevs need to be selectively altered for more than one
>> device in case of vfio devices. So, pass the whole list.
>>
>> Signed-off-by: Shivaprasad G Bhat <sbhat at linux.vnet.ibm.com>
>> ---
>>  src/util/virpci.c |   38 +++++++++++++++++++++-----------------
>>  1 file changed, 21 insertions(+), 17 deletions(-)
>>
>> diff --git a/src/util/virpci.c b/src/util/virpci.c
>> index 2709ddd..6c24a81 100644
>> --- a/src/util/virpci.c
>> +++ b/src/util/virpci.c
>> @@ -1129,7 +1129,9 @@ static int virPCIDeviceReprobeHostDriver(virPCIDevicePtr dev, char *driver, char
>>  }
>>
>>  static int
>> -virPCIDeviceUnbindFromStub(virPCIDevicePtr dev)
>> +virPCIDeviceUnbindFromStub(virPCIDevicePtr dev,
>> +                           virPCIDeviceListPtr activeDevs ATTRIBUTE_UNUSED,
>> +                           virPCIDeviceListPtr inactiveDevs)
>>  {
>>      int result = -1;
>>      char *drvdir = NULL;
>> @@ -1177,6 +1179,9 @@ virPCIDeviceUnbindFromStub(virPCIDevicePtr dev)
>>   reprobe:
>>      if (virPCIDeviceReprobeHostDriver(dev, driver, drvdir) < 0)
>>          goto cleanup;
>> +    /* Steal the dev from list inactiveDevs */
>> +    if (inactiveDevs)
>> +        virPCIDeviceListDel(inactiveDevs, dev);
>>
>>      result = 0;
>>
>> @@ -1195,7 +1200,9 @@ virPCIDeviceUnbindFromStub(virPCIDevicePtr dev)
>>
>>  static int
>>  virPCIDeviceBindToStub(virPCIDevicePtr dev,
>> -                       const char *stubDriverName)
>> +                       const char *stubDriverName,
>> +                       virPCIDeviceListPtr activeDevs,
>> +                       virPCIDeviceListPtr inactiveDevs)
>>  {
>>      int result = -1;
>>      bool reprobe = false;
>> @@ -1317,6 +1324,14 @@ virPCIDeviceBindToStub(virPCIDevicePtr dev,
>>          goto cleanup;
>>      }
>>
>> +    /* Add *a copy of* the dev into list inactiveDevs, if
>> +     * it's not already there.
>> +                     */
>
> Just close the comment in the previous line.
>
>> +    if (inactiveDevs && !virPCIDeviceListFind(inactiveDevs, dev) &&
>> +            virPCIDeviceListAddCopy(inactiveDevs, dev) < 0) {
>
> Really weird alignment here...
>
>> +        result = -1;
>> +    }
>> +
>>   cleanup:
>>      VIR_FREE(stubDriverPath);
>>      VIR_FREE(driverLink);
>> @@ -1324,7 +1339,7 @@ virPCIDeviceBindToStub(virPCIDevicePtr dev,
>>
>>      if (result < 0) {
>>          VIR_FREE(newDriverName);
>> -        virPCIDeviceUnbindFromStub(dev);
>> +        virPCIDeviceUnbindFromStub(dev, activeDevs, NULL);
>
> Why pass NULL instead of inactiveDevs here?
>

Correct. Will Pass the inactiveDevs. The devices are removed from
inactiveDevs before coming here. We need to restore if going on
error path.

>
>>      } else {
>>          VIR_FREE(dev->stubDriver);
>>          dev->stubDriver = newDriverName;
>> @@ -1371,16 +1386,9 @@ virPCIDeviceDetach(virPCIDevicePtr dev,
>>          return -1;
>>      }
>>
>> -    if (virPCIDeviceBindToStub(dev, dev->stubDriver) < 0)
>> -        return -1;
>> -
>> -    /* Add *a copy of* the dev into list inactiveDevs, if
>> -     * it's not already there.
>> -     */
>> -    if (inactiveDevs && !virPCIDeviceListFind(inactiveDevs, dev) &&
>> -        virPCIDeviceListAddCopy(inactiveDevs, dev) < 0) {
>> +    if (virPCIDeviceBindToStub(dev, dev->stubDriver,
>> +                               activeDevs, inactiveDevs) < 0)
>>          return -1;
>> -    }
>>
>>      return 0;
>>  }
>> @@ -1396,13 +1404,9 @@ virPCIDeviceReattach(virPCIDevicePtr dev,
>>          return -1;
>>      }
>>
>> -    if (virPCIDeviceUnbindFromStub(dev) < 0)
>> +    if (virPCIDeviceUnbindFromStub(dev, activeDevs, inactiveDevs) < 0)
>>          return -1;
>>
>> -    /* Steal the dev from list inactiveDevs */
>> -    if (inactiveDevs)
>> -        virPCIDeviceListDel(inactiveDevs, dev);
>> -
>>      return 0;
>>  }
>
> Cheers.
>
> --
> Andrea Bolognani
> Software Engineer - Virtualization Team