[libvirt] [PATCH REPOST 0/8] Privatize _virStoragePoolObj and _virStorageVolDefList (Batch #3)

Since the original series (19 patches): https://www.redhat.com/archives/libvir-list/2017-September/msg00594.html didn't garner any attention, I'm going with smaller patch piles to make forward progress. This is the last half of the storage backends plus one missed merge (because I broke things up <sigh>) John Ferlan (8): storage: Use virStoragePoolObjGetDef accessor for iSCSI backend storage: Use virStoragePoolObjGetDef accessor for MPATH backend storage: Use virStoragePoolObjGetDef accessor for RBD backend storage: Use virStoragePoolObjGetDef accessor for SCSI backend storage: Use virStoragePoolObjGetDef accessor for VSTORAGE backend storage: Use virStoragePoolObjGetDef accessor for ZFS backend storage: Use virStoragePoolObjGetDef accessor for new driver events storage: Privatize virStoragePoolObj and virStorageVolDefList src/conf/storage_conf.h | 4 --- src/conf/virstorageobj.c | 20 +++++++++++ src/conf/virstorageobj.h | 15 -------- src/storage/storage_backend_iscsi.c | 41 ++++++++++++---------- src/storage/storage_backend_mpath.c | 8 +++-- src/storage/storage_backend_rbd.c | 64 ++++++++++++++++++---------------- src/storage/storage_backend_scsi.c | 30 +++++++++------- src/storage/storage_backend_vstorage.c | 31 ++++++++-------- src/storage/storage_backend_zfs.c | 39 ++++++++++++--------- src/storage/storage_driver.c | 8 ++--- 10 files changed, 144 insertions(+), 116 deletions(-) -- 2.13.6

In preparation for privatizing the object, use the accessor. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_backend_iscsi.c | 41 +++++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c index 14f3e09a8f..b0c5096adb 100644 --- a/src/storage/storage_backend_iscsi.c +++ b/src/storage/storage_backend_iscsi.c @@ -83,7 +83,8 @@ static char * virStorageBackendISCSISession(virStoragePoolObjPtr pool, bool probe) { - return virISCSIGetSession(pool->def->source.devices[0].path, probe); + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); + return virISCSIGetSession(def->source.devices[0].path, probe); } @@ -235,25 +236,26 @@ static int virStorageBackendISCSICheckPool(virStoragePoolObjPtr pool, bool *isActive) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); char *session = NULL; int ret = -1; *isActive = false; - if (pool->def->source.nhost != 1) { + if (def->source.nhost != 1) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Expected exactly 1 host for the storage pool")); return -1; } - if (pool->def->source.hosts[0].name == NULL) { + if (def->source.hosts[0].name == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing source host")); return -1; } - if (pool->def->source.ndevice != 1 || - pool->def->source.devices[0].path == NULL) { + if (def->source.ndevice != 1 || + def->source.devices[0].path == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing source device")); return -1; @@ -327,44 +329,45 @@ static int virStorageBackendISCSIStartPool(virConnectPtr conn, virStoragePoolObjPtr pool) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); char *portal = NULL; char *session = NULL; int ret = -1; - if (pool->def->source.nhost != 1) { + if (def->source.nhost != 1) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Expected exactly 1 host for the storage pool")); return -1; } - if (pool->def->source.hosts[0].name == NULL) { + if (def->source.hosts[0].name == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing source host")); return -1; } - if (pool->def->source.ndevice != 1 || - pool->def->source.devices[0].path == NULL) { + if (def->source.ndevice != 1 || + def->source.devices[0].path == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing source device")); return -1; } if ((session = virStorageBackendISCSISession(pool, true)) == NULL) { - if ((portal = virStorageBackendISCSIPortal(&pool->def->source)) == NULL) + if ((portal = virStorageBackendISCSIPortal(&def->source)) == NULL) goto cleanup; /* Create a static node record for the IQN target. Must be done * in order for login to the target */ - if (virISCSINodeNew(portal, pool->def->source.devices[0].path) < 0) + if (virISCSINodeNew(portal, def->source.devices[0].path) < 0) goto cleanup; - if (virStorageBackendISCSISetAuth(portal, conn, &pool->def->source) < 0) + if (virStorageBackendISCSISetAuth(portal, conn, &def->source) < 0) goto cleanup; if (virISCSIConnectionLogin(portal, - pool->def->source.initiator.iqn, - pool->def->source.devices[0].path) < 0) + def->source.initiator.iqn, + def->source.devices[0].path) < 0) goto cleanup; } ret = 0; @@ -379,9 +382,10 @@ static int virStorageBackendISCSIRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); char *session = NULL; - pool->def->allocation = pool->def->capacity = pool->def->available = 0; + def->allocation = def->capacity = def->available = 0; if ((session = virStorageBackendISCSISession(pool, false)) == NULL) goto cleanup; @@ -403,6 +407,7 @@ static int virStorageBackendISCSIStopPool(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); char *portal; char *session; int ret = -1; @@ -411,12 +416,12 @@ virStorageBackendISCSIStopPool(virConnectPtr conn ATTRIBUTE_UNUSED, return 0; VIR_FREE(session); - if ((portal = virStorageBackendISCSIPortal(&pool->def->source)) == NULL) + if ((portal = virStorageBackendISCSIPortal(&def->source)) == NULL) return -1; if (virISCSIConnectionLogout(portal, - pool->def->source.initiator.iqn, - pool->def->source.devices[0].path) < 0) + def->source.initiator.iqn, + def->source.devices[0].path) < 0) goto cleanup; ret = 0; -- 2.13.6

