On Fri, Oct 30, 2015 at 9:13 PM, Andrea Bolognani <abologna@xxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxx>
>> ---
>> 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list