From nobody Mon Apr 29 07:29:34 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1534767034248437.3060505262466; Mon, 20 Aug 2018 05:10:34 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id EF980C02735D; Mon, 20 Aug 2018 12:10:29 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id A70C55E1C7; Mon, 20 Aug 2018 12:10:29 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 5EAE44E982; Mon, 20 Aug 2018 12:10:29 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id w7KCAEPc024596 for ; Mon, 20 Aug 2018 08:10:14 -0400 Received: by smtp.corp.redhat.com (Postfix) id 9EF062166BDB; Mon, 20 Aug 2018 12:10:14 +0000 (UTC) Received: from localhost.localdomain (ovpn-204-49.brq.redhat.com [10.40.204.49]) by smtp.corp.redhat.com (Postfix) with ESMTP id 292FF2166BA1 for ; Mon, 20 Aug 2018 12:10:14 +0000 (UTC) From: Michal Privoznik To: libvir-list@redhat.com Date: Mon, 20 Aug 2018 14:09:44 +0200 Message-Id: <170a88e3f1e72f3598d981e4f26dcac7ceebbb09.1534766849.git.mprivozn@redhat.com> In-Reply-To: References: In-Reply-To: References: X-Scanned-By: MIMEDefang 2.78 on 10.11.54.6 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH v2 1/8] storage_backend_rbd: Drop ATTRIBUTE_UNUSED for arguments that are used X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Mon, 20 Aug 2018 12:10:32 +0000 (UTC) X-ZohoMail: RDMRC_0 RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" In two places the passed pool object argument is marked as ATTRIBUTE_UNUSED even though it's used right away. Signed-off-by: Michal Privoznik Reviewed-by: John Ferlan --- src/storage/storage_backend_rbd.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backen= d_rbd.c index 642cacb673..c6fb791a81 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -1063,7 +1063,7 @@ virStorageBackendRBDBuildVolFrom(virStoragePoolObjPtr= pool, } =20 static int -virStorageBackendRBDRefreshVol(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, +virStorageBackendRBDRefreshVol(virStoragePoolObjPtr pool, virStorageVolDefPtr vol) { virStorageBackendRBDStatePtr ptr =3D NULL; @@ -1083,7 +1083,7 @@ virStorageBackendRBDRefreshVol(virStoragePoolObjPtr p= ool ATTRIBUTE_UNUSED, } =20 static int -virStorageBackendRBDResizeVol(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, +virStorageBackendRBDResizeVol(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, unsigned long long capacity, unsigned int flags) --=20 2.16.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list From nobody Mon Apr 29 07:29:34 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 153476703863613.705505358737128; Mon, 20 Aug 2018 05:10:38 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 580813082E25; Mon, 20 Aug 2018 12:10:35 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 1884D309132F; Mon, 20 Aug 2018 12:10:35 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id B454B4E987; Mon, 20 Aug 2018 12:10:34 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id w7KCAF20024604 for ; Mon, 20 Aug 2018 08:10:15 -0400 Received: by smtp.corp.redhat.com (Postfix) id 6CDCE2166BDB; Mon, 20 Aug 2018 12:10:15 +0000 (UTC) Received: from localhost.localdomain (ovpn-204-49.brq.redhat.com [10.40.204.49]) by smtp.corp.redhat.com (Postfix) with ESMTP id EB7EA2166BA1 for ; Mon, 20 Aug 2018 12:10:14 +0000 (UTC) From: Michal Privoznik To: libvir-list@redhat.com Date: Mon, 20 Aug 2018 14:09:45 +0200 Message-Id: <2d469b2a2158fdbd4c126fde10b20c91513bb60f.1534766849.git.mprivozn@redhat.com> In-Reply-To: References: In-Reply-To: References: X-Scanned-By: MIMEDefang 2.78 on 10.11.54.6 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH v2 2/8] virstorageobj: Move virStoragePoolObjIsDuplicate up X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.84 on 10.5.11.26 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.46]); Mon, 20 Aug 2018 12:10:37 +0000 (UTC) X-ZohoMail: RDMRC_0 RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" This function is going to be made static in used in virStoragePoolObjAssignDef(). Therefore move it a few lines up. Signed-off-by: Michal Privoznik Reviewed-by: John Ferlan --- src/conf/virstorageobj.c | 124 +++++++++++++++++++++++--------------------= ---- 1 file changed, 62 insertions(+), 62 deletions(-) diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c index 6f1e14ed3e..185822451b 100644 --- a/src/conf/virstorageobj.c +++ b/src/conf/virstorageobj.c @@ -1047,6 +1047,68 @@ virStoragePoolObjVolumeListExport(virConnectPtr conn, } =20 =20 +/* + * virStoragePoolObjIsDuplicate: + * @doms : virStoragePoolObjListPtr to search + * @def : virStoragePoolDefPtr definition of pool to lookup + * @check_active: If true, ensure that pool is not active + * + * Returns: -1 on error + * 0 if pool is new + * 1 if pool is a duplicate + */ +int +virStoragePoolObjIsDuplicate(virStoragePoolObjListPtr pools, + virStoragePoolDefPtr def, + bool check_active) +{ + int ret =3D -1; + virStoragePoolObjPtr obj =3D NULL; + + /* See if a Pool with matching UUID already exists */ + obj =3D virStoragePoolObjFindByUUID(pools, def->uuid); + if (obj) { + /* UUID matches, but if names don't match, refuse it */ + if (STRNEQ(obj->def->name, def->name)) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(obj->def->uuid, uuidstr); + virReportError(VIR_ERR_OPERATION_FAILED, + _("pool '%s' is already defined with uuid %s"), + obj->def->name, uuidstr); + goto cleanup; + } + + if (check_active) { + /* UUID & name match, but if Pool is already active, refuse it= */ + if (virStoragePoolObjIsActive(obj)) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("pool is already active as '%s'"), + obj->def->name); + goto cleanup; + } + } + + ret =3D 1; + } else { + /* UUID does not match, but if a name matches, refuse it */ + obj =3D virStoragePoolObjFindByName(pools, def->name); + if (obj) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(obj->def->uuid, uuidstr); + virReportError(VIR_ERR_OPERATION_FAILED, + _("pool '%s' already exists with uuid %s"), + def->name, uuidstr); + goto cleanup; + } + ret =3D 0; + } + + cleanup: + virStoragePoolObjEndAPI(&obj); + return ret; +} + + /** * virStoragePoolObjAssignDef: * @pools: Storage Pool object list pointer @@ -1452,68 +1514,6 @@ virStoragePoolObjGetNames(virStoragePoolObjListPtr p= ools, } =20 =20 -/* - * virStoragePoolObjIsDuplicate: - * @doms : virStoragePoolObjListPtr to search - * @def : virStoragePoolDefPtr definition of pool to lookup - * @check_active: If true, ensure that pool is not active - * - * Returns: -1 on error - * 0 if pool is new - * 1 if pool is a duplicate - */ -int -virStoragePoolObjIsDuplicate(virStoragePoolObjListPtr pools, - virStoragePoolDefPtr def, - bool check_active) -{ - int ret =3D -1; - virStoragePoolObjPtr obj =3D NULL; - - /* See if a Pool with matching UUID already exists */ - obj =3D virStoragePoolObjFindByUUID(pools, def->uuid); - if (obj) { - /* UUID matches, but if names don't match, refuse it */ - if (STRNEQ(obj->def->name, def->name)) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(obj->def->uuid, uuidstr); - virReportError(VIR_ERR_OPERATION_FAILED, - _("pool '%s' is already defined with uuid %s"), - obj->def->name, uuidstr); - goto cleanup; - } - - if (check_active) { - /* UUID & name match, but if Pool is already active, refuse it= */ - if (virStoragePoolObjIsActive(obj)) { - virReportError(VIR_ERR_OPERATION_INVALID, - _("pool is already active as '%s'"), - obj->def->name); - goto cleanup; - } - } - - ret =3D 1; - } else { - /* UUID does not match, but if a name matches, refuse it */ - obj =3D virStoragePoolObjFindByName(pools, def->name); - if (obj) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(obj->def->uuid, uuidstr); - virReportError(VIR_ERR_OPERATION_FAILED, - _("pool '%s' already exists with uuid %s"), - def->name, uuidstr); - goto cleanup; - } - ret =3D 0; - } - - cleanup: - virStoragePoolObjEndAPI(&obj); - return ret; -} - - static int getSCSIHostNumber(virStorageAdapterSCSIHostPtr scsi_host, unsigned int *hostnum) --=20 2.16.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list From nobody Mon Apr 29 07:29:34 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1534767043255323.12559797417987; Mon, 20 Aug 2018 05:10:43 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id AC475C059B76; Mon, 20 Aug 2018 12:10:39 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 751E1A6365; Mon, 20 Aug 2018 12:10:39 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 285B3181A12F; Mon, 20 Aug 2018 12:10:39 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id w7KCAGCv024611 for ; Mon, 20 Aug 2018 08:10:16 -0400 Received: by smtp.corp.redhat.com (Postfix) id 3D2042166BDB; Mon, 20 Aug 2018 12:10:16 +0000 (UTC) Received: from localhost.localdomain (ovpn-204-49.brq.redhat.com [10.40.204.49]) by smtp.corp.redhat.com (Postfix) with ESMTP id B9B792166BA1 for ; Mon, 20 Aug 2018 12:10:15 +0000 (UTC) From: Michal Privoznik To: libvir-list@redhat.com Date: Mon, 20 Aug 2018 14:09:46 +0200 Message-Id: In-Reply-To: References: In-Reply-To: References: X-Scanned-By: MIMEDefang 2.78 on 10.11.54.6 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH v2 3/8] virstorageobj: Check for duplicates from virStoragePoolObjAssignDef X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.84 on 10.5.11.27 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Mon, 20 Aug 2018 12:10:41 +0000 (UTC) X-ZohoMail: RDMRC_0 RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" Even though we do some checking is not as thorough as it should be. We already have virStoragePoolObjIsDuplicate but the way we use it is a typical TOCTOU. Imagine two threads trying to define two pools with the same name but different UUIDs. With the current code neither of them finds a duplicate and thus proceed to virStoragePoolObjAssignDef where only names are compared. Therefore both threads succeed which is obviously wrong. We should check for duplicates where we care for them. Signed-off-by: Michal Privoznik Reviewed-by: John Ferlan --- src/conf/virstorageobj.c | 39 +++++++++++++++++++++++++++------------ src/conf/virstorageobj.h | 8 ++------ src/libvirt_private.syms | 1 - src/storage/storage_driver.c | 10 ++-------- src/test/test_driver.c | 12 +++--------- 5 files changed, 34 insertions(+), 36 deletions(-) diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c index 185822451b..ea0ae6fd86 100644 --- a/src/conf/virstorageobj.c +++ b/src/conf/virstorageobj.c @@ -1052,22 +1052,28 @@ virStoragePoolObjVolumeListExport(virConnectPtr con= n, * @doms : virStoragePoolObjListPtr to search * @def : virStoragePoolDefPtr definition of pool to lookup * @check_active: If true, ensure that pool is not active + * @objRet: returned pool object * - * Returns: -1 on error + * Assumes @pools is locked by caller already. + * + * Returns: -1 on error (name or UUID mismatch) * 0 if pool is new - * 1 if pool is a duplicate + * 1 if pool is a duplicate (name and UUID match) */ -int +static int virStoragePoolObjIsDuplicate(virStoragePoolObjListPtr pools, virStoragePoolDefPtr def, - bool check_active) + bool check_active, + virStoragePoolObjPtr *objRet) { int ret =3D -1; virStoragePoolObjPtr obj =3D NULL; =20 /* See if a Pool with matching UUID already exists */ - obj =3D virStoragePoolObjFindByUUID(pools, def->uuid); + obj =3D virStoragePoolObjFindByUUIDLocked(pools, def->uuid); if (obj) { + virObjectLock(obj); + /* UUID matches, but if names don't match, refuse it */ if (STRNEQ(obj->def->name, def->name)) { char uuidstr[VIR_UUID_STRING_BUFLEN]; @@ -1088,11 +1094,14 @@ virStoragePoolObjIsDuplicate(virStoragePoolObjListP= tr pools, } } =20 + VIR_STEAL_PTR(*objRet, obj); ret =3D 1; } else { /* UUID does not match, but if a name matches, refuse it */ - obj =3D virStoragePoolObjFindByName(pools, def->name); + obj =3D virStoragePoolObjFindByNameLocked(pools, def->name); if (obj) { + virObjectLock(obj); + char uuidstr[VIR_UUID_STRING_BUFLEN]; virUUIDFormat(obj->def->uuid, uuidstr); virReportError(VIR_ERR_OPERATION_FAILED, @@ -1113,6 +1122,7 @@ virStoragePoolObjIsDuplicate(virStoragePoolObjListPtr= pools, * virStoragePoolObjAssignDef: * @pools: Storage Pool object list pointer * @def: Storage pool definition to add or update + * @check_active: If true, ensure that pool is not active * * Lookup the @def to see if it already exists in the @pools in order * to either update or add if it does not exist. @@ -1121,15 +1131,20 @@ virStoragePoolObjIsDuplicate(virStoragePoolObjListP= tr pools, */ virStoragePoolObjPtr virStoragePoolObjAssignDef(virStoragePoolObjListPtr pools, - virStoragePoolDefPtr def) + virStoragePoolDefPtr def, + bool check_active) { - virStoragePoolObjPtr obj; + virStoragePoolObjPtr obj =3D NULL; char uuidstr[VIR_UUID_STRING_BUFLEN]; + int rc; =20 virObjectRWLockWrite(pools); =20 - if ((obj =3D virStoragePoolObjFindByNameLocked(pools, def->name))) { - virObjectLock(obj); + rc =3D virStoragePoolObjIsDuplicate(pools, def, check_active, &obj); + + if (rc < 0) + goto error; + if (rc > 0) { if (!virStoragePoolObjIsActive(obj)) { virStoragePoolDefFree(obj->def); obj->def =3D def; @@ -1186,7 +1201,7 @@ virStoragePoolObjLoad(virStoragePoolObjListPtr pools, return NULL; } =20 - if (!(obj =3D virStoragePoolObjAssignDef(pools, def))) { + if (!(obj =3D virStoragePoolObjAssignDef(pools, def, false))) { virStoragePoolDefFree(def); return NULL; } @@ -1248,7 +1263,7 @@ virStoragePoolObjLoadState(virStoragePoolObjListPtr p= ools, } =20 /* create the object */ - if (!(obj =3D virStoragePoolObjAssignDef(pools, def))) + if (!(obj =3D virStoragePoolObjAssignDef(pools, def, true))) goto error; =20 /* XXX: future handling of some additional useful status data, diff --git a/src/conf/virstorageobj.h b/src/conf/virstorageobj.h index dd7001c4b2..9fabf34e4f 100644 --- a/src/conf/virstorageobj.h +++ b/src/conf/virstorageobj.h @@ -189,7 +189,8 @@ virStoragePoolObjVolumeListExport(virConnectPtr conn, =20 virStoragePoolObjPtr virStoragePoolObjAssignDef(virStoragePoolObjListPtr pools, - virStoragePoolDefPtr def); + virStoragePoolDefPtr def, + bool check_active); =20 int virStoragePoolObjSaveDef(virStorageDriverStatePtr driver, @@ -244,11 +245,6 @@ void virStoragePoolObjRemove(virStoragePoolObjListPtr pools, virStoragePoolObjPtr obj); =20 -int -virStoragePoolObjIsDuplicate(virStoragePoolObjListPtr pools, - virStoragePoolDefPtr def, - bool check_active); - int virStoragePoolObjSourceFindDuplicate(virConnectPtr conn, virStoragePoolObjListPtr pools, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index ca4a192a4a..572d1a1e22 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1140,7 +1140,6 @@ virStoragePoolObjGetVolumesCount; virStoragePoolObjIncrAsyncjobs; virStoragePoolObjIsActive; virStoragePoolObjIsAutostart; -virStoragePoolObjIsDuplicate; virStoragePoolObjListExport; virStoragePoolObjListForEach; virStoragePoolObjListNew; diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index c108f026ce..18a67ec8ac 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -705,16 +705,13 @@ storagePoolCreateXML(virConnectPtr conn, if (virStoragePoolCreateXMLEnsureACL(conn, newDef) < 0) goto cleanup; =20 - if (virStoragePoolObjIsDuplicate(driver->pools, newDef, true) < 0) - goto cleanup; - if (virStoragePoolObjSourceFindDuplicate(conn, driver->pools, newDef) = < 0) goto cleanup; =20 if ((backend =3D virStorageBackendForType(newDef->type)) =3D=3D NULL) goto cleanup; =20 - if (!(obj =3D virStoragePoolObjAssignDef(driver->pools, newDef))) + if (!(obj =3D virStoragePoolObjAssignDef(driver->pools, newDef, true))) goto cleanup; newDef =3D NULL; def =3D virStoragePoolObjGetDef(obj); @@ -799,16 +796,13 @@ storagePoolDefineXML(virConnectPtr conn, if (virStoragePoolDefineXMLEnsureACL(conn, newDef) < 0) goto cleanup; =20 - if (virStoragePoolObjIsDuplicate(driver->pools, newDef, false) < 0) - goto cleanup; - if (virStoragePoolObjSourceFindDuplicate(conn, driver->pools, newDef) = < 0) goto cleanup; =20 if (virStorageBackendForType(newDef->type) =3D=3D NULL) goto cleanup; =20 - if (!(obj =3D virStoragePoolObjAssignDef(driver->pools, newDef))) + if (!(obj =3D virStoragePoolObjAssignDef(driver->pools, newDef, false)= )) goto cleanup; newDef =3D virStoragePoolObjGetNewDef(obj); def =3D virStoragePoolObjGetDef(obj); diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 6697a7dfe6..c1f31b461c 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1108,7 +1108,7 @@ testParseStorage(testDriverPtr privconn, if (!def) goto error; =20 - if (!(obj =3D virStoragePoolObjAssignDef(privconn->pools, def))) { + if (!(obj =3D virStoragePoolObjAssignDef(privconn->pools, def, fal= se))) { virStoragePoolDefFree(def); goto error; } @@ -4515,10 +4515,7 @@ testStoragePoolCreateXML(virConnectPtr conn, if (!(newDef =3D virStoragePoolDefParseString(xml))) goto cleanup; =20 - if (virStoragePoolObjIsDuplicate(privconn->pools, newDef, true) < 0) - goto cleanup; - - if (!(obj =3D virStoragePoolObjAssignDef(privconn->pools, newDef))) + if (!(obj =3D virStoragePoolObjAssignDef(privconn->pools, newDef, true= ))) goto cleanup; newDef =3D NULL; def =3D virStoragePoolObjGetDef(obj); @@ -4589,10 +4586,7 @@ testStoragePoolDefineXML(virConnectPtr conn, newDef->allocation =3D defaultPoolAlloc; newDef->available =3D defaultPoolCap - defaultPoolAlloc; =20 - if (virStoragePoolObjIsDuplicate(privconn->pools, newDef, false) < 0) - goto cleanup; - - if (!(obj =3D virStoragePoolObjAssignDef(privconn->pools, newDef))) + if (!(obj =3D virStoragePoolObjAssignDef(privconn->pools, newDef, fals= e))) goto cleanup; newDef =3D NULL; def =3D virStoragePoolObjGetDef(obj); --=20 2.16.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list From nobody Mon Apr 29 07:29:34 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1534767026810347.0999372384191; Mon, 20 Aug 2018 05:10:26 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 303923086262; Mon, 20 Aug 2018 12:10:24 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id C8D1B309128B; Mon, 20 Aug 2018 12:10:23 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 7AD1E4BB75; Mon, 20 Aug 2018 12:10:23 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id w7KCAHnJ024616 for ; Mon, 20 Aug 2018 08:10:17 -0400 Received: by smtp.corp.redhat.com (Postfix) id 33CC42166BDB; Mon, 20 Aug 2018 12:10:17 +0000 (UTC) Received: from localhost.localdomain (ovpn-204-49.brq.redhat.com [10.40.204.49]) by smtp.corp.redhat.com (Postfix) with ESMTP id 8A1482166BA1 for ; Mon, 20 Aug 2018 12:10:16 +0000 (UTC) From: Michal Privoznik To: libvir-list@redhat.com Date: Mon, 20 Aug 2018 14:09:47 +0200 Message-Id: In-Reply-To: References: In-Reply-To: References: X-Scanned-By: MIMEDefang 2.78 on 10.11.54.6 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH v2 4/8] virstorageobj: Move virStoragePoolObjSourceFindDuplicate and friends up X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.84 on 10.5.11.26 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.49]); Mon, 20 Aug 2018 12:10:25 +0000 (UTC) X-ZohoMail: RDMRC_0 RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" This function is going to be made static in used in virStoragePoolObjAssignDef(). Therefore move it and all the static functions it calls a few lines up. Signed-off-by: Michal Privoznik Reviewed-by: John Ferlan --- src/conf/virstorageobj.c | 788 +++++++++++++++++++++++--------------------= ---- 1 file changed, 394 insertions(+), 394 deletions(-) diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c index ea0ae6fd86..b710ec652d 100644 --- a/src/conf/virstorageobj.c +++ b/src/conf/virstorageobj.c @@ -1118,6 +1118,400 @@ virStoragePoolObjIsDuplicate(virStoragePoolObjListP= tr pools, } =20 =20 +static int +getSCSIHostNumber(virStorageAdapterSCSIHostPtr scsi_host, + unsigned int *hostnum) +{ + int ret =3D -1; + unsigned int num; + char *name =3D NULL; + + if (scsi_host->has_parent) { + virPCIDeviceAddressPtr addr =3D &scsi_host->parentaddr; + unsigned int unique_id =3D scsi_host->unique_id; + + if (!(name =3D virSCSIHostGetNameByParentaddr(addr->domain, + addr->bus, + addr->slot, + addr->function, + unique_id))) + goto cleanup; + if (virSCSIHostGetNumber(name, &num) < 0) + goto cleanup; + } else { + if (virSCSIHostGetNumber(scsi_host->name, &num) < 0) + goto cleanup; + } + + *hostnum =3D num; + ret =3D 0; + + cleanup: + VIR_FREE(name); + return ret; +} + + +static bool +virStorageIsSameHostnum(const char *name, + unsigned int scsi_hostnum) +{ + unsigned int fc_hostnum; + + if (virSCSIHostGetNumber(name, &fc_hostnum) =3D=3D 0 && + scsi_hostnum =3D=3D fc_hostnum) + return true; + + return false; +} + + +/* + * matchFCHostToSCSIHost: + * + * @conn: Connection pointer + * @fchost: fc_host adapter ptr (either def or pool->def) + * @scsi_hostnum: Already determined "scsi_pool" hostnum + * + * Returns true/false whether there is a match between the incoming + * fc_adapter host# and the scsi_host host# + */ +static bool +matchFCHostToSCSIHost(virConnectPtr conn, + virStorageAdapterFCHostPtr fchost, + unsigned int scsi_hostnum) +{ + bool ret =3D false; + char *name =3D NULL; + char *scsi_host_name =3D NULL; + char *parent_name =3D NULL; + + /* If we have a parent defined, get its hostnum, and compare to the + * scsi_hostnum. If they are the same, then we have a match + */ + if (fchost->parent && + virStorageIsSameHostnum(fchost->parent, scsi_hostnum)) + return true; + + /* If we find an fc adapter name, then either libvirt created a vHBA + * for this fc_host or a 'virsh nodedev-create' generated a vHBA. + */ + if ((name =3D virVHBAGetHostByWWN(NULL, fchost->wwnn, fchost->wwpn))) { + + /* Get the scsi_hostN for the vHBA in order to see if it + * matches our scsi_hostnum + */ + if (virStorageIsSameHostnum(name, scsi_hostnum)) { + ret =3D true; + goto cleanup; + } + + /* We weren't provided a parent, so we have to query the node + * device driver in order to ascertain the parent of the vHBA. + * If the parent fc_hostnum is the same as the scsi_hostnum, we + * have a match. + */ + if (conn && !fchost->parent) { + if (virAsprintf(&scsi_host_name, "scsi_%s", name) < 0) + goto cleanup; + if ((parent_name =3D virNodeDeviceGetParentName(conn, + scsi_host_name))= ) { + if (virStorageIsSameHostnum(parent_name, scsi_hostnum)) { + ret =3D true; + goto cleanup; + } + } else { + /* Throw away the error and fall through */ + virResetLastError(); + VIR_DEBUG("Could not determine parent vHBA"); + } + } + } + + /* NB: Lack of a name means that this vHBA hasn't yet been created, + * which means our scsi_host cannot be using the vHBA. Furthermore, + * lack of a provided parent means libvirt is going to choose the + * "best" fc_host capable adapter based on availabilty. That could + * conflict with an existing scsi_host definition, but there's no + * way to know that now. + */ + + cleanup: + VIR_FREE(name); + VIR_FREE(parent_name); + VIR_FREE(scsi_host_name); + return ret; +} + + +static bool +matchSCSIAdapterParent(virStorageAdapterSCSIHostPtr pool_scsi_host, + virStorageAdapterSCSIHostPtr def_scsi_host) +{ + virPCIDeviceAddressPtr pooladdr =3D &pool_scsi_host->parentaddr; + virPCIDeviceAddressPtr defaddr =3D &def_scsi_host->parentaddr; + + if (pooladdr->domain =3D=3D defaddr->domain && + pooladdr->bus =3D=3D defaddr->bus && + pooladdr->slot =3D=3D defaddr->slot && + pooladdr->function =3D=3D defaddr->function && + pool_scsi_host->unique_id =3D=3D def_scsi_host->unique_id) + return true; + + return false; +} + + +static bool +virStoragePoolSourceMatchSingleHost(virStoragePoolSourcePtr poolsrc, + virStoragePoolSourcePtr defsrc) +{ + if (poolsrc->nhost !=3D 1 && defsrc->nhost !=3D 1) + return false; + + if (defsrc->hosts[0].port && + poolsrc->hosts[0].port !=3D defsrc->hosts[0].port) + return false; + + return STREQ(poolsrc->hosts[0].name, defsrc->hosts[0].name); +} + + +static bool +virStoragePoolSourceISCSIMatch(virStoragePoolObjPtr obj, + virStoragePoolDefPtr def) +{ + virStoragePoolSourcePtr poolsrc =3D &obj->def->source; + virStoragePoolSourcePtr defsrc =3D &def->source; + + /* NB: Do not check the source host name */ + if (STRNEQ_NULLABLE(poolsrc->initiator.iqn, defsrc->initiator.iqn)) + return false; + + return true; +} + + +static virStoragePoolObjPtr +virStoragePoolObjSourceMatchTypeDIR(virStoragePoolObjPtr obj, + virStoragePoolDefPtr def) +{ + if (obj->def->type =3D=3D VIR_STORAGE_POOL_DIR) { + if (STREQ(obj->def->target.path, def->target.path)) + return obj; + } else if (obj->def->type =3D=3D VIR_STORAGE_POOL_GLUSTER) { + if (STREQ(obj->def->source.name, def->source.name) && + STREQ_NULLABLE(obj->def->source.dir, def->source.dir) && + virStoragePoolSourceMatchSingleHost(&obj->def->source, + &def->source)) + return obj; + } else if (obj->def->type =3D=3D VIR_STORAGE_POOL_NETFS) { + if (STREQ(obj->def->source.dir, def->source.dir) && + virStoragePoolSourceMatchSingleHost(&obj->def->source, + &def->source)) + return obj; + } + + return NULL; +} + + +static virStoragePoolObjPtr +virStoragePoolObjSourceMatchTypeISCSI(virStoragePoolObjPtr obj, + virStoragePoolDefPtr def, + virConnectPtr conn) +{ + virStorageAdapterPtr pool_adapter =3D &obj->def->source.adapter; + virStorageAdapterPtr def_adapter =3D &def->source.adapter; + virStorageAdapterSCSIHostPtr pool_scsi_host; + virStorageAdapterSCSIHostPtr def_scsi_host; + virStorageAdapterFCHostPtr pool_fchost; + virStorageAdapterFCHostPtr def_fchost; + unsigned int pool_hostnum; + unsigned int def_hostnum; + unsigned int scsi_hostnum; + + if (pool_adapter->type =3D=3D VIR_STORAGE_ADAPTER_TYPE_FC_HOST && + def_adapter->type =3D=3D VIR_STORAGE_ADAPTER_TYPE_FC_HOST) { + pool_fchost =3D &pool_adapter->data.fchost; + def_fchost =3D &def_adapter->data.fchost; + + if (STREQ(pool_fchost->wwnn, def_fchost->wwnn) && + STREQ(pool_fchost->wwpn, def_fchost->wwpn)) + return obj; + } else if (pool_adapter->type =3D=3D VIR_STORAGE_ADAPTER_TYPE_SCSI_HOS= T && + def_adapter->type =3D=3D VIR_STORAGE_ADAPTER_TYPE_SCSI_HOST= ) { + pool_scsi_host =3D &pool_adapter->data.scsi_host; + def_scsi_host =3D &def_adapter->data.scsi_host; + + if (pool_scsi_host->has_parent && + def_scsi_host->has_parent && + matchSCSIAdapterParent(pool_scsi_host, def_scsi_host)) + return obj; + + if (getSCSIHostNumber(pool_scsi_host, &pool_hostnum) < 0 || + getSCSIHostNumber(def_scsi_host, &def_hostnum) < 0) + return NULL; + if (pool_hostnum =3D=3D def_hostnum) + return obj; + } else if (pool_adapter->type =3D=3D VIR_STORAGE_ADAPTER_TYPE_FC_HOST = && + def_adapter->type =3D=3D VIR_STORAGE_ADAPTER_TYPE_SCSI_HOST= ) { + pool_fchost =3D &pool_adapter->data.fchost; + def_scsi_host =3D &def_adapter->data.scsi_host; + + /* Get the scsi_hostN for the scsi_host source adapter def */ + if (getSCSIHostNumber(def_scsi_host, &scsi_hostnum) < 0) + return NULL; + + if (matchFCHostToSCSIHost(conn, pool_fchost, scsi_hostnum)) + return obj; + + } else if (pool_adapter->type =3D=3D VIR_STORAGE_ADAPTER_TYPE_SCSI_HOS= T && + def_adapter->type =3D=3D VIR_STORAGE_ADAPTER_TYPE_FC_HOST) { + pool_scsi_host =3D &pool_adapter->data.scsi_host; + def_fchost =3D &def_adapter->data.fchost; + + if (getSCSIHostNumber(pool_scsi_host, &scsi_hostnum) < 0) + return NULL; + + if (matchFCHostToSCSIHost(conn, def_fchost, scsi_hostnum)) + return obj; + } + + return NULL; +} + + +static virStoragePoolObjPtr +virStoragePoolObjSourceMatchTypeDEVICE(virStoragePoolObjPtr obj, + virStoragePoolDefPtr def) +{ + virStoragePoolObjPtr matchobj =3D NULL; + + if (obj->def->type =3D=3D VIR_STORAGE_POOL_ISCSI) { + if (def->type !=3D VIR_STORAGE_POOL_ISCSI) + return NULL; + + if ((matchobj =3D virStoragePoolSourceFindDuplicateDevices(obj, de= f))) { + if (!virStoragePoolSourceISCSIMatch(matchobj, def)) + return NULL; + } + return matchobj; + } + + if (def->type =3D=3D VIR_STORAGE_POOL_ISCSI) + return NULL; + + /* VIR_STORAGE_POOL_FS + * VIR_STORAGE_POOL_LOGICAL + * VIR_STORAGE_POOL_DISK + * VIR_STORAGE_POOL_ZFS */ + return virStoragePoolSourceFindDuplicateDevices(obj, def); +} + + +struct _virStoragePoolObjFindDuplicateData { + virConnectPtr conn; + virStoragePoolDefPtr def; +}; + +static int +virStoragePoolObjSourceFindDuplicateCb(const void *payload, + const void *name ATTRIBUTE_UNUSED, + const void *opaque) +{ + virStoragePoolObjPtr obj =3D (virStoragePoolObjPtr) payload; + struct _virStoragePoolObjFindDuplicateData *data =3D + (struct _virStoragePoolObjFindDuplicateData *) opaque; + + /* Don't match against ourself if re-defining existing pool ! */ + if (STREQ(obj->def->name, data->def->name)) + return 0; + + switch ((virStoragePoolType)obj->def->type) { + case VIR_STORAGE_POOL_DIR: + case VIR_STORAGE_POOL_GLUSTER: + case VIR_STORAGE_POOL_NETFS: + if (data->def->type =3D=3D obj->def->type && + virStoragePoolObjSourceMatchTypeDIR(obj, data->def)) + return 1; + break; + + case VIR_STORAGE_POOL_SCSI: + if (data->def->type =3D=3D obj->def->type && + virStoragePoolObjSourceMatchTypeISCSI(obj, data->def, data->co= nn)) + return 1; + break; + + case VIR_STORAGE_POOL_ISCSI: + case VIR_STORAGE_POOL_ISCSI_DIRECT: + case VIR_STORAGE_POOL_FS: + case VIR_STORAGE_POOL_LOGICAL: + case VIR_STORAGE_POOL_DISK: + case VIR_STORAGE_POOL_ZFS: + if ((data->def->type =3D=3D VIR_STORAGE_POOL_ISCSI || + data->def->type =3D=3D VIR_STORAGE_POOL_ISCSI_DIRECT || + data->def->type =3D=3D VIR_STORAGE_POOL_FS || + data->def->type =3D=3D VIR_STORAGE_POOL_LOGICAL || + data->def->type =3D=3D VIR_STORAGE_POOL_DISK || + data->def->type =3D=3D VIR_STORAGE_POOL_ZFS) && + virStoragePoolObjSourceMatchTypeDEVICE(obj, data->def)) + return 1; + break; + + case VIR_STORAGE_POOL_SHEEPDOG: + if (data->def->type =3D=3D obj->def->type && + virStoragePoolSourceMatchSingleHost(&obj->def->source, + &data->def->source)) + return 1; + break; + + case VIR_STORAGE_POOL_MPATH: + /* Only one mpath pool is valid per host */ + if (data->def->type =3D=3D obj->def->type) + return 1; + break; + + case VIR_STORAGE_POOL_VSTORAGE: + if (data->def->type =3D=3D obj->def->type && + STREQ(obj->def->source.name, data->def->source.name)) + return 1; + break; + + case VIR_STORAGE_POOL_RBD: + case VIR_STORAGE_POOL_LAST: + break; + } + + return 0; +} + + +int +virStoragePoolObjSourceFindDuplicate(virConnectPtr conn, + virStoragePoolObjListPtr pools, + virStoragePoolDefPtr def) +{ + struct _virStoragePoolObjFindDuplicateData data =3D { .conn =3D conn, + .def =3D def }; + virStoragePoolObjPtr obj =3D NULL; + + virObjectRWLockRead(pools); + obj =3D virHashSearch(pools->objs, virStoragePoolObjSourceFindDuplicat= eCb, + &data, NULL); + virObjectRWUnlock(pools); + + if (obj) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("Storage source conflict with pool: '%s'"), + obj->def->name); + return -1; + } + + return 0; +} + + /** * virStoragePoolObjAssignDef: * @pools: Storage Pool object list pointer @@ -1529,400 +1923,6 @@ virStoragePoolObjGetNames(virStoragePoolObjListPtr = pools, } =20 =20 -static int -getSCSIHostNumber(virStorageAdapterSCSIHostPtr scsi_host, - unsigned int *hostnum) -{ - int ret =3D -1; - unsigned int num; - char *name =3D NULL; - - if (scsi_host->has_parent) { - virPCIDeviceAddressPtr addr =3D &scsi_host->parentaddr; - unsigned int unique_id =3D scsi_host->unique_id; - - if (!(name =3D virSCSIHostGetNameByParentaddr(addr->domain, - addr->bus, - addr->slot, - addr->function, - unique_id))) - goto cleanup; - if (virSCSIHostGetNumber(name, &num) < 0) - goto cleanup; - } else { - if (virSCSIHostGetNumber(scsi_host->name, &num) < 0) - goto cleanup; - } - - *hostnum =3D num; - ret =3D 0; - - cleanup: - VIR_FREE(name); - return ret; -} - - -static bool -virStorageIsSameHostnum(const char *name, - unsigned int scsi_hostnum) -{ - unsigned int fc_hostnum; - - if (virSCSIHostGetNumber(name, &fc_hostnum) =3D=3D 0 && - scsi_hostnum =3D=3D fc_hostnum) - return true; - - return false; -} - - -/* - * matchFCHostToSCSIHost: - * - * @conn: Connection pointer - * @fchost: fc_host adapter ptr (either def or pool->def) - * @scsi_hostnum: Already determined "scsi_pool" hostnum - * - * Returns true/false whether there is a match between the incoming - * fc_adapter host# and the scsi_host host# - */ -static bool -matchFCHostToSCSIHost(virConnectPtr conn, - virStorageAdapterFCHostPtr fchost, - unsigned int scsi_hostnum) -{ - bool ret =3D false; - char *name =3D NULL; - char *scsi_host_name =3D NULL; - char *parent_name =3D NULL; - - /* If we have a parent defined, get its hostnum, and compare to the - * scsi_hostnum. If they are the same, then we have a match - */ - if (fchost->parent && - virStorageIsSameHostnum(fchost->parent, scsi_hostnum)) - return true; - - /* If we find an fc adapter name, then either libvirt created a vHBA - * for this fc_host or a 'virsh nodedev-create' generated a vHBA. - */ - if ((name =3D virVHBAGetHostByWWN(NULL, fchost->wwnn, fchost->wwpn))) { - - /* Get the scsi_hostN for the vHBA in order to see if it - * matches our scsi_hostnum - */ - if (virStorageIsSameHostnum(name, scsi_hostnum)) { - ret =3D true; - goto cleanup; - } - - /* We weren't provided a parent, so we have to query the node - * device driver in order to ascertain the parent of the vHBA. - * If the parent fc_hostnum is the same as the scsi_hostnum, we - * have a match. - */ - if (conn && !fchost->parent) { - if (virAsprintf(&scsi_host_name, "scsi_%s", name) < 0) - goto cleanup; - if ((parent_name =3D virNodeDeviceGetParentName(conn, - scsi_host_name))= ) { - if (virStorageIsSameHostnum(parent_name, scsi_hostnum)) { - ret =3D true; - goto cleanup; - } - } else { - /* Throw away the error and fall through */ - virResetLastError(); - VIR_DEBUG("Could not determine parent vHBA"); - } - } - } - - /* NB: Lack of a name means that this vHBA hasn't yet been created, - * which means our scsi_host cannot be using the vHBA. Furthermore, - * lack of a provided parent means libvirt is going to choose the - * "best" fc_host capable adapter based on availabilty. That could - * conflict with an existing scsi_host definition, but there's no - * way to know that now. - */ - - cleanup: - VIR_FREE(name); - VIR_FREE(parent_name); - VIR_FREE(scsi_host_name); - return ret; -} - - -static bool -matchSCSIAdapterParent(virStorageAdapterSCSIHostPtr pool_scsi_host, - virStorageAdapterSCSIHostPtr def_scsi_host) -{ - virPCIDeviceAddressPtr pooladdr =3D &pool_scsi_host->parentaddr; - virPCIDeviceAddressPtr defaddr =3D &def_scsi_host->parentaddr; - - if (pooladdr->domain =3D=3D defaddr->domain && - pooladdr->bus =3D=3D defaddr->bus && - pooladdr->slot =3D=3D defaddr->slot && - pooladdr->function =3D=3D defaddr->function && - pool_scsi_host->unique_id =3D=3D def_scsi_host->unique_id) - return true; - - return false; -} - - -static bool -virStoragePoolSourceMatchSingleHost(virStoragePoolSourcePtr poolsrc, - virStoragePoolSourcePtr defsrc) -{ - if (poolsrc->nhost !=3D 1 && defsrc->nhost !=3D 1) - return false; - - if (defsrc->hosts[0].port && - poolsrc->hosts[0].port !=3D defsrc->hosts[0].port) - return false; - - return STREQ(poolsrc->hosts[0].name, defsrc->hosts[0].name); -} - - -static bool -virStoragePoolSourceISCSIMatch(virStoragePoolObjPtr obj, - virStoragePoolDefPtr def) -{ - virStoragePoolSourcePtr poolsrc =3D &obj->def->source; - virStoragePoolSourcePtr defsrc =3D &def->source; - - /* NB: Do not check the source host name */ - if (STRNEQ_NULLABLE(poolsrc->initiator.iqn, defsrc->initiator.iqn)) - return false; - - return true; -} - - -static virStoragePoolObjPtr -virStoragePoolObjSourceMatchTypeDIR(virStoragePoolObjPtr obj, - virStoragePoolDefPtr def) -{ - if (obj->def->type =3D=3D VIR_STORAGE_POOL_DIR) { - if (STREQ(obj->def->target.path, def->target.path)) - return obj; - } else if (obj->def->type =3D=3D VIR_STORAGE_POOL_GLUSTER) { - if (STREQ(obj->def->source.name, def->source.name) && - STREQ_NULLABLE(obj->def->source.dir, def->source.dir) && - virStoragePoolSourceMatchSingleHost(&obj->def->source, - &def->source)) - return obj; - } else if (obj->def->type =3D=3D VIR_STORAGE_POOL_NETFS) { - if (STREQ(obj->def->source.dir, def->source.dir) && - virStoragePoolSourceMatchSingleHost(&obj->def->source, - &def->source)) - return obj; - } - - return NULL; -} - - -static virStoragePoolObjPtr -virStoragePoolObjSourceMatchTypeISCSI(virStoragePoolObjPtr obj, - virStoragePoolDefPtr def, - virConnectPtr conn) -{ - virStorageAdapterPtr pool_adapter =3D &obj->def->source.adapter; - virStorageAdapterPtr def_adapter =3D &def->source.adapter; - virStorageAdapterSCSIHostPtr pool_scsi_host; - virStorageAdapterSCSIHostPtr def_scsi_host; - virStorageAdapterFCHostPtr pool_fchost; - virStorageAdapterFCHostPtr def_fchost; - unsigned int pool_hostnum; - unsigned int def_hostnum; - unsigned int scsi_hostnum; - - if (pool_adapter->type =3D=3D VIR_STORAGE_ADAPTER_TYPE_FC_HOST && - def_adapter->type =3D=3D VIR_STORAGE_ADAPTER_TYPE_FC_HOST) { - pool_fchost =3D &pool_adapter->data.fchost; - def_fchost =3D &def_adapter->data.fchost; - - if (STREQ(pool_fchost->wwnn, def_fchost->wwnn) && - STREQ(pool_fchost->wwpn, def_fchost->wwpn)) - return obj; - } else if (pool_adapter->type =3D=3D VIR_STORAGE_ADAPTER_TYPE_SCSI_HOS= T && - def_adapter->type =3D=3D VIR_STORAGE_ADAPTER_TYPE_SCSI_HOST= ) { - pool_scsi_host =3D &pool_adapter->data.scsi_host; - def_scsi_host =3D &def_adapter->data.scsi_host; - - if (pool_scsi_host->has_parent && - def_scsi_host->has_parent && - matchSCSIAdapterParent(pool_scsi_host, def_scsi_host)) - return obj; - - if (getSCSIHostNumber(pool_scsi_host, &pool_hostnum) < 0 || - getSCSIHostNumber(def_scsi_host, &def_hostnum) < 0) - return NULL; - if (pool_hostnum =3D=3D def_hostnum) - return obj; - } else if (pool_adapter->type =3D=3D VIR_STORAGE_ADAPTER_TYPE_FC_HOST = && - def_adapter->type =3D=3D VIR_STORAGE_ADAPTER_TYPE_SCSI_HOST= ) { - pool_fchost =3D &pool_adapter->data.fchost; - def_scsi_host =3D &def_adapter->data.scsi_host; - - /* Get the scsi_hostN for the scsi_host source adapter def */ - if (getSCSIHostNumber(def_scsi_host, &scsi_hostnum) < 0) - return NULL; - - if (matchFCHostToSCSIHost(conn, pool_fchost, scsi_hostnum)) - return obj; - - } else if (pool_adapter->type =3D=3D VIR_STORAGE_ADAPTER_TYPE_SCSI_HOS= T && - def_adapter->type =3D=3D VIR_STORAGE_ADAPTER_TYPE_FC_HOST) { - pool_scsi_host =3D &pool_adapter->data.scsi_host; - def_fchost =3D &def_adapter->data.fchost; - - if (getSCSIHostNumber(pool_scsi_host, &scsi_hostnum) < 0) - return NULL; - - if (matchFCHostToSCSIHost(conn, def_fchost, scsi_hostnum)) - return obj; - } - - return NULL; -} - - -static virStoragePoolObjPtr -virStoragePoolObjSourceMatchTypeDEVICE(virStoragePoolObjPtr obj, - virStoragePoolDefPtr def) -{ - virStoragePoolObjPtr matchobj =3D NULL; - - if (obj->def->type =3D=3D VIR_STORAGE_POOL_ISCSI) { - if (def->type !=3D VIR_STORAGE_POOL_ISCSI) - return NULL; - - if ((matchobj =3D virStoragePoolSourceFindDuplicateDevices(obj, de= f))) { - if (!virStoragePoolSourceISCSIMatch(matchobj, def)) - return NULL; - } - return matchobj; - } - - if (def->type =3D=3D VIR_STORAGE_POOL_ISCSI) - return NULL; - - /* VIR_STORAGE_POOL_FS - * VIR_STORAGE_POOL_LOGICAL - * VIR_STORAGE_POOL_DISK - * VIR_STORAGE_POOL_ZFS */ - return virStoragePoolSourceFindDuplicateDevices(obj, def); -} - - -struct _virStoragePoolObjFindDuplicateData { - virConnectPtr conn; - virStoragePoolDefPtr def; -}; - -static int -virStoragePoolObjSourceFindDuplicateCb(const void *payload, - const void *name ATTRIBUTE_UNUSED, - const void *opaque) -{ - virStoragePoolObjPtr obj =3D (virStoragePoolObjPtr) payload; - struct _virStoragePoolObjFindDuplicateData *data =3D - (struct _virStoragePoolObjFindDuplicateData *) opaque; - - /* Don't match against ourself if re-defining existing pool ! */ - if (STREQ(obj->def->name, data->def->name)) - return 0; - - switch ((virStoragePoolType)obj->def->type) { - case VIR_STORAGE_POOL_DIR: - case VIR_STORAGE_POOL_GLUSTER: - case VIR_STORAGE_POOL_NETFS: - if (data->def->type =3D=3D obj->def->type && - virStoragePoolObjSourceMatchTypeDIR(obj, data->def)) - return 1; - break; - - case VIR_STORAGE_POOL_SCSI: - if (data->def->type =3D=3D obj->def->type && - virStoragePoolObjSourceMatchTypeISCSI(obj, data->def, data->co= nn)) - return 1; - break; - - case VIR_STORAGE_POOL_ISCSI: - case VIR_STORAGE_POOL_ISCSI_DIRECT: - case VIR_STORAGE_POOL_FS: - case VIR_STORAGE_POOL_LOGICAL: - case VIR_STORAGE_POOL_DISK: - case VIR_STORAGE_POOL_ZFS: - if ((data->def->type =3D=3D VIR_STORAGE_POOL_ISCSI || - data->def->type =3D=3D VIR_STORAGE_POOL_ISCSI_DIRECT || - data->def->type =3D=3D VIR_STORAGE_POOL_FS || - data->def->type =3D=3D VIR_STORAGE_POOL_LOGICAL || - data->def->type =3D=3D VIR_STORAGE_POOL_DISK || - data->def->type =3D=3D VIR_STORAGE_POOL_ZFS) && - virStoragePoolObjSourceMatchTypeDEVICE(obj, data->def)) - return 1; - break; - - case VIR_STORAGE_POOL_SHEEPDOG: - if (data->def->type =3D=3D obj->def->type && - virStoragePoolSourceMatchSingleHost(&obj->def->source, - &data->def->source)) - return 1; - break; - - case VIR_STORAGE_POOL_MPATH: - /* Only one mpath pool is valid per host */ - if (data->def->type =3D=3D obj->def->type) - return 1; - break; - - case VIR_STORAGE_POOL_VSTORAGE: - if (data->def->type =3D=3D obj->def->type && - STREQ(obj->def->source.name, data->def->source.name)) - return 1; - break; - - case VIR_STORAGE_POOL_RBD: - case VIR_STORAGE_POOL_LAST: - break; - } - - return 0; -} - - -int -virStoragePoolObjSourceFindDuplicate(virConnectPtr conn, - virStoragePoolObjListPtr pools, - virStoragePoolDefPtr def) -{ - struct _virStoragePoolObjFindDuplicateData data =3D { .conn =3D conn, - .def =3D def }; - virStoragePoolObjPtr obj =3D NULL; - - virObjectRWLockRead(pools); - obj =3D virHashSearch(pools->objs, virStoragePoolObjSourceFindDuplicat= eCb, - &data, NULL); - virObjectRWUnlock(pools); - - if (obj) { - virReportError(VIR_ERR_OPERATION_FAILED, - _("Storage source conflict with pool: '%s'"), - obj->def->name); - return -1; - } - - return 0; -} - - #define MATCH(FLAG) (flags & (FLAG)) static bool virStoragePoolObjMatch(virStoragePoolObjPtr obj, --=20 2.16.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list From nobody Mon Apr 29 07:29:34 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 15347671690411015.8752938675356; Mon, 20 Aug 2018 05:12:49 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 0204683F4C; Mon, 20 Aug 2018 12:12:47 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id B90009D660; Mon, 20 Aug 2018 12:12:46 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 7219518005CF; Mon, 20 Aug 2018 12:12:46 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id w7KCAIan024629 for ; Mon, 20 Aug 2018 08:10:19 -0400 Received: by smtp.corp.redhat.com (Postfix) id D094B2166BDB; Mon, 20 Aug 2018 12:10:18 +0000 (UTC) Received: from localhost.localdomain (ovpn-204-49.brq.redhat.com [10.40.204.49]) by smtp.corp.redhat.com (Postfix) with ESMTP id 5A4B22166BA1 for ; Mon, 20 Aug 2018 12:10:17 +0000 (UTC) From: Michal Privoznik To: libvir-list@redhat.com Date: Mon, 20 Aug 2018 14:09:48 +0200 Message-Id: In-Reply-To: References: In-Reply-To: References: X-Scanned-By: MIMEDefang 2.78 on 10.11.54.6 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH v2 5/8] virStoragePoolObjSourceFindDuplicate: Drop @conn argument X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.84 on 10.5.11.27 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Mon, 20 Aug 2018 12:12:47 +0000 (UTC) X-ZohoMail: RDMRC_0 RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" The @conn argument is needed only to do some source matching in case of iSCSI source. Anyway, it's used just for node device driver and as such can be replaced with virGetConnectNodeDev(). At the same time, the @conn struct member is dropped from _virStoragePoolObjFindDuplicateData. Signed-off-by: Michal Privoznik Reviewed-by: John Ferlan --- src/conf/virstorageobj.c | 25 +++++++++++-------------- src/conf/virstorageobj.h | 3 +-- src/storage/storage_driver.c | 4 ++-- 3 files changed, 14 insertions(+), 18 deletions(-) diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c index b710ec652d..dce45ce870 100644 --- a/src/conf/virstorageobj.c +++ b/src/conf/virstorageobj.c @@ -1169,7 +1169,6 @@ virStorageIsSameHostnum(const char *name, /* * matchFCHostToSCSIHost: * - * @conn: Connection pointer * @fchost: fc_host adapter ptr (either def or pool->def) * @scsi_hostnum: Already determined "scsi_pool" hostnum * @@ -1177,10 +1176,10 @@ virStorageIsSameHostnum(const char *name, * fc_adapter host# and the scsi_host host# */ static bool -matchFCHostToSCSIHost(virConnectPtr conn, - virStorageAdapterFCHostPtr fchost, +matchFCHostToSCSIHost(virStorageAdapterFCHostPtr fchost, unsigned int scsi_hostnum) { + virConnectPtr conn =3D NULL; bool ret =3D false; char *name =3D NULL; char *scsi_host_name =3D NULL; @@ -1211,7 +1210,8 @@ matchFCHostToSCSIHost(virConnectPtr conn, * If the parent fc_hostnum is the same as the scsi_hostnum, we * have a match. */ - if (conn && !fchost->parent) { + if (!fchost->parent && + (conn =3D virGetConnectNodeDev())) { if (virAsprintf(&scsi_host_name, "scsi_%s", name) < 0) goto cleanup; if ((parent_name =3D virNodeDeviceGetParentName(conn, @@ -1240,6 +1240,7 @@ matchFCHostToSCSIHost(virConnectPtr conn, VIR_FREE(name); VIR_FREE(parent_name); VIR_FREE(scsi_host_name); + virConnectClose(conn); return ret; } =20 @@ -1318,8 +1319,7 @@ virStoragePoolObjSourceMatchTypeDIR(virStoragePoolObj= Ptr obj, =20 static virStoragePoolObjPtr virStoragePoolObjSourceMatchTypeISCSI(virStoragePoolObjPtr obj, - virStoragePoolDefPtr def, - virConnectPtr conn) + virStoragePoolDefPtr def) { virStorageAdapterPtr pool_adapter =3D &obj->def->source.adapter; virStorageAdapterPtr def_adapter =3D &def->source.adapter; @@ -1363,7 +1363,7 @@ virStoragePoolObjSourceMatchTypeISCSI(virStoragePoolO= bjPtr obj, if (getSCSIHostNumber(def_scsi_host, &scsi_hostnum) < 0) return NULL; =20 - if (matchFCHostToSCSIHost(conn, pool_fchost, scsi_hostnum)) + if (matchFCHostToSCSIHost(pool_fchost, scsi_hostnum)) return obj; =20 } else if (pool_adapter->type =3D=3D VIR_STORAGE_ADAPTER_TYPE_SCSI_HOS= T && @@ -1374,7 +1374,7 @@ virStoragePoolObjSourceMatchTypeISCSI(virStoragePoolO= bjPtr obj, if (getSCSIHostNumber(pool_scsi_host, &scsi_hostnum) < 0) return NULL; =20 - if (matchFCHostToSCSIHost(conn, def_fchost, scsi_hostnum)) + if (matchFCHostToSCSIHost(def_fchost, scsi_hostnum)) return obj; } =20 @@ -1411,7 +1411,6 @@ virStoragePoolObjSourceMatchTypeDEVICE(virStoragePool= ObjPtr obj, =20 =20 struct _virStoragePoolObjFindDuplicateData { - virConnectPtr conn; virStoragePoolDefPtr def; }; =20 @@ -1439,7 +1438,7 @@ virStoragePoolObjSourceFindDuplicateCb(const void *pa= yload, =20 case VIR_STORAGE_POOL_SCSI: if (data->def->type =3D=3D obj->def->type && - virStoragePoolObjSourceMatchTypeISCSI(obj, data->def, data->co= nn)) + virStoragePoolObjSourceMatchTypeISCSI(obj, data->def)) return 1; break; =20 @@ -1488,12 +1487,10 @@ virStoragePoolObjSourceFindDuplicateCb(const void *= payload, =20 =20 int -virStoragePoolObjSourceFindDuplicate(virConnectPtr conn, - virStoragePoolObjListPtr pools, +virStoragePoolObjSourceFindDuplicate(virStoragePoolObjListPtr pools, virStoragePoolDefPtr def) { - struct _virStoragePoolObjFindDuplicateData data =3D { .conn =3D conn, - .def =3D def }; + struct _virStoragePoolObjFindDuplicateData data =3D {.def =3D def}; virStoragePoolObjPtr obj =3D NULL; =20 virObjectRWLockRead(pools); diff --git a/src/conf/virstorageobj.h b/src/conf/virstorageobj.h index 9fabf34e4f..bc24db1928 100644 --- a/src/conf/virstorageobj.h +++ b/src/conf/virstorageobj.h @@ -246,8 +246,7 @@ virStoragePoolObjRemove(virStoragePoolObjListPtr pools, virStoragePoolObjPtr obj); =20 int -virStoragePoolObjSourceFindDuplicate(virConnectPtr conn, - virStoragePoolObjListPtr pools, +virStoragePoolObjSourceFindDuplicate(virStoragePoolObjListPtr pools, virStoragePoolDefPtr def); =20 int diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 18a67ec8ac..df4f86c4bd 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -705,7 +705,7 @@ storagePoolCreateXML(virConnectPtr conn, if (virStoragePoolCreateXMLEnsureACL(conn, newDef) < 0) goto cleanup; =20 - if (virStoragePoolObjSourceFindDuplicate(conn, driver->pools, newDef) = < 0) + if (virStoragePoolObjSourceFindDuplicate(driver->pools, newDef) < 0) goto cleanup; =20 if ((backend =3D virStorageBackendForType(newDef->type)) =3D=3D NULL) @@ -796,7 +796,7 @@ storagePoolDefineXML(virConnectPtr conn, if (virStoragePoolDefineXMLEnsureACL(conn, newDef) < 0) goto cleanup; =20 - if (virStoragePoolObjSourceFindDuplicate(conn, driver->pools, newDef) = < 0) + if (virStoragePoolObjSourceFindDuplicate(driver->pools, newDef) < 0) goto cleanup; =20 if (virStorageBackendForType(newDef->type) =3D=3D NULL) --=20 2.16.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list From nobody Mon Apr 29 07:29:34 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1534767034126103.8514033735944; Mon, 20 Aug 2018 05:10:34 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id EBEC4308218F; Mon, 20 Aug 2018 12:10:30 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id BCAE2309128B; Mon, 20 Aug 2018 12:10:30 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 6B4F34E985; Mon, 20 Aug 2018 12:10:30 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id w7KCAJSb024637 for ; Mon, 20 Aug 2018 08:10:19 -0400 Received: by smtp.corp.redhat.com (Postfix) id A90492166BDB; Mon, 20 Aug 2018 12:10:19 +0000 (UTC) Received: from localhost.localdomain (ovpn-204-49.brq.redhat.com [10.40.204.49]) by smtp.corp.redhat.com (Postfix) with ESMTP id 2A7CB2166BA1 for ; Mon, 20 Aug 2018 12:10:18 +0000 (UTC) From: Michal Privoznik To: libvir-list@redhat.com Date: Mon, 20 Aug 2018 14:09:49 +0200 Message-Id: <5395d786ba1c64a74c4da274b66c875ff2eb572a.1534766849.git.mprivozn@redhat.com> In-Reply-To: References: In-Reply-To: References: X-Scanned-By: MIMEDefang 2.78 on 10.11.54.6 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH v2 6/8] virstorageobj: Check for source duplicates from virStoragePoolObjAssignDef X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.84 on 10.5.11.26 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.47]); Mon, 20 Aug 2018 12:10:32 +0000 (UTC) X-ZohoMail: RDMRC_0 RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" Just like a few commits earlier, checking for pool source duplicates and unlocking pools list afterwards is a buggy pattern. The check must go into virStoragePoolObjAssignDef. Signed-off-by: Michal Privoznik Reviewed-by: John Ferlan --- src/conf/virstorageobj.c | 7 ++++--- src/conf/virstorageobj.h | 4 ---- src/libvirt_private.syms | 1 - src/storage/storage_driver.c | 6 ------ 4 files changed, 4 insertions(+), 14 deletions(-) diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c index dce45ce870..c717176133 100644 --- a/src/conf/virstorageobj.c +++ b/src/conf/virstorageobj.c @@ -1486,17 +1486,15 @@ virStoragePoolObjSourceFindDuplicateCb(const void *= payload, } =20 =20 -int +static int virStoragePoolObjSourceFindDuplicate(virStoragePoolObjListPtr pools, virStoragePoolDefPtr def) { struct _virStoragePoolObjFindDuplicateData data =3D {.def =3D def}; virStoragePoolObjPtr obj =3D NULL; =20 - virObjectRWLockRead(pools); obj =3D virHashSearch(pools->objs, virStoragePoolObjSourceFindDuplicat= eCb, &data, NULL); - virObjectRWUnlock(pools); =20 if (obj) { virReportError(VIR_ERR_OPERATION_FAILED, @@ -1531,6 +1529,9 @@ virStoragePoolObjAssignDef(virStoragePoolObjListPtr p= ools, =20 virObjectRWLockWrite(pools); =20 + if (virStoragePoolObjSourceFindDuplicate(pools, def) < 0) + goto error; + rc =3D virStoragePoolObjIsDuplicate(pools, def, check_active, &obj); =20 if (rc < 0) diff --git a/src/conf/virstorageobj.h b/src/conf/virstorageobj.h index bc24db1928..abd6166d88 100644 --- a/src/conf/virstorageobj.h +++ b/src/conf/virstorageobj.h @@ -245,10 +245,6 @@ void virStoragePoolObjRemove(virStoragePoolObjListPtr pools, virStoragePoolObjPtr obj); =20 -int -virStoragePoolObjSourceFindDuplicate(virStoragePoolObjListPtr pools, - virStoragePoolDefPtr def); - int virStoragePoolObjListExport(virConnectPtr conn, virStoragePoolObjListPtr poolobjs, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 572d1a1e22..e107efedcc 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1157,7 +1157,6 @@ virStoragePoolObjSetActive; virStoragePoolObjSetAutostart; virStoragePoolObjSetConfigFile; virStoragePoolObjSetDef; -virStoragePoolObjSourceFindDuplicate; virStoragePoolObjVolumeGetNames; virStoragePoolObjVolumeListExport; =20 diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index df4f86c4bd..e7085e4773 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -705,9 +705,6 @@ storagePoolCreateXML(virConnectPtr conn, if (virStoragePoolCreateXMLEnsureACL(conn, newDef) < 0) goto cleanup; =20 - if (virStoragePoolObjSourceFindDuplicate(driver->pools, newDef) < 0) - goto cleanup; - if ((backend =3D virStorageBackendForType(newDef->type)) =3D=3D NULL) goto cleanup; =20 @@ -796,9 +793,6 @@ storagePoolDefineXML(virConnectPtr conn, if (virStoragePoolDefineXMLEnsureACL(conn, newDef) < 0) goto cleanup; =20 - if (virStoragePoolObjSourceFindDuplicate(driver->pools, newDef) < 0) - goto cleanup; - if (virStorageBackendForType(newDef->type) =3D=3D NULL) goto cleanup; =20 --=20 2.16.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list From nobody Mon Apr 29 07:29:34 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1534767174813328.1103707207599; Mon, 20 Aug 2018 05:12:54 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 5139585547; Mon, 20 Aug 2018 12:12:52 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 1F6731001918; Mon, 20 Aug 2018 12:12:52 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id B9BEC4BB75; Mon, 20 Aug 2018 12:12:51 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id w7KCAKWN024646 for ; Mon, 20 Aug 2018 08:10:20 -0400 Received: by smtp.corp.redhat.com (Postfix) id 7B27E2166BDB; Mon, 20 Aug 2018 12:10:20 +0000 (UTC) Received: from localhost.localdomain (ovpn-204-49.brq.redhat.com [10.40.204.49]) by smtp.corp.redhat.com (Postfix) with ESMTP id F2A882166BA1 for ; Mon, 20 Aug 2018 12:10:19 +0000 (UTC) From: Michal Privoznik To: libvir-list@redhat.com Date: Mon, 20 Aug 2018 14:09:50 +0200 Message-Id: <2d2957c90180f29814fdb63cf9983cdf598898cc.1534766849.git.mprivozn@redhat.com> In-Reply-To: References: In-Reply-To: References: X-Scanned-By: MIMEDefang 2.78 on 10.11.54.6 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH v2 7/8] storage_driver: Mark volume as 'in use' for some operations X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Mon, 20 Aug 2018 12:12:53 +0000 (UTC) X-ZohoMail: RDMRC_0 RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" There are few operations in the storage driver that read/write data onto volumes. Such operations can take very long time to finish. During that time the storage pool object is locked which has bad performance impacts (other threads can't fetch its XML for instance). This commit prepares the storage driver for releasing the lock during those operations (downloadVol, uploadVol, wipeVol). Signed-off-by: Michal Privoznik Reviewed-by: John Ferlan --- src/storage/storage_driver.c | 34 ++++++++++++++++++++++++++++------ 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index e7085e4773..9edd5df119 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -2176,8 +2176,13 @@ storageVolDownload(virStorageVolPtr vol, goto cleanup; } =20 - ret =3D backend->downloadVol(obj, voldef, stream, - offset, length, flags); + virStoragePoolObjIncrAsyncjobs(obj); + voldef->in_use++; + + ret =3D backend->downloadVol(obj, voldef, stream, offset, length, flag= s); + + voldef->in_use--; + virStoragePoolObjDecrAsyncjobs(obj); =20 cleanup: virStoragePoolObjEndAPI(&obj); @@ -2326,6 +2331,7 @@ storageVolUpload(virStorageVolPtr vol, virStoragePoolDefPtr def; virStorageVolDefPtr voldef =3D NULL; virStorageVolStreamInfoPtr cbdata =3D NULL; + int rc; int ret =3D -1; =20 virCheckFlags(VIR_STORAGE_VOL_UPLOAD_SPARSE_STREAM, -1); @@ -2370,8 +2376,15 @@ storageVolUpload(virStorageVolPtr vol, VIR_STRDUP(cbdata->vol_path, voldef->target.path) < 0) goto cleanup; =20 - if ((ret =3D backend->uploadVol(obj, voldef, stream, - offset, length, flags)) < 0) + virStoragePoolObjIncrAsyncjobs(obj); + voldef->in_use++; + + rc =3D backend->uploadVol(obj, voldef, stream, offset, length, flags); + + voldef->in_use--; + virStoragePoolObjDecrAsyncjobs(obj); + + if (rc < 0) goto cleanup; =20 /* Add cleanup callback - call after uploadVol since the stream @@ -2381,7 +2394,7 @@ storageVolUpload(virStorageVolPtr vol, virStorageVolFDStreamCloseCb, cbdata, NULL); cbdata =3D NULL; - + ret =3D 0; cleanup: virStoragePoolObjEndAPI(&obj); if (cbdata) @@ -2499,6 +2512,7 @@ storageVolWipePattern(virStorageVolPtr vol, virStorageBackendPtr backend; virStoragePoolObjPtr obj =3D NULL; virStorageVolDefPtr voldef =3D NULL; + int rc; int ret =3D -1; =20 virCheckFlags(0, -1); @@ -2538,7 +2552,15 @@ storageVolWipePattern(virStorageVolPtr vol, goto cleanup; } =20 - if (backend->wipeVol(obj, voldef, algorithm, flags) < 0) + virStoragePoolObjIncrAsyncjobs(obj); + voldef->in_use++; + + rc =3D backend->wipeVol(obj, voldef, algorithm, flags); + + voldef->in_use--; + virStoragePoolObjDecrAsyncjobs(obj); + + if (rc < 0) goto cleanup; =20 /* Instead of using the refreshVol, since much changes on the target --=20 2.16.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list From nobody Mon Apr 29 07:29:34 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1534767178906127.40458649855589; Mon, 20 Aug 2018 05:12:58 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 4728F308FC4F; Mon, 20 Aug 2018 12:12:56 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 1472C19034; Mon, 20 Aug 2018 12:12:56 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id BCC97181A12E; Mon, 20 Aug 2018 12:12:55 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id w7KCALrT024651 for ; Mon, 20 Aug 2018 08:10:21 -0400 Received: by smtp.corp.redhat.com (Postfix) id 4DEA72166BDB; Mon, 20 Aug 2018 12:10:21 +0000 (UTC) Received: from localhost.localdomain (ovpn-204-49.brq.redhat.com [10.40.204.49]) by smtp.corp.redhat.com (Postfix) with ESMTP id C8DFC2166BA1 for ; Mon, 20 Aug 2018 12:10:20 +0000 (UTC) From: Michal Privoznik To: libvir-list@redhat.com Date: Mon, 20 Aug 2018 14:09:51 +0200 Message-Id: In-Reply-To: References: In-Reply-To: References: X-Scanned-By: MIMEDefang 2.78 on 10.11.54.6 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH v2 8/8] storage_driver: Release pool object lock for some long running jobs X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.43]); Mon, 20 Aug 2018 12:12:57 +0000 (UTC) X-ZohoMail: RDMRC_0 RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" As advertised in previous commit, there are three APIs that might run for quite some time (because they read/write data from/to a volume) and these three are: downloadVol, uploadVol, wipeVol. Release pool object lock and reacquire it later to allow more concurrency. Signed-off-by: Michal Privoznik Reviewed-by: John Ferlan --- src/storage/storage_backend_iscsi_direct.c | 6 +++++- src/storage/storage_backend_rbd.c | 8 ++++++-- src/storage/storage_driver.c | 6 ++++++ 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/src/storage/storage_backend_iscsi_direct.c b/src/storage/stora= ge_backend_iscsi_direct.c index 0d7d6ba9c3..5c1b251a17 100644 --- a/src/storage/storage_backend_iscsi_direct.c +++ b/src/storage/storage_backend_iscsi_direct.c @@ -693,7 +693,11 @@ virStorageBackenISCSIDirectWipeVol(virStoragePoolObjPt= r pool, =20 virCheckFlags(0, -1); =20 - if (!(iscsi =3D virStorageBackendISCSIDirectSetConnection(pool, NULL))) + virObjectLock(pool); + iscsi =3D virStorageBackendISCSIDirectSetConnection(pool, NULL); + virObjectUnlock(pool); + + if (!iscsi) return -1; =20 switch ((virStorageVolWipeAlgorithm) algorithm) { diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backen= d_rbd.c index c6fb791a81..2cba678b72 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -1200,7 +1200,7 @@ virStorageBackendRBDVolWipe(virStoragePoolObjPtr pool, unsigned int flags) { virStorageBackendRBDStatePtr ptr =3D NULL; - virStoragePoolDefPtr def =3D virStoragePoolObjGetDef(pool); + virStoragePoolDefPtr def; rbd_image_t image =3D NULL; rbd_image_info_t info; uint64_t stripe_count; @@ -1209,9 +1209,13 @@ virStorageBackendRBDVolWipe(virStoragePoolObjPtr poo= l, =20 virCheckFlags(0, -1); =20 + virObjectLock(pool); + def =3D virStoragePoolObjGetDef(pool); VIR_DEBUG("Wiping RBD image %s/%s", def->source.name, vol->name); + ptr =3D virStorageBackendRBDNewState(pool); + virObjectUnlock(pool); =20 - if (!(ptr =3D virStorageBackendRBDNewState(pool))) + if (!ptr) goto cleanup; =20 if ((r =3D rbd_open(ptr->ioctx, vol->name, &image, NULL)) < 0) { diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 9edd5df119..8943df1f84 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -2178,9 +2178,11 @@ storageVolDownload(virStorageVolPtr vol, =20 virStoragePoolObjIncrAsyncjobs(obj); voldef->in_use++; + virObjectUnlock(obj); =20 ret =3D backend->downloadVol(obj, voldef, stream, offset, length, flag= s); =20 + virObjectLock(obj); voldef->in_use--; virStoragePoolObjDecrAsyncjobs(obj); =20 @@ -2378,9 +2380,11 @@ storageVolUpload(virStorageVolPtr vol, =20 virStoragePoolObjIncrAsyncjobs(obj); voldef->in_use++; + virObjectUnlock(obj); =20 rc =3D backend->uploadVol(obj, voldef, stream, offset, length, flags); =20 + virObjectLock(obj); voldef->in_use--; virStoragePoolObjDecrAsyncjobs(obj); =20 @@ -2554,9 +2558,11 @@ storageVolWipePattern(virStorageVolPtr vol, =20 virStoragePoolObjIncrAsyncjobs(obj); voldef->in_use++; + virObjectUnlock(obj); =20 rc =3D backend->wipeVol(obj, voldef, algorithm, flags); =20 + virObjectLock(obj); voldef->in_use--; virStoragePoolObjDecrAsyncjobs(obj); =20 --=20 2.16.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list