conf: Revert some code to resolve issues for hostdev hotplug

This patch reverts parts of commits 0d8b24f6b and 0785966d dealing with
the addition of a controller during virDomainHostdevAssignAddress. This
caused a regression for the hostdev hotplug path which assumes the
qemuDomainFindOrCreateSCSIDiskController will add the new controller
during qemuDomainAttachHostSCSIDevice to both the running domain and
the domain def controller list when the controller doesn't yet exist
(whether due to no SCSI controllers existing or the addition of a new
controller because existing ones are full).

Since commit id 0d8b24f6 will call virDomainHostdevAssignAddress during
virDomainDeviceDefPostParseInternal which is called either during domain
definition post processing (via an iterator during virDomainDefPostParse)
or directly from virDomainDeviceDefParse during hotplug, the change
broke the "side effect" of being able to add both a hostdev and controller
to the running domain.

The regression would only be seen if the running domain didn't have a
SCSI controller already defined or if the existing SCSI controller was
"full" of devices and a new controller needed to be created.

This patch will also add some extra comments to the code to avoid a
similar future change.

Signed-off-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Reviewed-by: Stefan Zimmermann <stzi@linux.vnet.ibm.com>

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index e6102a0b4254f4e88fc454c2d49642f2ad896c74..2f5c0ed4b7f4a45c097b6fa9b45c89af3d862bed 100644 (file)
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3845,7 +3845,7 @@ virDomainHostdevAssignAddress(virDomainXMLOptionPtr xmlopt,
     int next_unit = 0;
     unsigned controller = 0;
     size_t i;
-    int ret = -1;
+    int ret;
 
     for (i = 0; i < def->ncontrollers; i++) {
         if (def->controllers[i]->type != VIR_DOMAIN_CONTROLLER_TYPE_SCSI)
@@ -3864,16 +3864,13 @@ virDomainHostdevAssignAddress(virDomainXMLOptionPtr xmlopt,
         }
     }
 
-    /* If failed to find any VIR_DOMAIN_CONTROLLER_TYPE_SCSI or any space
-     * on existing VIR_DOMAIN_CONTROLLER_TYPE_SCSI controller(s), then
-     * try to add a new controller resulting in placement of this entry
-     * as unit=0
+    /* NB: Do not attempt calling virDomainDefMaybeAddController to
+     * automagically add a "new" controller. Doing so will result in
+     * qemuDomainFindOrCreateSCSIDiskController "finding" the controller
+     * in the domain def list and thus not hotplugging the controller as
+     * well as the hostdev in the event that there are either no SCSI
+     * controllers defined or there was no space on an existing one.
      */
-    if (ret == -1 &&
-        virDomainDefMaybeAddController((virDomainDefPtr) def,
-                                           VIR_DOMAIN_CONTROLLER_TYPE_SCSI,
-                                           controller, -1) < 0)
-        return -1;
 
     hostdev->info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE;
     hostdev->info->addr.drive.controller = controller;
@@ -15965,6 +15962,15 @@ virDomainDefParseXML(xmlDocPtr xml,
         }
 
         def->hostdevs[def->nhostdevs++] = hostdev;
+
+        /* For a domain definition, we need to check if the controller
+         * for this hostdev exists yet and if not add it. This cannot be
+         * done during virDomainHostdevAssignAddress (as part of device
+         * post processing) because that will result in the failure to
+         * load the controller during hostdev hotplug.
+         */
+        if (virDomainDefMaybeAddHostdevSCSIcontroller(def) < 0)
+            goto error;
     }
     VIR_FREE(nodes);