In preparation for privatizing the object, use the accessor. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_backend_mpath.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/storage/storage_backend_mpath.c b/src/storage/storage_backend_mpath.c index 46818ef2cc..4bc39c24eb 100644 --- a/src/storage/storage_backend_mpath.c +++ b/src/storage/storage_backend_mpath.c @@ -48,6 +48,7 @@ virStorageBackendMpathNewVol(virStoragePoolObjPtr pool, const int devnum, const char *dev) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); virStorageVolDefPtr vol; int ret = -1; @@ -74,8 +75,8 @@ virStorageBackendMpathNewVol(virStoragePoolObjPtr pool, if (virStoragePoolObjAddVol(pool, vol) < 0) goto cleanup; - pool->def->capacity += vol->target.capacity; - pool->def->allocation += vol->target.allocation; + def->capacity += vol->target.capacity; + def->allocation += vol->target.allocation; ret = 0; cleanup: @@ -259,10 +260,11 @@ virStorageBackendMpathRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool) { int retval = 0; + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); VIR_DEBUG("conn=%p, pool=%p", conn, pool); - pool->def->allocation = pool->def->capacity = pool->def->available = 0; + def->allocation = def->capacity = def->available = 0; virWaitForDevices(); -- 2.13.6

In preparation for privatizing the object, use the accessor. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_backend_rbd.c | 64 +++++++++++++++++++++------------------ 1 file changed, 34 insertions(+), 30 deletions(-) diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 6731677851..7f9597cabe 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -210,10 +210,11 @@ static int virStorageBackendRBDOpenIoCTX(virStorageBackendRBDStatePtr ptr, virStoragePoolObjPtr pool) { - int r = rados_ioctx_create(ptr->cluster, pool->def->source.name, &ptr->ioctx); + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); + int r = rados_ioctx_create(ptr->cluster, def->source.name, &ptr->ioctx); if (r < 0) { virReportSystemError(-r, _("failed to create the RBD IoCTX. Does the pool '%s' exist?"), - pool->def->source.name); + def->source.name); } return r; } @@ -255,11 +256,12 @@ virStorageBackendRBDNewState(virConnectPtr conn, virStoragePoolObjPtr pool) { virStorageBackendRBDStatePtr ptr; + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); if (VIR_ALLOC(ptr) < 0) return NULL; - if (virStorageBackendRBDOpenRADOSConn(ptr, conn, &pool->def->source) < 0) + if (virStorageBackendRBDOpenRADOSConn(ptr, conn, &def->source) < 0) goto error; if (virStorageBackendRBDOpenIoCTX(ptr, pool) < 0) @@ -359,6 +361,7 @@ volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr vol, virStorageBackendRBDStatePtr ptr) { int ret = -1; + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); int r = 0; rbd_image_t image = NULL; rbd_image_info_t info; @@ -388,7 +391,7 @@ volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr vol, if (volStorageBackendRBDUseFastDiff(features)) { VIR_DEBUG("RBD image %s/%s has fast-diff feature enabled. " "Querying for actual allocation", - pool->def->source.name, vol->name); + def->source.name, vol->name); if (virStorageBackendRBDSetAllocation(vol, image, &info) < 0) goto cleanup; @@ -398,19 +401,17 @@ volStorageBackendRBDRefreshVolInfo(virStorageVolDefPtr vol, VIR_DEBUG("Refreshed RBD image %s/%s (capacity: %llu allocation: %llu " "obj_size: %"PRIu64" num_objs: %"PRIu64")", - pool->def->source.name, vol->name, vol->target.capacity, + def->source.name, vol->name, vol->target.capacity, vol->target.allocation, info.obj_size, info.num_objs); VIR_FREE(vol->target.path); if (virAsprintf(&vol->target.path, "%s/%s", - pool->def->source.name, - vol->name) < 0) + def->source.name, vol->name) < 0) goto cleanup; VIR_FREE(vol->key); if (virAsprintf(&vol->key, "%s/%s", - pool->def->source.name, - vol->name) < 0) + def->source.name, vol->name) < 0) goto cleanup; ret = 0; @@ -430,6 +431,7 @@ virStorageBackendRBDRefreshPool(virConnectPtr conn, int len = -1; int r = 0; char *name, *names = NULL; + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); virStorageBackendRBDStatePtr ptr = NULL; struct rados_cluster_stat_t clusterstat; struct rados_pool_stat_t poolstat; @@ -444,17 +446,17 @@ virStorageBackendRBDRefreshPool(virConnectPtr conn, if ((r = rados_ioctx_pool_stat(ptr->ioctx, &poolstat)) < 0) { virReportSystemError(-r, _("failed to stat the RADOS pool '%s'"), - pool->def->source.name); + def->source.name); goto cleanup; } - pool->def->capacity = clusterstat.kb * 1024; - pool->def->available = clusterstat.kb_avail * 1024; - pool->def->allocation = poolstat.num_bytes; + def->capacity = clusterstat.kb * 1024; + def->available = clusterstat.kb_avail * 1024; + def->allocation = poolstat.num_bytes; VIR_DEBUG("Utilization of RBD pool %s: (kb: %"PRIu64" kb_avail: %"PRIu64 " num_bytes: %"PRIu64")", - pool->def->source.name, clusterstat.kb, clusterstat.kb_avail, + def->source.name, clusterstat.kb, clusterstat.kb_avail, poolstat.num_bytes); while (true) { @@ -514,7 +516,7 @@ virStorageBackendRBDRefreshPool(virConnectPtr conn, } VIR_DEBUG("Found %zu images in RBD pool %s", - virStoragePoolObjGetVolumesCount(pool), pool->def->source.name); + virStoragePoolObjGetVolumesCount(pool), def->source.name); ret = 0; @@ -610,12 +612,13 @@ virStorageBackendRBDDeleteVol(virConnectPtr conn, { int ret = -1; int r = 0; + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); virStorageBackendRBDStatePtr ptr = NULL; virCheckFlags(VIR_STORAGE_VOL_DELETE_ZEROED | VIR_STORAGE_VOL_DELETE_WITH_SNAPSHOTS, -1); - VIR_DEBUG("Removing RBD image %s/%s", pool->def->source.name, vol->name); + VIR_DEBUG("Removing RBD image %s/%s", def->source.name, vol->name); if (flags & VIR_STORAGE_VOL_DELETE_ZEROED) VIR_WARN("%s", "This storage backend does not support zeroed removal of volumes"); @@ -624,17 +627,17 @@ virStorageBackendRBDDeleteVol(virConnectPtr conn, goto cleanup; if (flags & VIR_STORAGE_VOL_DELETE_WITH_SNAPSHOTS) { - if (virStorageBackendRBDCleanupSnapshots(ptr->ioctx, &pool->def->source, + if (virStorageBackendRBDCleanupSnapshots(ptr->ioctx, &def->source, vol) < 0) goto cleanup; } - VIR_DEBUG("Removing volume %s/%s", pool->def->source.name, vol->name); + VIR_DEBUG("Removing volume %s/%s", def->source.name, vol->name); r = rbd_remove(ptr->ioctx, vol->name); if (r < 0 && (-r) != ENOENT) { virReportSystemError(-r, _("failed to remove volume '%s/%s'"), - pool->def->source.name, vol->name); + def->source.name, vol->name); goto cleanup; } @@ -651,6 +654,8 @@ virStorageBackendRBDCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool, virStorageVolDefPtr vol) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); + vol->type = VIR_STORAGE_VOL_NETWORK; if (vol->target.format != VIR_STORAGE_FILE_RAW) { @@ -661,14 +666,12 @@ virStorageBackendRBDCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED, VIR_FREE(vol->target.path); if (virAsprintf(&vol->target.path, "%s/%s", - pool->def->source.name, - vol->name) < 0) + def->source.name, vol->name) < 0) return -1; VIR_FREE(vol->key); if (virAsprintf(&vol->key, "%s/%s", - pool->def->source.name, - vol->name) < 0) + def->source.name, vol->name) < 0) return -1; return 0; @@ -687,13 +690,13 @@ virStorageBackendRBDBuildVol(virConnectPtr conn, virStorageVolDefPtr vol, unsigned int flags) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); virStorageBackendRBDStatePtr ptr = NULL; int ret = -1; int r = 0; VIR_DEBUG("Creating RBD image %s/%s with size %llu", - pool->def->source.name, - vol->name, vol->target.capacity); + def->source.name, vol->name, vol->target.capacity); virCheckFlags(0, -1); @@ -721,8 +724,7 @@ virStorageBackendRBDBuildVol(virConnectPtr conn, if ((r = virStorageBackendRBDCreateImage(ptr->ioctx, vol->name, vol->target.capacity)) < 0) { virReportSystemError(-r, _("failed to create volume '%s/%s'"), - pool->def->source.name, - vol->name); + def->source.name, vol->name); goto cleanup; } @@ -1045,11 +1047,12 @@ virStorageBackendRBDBuildVolFrom(virConnectPtr conn, virStorageVolDefPtr origvol, unsigned int flags) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); virStorageBackendRBDStatePtr ptr = NULL; int ret = -1; VIR_DEBUG("Creating clone of RBD image %s/%s with name %s", - pool->def->source.name, origvol->name, newvol->name); + def->source.name, origvol->name, newvol->name); virCheckFlags(0, -1); @@ -1208,6 +1211,7 @@ virStorageBackendRBDVolWipe(virConnectPtr conn, unsigned int flags) { virStorageBackendRBDStatePtr ptr = NULL; + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); rbd_image_t image = NULL; rbd_image_info_t info; uint64_t stripe_count; @@ -1216,7 +1220,7 @@ virStorageBackendRBDVolWipe(virConnectPtr conn, virCheckFlags(0, -1); - VIR_DEBUG("Wiping RBD image %s/%s", pool->def->source.name, vol->name); + VIR_DEBUG("Wiping RBD image %s/%s", def->source.name, vol->name); if (!(ptr = virStorageBackendRBDNewState(conn, pool))) goto cleanup; @@ -1240,7 +1244,7 @@ virStorageBackendRBDVolWipe(virConnectPtr conn, } VIR_DEBUG("Need to wipe %"PRIu64" bytes from RBD image %s/%s", - info.size, pool->def->source.name, vol->name); + info.size, def->source.name, vol->name); switch ((virStorageVolWipeAlgorithm) algorithm) { case VIR_STORAGE_VOL_WIPE_ALG_ZERO: -- 2.13.6

In preparation for privatizing the object, use the accessor. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_backend_scsi.c | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index 02fd4b643c..ee79ad72f5 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -139,6 +139,7 @@ virStoragePoolFCRefreshThread(void *opaque) const char *fchost_name = cbdata->fchost_name; const unsigned char *pool_uuid = cbdata->pool_uuid; virStoragePoolObjPtr pool = NULL; + virStoragePoolDefPtr def; unsigned int host; int found = 0; int tries = 2; @@ -149,14 +150,15 @@ virStoragePoolFCRefreshThread(void *opaque) /* Let's see if the pool still exists - */ if (!(pool = virStoragePoolObjFindPoolByUUID(pool_uuid))) break; + def = virStoragePoolObjGetDef(pool); /* Return with pool lock, if active, we can get the host number, * successfully, rescan, and find LUN's, then we are happy */ VIR_DEBUG("Attempt FC Refresh for pool='%s' name='%s' tries='%d'", - pool->def->name, fchost_name, tries); + def->name, fchost_name, tries); - pool->def->allocation = pool->def->capacity = pool->def->available = 0; + def->allocation = def->capacity = def->available = 0; if (virStoragePoolObjIsActive(pool) && virSCSIHostGetNumber(fchost_name, &host) == 0 && @@ -371,6 +373,7 @@ static int virStorageBackendSCSICheckPool(virStoragePoolObjPtr pool, bool *isActive) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); char *path = NULL; char *name = NULL; unsigned int host; @@ -378,13 +381,12 @@ virStorageBackendSCSICheckPool(virStoragePoolObjPtr pool, *isActive = false; - if (!(name = getAdapterName(&pool->def->source.adapter))) { + if (!(name = getAdapterName(&def->source.adapter))) { /* It's normal for the pool with "fc_host" type source * adapter fails to get the adapter name, since the vHBA * the adapter based on might be not created yet. */ - if (pool->def->source.adapter.type == - VIR_STORAGE_ADAPTER_TYPE_FC_HOST) { + if (def->source.adapter.type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST) { virResetLastError(); return 0; } else { @@ -412,13 +414,14 @@ static int virStorageBackendSCSIRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); char *name = NULL; unsigned int host; int ret = -1; - pool->def->allocation = pool->def->capacity = pool->def->available = 0; + def->allocation = def->capacity = def->available = 0; - if (!(name = getAdapterName(&pool->def->source.adapter))) + if (!(name = getAdapterName(&def->source.adapter))) return -1; if (virSCSIHostGetNumber(name, &host) < 0) @@ -443,11 +446,12 @@ static int virStorageBackendSCSIStartPool(virConnectPtr conn, virStoragePoolObjPtr pool) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); const char *configFile = virStoragePoolObjGetConfigFile(pool); - if (pool->def->source.adapter.type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST) - return createVport(conn, pool->def, configFile, - &pool->def->source.adapter.data.fchost); + if (def->source.adapter.type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST) + return createVport(conn, def, configFile, + &def->source.adapter.data.fchost); return 0; } @@ -457,9 +461,11 @@ static int virStorageBackendSCSIStopPool(virConnectPtr conn, virStoragePoolObjPtr pool) { - if (pool->def->source.adapter.type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST) + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); + + if (def->source.adapter.type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST) return virNodeDeviceDeleteVport(conn, - &pool->def->source.adapter.data.fchost); + &def->source.adapter.data.fchost); return 0; } -- 2.13.6

In preparation for privatizing the object, use the accessor. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_backend_vstorage.c | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/src/storage/storage_backend_vstorage.c b/src/storage/storage_backend_vstorage.c index fb06138538..2dc26af387 100644 --- a/src/storage/storage_backend_vstorage.c +++ b/src/storage/storage_backend_vstorage.c @@ -41,33 +41,34 @@ virStorageBackendVzPoolStart(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool) { int ret = -1; + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); virCommandPtr cmd = NULL; char *grp_name = NULL; char *usr_name = NULL; char *mode = NULL; /* Check the permissions */ - if (pool->def->target.perms.mode == (mode_t) - 1) - pool->def->target.perms.mode = VIR_STORAGE_DEFAULT_POOL_PERM_MODE; - if (pool->def->target.perms.uid == (uid_t) -1) - pool->def->target.perms.uid = geteuid(); - if (pool->def->target.perms.gid == (gid_t) -1) - pool->def->target.perms.gid = getegid(); + if (def->target.perms.mode == (mode_t) - 1) + def->target.perms.mode = VIR_STORAGE_DEFAULT_POOL_PERM_MODE; + if (def->target.perms.uid == (uid_t) -1) + def->target.perms.uid = geteuid(); + if (def->target.perms.gid == (gid_t) -1) + def->target.perms.gid = getegid(); /* Convert ids to names because vstorage uses names */ - if (!(grp_name = virGetGroupName(pool->def->target.perms.gid))) + if (!(grp_name = virGetGroupName(def->target.perms.gid))) goto cleanup; - if (!(usr_name = virGetUserName(pool->def->target.perms.uid))) + if (!(usr_name = virGetUserName(def->target.perms.uid))) goto cleanup; - if (virAsprintf(&mode, "%o", pool->def->target.perms.mode) < 0) + if (virAsprintf(&mode, "%o", def->target.perms.mode) < 0) goto cleanup; cmd = virCommandNewArgList(VSTORAGE_MOUNT, - "-c", pool->def->source.name, - pool->def->target.path, + "-c", def->source.name, + def->target.path, "-m", mode, "-g", grp_name, "-u", usr_name, NULL); @@ -89,12 +90,13 @@ static int virStorageBackendVzIsMounted(virStoragePoolObjPtr pool) { int ret = -1; + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); FILE *mtab; struct mntent ent; char buf[1024]; char *cluster = NULL; - if (virAsprintf(&cluster, "vstorage://%s", pool->def->source.name) < 0) + if (virAsprintf(&cluster, "vstorage://%s", def->source.name) < 0) return -1; if ((mtab = fopen(_PATH_MOUNTED, "r")) == NULL) { @@ -106,7 +108,7 @@ virStorageBackendVzIsMounted(virStoragePoolObjPtr pool) while ((getmntent_r(mtab, &ent, buf, sizeof(buf))) != NULL) { - if (STREQ(ent.mnt_dir, pool->def->target.path) && + if (STREQ(ent.mnt_dir, def->target.path) && STREQ(ent.mnt_fsname, cluster)) { ret = 1; goto cleanup; @@ -126,6 +128,7 @@ static int virStorageBackendVzPoolStop(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); virCommandPtr cmd = NULL; int ret = -1; int rc; @@ -134,7 +137,7 @@ virStorageBackendVzPoolStop(virConnectPtr conn ATTRIBUTE_UNUSED, if ((rc = virStorageBackendVzIsMounted(pool)) != 1) return rc; - cmd = virCommandNewArgList(UMOUNT, pool->def->target.path, NULL); + cmd = virCommandNewArgList(UMOUNT, def->target.path, NULL); if (virCommandRun(cmd, NULL) < 0) goto cleanup; -- 2.13.6

In preparation for privatizing the object, use the accessor. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_backend_zfs.c | 39 +++++++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/src/storage/storage_backend_zfs.c b/src/storage/storage_backend_zfs.c index c2662815a1..198c788aca 100644 --- a/src/storage/storage_backend_zfs.c +++ b/src/storage/storage_backend_zfs.c @@ -86,10 +86,11 @@ static int virStorageBackendZFSCheckPool(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, bool *isActive) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); char *devpath; if (virAsprintf(&devpath, "/dev/zvol/%s", - pool->def->source.name) < 0) + def->source.name) < 0) return -1; *isActive = virFileIsDir(devpath); VIR_FREE(devpath); @@ -109,6 +110,7 @@ virStorageBackendZFSParseVol(virStoragePoolObjPtr pool, char *vol_name; bool is_new_vol = false; virStorageVolDefPtr volume = NULL; + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); if (!(tokens = virStringSplitCount(volume_string, "\t", 0, &count))) return -1; @@ -142,7 +144,7 @@ virStorageBackendZFSParseVol(virStoragePoolObjPtr pool, if (volume->target.path == NULL) { if (virAsprintf(&volume->target.path, "%s/%s", - pool->def->target.path, volume->name) < 0) + def->target.path, volume->name) < 0) goto cleanup; } @@ -178,6 +180,7 @@ static int virStorageBackendZFSFindVols(virStoragePoolObjPtr pool, virStorageVolDefPtr vol) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); virCommandPtr cmd = NULL; char *volumes_list = NULL; char **lines = NULL; @@ -199,7 +202,7 @@ virStorageBackendZFSFindVols(virStoragePoolObjPtr pool, "list", "-Hp", "-t", "volume", "-r", "-o", "name,volsize,refreservation", - pool->def->source.name, + def->source.name, NULL); virCommandSetOutputBuffer(cmd, &volumes_list); if (virCommandRun(cmd, NULL) < 0) @@ -228,6 +231,7 @@ static int virStorageBackendZFSRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool ATTRIBUTE_UNUSED) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); virCommandPtr cmd = NULL; char *zpool_props = NULL; char **lines = NULL; @@ -247,7 +251,7 @@ virStorageBackendZFSRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, cmd = virCommandNewArgList(ZPOOL, "get", "-Hp", "health,size,free,allocated", - pool->def->source.name, + def->source.name, NULL); virCommandSetOutputBuffer(cmd, &zpool_props); if (virCommandRun(cmd, NULL) < 0) @@ -279,11 +283,11 @@ virStorageBackendZFSRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, goto cleanup; if (STREQ(prop_name, "free")) - pool->def->available = value; + def->available = value; else if (STREQ(prop_name, "size")) - pool->def->capacity = value; + def->capacity = value; else if (STREQ(prop_name, "allocated")) - pool->def->allocation = value; + def->allocation = value; } } @@ -305,6 +309,7 @@ virStorageBackendZFSCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool, virStorageVolDefPtr vol) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); virCommandPtr cmd = NULL; int ret = -1; int volmode_needed = -1; @@ -320,7 +325,7 @@ virStorageBackendZFSCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED, VIR_FREE(vol->target.path); if (virAsprintf(&vol->target.path, "%s/%s", - pool->def->target.path, vol->name) < 0) + def->target.path, vol->name) < 0) return -1; if (VIR_STRDUP(vol->key, vol->target.path) < 0) @@ -356,8 +361,7 @@ virStorageBackendZFSCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED, virCommandAddArg(cmd, "-V"); virCommandAddArgFormat(cmd, "%lluK", VIR_DIV_UP(vol->target.capacity, 1024)); - virCommandAddArgFormat(cmd, "%s/%s", - pool->def->source.name, vol->name); + virCommandAddArgFormat(cmd, "%s/%s", def->source.name, vol->name); if (virCommandRun(cmd, NULL) < 0) goto cleanup; @@ -379,6 +383,7 @@ virStorageBackendZFSDeleteVol(virConnectPtr conn ATTRIBUTE_UNUSED, unsigned int flags) { int ret = -1; + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); virCommandPtr destroy_cmd = NULL; virCheckFlags(0, -1); @@ -386,7 +391,7 @@ virStorageBackendZFSDeleteVol(virConnectPtr conn ATTRIBUTE_UNUSED, destroy_cmd = virCommandNewArgList(ZFS, "destroy", NULL); virCommandAddArgFormat(destroy_cmd, "%s/%s", - pool->def->source.name, vol->name); + def->source.name, vol->name); if (virCommandRun(destroy_cmd, NULL) < 0) goto cleanup; @@ -402,23 +407,24 @@ virStorageBackendZFSBuildPool(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool, unsigned int flags) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); virCommandPtr cmd = NULL; size_t i; int ret = -1; virCheckFlags(0, -1); - if (pool->def->source.ndevice == 0) { + if (def->source.ndevice == 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("missing source devices")); return -1; } cmd = virCommandNewArgList(ZPOOL, "create", - pool->def->source.name, NULL); + def->source.name, NULL); - for (i = 0; i < pool->def->source.ndevice; i++) - virCommandAddArg(cmd, pool->def->source.devices[i].path); + for (i = 0; i < def->source.ndevice; i++) + virCommandAddArg(cmd, def->source.devices[i].path); if (virCommandRun(cmd, NULL) < 0) goto cleanup; @@ -435,13 +441,14 @@ virStorageBackendZFSDeletePool(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool, unsigned int flags) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); virCommandPtr cmd = NULL; int ret = -1; virCheckFlags(0, -1); cmd = virCommandNewArgList(ZPOOL, "destroy", - pool->def->source.name, NULL); + def->source.name, NULL); if (virCommandRun(cmd, NULL) < 0) goto cleanup; -- 2.13.6

Missed from merge from commit id 'b0652192' into commit id 'bfcd8fc92' were a couple of obj->def-> references. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_driver.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index b0edf9f885..2b7a299706 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -994,8 +994,8 @@ storagePoolBuild(virStoragePoolPtr pool, backend->buildPool(pool->conn, obj, flags) < 0) goto cleanup; - event = virStoragePoolEventLifecycleNew(obj->def->name, - obj->def->uuid, + event = virStoragePoolEventLifecycleNew(def->name, + def->uuid, VIR_STORAGE_POOL_EVENT_CREATED, 0); @@ -1128,8 +1128,8 @@ storagePoolDelete(virStoragePoolPtr pool, if (backend->deletePool(pool->conn, obj, flags) < 0) goto cleanup; - event = virStoragePoolEventLifecycleNew(obj->def->name, - obj->def->uuid, + event = virStoragePoolEventLifecycleNew(def->name, + def->uuid, VIR_STORAGE_POOL_EVENT_DELETED, 0); -- 2.13.6

Move the structures into virstorageobj so that both are known within virstorageobj.c. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/storage_conf.h | 4 ---- src/conf/virstorageobj.c | 20 ++++++++++++++++++++ src/conf/virstorageobj.h | 15 --------------- 3 files changed, 20 insertions(+), 19 deletions(-) diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 8ac6796a61..b349783d26 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -75,10 +75,6 @@ struct _virStorageVolDef { typedef struct _virStorageVolDefList virStorageVolDefList; typedef virStorageVolDefList *virStorageVolDefListPtr; -struct _virStorageVolDefList { - size_t count; - virStorageVolDefPtr *objs; -}; VIR_ENUM_DECL(virStorageVol) diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c index ff04c9efe4..50dbd7bf4d 100644 --- a/src/conf/virstorageobj.c +++ b/src/conf/virstorageobj.c @@ -37,6 +37,26 @@ VIR_LOG_INIT("conf.virstorageobj"); +struct _virStorageVolDefList { + size_t count; + virStorageVolDefPtr *objs; +}; + +struct _virStoragePoolObj { + virMutex lock; + + char *configFile; + char *autostartLink; + bool active; + bool autostart; + unsigned int asyncjobs; + + virStoragePoolDefPtr def; + virStoragePoolDefPtr newDef; + + virStorageVolDefList volumes; +}; + virStoragePoolObjPtr virStoragePoolObjNew(void) { diff --git a/src/conf/virstorageobj.h b/src/conf/virstorageobj.h index cf7ee06cd1..69e737226b 100644 --- a/src/conf/virstorageobj.h +++ b/src/conf/virstorageobj.h @@ -27,21 +27,6 @@ typedef struct _virStoragePoolObj virStoragePoolObj; typedef virStoragePoolObj *virStoragePoolObjPtr; -struct _virStoragePoolObj { - virMutex lock; - - char *configFile; - char *autostartLink; - bool active; - bool autostart; - unsigned int asyncjobs; - - virStoragePoolDefPtr def; - virStoragePoolDefPtr newDef; - - virStorageVolDefList volumes; -}; - typedef struct _virStoragePoolObjList virStoragePoolObjList; typedef virStoragePoolObjList *virStoragePoolObjListPtr; struct _virStoragePoolObjList { -- 2.13.6

ping? Tks - John On 10/06/2017 10:42 AM, John Ferlan wrote:
Since the original series (19 patches):
https://www.redhat.com/archives/libvir-list/2017-September/msg00594.html
didn't garner any attention, I'm going with smaller patch piles to make forward progress.
This is the last half of the storage backends plus one missed merge (because I broke things up <sigh>)
John Ferlan (8): storage: Use virStoragePoolObjGetDef accessor for iSCSI backend storage: Use virStoragePoolObjGetDef accessor for MPATH backend storage: Use virStoragePoolObjGetDef accessor for RBD backend storage: Use virStoragePoolObjGetDef accessor for SCSI backend storage: Use virStoragePoolObjGetDef accessor for VSTORAGE backend storage: Use virStoragePoolObjGetDef accessor for ZFS backend storage: Use virStoragePoolObjGetDef accessor for new driver events storage: Privatize virStoragePoolObj and virStorageVolDefList
src/conf/storage_conf.h | 4 --- src/conf/virstorageobj.c | 20 +++++++++++ src/conf/virstorageobj.h | 15 -------- src/storage/storage_backend_iscsi.c | 41 ++++++++++++---------- src/storage/storage_backend_mpath.c | 8 +++-- src/storage/storage_backend_rbd.c | 64 ++++++++++++++++++---------------- src/storage/storage_backend_scsi.c | 30 +++++++++------- src/storage/storage_backend_vstorage.c | 31 ++++++++-------- src/storage/storage_backend_zfs.c | 39 ++++++++++++--------- src/storage/storage_driver.c | 8 ++--- 10 files changed, 144 insertions(+), 116 deletions(-)

ping^2, TKs, John On 10/19/2017 11:02 AM, John Ferlan wrote:
ping?
Tks -
John
On 10/06/2017 10:42 AM, John Ferlan wrote:
Since the original series (19 patches):
https://www.redhat.com/archives/libvir-list/2017-September/msg00594.html
didn't garner any attention, I'm going with smaller patch piles to make forward progress.
This is the last half of the storage backends plus one missed merge (because I broke things up <sigh>)
John Ferlan (8): storage: Use virStoragePoolObjGetDef accessor for iSCSI backend storage: Use virStoragePoolObjGetDef accessor for MPATH backend storage: Use virStoragePoolObjGetDef accessor for RBD backend storage: Use virStoragePoolObjGetDef accessor for SCSI backend storage: Use virStoragePoolObjGetDef accessor for VSTORAGE backend storage: Use virStoragePoolObjGetDef accessor for ZFS backend storage: Use virStoragePoolObjGetDef accessor for new driver events storage: Privatize virStoragePoolObj and virStorageVolDefList
src/conf/storage_conf.h | 4 --- src/conf/virstorageobj.c | 20 +++++++++++ src/conf/virstorageobj.h | 15 -------- src/storage/storage_backend_iscsi.c | 41 ++++++++++++---------- src/storage/storage_backend_mpath.c | 8 +++-- src/storage/storage_backend_rbd.c | 64 ++++++++++++++++++---------------- src/storage/storage_backend_scsi.c | 30 +++++++++------- src/storage/storage_backend_vstorage.c | 31 ++++++++-------- src/storage/storage_backend_zfs.c | 39 ++++++++++++--------- src/storage/storage_driver.c | 8 ++--- 10 files changed, 144 insertions(+), 116 deletions(-)
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Fri, Oct 06, 2017 at 10:42:54AM -0400, John Ferlan wrote:
Since the original series (19 patches):
https://www.redhat.com/archives/libvir-list/2017-September/msg00594.html
didn't garner any attention, I'm going with smaller patch piles to make forward progress.
This is the last half of the storage backends plus one missed merge (because I broke things up <sigh>)
John Ferlan (8): storage: Use virStoragePoolObjGetDef accessor for iSCSI backend storage: Use virStoragePoolObjGetDef accessor for MPATH backend storage: Use virStoragePoolObjGetDef accessor for RBD backend storage: Use virStoragePoolObjGetDef accessor for SCSI backend storage: Use virStoragePoolObjGetDef accessor for VSTORAGE backend storage: Use virStoragePoolObjGetDef accessor for ZFS backend storage: Use virStoragePoolObjGetDef accessor for new driver events storage: Privatize virStoragePoolObj and virStorageVolDefList
src/conf/storage_conf.h | 4 --- src/conf/virstorageobj.c | 20 +++++++++++ src/conf/virstorageobj.h | 15 -------- src/storage/storage_backend_iscsi.c | 41 ++++++++++++---------- src/storage/storage_backend_mpath.c | 8 +++-- src/storage/storage_backend_rbd.c | 64 ++++++++++++++++++---------------- src/storage/storage_backend_scsi.c | 30 +++++++++------- src/storage/storage_backend_vstorage.c | 31 ++++++++-------- src/storage/storage_backend_zfs.c | 39 ++++++++++++--------- src/storage/storage_driver.c | 8 ++--- 10 files changed, 144 insertions(+), 116 deletions(-)
Reviewed-by: Erik Skultety <eskultet@redhat.com>
participants (2)
-
Erik Skultety
-
John Ferlan