[libvirt] [PATCH v2 0/4] hostdev: Cleanups and fixes

Respin of patches 20, 22, 23 and 24 from https://www.redhat.com/archives/libvir-list/2016-March/msg00209.html Patches 01-19 have already been pushed; patch 21 has been converted into the tiny series https://www.redhat.com/archives/libvir-list/2016-March/msg00792.html which this one depends on. Not many changes, basically just shuffling patches around to avoid a problem found during review and other small nits. Cheers. Andrea Bolognani (4): hostdev: Streamline device ownership tracking hostdev: Save netdev configuration of actual device hostdev: Use actual device when reattaching tests: hostdev: Add more tests src/util/virhostdev.c | 181 +++++++++++++++++++++++++++---------------------- tests/virhostdevtest.c | 84 +++++++++++++++++++++-- 2 files changed, 178 insertions(+), 87 deletions(-) -- 2.5.0

After this patch, ownership of virPCIDevice instances is very easy to keep track of: for each host PCI device, the only instance that actually matters is the one inside one of the bookkeeping list. Whenever some operation needs to be performed on a PCI device, the actual device is looked up first; when this is not the case, a comment explains the reason. --- src/util/virhostdev.c | 130 ++++++++++++++++++++++++++++---------------------- 1 file changed, 72 insertions(+), 58 deletions(-) diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index 572aec0..e8efc53 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -527,10 +527,7 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr mgr, * impacts all devices on it. Also, all devices must be reset * before being marked as active */ - /* Step 1: validate that non-managed device isn't in use, eg - * by checking that device is either un-bound, or bound - * to pci-stub.ko - */ + /* Step 1: Perform some initial checks on the devices */ for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) { virPCIDevicePtr pci = virPCIDeviceListGet(pcidevs, i); bool strict_acs_check = !!(flags & VIR_HOSTDEV_STRICT_ACS_CHECK); @@ -659,28 +656,22 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr mgr, last_processed_hostdev_vf = i; } - /* Step 5: Now mark all the devices as active */ + /* Step 5: Move devices from the inactive list to the active list */ for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) { virPCIDevicePtr pci = virPCIDeviceListGet(pcidevs, i); + virPCIDevicePtr actual; - VIR_DEBUG("Adding PCI device %s to active list", + VIR_DEBUG("Removing PCI device %s from inactive list", virPCIDeviceGetName(pci)); - if (virPCIDeviceListAdd(mgr->activePCIHostdevs, pci) < 0) - goto inactivedevs; - } - - /* Step 6: Now remove the devices from inactive list. */ - for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) { - virPCIDevicePtr pci = virPCIDeviceListGet(pcidevs, i); + actual = virPCIDeviceListSteal(mgr->inactivePCIHostdevs, pci); - VIR_DEBUG("Removing PCI device %s from inactive list", + VIR_DEBUG("Adding PCI device %s to active list", virPCIDeviceGetName(pci)); - virPCIDeviceListDel(mgr->inactivePCIHostdevs, pci); + if (!actual || virPCIDeviceListAdd(mgr->activePCIHostdevs, actual) < 0) + goto inactivedevs; } - /* Step 7: Now set the used_by_domain of the device in - * activePCIHostdevs as domain name. - */ + /* Step 6: Set driver and domain information */ for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) { virPCIDevicePtr pci, actual; @@ -696,7 +687,7 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr mgr, virPCIDeviceSetUsedBy(actual, drv_name, dom_name); } - /* Step 8: Now set the original states for hostdev def */ + /* Step 7: Now set the original states for hostdev def */ for (i = 0; i < nhostdevs; i++) { virPCIDevicePtr pci; virDomainHostdevDefPtr hostdev = hostdevs[i]; @@ -728,23 +719,26 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr mgr, } } - /* Step 9: Now steal all the devices from pcidevs */ - while (virPCIDeviceListCount(pcidevs) > 0) - virPCIDeviceListStealIndex(pcidevs, 0); - ret = 0; goto cleanup; inactivedevs: - /* Only steal all the devices from activePCIHostdevs. We will - * free them in virObjectUnref(). - */ + /* Move devices back to the inactive list so that they can be + * processed properly below (reattachdevs label) */ for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) { virPCIDevicePtr pci = virPCIDeviceListGet(pcidevs, i); + virPCIDevicePtr actual; VIR_DEBUG("Removing PCI device %s from active list", virPCIDeviceGetName(pci)); - virPCIDeviceListSteal(mgr->activePCIHostdevs, pci); + if (!(actual = virPCIDeviceListSteal(mgr->activePCIHostdevs, pci))) + continue; + + VIR_DEBUG("Adding PCI device %s to inactive list", + virPCIDeviceGetName(pci)); + if (virPCIDeviceListAdd(mgr->inactivePCIHostdevs, actual) < 0) + VIR_WARN("Failed to add PCI device %s to the inactive list", + virPCIDeviceGetName(pci)); } resetvfnetconfig: @@ -756,11 +750,17 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr mgr, reattachdevs: for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) { virPCIDevicePtr pci = virPCIDeviceListGet(pcidevs, i); + virPCIDevicePtr actual; - if (virPCIDeviceGetManaged(pci)) { + /* We need to look up the actual device because that's what + * virPCIDeviceReattach() exepects as its argument */ + if (!(actual = virPCIDeviceListFind(mgr->inactivePCIHostdevs, pci))) + continue; + + if (virPCIDeviceGetManaged(actual)) { VIR_DEBUG("Reattaching managed PCI device %s", virPCIDeviceGetName(pci)); - ignore_value(virPCIDeviceReattach(pci, + ignore_value(virPCIDeviceReattach(actual, mgr->activePCIHostdevs, mgr->inactivePCIHostdevs)); } else { @@ -785,17 +785,6 @@ static void virHostdevReattachPCIDevice(virHostdevManagerPtr mgr, virPCIDevicePtr actual) { - /* If the device is not managed and was attached to guest - * successfully, it must have been inactive. - */ - if (!virPCIDeviceGetManaged(actual)) { - VIR_DEBUG("Adding unmanaged PCI device %s to inactive list", - virPCIDeviceGetName(actual)); - if (virPCIDeviceListAdd(mgr->inactivePCIHostdevs, actual) < 0) - virPCIDeviceFree(actual); - return; - } - /* Wait for device cleanup if it is qemu/kvm */ if (virPCIDeviceGetStubDriver(actual) == VIR_PCI_STUB_DRIVER_KVM) { int retries = 100; @@ -814,7 +803,6 @@ virHostdevReattachPCIDevice(virHostdevManagerPtr mgr, err ? err->message : _("unknown error")); virResetError(err); } - virPCIDeviceFree(actual); } /* @oldStateDir: @@ -848,9 +836,8 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr mgr, /* Reattaching devices to the host involves several steps; each * of them is described at length below */ - /* Step 1: verify that each device in the hostdevs list really was in use - * by this domain, and remove them all from the activePCIHostdevs list. - */ + /* Step 1: Filter out all devices that are either not active or not + * used by the current domain and driver */ i = 0; while (i < virPCIDeviceListCount(pcidevs)) { virPCIDevicePtr pci = virPCIDeviceListGet(pcidevs, i); @@ -876,17 +863,34 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr mgr, continue; } + i++; + } + + /* Step 2: Move devices from the active list to the inactive list */ + for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) { + virPCIDevicePtr pci = virPCIDeviceListGet(pcidevs, i); + virPCIDevicePtr actual; + VIR_DEBUG("Removing PCI device %s from active list", virPCIDeviceGetName(pci)); - virPCIDeviceListDel(mgr->activePCIHostdevs, pci); - i++; + actual = virPCIDeviceListSteal(mgr->activePCIHostdevs, pci); + + VIR_DEBUG("Adding PCI device %s to inactive list", + virPCIDeviceGetName(pci)); + if (!actual || + virPCIDeviceListAdd(mgr->inactivePCIHostdevs, actual) < 0) { + + virErrorPtr err = virGetLastError(); + VIR_ERROR(_("Failed to add PCI device %s to inactive list"), + err ? err->message : _("unknown error")); + virResetError(err); + } } - /* At this point, any device that had been used by the guest is in - * pcidevs, but has been removed from activePCIHostdevs. - */ + /* At this point, any device that had been used by the guest has been + * moved to the inactive list */ - /* Step 2: restore original network config of hostdevs that used + /* Step 3: restore original network config of hostdevs that used * <interface type='hostdev'> */ for (i = 0; i < nhostdevs; i++) { @@ -911,7 +915,7 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr mgr, } } - /* Step 3: perform a PCI Reset on all devices */ + /* Step 4: perform a PCI Reset on all devices */ for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) { virPCIDevicePtr pci = virPCIDeviceListGet(pcidevs, i); @@ -928,16 +932,26 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr mgr, } } - /* Step 4: reattach devices to their host drivers (if managed) or place - * them on the inactive list (if not managed) - */ - while (virPCIDeviceListCount(pcidevs) > 0) { - virPCIDevicePtr pci = virPCIDeviceListStealIndex(pcidevs, 0); - virHostdevReattachPCIDevice(mgr, pci); + /* Step 5: Reattach managed devices to their host drivers; unmanaged + * devices don't need to be processed further */ + for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) { + virPCIDevicePtr pci = virPCIDeviceListGet(pcidevs, i); + virPCIDevicePtr actual; + + /* We need to look up the actual device because that's what + * virHostdevReattachPCIDevice() expects as its argument */ + if (!(actual = virPCIDeviceListFind(mgr->inactivePCIHostdevs, pci))) + continue; + + if (virPCIDeviceGetManaged(actual)) + virHostdevReattachPCIDevice(mgr, actual); + else + VIR_DEBUG("Not reattaching unmanaged PCI device %s", + virPCIDeviceGetName(actual)); } - virObjectUnref(pcidevs); cleanup: + virObjectUnref(pcidevs); virObjectUnlock(mgr->activePCIHostdevs); virObjectUnlock(mgr->inactivePCIHostdevs); } -- 2.5.0

