qemu: Improve qemuDomainDefaultSCSIControllerModel()

Make the helper stateless. This requires the caller to check
whether it needs to be called in the first place instead of
adding this check inside the function, which makes for more
readable, if a little more verbose, code.

We also update callers to check the return value against
VIR_DOMAIN_CONTROLLER_MODEL_SCSI_DEFAULT instead of a
functionally equivalent, but semantically less meaningful,
check for whether the return value is negative.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 58228ce7d4eddefd796f421963ab7595f65e3c8e..cdf43e788a0a66d9db1f113e7c8892b5e7700c80 100644 (file)
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4234,22 +4234,27 @@ qemuDomainDefAddDefaultAudioBackend(virQEMUDriver *driver,
 
 /**
  * @def: Domain definition
- * @cont: Domain controller def
  * @qemuCaps: qemu capabilities
  *
- * If the controller model is already defined, return it immediately;
- * otherwise, based on the @qemuCaps return a default model value.
+ * Choose a reasonable model to use for a SCSI controller where a
+ * specific one hasn't been provided by the user.
  *
- * Returns model on success, -1 on failure.
+ * The choice is based on a number of factors, including the guest's
+ * architecture and machine type. @qemuCaps, if provided, might be
+ * taken into consideration too.
+ *
+ * If no sensible choice can be made for the controller model,
+ * VIR_DOMAIN_CONTROLLER_MODEL_SCSI_DEFAULT will be returned. It's
+ * likely that a failure will need to be reported in this scenario,
+ * but the handling is entirely up to the caller.
+ *
+ * Returns: a valid virDomainControllerModelSCSI value if one could
+ *          be determined, or VIR_DOMAIN_CONTROLLER_MODEL_SCSI_DEFAULT
  */
-int
+virDomainControllerModelSCSI
 qemuDomainDefaultSCSIControllerModel(const virDomainDef *def,
-                                     const virDomainControllerDef *cont,
                                      virQEMUCaps *qemuCaps)
 {
-    if (cont->model > 0)
-        return cont->model;
-
     if (qemuDomainIsPSeries(def))
         return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI;
     if (ARCH_IS_S390(def->os.arch) || qemuDomainIsARMVirt(def) ||
@@ -4262,7 +4267,7 @@ qemuDomainDefaultSCSIControllerModel(const virDomainDef *def,
     if (qemuDomainHasBuiltinESP(def))
         return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_NCR53C90;
 
-    return -1;
+    return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_DEFAULT;
 }
 
 

diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 751607c12b9d1b35a8f84af0bc317b8a96b63677..49f83613e31766604243333737a0711deae15c4b 100644 (file)
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -838,9 +838,8 @@ bool qemuDomainHasBuiltinESP(const virDomainDef *def);
 bool qemuDomainNeedsFDC(const virDomainDef *def);
 bool qemuDomainSupportsPCI(const virDomainDef *def);
 bool qemuDomainSupportsPCIMultibus(const virDomainDef *def);
-int qemuDomainDefaultSCSIControllerModel(const virDomainDef *def,
-                                         const virDomainControllerDef *cont,
-                                         virQEMUCaps *qemuCaps);
+virDomainControllerModelSCSI qemuDomainDefaultSCSIControllerModel(const virDomainDef *def,
+                                                                  virQEMUCaps *qemuCaps);
 
 int qemuDomainDefAddDefaultAudioBackend(virQEMUDriver *driver,
                                         virDomainDef *def);

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index c7f31ab2648745d82bb92f90f8c858cbc8bccd83..073bd97d3af6fb9a665693d5956a520ef699b3f7 100644 (file)
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -930,7 +930,7 @@ qemuDomainFindOrCreateSCSIDiskController(virDomainObj *vm,
     size_t i;
     virDomainControllerDef *cont;
     qemuDomainObjPrivate *priv = vm->privateData;
-    int model = -1;
+    virDomainControllerModelSCSI model = VIR_DOMAIN_CONTROLLER_MODEL_SCSI_DEFAULT;
 
     for (i = 0; i < vm->def->ncontrollers; i++) {
         cont = vm->def->controllers[i];
@@ -956,13 +956,15 @@ qemuDomainFindOrCreateSCSIDiskController(virDomainObj *vm,
      * now hotplug a controller */
     cont = virDomainControllerDefNew(VIR_DOMAIN_CONTROLLER_TYPE_SCSI);
     cont->idx = controller;
+    cont->model = model;
 
-    if (model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_DEFAULT)
-        cont->model = qemuDomainDefaultSCSIControllerModel(vm->def, cont, priv->qemuCaps);
-    else
-        cont->model = model;
-
-    if (cont->model < 0) {
+    /* If no model has been discovered by looking at existing SCSI
+     * controllers, try to come up with a reasonable default. If one
+     * cannot be determined, error out */
+    if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_DEFAULT) {
+        cont->model = qemuDomainDefaultSCSIControllerModel(vm->def, priv->qemuCaps);
+    }
+    if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_DEFAULT) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("Unable to determine model for SCSI controller idx=%1$d"),
                        cont->idx);

diff --git a/src/qemu/qemu_postparse.c b/src/qemu/qemu_postparse.c
index 3f9a1064c73d201c863419ecfed17fd3d189e28b..9c2427970dd6b44b1554b9cec9a35caf6be09db2 100644 (file)
--- a/src/qemu/qemu_postparse.c
+++ b/src/qemu/qemu_postparse.c
@@ -343,10 +343,13 @@ qemuDomainControllerDefPostParse(virDomainControllerDef *cont,
 {
     switch (cont->type) {
     case VIR_DOMAIN_CONTROLLER_TYPE_SCSI:
-        /* Set the default SCSI controller model if not already set */
-        cont->model = qemuDomainDefaultSCSIControllerModel(def, cont, qemuCaps);
-
-        if (cont->model < 0) {
+        /* If no model is set, try to come up with a reasonable
+         * default. If one cannot be determined, error out */
+        if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_DEFAULT ||
+            cont->model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO) {
+            cont->model = qemuDomainDefaultSCSIControllerModel(def, qemuCaps);
+        }
+        if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_DEFAULT) {
             virReportError(VIR_ERR_INTERNAL_ERROR,
                            _("Unable to determine model for SCSI controller idx=%1$d"),
                            cont->idx);