From nobody Thu May 15 19:25:25 2025 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 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 15035801761706.651729247114986; Thu, 24 Aug 2017 06:09:36 -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 6F9CC1F57B; Thu, 24 Aug 2017 13:09:34 +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 471F1729C8; Thu, 24 Aug 2017 13:09:34 +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 0C7A83FAD7; Thu, 24 Aug 2017 13:09:34 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id v7OD9DWc027791 for ; Thu, 24 Aug 2017 09:09:13 -0400 Received: by smtp.corp.redhat.com (Postfix) id 3A5E77B9C8; Thu, 24 Aug 2017 13:09:13 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-123-165.rdu2.redhat.com [10.10.123.165]) by smtp.corp.redhat.com (Postfix) with ESMTP id 062FD7B8D4 for ; Thu, 24 Aug 2017 13:09:12 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 6F9CC1F57B Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=libvir-list-bounces@redhat.com From: John Ferlan To: libvir-list@redhat.com Date: Thu, 24 Aug 2017 09:08:57 -0400 Message-Id: <20170824130905.5199-5-jferlan@redhat.com> In-Reply-To: <20170824130905.5199-1-jferlan@redhat.com> References: <20170824130905.5199-1-jferlan@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH v2 04/12] storage: Introduce storage volume add, delete, count APIs 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.30]); Thu, 24 Aug 2017 13:09:35 +0000 (UTC) X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" Create/use virStoragePoolObjAddVol in order to add volumes onto list. Create/use virStoragePoolObjRemoveVol in order to remove volumes from list. Create/use virStoragePoolObjGetVolumesCount to get count of volumes on list. For the storage driver, the logic alters when the volumes.obj list grows to after we've fetched the volobj. This is an optimization of sorts, but also doesn't "needlessly" grow the volumes.objs list and then just decr the count if the virGetStorageVol fails. Signed-off-by: John Ferlan --- src/conf/virstorageobj.c | 37 ++++++++++++++++++++++++ src/conf/virstorageobj.h | 11 +++++++ src/libvirt_private.syms | 3 ++ src/storage/storage_backend_disk.c | 5 ++-- src/storage/storage_backend_gluster.c | 3 +- src/storage/storage_backend_logical.c | 4 +-- src/storage/storage_backend_mpath.c | 3 +- src/storage/storage_backend_rbd.c | 4 +-- src/storage/storage_backend_sheepdog.c | 4 +-- src/storage/storage_backend_zfs.c | 6 ++-- src/storage/storage_driver.c | 53 +++++++++---------------------= ---- src/storage/storage_util.c | 8 ++--- src/test/test_driver.c | 18 ++++-------- 13 files changed, 85 insertions(+), 74 deletions(-) diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c index a9fa190..912c27a 100644 --- a/src/conf/virstorageobj.c +++ b/src/conf/virstorageobj.c @@ -282,6 +282,43 @@ virStoragePoolObjClearVols(virStoragePoolObjPtr obj) } =20 =20 +int +virStoragePoolObjAddVol(virStoragePoolObjPtr obj, + virStorageVolDefPtr voldef) +{ + if (VIR_APPEND_ELEMENT(obj->volumes.objs, obj->volumes.count, voldef) = < 0) + return -1; + return 0; +} + + +void +virStoragePoolObjRemoveVol(virStoragePoolObjPtr obj, + virStorageVolDefPtr voldef) +{ + virStoragePoolDefPtr def =3D virStoragePoolObjGetDef(obj); + size_t i; + + for (i =3D 0; i < obj->volumes.count; i++) { + if (obj->volumes.objs[i] =3D=3D voldef) { + VIR_INFO("Deleting volume '%s' from storage pool '%s'", + voldef->name, def->name); + virStorageVolDefFree(voldef); + + VIR_DELETE_ELEMENT(obj->volumes.objs, i, obj->volumes.count); + return; + } + } +} + + +size_t +virStoragePoolObjGetVolumesCount(virStoragePoolObjPtr obj) +{ + return obj->volumes.count; +} + + virStorageVolDefPtr virStorageVolDefFindByKey(virStoragePoolObjPtr obj, const char *key) diff --git a/src/conf/virstorageobj.h b/src/conf/virstorageobj.h index 401c4d5..d1a1247 100644 --- a/src/conf/virstorageobj.h +++ b/src/conf/virstorageobj.h @@ -136,6 +136,17 @@ virStoragePoolObjPtr virStoragePoolObjFindByName(virStoragePoolObjListPtr pools, const char *name); =20 +int +virStoragePoolObjAddVol(virStoragePoolObjPtr obj, + virStorageVolDefPtr voldef); + +void +virStoragePoolObjRemoveVol(virStoragePoolObjPtr obj, + virStorageVolDefPtr voldef); + +size_t +virStoragePoolObjGetVolumesCount(virStoragePoolObjPtr obj); + virStorageVolDefPtr virStorageVolDefFindByKey(virStoragePoolObjPtr obj, const char *key); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index fc884fa..a35299b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1049,6 +1049,7 @@ virSecretObjSetValueSize; =20 =20 # conf/virstorageobj.h +virStoragePoolObjAddVol; virStoragePoolObjAssignDef; virStoragePoolObjClearVols; virStoragePoolObjDecrAsyncjobs; @@ -1062,6 +1063,7 @@ virStoragePoolObjGetConfigFile; virStoragePoolObjGetDef; virStoragePoolObjGetNames; virStoragePoolObjGetNewDef; +virStoragePoolObjGetVolumesCount; virStoragePoolObjIncrAsyncjobs; virStoragePoolObjIsActive; virStoragePoolObjIsAutostart; @@ -1075,6 +1077,7 @@ virStoragePoolObjNew; virStoragePoolObjNumOfStoragePools; virStoragePoolObjNumOfVolumes; virStoragePoolObjRemove; +virStoragePoolObjRemoveVol; virStoragePoolObjSaveDef; virStoragePoolObjSetActive; virStoragePoolObjSetAutostart; diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backe= nd_disk.c index e8f67bb..0bf5567 100644 --- a/src/storage/storage_backend_disk.c +++ b/src/storage/storage_backend_disk.c @@ -65,8 +65,7 @@ virStorageBackendDiskMakeDataVol(virStoragePoolObjPtr poo= l, if (VIR_ALLOC(vol) < 0) return -1; if (VIR_STRDUP(vol->name, partname) < 0 || - VIR_APPEND_ELEMENT_COPY(pool->volumes.objs, - pool->volumes.count, vol) < 0) { + virStoragePoolObjAddVol(pool, vol) < 0) { virStorageVolDefFree(vol); return -1; } @@ -595,7 +594,7 @@ virStorageBackendDiskPartFormat(virStoragePoolObjPtr po= ol, break; } } - if (i =3D=3D pool->volumes.count) { + if (i =3D=3D virStoragePoolObjGetVolumesCount(pool)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("no extended partition found an= d no primary partition available")); return -1; diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_ba= ckend_gluster.c index 92038c1..90f31b0 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -386,8 +386,7 @@ virStorageBackendGlusterRefreshPool(virConnectPtr conn = ATTRIBUTE_UNUSED, =20 if (okay < 0) goto cleanup; - if (vol && VIR_APPEND_ELEMENT(pool->volumes.objs, pool->volumes.co= unt, - vol) < 0) + if (vol && virStoragePoolObjAddVol(pool, vol) < 0) goto cleanup; } if (errno) { diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_ba= ckend_logical.c index 67f70e5..7bfe211 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -356,9 +356,9 @@ virStorageBackendLogicalMakeVol(char **const groups, if (virStorageBackendLogicalParseVolExtents(vol, groups) < 0) goto cleanup; =20 - if (is_new_vol && - VIR_APPEND_ELEMENT(pool->volumes.objs, pool->volumes.count, vol) <= 0) + if (is_new_vol && virStoragePoolObjAddVol(pool, vol) < 0) goto cleanup; + vol =3D NULL; =20 ret =3D 0; =20 diff --git a/src/storage/storage_backend_mpath.c b/src/storage/storage_back= end_mpath.c index 434a477..46818ef 100644 --- a/src/storage/storage_backend_mpath.c +++ b/src/storage/storage_backend_mpath.c @@ -71,8 +71,9 @@ virStorageBackendMpathNewVol(virStoragePoolObjPtr pool, if (VIR_STRDUP(vol->key, vol->target.path) < 0) goto cleanup; =20 - if (VIR_APPEND_ELEMENT_COPY(pool->volumes.objs, pool->volumes.count, v= ol) < 0) + if (virStoragePoolObjAddVol(pool, vol) < 0) goto cleanup; + pool->def->capacity +=3D vol->target.capacity; pool->def->allocation +=3D vol->target.allocation; ret =3D 0; diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backen= d_rbd.c index 7b8887b..6731677 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -506,7 +506,7 @@ virStorageBackendRBDRefreshPool(virConnectPtr conn, goto cleanup; } =20 - if (VIR_APPEND_ELEMENT(pool->volumes.objs, pool->volumes.count, vo= l) < 0) { + if (virStoragePoolObjAddVol(pool, vol) < 0) { virStorageVolDefFree(vol); virStoragePoolObjClearVols(pool); goto cleanup; @@ -514,7 +514,7 @@ virStorageBackendRBDRefreshPool(virConnectPtr conn, } =20 VIR_DEBUG("Found %zu images in RBD pool %s", - pool->volumes.count, pool->def->source.name); + virStoragePoolObjGetVolumesCount(pool), pool->def->source.na= me); =20 ret =3D 0; =20 diff --git a/src/storage/storage_backend_sheepdog.c b/src/storage/storage_b= ackend_sheepdog.c index b55d96a..e72dcda 100644 --- a/src/storage/storage_backend_sheepdog.c +++ b/src/storage/storage_backend_sheepdog.c @@ -130,11 +130,9 @@ virStorageBackendSheepdogAddVolume(virConnectPtr conn = ATTRIBUTE_UNUSED, if (virStorageBackendSheepdogRefreshVol(conn, pool, vol) < 0) goto error; =20 - if (VIR_EXPAND_N(pool->volumes.objs, pool->volumes.count, 1) < 0) + if (virStoragePoolObjAddVol(pool, vol) < 0) goto error; =20 - pool->volumes.objs[pool->volumes.count - 1] =3D vol; - return 0; =20 error: diff --git a/src/storage/storage_backend_zfs.c b/src/storage/storage_backen= d_zfs.c index c6dae71..c266281 100644 --- a/src/storage/storage_backend_zfs.c +++ b/src/storage/storage_backend_zfs.c @@ -161,11 +161,9 @@ virStorageBackendZFSParseVol(virStoragePoolObjPtr pool, if (volume->target.allocation < volume->target.capacity) volume->target.sparse =3D true; =20 - if (is_new_vol && - VIR_APPEND_ELEMENT(pool->volumes.objs, - pool->volumes.count, - volume) < 0) + if (is_new_vol && virStoragePoolObjAddVol(pool, volume) < 0) goto cleanup; + volume =3D NULL; =20 ret =3D 0; cleanup: diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 8552120..b780f9a 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1626,25 +1626,6 @@ storagePoolLookupByTargetPath(virConnectPtr conn, } =20 =20 -static void -storageVolRemoveFromPool(virStoragePoolObjPtr obj, - virStorageVolDefPtr voldef) -{ - size_t i; - - for (i =3D 0; i < obj->volumes.count; i++) { - if (obj->volumes.objs[i] =3D=3D voldef) { - VIR_INFO("Deleting volume '%s' from storage pool '%s'", - voldef->name, obj->def->name); - virStorageVolDefFree(voldef); - - VIR_DELETE_ELEMENT(obj->volumes.objs, i, obj->volumes.count); - break; - } - } -} - - static int storageVolDeleteInternal(virStorageVolPtr vol, virStorageBackendPtr backend, @@ -1676,7 +1657,7 @@ storageVolDeleteInternal(virStorageVolPtr vol, } } =20 - storageVolRemoveFromPool(obj, voldef); + virStoragePoolObjRemoveVol(obj, voldef); ret =3D 0; =20 cleanup: @@ -1815,24 +1796,19 @@ storageVolCreateXML(virStoragePoolPtr pool, goto cleanup; } =20 - if (VIR_REALLOC_N(obj->volumes.objs, - obj->volumes.count + 1) < 0) - goto cleanup; - /* Wipe any key the user may have suggested, as volume creation * will generate the canonical key. */ VIR_FREE(voldef->key); if (backend->createVol(pool->conn, obj, voldef) < 0) goto cleanup; =20 - obj->volumes.objs[obj->volumes.count++] =3D voldef; - newvol =3D virGetStorageVol(pool->conn, obj->def->name, voldef->name, - voldef->key, NULL, NULL); - if (!newvol) { - obj->volumes.count--; + if (!(newvol =3D virGetStorageVol(pool->conn, obj->def->name, voldef->= name, + voldef->key, NULL, NULL))) goto cleanup; - } =20 + /* NB: Upon success voldef "owned" by storage pool for deletion purpos= es */ + if (virStoragePoolObjAddVol(obj, voldef) < 0) + goto cleanup; =20 if (backend->buildVol) { int buildret; @@ -1867,7 +1843,7 @@ storageVolCreateXML(virStoragePoolPtr pool, =20 if (buildret < 0) { /* buildVol handles deleting volume on failure */ - storageVolRemoveFromPool(obj, voldef); + virStoragePoolObjRemoveVol(obj, voldef); voldef =3D NULL; goto cleanup; } @@ -2018,9 +1994,6 @@ storageVolCreateXMLFrom(virStoragePoolPtr pool, backend->refreshVol(pool->conn, obj, voldefsrc) < 0) goto cleanup; =20 - if (VIR_REALLOC_N(obj->volumes.objs, obj->volumes.count + 1) < 0) - goto cleanup; - /* 'Define' the new volume so we get async progress reporting. * Wipe any key the user may have suggested, as volume creation * will generate the canonical key. */ @@ -2037,13 +2010,13 @@ storageVolCreateXMLFrom(virStoragePoolPtr pool, =20 memcpy(shadowvol, voldef, sizeof(*voldef)); =20 - obj->volumes.objs[obj->volumes.count++] =3D voldef; - newvol =3D virGetStorageVol(pool->conn, obj->def->name, voldef->name, - voldef->key, NULL, NULL); - if (!newvol) { - obj->volumes.count--; + if (!(newvol =3D virGetStorageVol(pool->conn, obj->def->name, voldef->= name, + voldef->key, NULL, NULL))) + goto cleanup; + + /* NB: Upon success voldef "owned" by storage pool for deletion purpos= es */ + if (virStoragePoolObjAddVol(obj, voldef) < 0) goto cleanup; - } =20 /* Drop the pool lock during volume allocation */ obj->asyncjobs++; diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index e1fe162..3efa181 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -3597,13 +3597,13 @@ virStorageBackendRefreshLocal(virConnectPtr conn AT= TRIBUTE_UNUSED, * An error message was raised, but we just continue. */ } =20 - if (VIR_APPEND_ELEMENT(pool->volumes.objs, pool->volumes.count, vo= l) < 0) + if (virStoragePoolObjAddVol(pool, vol) < 0) goto cleanup; + vol =3D NULL; } if (direrr < 0) goto cleanup; VIR_DIR_CLOSE(dir); - vol =3D NULL; =20 if (VIR_ALLOC(target)) goto cleanup; @@ -3785,10 +3785,10 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr po= ol, pool->def->capacity +=3D vol->target.capacity; pool->def->allocation +=3D vol->target.allocation; =20 - if (VIR_APPEND_ELEMENT(pool->volumes.objs, pool->volumes.count, vol) <= 0) + if (virStoragePoolObjAddVol(pool, vol) < 0) goto cleanup; - vol =3D NULL; + retval =3D 0; =20 cleanup: diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 989c3a8..67b115f 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1079,7 +1079,8 @@ testOpenVolumesForPool(const char *file, =20 if (!def->key && VIR_STRDUP(def->key, def->target.path) < 0) goto error; - if (VIR_APPEND_ELEMENT_COPY(obj->volumes.objs, obj->volumes.count,= def) < 0) + + if (virStoragePoolObjAddVol(obj, def) < 0) goto error; =20 obj->def->allocation +=3D def->target.allocation; @@ -4995,8 +4996,7 @@ testStorageVolCreateXML(virStoragePoolPtr pool, goto cleanup; =20 if (VIR_STRDUP(privvol->key, privvol->target.path) < 0 || - VIR_APPEND_ELEMENT_COPY(obj->volumes.objs, - obj->volumes.count, privvol) < 0) + virStoragePoolObjAddVol(obj, privvol) < 0) goto cleanup; =20 obj->def->allocation +=3D privvol->target.allocation; @@ -5063,8 +5063,7 @@ testStorageVolCreateXMLFrom(virStoragePoolPtr pool, goto cleanup; =20 if (VIR_STRDUP(privvol->key, privvol->target.path) < 0 || - VIR_APPEND_ELEMENT_COPY(obj->volumes.objs, - obj->volumes.count, privvol) < 0) + virStoragePoolObjAddVol(obj, privvol) < 0) goto cleanup; =20 obj->def->allocation +=3D privvol->target.allocation; @@ -5089,7 +5088,6 @@ testStorageVolDelete(virStorageVolPtr vol, testDriverPtr privconn =3D vol->conn->privateData; virStoragePoolObjPtr obj; virStorageVolDefPtr privvol; - size_t i; int ret =3D -1; =20 virCheckFlags(0, -1); @@ -5103,14 +5101,8 @@ testStorageVolDelete(virStorageVolPtr vol, obj->def->allocation -=3D privvol->target.allocation; obj->def->available =3D (obj->def->capacity - obj->def->allocation); =20 - for (i =3D 0; i < obj->volumes.count; i++) { - if (obj->volumes.objs[i] =3D=3D privvol) { - virStorageVolDefFree(privvol); + virStoragePoolObjRemoveVol(obj, privvol); =20 - VIR_DELETE_ELEMENT(obj->volumes.objs, i, obj->volumes.count); - break; - } - } ret =3D 0; =20 cleanup: --=20 2.9.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list