On 03/18/2016 01:03 PM, Andrea Bolognani wrote:
After this patch, ownership of virPCIDevice instances is very easy to keep track of: for each host PCI device, the only instance that actually matters is the one inside one of the bookkeeping list.
Whenever some operation needs to be performed on a PCI device, the actual device is looked up first; when this is not the case, a comment explains the reason. --- src/util/virhostdev.c | 130 ++++++++++++++++++++++++++++---------------------- 1 file changed, 72 insertions(+), 58 deletions(-)
ACK - couple of minor textual edits if you're interested below John
diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index 572aec0..e8efc53 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c
[...]
inactivedevs: - /* Only steal all the devices from activePCIHostdevs. We will - * free them in virObjectUnref(). - */ + /* Move devices back to the inactive list so that they can be + * processed properly below (reattachdevs label) */ for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) { virPCIDevicePtr pci = virPCIDeviceListGet(pcidevs, i); + virPCIDevicePtr actual;
VIR_DEBUG("Removing PCI device %s from active list", virPCIDeviceGetName(pci)); - virPCIDeviceListSteal(mgr->activePCIHostdevs, pci); + if (!(actual = virPCIDeviceListSteal(mgr->activePCIHostdevs, pci))) + continue; + + VIR_DEBUG("Adding PCI device %s to inactive list", + virPCIDeviceGetName(pci)); + if (virPCIDeviceListAdd(mgr->inactivePCIHostdevs, actual) < 0) + VIR_WARN("Failed to add PCI device %s to the inactive list",
or "Failed to return PCI device ..."
+ virPCIDeviceGetName(pci)); }
[...]
+ /* Step 2: Move devices from the active list to the inactive list */ + for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) { + virPCIDevicePtr pci = virPCIDeviceListGet(pcidevs, i); + virPCIDevicePtr actual; + VIR_DEBUG("Removing PCI device %s from active list", virPCIDeviceGetName(pci)); - virPCIDeviceListDel(mgr->activePCIHostdevs, pci); - i++; + actual = virPCIDeviceListSteal(mgr->activePCIHostdevs, pci); + + VIR_DEBUG("Adding PCI device %s to inactive list",
...to the inactive list
+ virPCIDeviceGetName(pci)); + if (!actual || + virPCIDeviceListAdd(mgr->inactivePCIHostdevs, actual) < 0) { + + virErrorPtr err = virGetLastError(); + VIR_ERROR(_("Failed to add PCI device %s to inactive list"), + err ? err->message : _("unknown error")); + virResetError(err); + } }

