From nobody Thu Apr 16 15:43:17 2026 Delivered-To: importer@patchew.org Received-SPF: none (zohomail.com: 8.43.85.245 is neither permitted nor denied by domain of lists.libvirt.org) client-ip=8.43.85.245; envelope-from=devel-bounces@lists.libvirt.org; helo=lists.libvirt.org; Authentication-Results: mx.zohomail.com; spf=none (zohomail.com: 8.43.85.245 is neither permitted nor denied by domain of lists.libvirt.org) smtp.mailfrom=devel-bounces@lists.libvirt.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.libvirt.org (lists.libvirt.org [8.43.85.245]) by mx.zohomail.com with SMTPS id 1707931383749605.1044663225694; Wed, 14 Feb 2024 09:23:03 -0800 (PST) Received: by lists.libvirt.org (Postfix, from userid 996) id B0D151A21; Wed, 14 Feb 2024 12:23:02 -0500 (EST) Received: from lists.libvirt.org.85.43.8.in-addr.arpa (localhost [IPv6:::1]) by lists.libvirt.org (Postfix) with ESMTP id 33ADA1B9D; Wed, 14 Feb 2024 12:12:38 -0500 (EST) Received: by lists.libvirt.org (Postfix, from userid 996) id DFA0A19F7; Wed, 14 Feb 2024 12:11:35 -0500 (EST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by lists.libvirt.org (Postfix) with ESMTPS id B037719EB for ; Wed, 14 Feb 2024 12:11:32 -0500 (EST) Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-145-4HeuqHbyMGCw6c0M54m4WQ-1; Wed, 14 Feb 2024 12:11:30 -0500 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 7DAE184AEF2 for ; Wed, 14 Feb 2024 17:11:29 +0000 (UTC) Received: from harajuku.usersys.redhat.com (unknown [10.45.226.93]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 141DE112131D for ; Wed, 14 Feb 2024 17:11:28 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on lists.libvirt.org X-Spam-Level: X-Spam-Status: No, score=-0.8 required=5.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE, T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.4 X-MC-Unique: 4HeuqHbyMGCw6c0M54m4WQ-1 From: Andrea Bolognani To: devel@lists.libvirt.org Subject: [PATCH v2 04/17] qemu: Improve qemuDomainDefaultSCSIControllerModel() Date: Wed, 14 Feb 2024 18:11:11 +0100 Message-ID: <20240214171124.508000-5-abologna@redhat.com> In-Reply-To: <20240214171124.508000-1-abologna@redhat.com> References: <20240214171124.508000-1-abologna@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.3 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Message-ID-Hash: O4XWNKFPM6NK5JEFZNKSG2SISAXJ4ZL7 X-Message-ID-Hash: O4XWNKFPM6NK5JEFZNKSG2SISAXJ4ZL7 X-MailFrom: abologna@redhat.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; header-match-config-1; header-match-config-2; header-match-config-3; header-match-devel.lists.libvirt.org-0; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; suspicious-header X-Mailman-Version: 3.2.2 Precedence: list List-Id: Development discussions about the libvirt library & tools Archived-At: List-Archive: List-Help: List-Post: List-Subscribe: List-Unsubscribe: Content-Type: text/plain; charset="utf-8"; x-default="true" Content-Transfer-Encoding: quoted-printable X-ZM-MESSAGEID: 1707931384460100001 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 Reviewed-by: Peter Krempa --- 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 *d= river, } =20 =20 +/* 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 =3D qemuDomainDefaultSCSIControllerModelInternal(def, qemuCaps); + return 0; } =20 =20 @@ -5610,10 +5617,12 @@ qemuDomainControllerDefPostParse(virDomainControlle= rDef *cont, { switch (cont->type) { case VIR_DOMAIN_CONTROLLER_TYPE_SCSI: - /* Set the default SCSI controller model if not already set */ - cont->model =3D qemuDomainDefaultSCSIControllerModel(def, cont, qe= muCaps); - - 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 =3D=3D VIR_DOMAIN_CONTROLLER_MODEL_SCSI_DEFAULT || + cont->model =3D=3D VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO) && + (qemuDomainDefaultSCSIControllerModel(&cont->model, def, qemuC= aps) < 0 || + cont->model =3D=3D VIR_DOMAIN_CONTROLLER_MODEL_SCSI_DEFAULT))= { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to determine model for SCSI controlle= r idx=3D%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 *con= t, +int qemuDomainDefaultSCSIControllerModel(virDomainControllerModelSCSI *mod= el, + const virDomainDef *def, virQEMUCaps *qemuCaps); =20 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 =3D vm->privateData; - int model =3D -1; + virDomainControllerModelSCSI model =3D VIR_DOMAIN_CONTROLLER_MODEL_SCS= I_DEFAULT; =20 for (i =3D 0; i < vm->def->ncontrollers; i++) { cont =3D vm->def->controllers[i]; @@ -876,13 +876,15 @@ qemuDomainFindOrCreateSCSIDiskController(virDomainObj= *vm, * now hotplug a controller */ cont =3D virDomainControllerDefNew(VIR_DOMAIN_CONTROLLER_TYPE_SCSI); cont->idx =3D controller; - - if (model =3D=3D VIR_DOMAIN_CONTROLLER_MODEL_SCSI_DEFAULT) - cont->model =3D qemuDomainDefaultSCSIControllerModel(vm->def, cont= , priv->qemuCaps); - else - cont->model =3D model; - - if (cont->model < 0) { + cont->model =3D 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 =3D=3D VIR_DOMAIN_CONTROLLER_MODEL_SCSI_DEFAULT || + cont->model =3D=3D VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO) && + (qemuDomainDefaultSCSIControllerModel(&cont->model, vm->def, priv-= >qemuCaps) < 0 || + cont->model =3D=3D VIR_DOMAIN_CONTROLLER_MODEL_SCSI_DEFAULT)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to determine model for SCSI controller id= x=3D%1$d"), cont->idx); --=20 2.43.0 _______________________________________________ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-leave@lists.libvirt.org