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?
> } 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