On Fri, Oct 30, 2015 at 9:13 PM, Andrea Bolognani <abologna@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@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