[PATCH v2 04/17] 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. 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(a)redhat.com&gt; --- 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