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.
At the same time, change it so that it uses an out argument to
return the selected model back to the caller, and only use the
return value to signal whether the function completed its task
successfully. This is consistent with most APIs in libvirt, and
removes the ambiguity caused by the fact that the value of
VIR_DOMAIN_CONTROLLER_MODEL_SCSI_DEFAULT is -1.
In order to have a nice API while still keeping the
implementation short and readable, a small wrapper is required.
Signed-off-by: Andrea Bolognani <abologna@xxxxxxxxxx>
---
src/qemu/qemu_domain.c | 51 ++++++++++++++++++++++++-----------------
src/qemu/qemu_domain.h | 4 ++--
src/qemu/qemu_hotplug.c | 18 ++++++++-------
3 files changed, 42 insertions(+), 31 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 22e6b9fd3e..ab2cc427a2 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4096,6 +4096,26 @@ qemuDomainDefAddDefaultAudioBackend(virQEMUDriver *driver,
}
+/* Internal. Use qemuDomainDefaultSCSIControllerModel() instead */
+static virDomainControllerModelSCSI
+qemuDomainDefaultSCSIControllerModelInternal(const virDomainDef *def,
+ virQEMUCaps *qemuCaps)
+{
+ if (qemuDomainIsPSeries(def))
+ return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI;
+ if (ARCH_IS_S390(def->os.arch))
+ return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI;
+ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_LSI))
+ return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC;
+ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_SCSI))
+ return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI;
+ if (qemuDomainHasBuiltinESP(def))
+ return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_NCR53C90;
+
+ return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_DEFAULT;
+}
+
+
/**
* @def: Domain definition
* @cont: Domain controller def
@@ -4107,25 +4127,12 @@ qemuDomainDefAddDefaultAudioBackend(virQEMUDriver *driver,
* Returns model on success, -1 on failure with error set.
*/
int
-qemuDomainDefaultSCSIControllerModel(const virDomainDef *def,
- const virDomainControllerDef *cont,
+qemuDomainDefaultSCSIControllerModel(virDomainControllerModelSCSI *model,
+ const virDomainDef *def,
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))
- return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI;
- if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_LSI))
- return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC;
- if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_SCSI))
- return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI;
- if (qemuDomainHasBuiltinESP(def))
- return VIR_DOMAIN_CONTROLLER_MODEL_SCSI_NCR53C90;
-
- return -1;
+ *model = qemuDomainDefaultSCSIControllerModelInternal(def, qemuCaps);
+ return 0;
}
@@ -5610,10 +5617,12 @@ 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 can't be determined, error out */
+ if ((cont->model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_DEFAULT ||
+ cont->model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO) &&
+ (qemuDomainDefaultSCSIControllerModel(&cont->model, def, qemuCaps) < 0 ||
+ 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_domain.h b/src/qemu/qemu_domain.h
index f3aad2ef5c..dae817d655 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -838,8 +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,
+int qemuDomainDefaultSCSIControllerModel(virDomainControllerModelSCSI *model,
+ const virDomainDef *def,
virQEMUCaps *qemuCaps);
void qemuDomainUpdateCurrentMemorySize(virDomainObj *vm);
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index a9dbc37915..b96c706804 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -850,7 +850,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];
@@ -876,13 +876,15 @@ qemuDomainFindOrCreateSCSIDiskController(virDomainObj *vm,
* now hotplug a controller */
cont = virDomainControllerDefNew(VIR_DOMAIN_CONTROLLER_TYPE_SCSI);
cont->idx = controller;
-
- if (model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_DEFAULT)
- cont->model = qemuDomainDefaultSCSIControllerModel(vm->def, cont, priv->qemuCaps);
- else
- cont->model = model;
-
- if (cont->model < 0) {
+ cont->model = model;
+
+ /* If no model has been discovered by looking at existing SCSI
+ * controllers, try to come up with a reasonable default. If one
+ * can't be determined, error out */
+ if ((cont->model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_DEFAULT ||
+ cont->model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO) &&
+ (qemuDomainDefaultSCSIControllerModel(&cont->model, vm->def, priv->qemuCaps) < 0 ||
+ 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);
--
2.43.0
_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx
