[libvirt] [PATCH v2 7/9] hostdev: Update comments

Andrea Bolognani abologna at redhat.com
Mon Jan 25 16:21:02 UTC 2016


Some of the comments are no longer accurate after the recent changes,
others have been expanded and hopefully improved.

The initial summary of the operations has been removed so that two
separate edits are not needed when making changes.
---
 src/util/virhostdev.c | 68 +++++++++++++++++++++++----------------------------
 1 file changed, 30 insertions(+), 38 deletions(-)

diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index ab17547..0892dbf 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -675,11 +675,10 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr hostdev_mgr,
      * must be reset before being marked as active.
      */
 
-    /* Loop 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
-     */
+    /* Detaching devices from the host involves several steps; each of them
+     * is described at length below */
 
+    /* Step 1: perform safety checks, eg. ensure the devices are assignable */
     for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) {
         virPCIDevicePtr dev = virPCIDeviceListGet(pcidevs, i);
         bool strict_acs_check = !!(flags & VIR_HOSTDEV_STRICT_ACS_CHECK);
@@ -710,7 +709,8 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr hostdev_mgr,
         }
     }
 
-    /* Loop 2: detach managed devices (i.e. bind to appropriate stub driver) */
+    /* Step 2: detach managed devices (i.e. bind to appropriate stub driver).
+     *         detachPCIDevices() will also mark devices as inactive */
     for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) {
         virPCIDevicePtr dev = virPCIDeviceListGet(pcidevs, i);
 
@@ -718,11 +718,10 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr hostdev_mgr,
             goto reattachdevs;
     }
 
-    /* At this point, all devices are attached to the stub driver and have
+    /* At this point, devices are attached to the stub driver and have
      * been marked as inactive */
 
-    /* Loop 3: Now that all the PCI hostdevs have been detached, we
-     * can safely reset them */
+    /* Step 3: perform a PCI reset on all devices */
     for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) {
         virPCIDevicePtr dev = virPCIDeviceListGet(pcidevs, i);
 
@@ -732,8 +731,7 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr hostdev_mgr,
             goto reattachdevs;
     }
 
-    /* Loop 4: For SRIOV network devices, Now that we have detached the
-     * the network device, set the netdev config */
+    /* Step 4: set the netdev config for SRIOV network devices */
     for (i = 0; i < nhostdevs; i++) {
          virDomainHostdevDefPtr hostdev = hostdevs[i];
          if (!virHostdevIsPCINetDevice(hostdev))
@@ -762,9 +760,7 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr hostdev_mgr,
             goto inactivedevs;
     }
 
-    /* Loop 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 dev, activeDev;
 
@@ -777,7 +773,7 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr hostdev_mgr,
             virPCIDeviceSetUsedBy(activeDev, drv_name, dom_name);
     }
 
-    /* Loop 8: Now set the original states for hostdev def */
+    /* Step 7: set the original states for hostdev def */
     for (i = 0; i < nhostdevs; i++) {
         virPCIDevicePtr dev;
         virPCIDevicePtr pcidev;
@@ -792,10 +788,9 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr hostdev_mgr,
         dev = virPCIDeviceNew(pcisrc->addr.domain, pcisrc->addr.bus,
                               pcisrc->addr.slot, pcisrc->addr.function);
 
-        /* original states "unbind_from_stub", "remove_slot",
-         * "reprobe" were already set by pciDettachDevice in
-         * loop 2.
-         */
+        /* original states for "unbind_from_stub", "remove_slot" and
+         * "reprobe" (used when reattaching) were already set by
+         * detachPCIDevices() in a previous step */
         VIR_DEBUG("Saving network configuration of PCI device %s",
                   virPCIDeviceGetName(dev));
         if ((pcidev = virPCIDeviceListFind(pcidevs, dev))) {
@@ -875,16 +870,11 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr hostdev_mgr,
         goto cleanup;
     }
 
-    /* Loop through the assigned devices 4 times: 1) delete them all from
-     * activePCIHostdevs, 2) restore network config of SRIOV netdevs, 3) Do a
-     * PCI reset on each device, 4) reattach the devices to their host drivers
-     * (managed) or add them to inactivePCIHostdevs (!managed).
-     */
+    /* Reattaching devices to the host involves several steps; each of them
+     * is described at length below */
 
-    /*
-     * Loop 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: verify that each device in the hostdevs list really was in use
+     *         by this domain */
     i = 0;
     while (i < virPCIDeviceListCount(pcidevs)) {
         virPCIDevicePtr dev = virPCIDeviceListGet(pcidevs, i);
@@ -922,14 +912,11 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr hostdev_mgr,
             goto cleanup;
     }
 
-    /* At this point, any device that had been used by the guest is in
-     * pcidevs, but has been removed from activePCIHostdevs.
-     */
+    /* At this point, devices are no longer marked as active and have been
+     * marked as inactive instead */
 
-    /*
-     * Loop 2: restore original network config of hostdevs that used
-     * <interface type='hostdev'>
-     */
+    /* Step 3: restore original network config of hostdevs that used
+     *         <interface type='hostdev'> */
     for (i = 0; i < nhostdevs; i++) {
         virDomainHostdevDefPtr hostdev = hostdevs[i];
 
@@ -950,7 +937,7 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr hostdev_mgr,
         }
     }
 
-    /* Loop 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 dev = virPCIDeviceListGet(pcidevs, i);
 
@@ -964,15 +951,20 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr hostdev_mgr,
         }
     }
 
-    /* Loop 4: reattach devices to their host drivers (if managed) or place
-     * them on the inactive list (if not managed)
-     */
+    /* Step 5: reattach managed devices to their host drivers; unmanaged
+     *         devices need no further processing. reattachPCIDevices() will
+     *         remove devices from the inactive list as they are reattached
+     *         to the host */
     for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) {
         virPCIDevicePtr dev = virPCIDeviceListGet(pcidevs, i);
 
         ignore_value(virHostdevOnlyReattachPCIDevice(hostdev_mgr, dev, true));
     }
 
+    /* At this point, all devices are either marked as inactive (if they
+     * were unmanaged), or have been reattached to the host driver (if they
+     * were managed) and are no longer tracked by any of our device lists */
+
  cleanup:
     virObjectUnref(pcidevs);
     virObjectUnlock(hostdev_mgr->activePCIHostdevs);
-- 
2.5.0




More information about the libvir-list mailing list