On Tue, 2016-03-22 at 15:03 -0400, John Ferlan wrote:
- virPCIDeviceListDel(mgr->activePCIHostdevs, pci); - i++; + actual = virPCIDeviceListSteal(mgr->activePCIHostdevs, pci); + + VIR_DEBUG("Adding PCI device %s to inactive list", ...to the inactive list
There are a bunch of other instances of the same message, so if we wanted to update them it would be better done in a separate patch. I've pushed the series, thanks for sticking with it! :) -- Andrea Bolognani Software Engineer - Virtualization Team

We would be just fine looking up the information in pcidevs most of the time; however, some corner cases would not be handled properly, so look up the actual device instead. --- src/util/virhostdev.c | 41 ++++++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index e8efc53..b21aa22 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -689,7 +689,7 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr mgr, /* Step 7: Now set the original states for hostdev def */ for (i = 0; i < nhostdevs; i++) { - virPCIDevicePtr pci; + virPCIDevicePtr actual; virDomainHostdevDefPtr hostdev = hostdevs[i]; virDomainHostdevSubsysPCIPtr pcisrc = &hostdev->source.subsys.u.pci; @@ -698,24 +698,27 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr mgr, if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) continue; - pci = virPCIDeviceListFindByIDs(pcidevs, - pcisrc->addr.domain, - pcisrc->addr.bus, - pcisrc->addr.slot, - pcisrc->addr.function); + /* We need to look up the actual device because it's the one + * that contains the information we care about (unbind_from_stub, + * remove_slot, reprobe) */ + actual = virPCIDeviceListFindByIDs(mgr->activePCIHostdevs, + pcisrc->addr.domain, + pcisrc->addr.bus, + pcisrc->addr.slot, + pcisrc->addr.function); /* Appropriate values for the unbind_from_stub, remove_slot * and reprobe properties of the device were set earlier * by virPCIDeviceDetach() */ - if (pci) { + if (actual) { VIR_DEBUG("Saving network configuration of PCI device %s", - virPCIDeviceGetName(pci)); + virPCIDeviceGetName(actual)); hostdev->origstates.states.pci.unbind_from_stub = - virPCIDeviceGetUnbindFromStub(pci); + virPCIDeviceGetUnbindFromStub(actual); hostdev->origstates.states.pci.remove_slot = - virPCIDeviceGetRemoveSlot(pci); + virPCIDeviceGetRemoveSlot(actual); hostdev->origstates.states.pci.reprobe = - virPCIDeviceGetReprobe(pci); + virPCIDeviceGetReprobe(actual); } } @@ -898,17 +901,17 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr mgr, if (virHostdevIsPCINetDevice(hostdev)) { virDomainHostdevSubsysPCIPtr pcisrc = &hostdev->source.subsys.u.pci; - virPCIDevicePtr pci; + virPCIDevicePtr actual; - pci = virPCIDeviceListFindByIDs(pcidevs, - pcisrc->addr.domain, - pcisrc->addr.bus, - pcisrc->addr.slot, - pcisrc->addr.function); + actual = virPCIDeviceListFindByIDs(mgr->inactivePCIHostdevs, + pcisrc->addr.domain, + pcisrc->addr.bus, + pcisrc->addr.slot, + pcisrc->addr.function); - if (pci) { + if (actual) { VIR_DEBUG("Restoring network configuration of PCI device %s", - virPCIDeviceGetName(pci)); + virPCIDeviceGetName(actual)); virHostdevNetConfigRestore(hostdev, mgr->stateDir, oldStateDir); } -- 2.5.0

On 03/18/2016 01:03 PM, Andrea Bolognani wrote:
We would be just fine looking up the information in pcidevs most of the time; however, some corner cases would not be handled properly, so look up the actual device instead. --- src/util/virhostdev.c | 41 ++++++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 19 deletions(-)
ACK John

Instead of forcing the values for the unbind_from_stub, remove_slot and reprobe properties, look up the actual device and use that when calling virPCIDeviceReattach(). This ensures the device is restored to its original state after reattach: for example, if it was not bound to any driver before detach, it will not be bound forcefully during reattach. --- src/util/virhostdev.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index b21aa22..0973fb1 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -1658,6 +1658,7 @@ virHostdevPCINodeDeviceReAttach(virHostdevManagerPtr mgr, virPCIDevicePtr pci) { struct virHostdevIsPCINodeDeviceUsedData data = { mgr, NULL, false }; + virPCIDevicePtr actual; int ret = -1; virObjectLock(mgr->activePCIHostdevs); @@ -1666,11 +1667,12 @@ virHostdevPCINodeDeviceReAttach(virHostdevManagerPtr mgr, if (virHostdevIsPCINodeDeviceUsed(virPCIDeviceGetAddress(pci), &data)) goto cleanup; - virPCIDeviceSetUnbindFromStub(pci, true); - virPCIDeviceSetRemoveSlot(pci, true); - virPCIDeviceSetReprobe(pci, true); + /* We need to look up the actual device because that's what + * virPCIDeviceReattach() expects as its argument */ + if (!(actual = virPCIDeviceListFind(mgr->inactivePCIHostdevs, pci))) + goto cleanup; - if (virPCIDeviceReattach(pci, mgr->activePCIHostdevs, + if (virPCIDeviceReattach(actual, mgr->activePCIHostdevs, mgr->inactivePCIHostdevs) < 0) goto cleanup; -- 2.5.0

On 03/18/2016 01:03 PM, Andrea Bolognani wrote:
Instead of forcing the values for the unbind_from_stub, remove_slot and reprobe properties, look up the actual device and use that when calling virPCIDeviceReattach().
This ensures the device is restored to its original state after reattach: for example, if it was not bound to any driver before detach, it will not be bound forcefully during reattach. --- src/util/virhostdev.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
ACK John

Ensure the code behaves properly even for situations that were not being considered before, such as simply detaching devices from the host without attaching them to a guest and attaching devices as managed even though they had already been manually detached from the host. --- tests/virhostdevtest.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 78 insertions(+), 6 deletions(-) diff --git a/tests/virhostdevtest.c b/tests/virhostdevtest.c index 5eb2e7e..610b02a 100644 --- a/tests/virhostdevtest.c +++ b/tests/virhostdevtest.c @@ -253,7 +253,7 @@ testVirHostdevReAttachPCIHostdevs_unmanaged(void) } static int -testVirHostdevPreparePCIHostdevs_managed(void) +testVirHostdevPreparePCIHostdevs_managed(bool mixed) { int ret = -1; size_t active_count, inactive_count, i; @@ -270,7 +270,13 @@ testVirHostdevPreparePCIHostdevs_managed(void) hostdevs, nhostdevs, 0) < 0) goto cleanup; CHECK_LIST_COUNT(mgr->activePCIHostdevs, active_count + nhostdevs); - CHECK_LIST_COUNT(mgr->inactivePCIHostdevs, inactive_count); + /* If testing a mixed roundtrip, devices are already in the inactive list + * before we start and are removed from it as soon as we attach them to + * the guest */ + if (mixed) + CHECK_LIST_COUNT(mgr->inactivePCIHostdevs, inactive_count - nhostdevs); + else + CHECK_LIST_COUNT(mgr->inactivePCIHostdevs, inactive_count); /* Test conflict */ active_count = virPCIDeviceListCount(mgr->activePCIHostdevs); @@ -304,7 +310,7 @@ testVirHostdevPreparePCIHostdevs_managed(void) } static int -testVirHostdevReAttachPCIHostdevs_managed(void) +testVirHostdevReAttachPCIHostdevs_managed(bool mixed) { int ret = -1; size_t active_count, inactive_count, i; @@ -328,7 +334,12 @@ testVirHostdevReAttachPCIHostdevs_managed(void) virHostdevReAttachPCIDevices(mgr, drv_name, dom_name, hostdevs, nhostdevs, NULL); CHECK_LIST_COUNT(mgr->activePCIHostdevs, active_count - nhostdevs); - CHECK_LIST_COUNT(mgr->inactivePCIHostdevs, inactive_count); + /* If testing a mixed roundtrip, devices are added back to the inactive + * list as soon as we detach from the guest */ + if (mixed) + CHECK_LIST_COUNT(mgr->inactivePCIHostdevs, inactive_count + nhostdevs); + else + CHECK_LIST_COUNT(mgr->inactivePCIHostdevs, inactive_count); ret = 0; @@ -432,6 +443,31 @@ testVirHostdevUpdateActivePCIHostdevs(void) } /** + * testVirHostdevRoundtripNoGuest: + * @opaque: unused + * + * Perform a roundtrip without ever assigning devices to the guest. + * + * 1. Detach devices from the host + * 2. Reattach devices to the host + */ +static int +testVirHostdevRoundtripNoGuest(const void *opaque ATTRIBUTE_UNUSED) +{ + int ret = -1; + + if (testVirHostdevDetachPCINodeDevice() < 0) + goto out; + if (testVirHostdevReAttachPCINodeDevice() < 0) + goto out; + + ret = 0; + + out: + return ret; +} + +/** * testVirHostdevRoundtripUnmanaged: * @opaque: unused * @@ -479,9 +515,9 @@ testVirHostdevRoundtripManaged(const void *opaque ATTRIBUTE_UNUSED) int ret = -1; if (virHostdevHostSupportsPassthroughKVM()) { - if (testVirHostdevPreparePCIHostdevs_managed() < 0) + if (testVirHostdevPreparePCIHostdevs_managed(false) < 0) goto out; - if (testVirHostdevReAttachPCIHostdevs_managed() < 0) + if (testVirHostdevReAttachPCIHostdevs_managed(false) < 0) goto out; } @@ -492,6 +528,40 @@ testVirHostdevRoundtripManaged(const void *opaque ATTRIBUTE_UNUSED) } /** + * testVirHostdevRoundtripMixed: + * @opaque: unused + * + * Perform a roundtrip with managed devices but manually detach the devices + * from the host first. + * + * 1. Detach devices from the host + * 2. Attach devices to the guest as managed + * 3. Detach devices from the guest as managed + * 4. Reattach devices to the host + */ +static int +testVirHostdevRoundtripMixed(const void *opaque ATTRIBUTE_UNUSED) +{ + int ret = -1; + + if (testVirHostdevDetachPCINodeDevice() < 0) + goto out; + if (virHostdevHostSupportsPassthroughKVM()) { + if (testVirHostdevPreparePCIHostdevs_managed(true) < 0) + goto out; + if (testVirHostdevReAttachPCIHostdevs_managed(true) < 0) + goto out; + } + if (testVirHostdevReAttachPCINodeDevice() < 0) + goto out; + + ret = 0; + + out: + return ret; +} + +/** * testVirHostdevOther: * @opaque: unused * @@ -546,8 +616,10 @@ mymain(void) if (myInit() < 0) fprintf(stderr, "Init data structures failed."); + DO_TEST(testVirHostdevRoundtripNoGuest); DO_TEST(testVirHostdevRoundtripUnmanaged); DO_TEST(testVirHostdevRoundtripManaged); + DO_TEST(testVirHostdevRoundtripMixed); DO_TEST(testVirHostdevOther); myCleanup(); -- 2.5.0

On 03/18/2016 01:03 PM, Andrea Bolognani wrote:
Ensure the code behaves properly even for situations that were not being considered before, such as simply detaching devices from the host without attaching them to a guest and attaching devices as managed even though they had already been manually detached from the host. --- tests/virhostdevtest.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 78 insertions(+), 6 deletions(-)
ACK - although I thought much less hard about this ;-) John
participants (2)
-
Andrea Bolognani
-
John Ferlan