[libvirt] [PATCH REPOST 0/6] Privatize _virStoragePoolObj and _virStorageVolDefList (Batch #2)
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 series is the first half of the storage backend changes to use the virStoragePoolObjGetDef accessor. The next/last series will be the next half, plus the change to move virStoragePoolObj and virStorageVolDefList into virstoragepoolobj.c. After that adjustments to virstoragepoolobj similar to other drivers and perhaps even generating a virStorageVolObjPtr - e.g. a hash table of volume defs rather than a linked list of defs. John Ferlan (6): storage: Use virStoragePoolObjGetDef accessor for storage_util storage: Use virStoragePoolObjGetDef accessor for Disk backend storage: Use virStoragePoolObjGetDef accessor for Logical backend storage: Use virStoragePoolObjGetDef accessor for Sheepdog backend storage: Use virStoragePoolObjGetDef accessor for FS backend storage: Use virStoragePoolObjGetDef accessor for Gluster backend src/storage/storage_backend_disk.c | 98 ++++++++++++++++------------ src/storage/storage_backend_fs.c | 90 +++++++++++++------------ src/storage/storage_backend_gluster.c | 20 +++--- src/storage/storage_backend_logical.c | 63 ++++++++++-------- src/storage/storage_backend_sheepdog.c | 23 ++++--- src/storage/storage_util.c | 116 ++++++++++++++++++--------------- 6 files changed, 230 insertions(+), 180 deletions(-) -- 2.13.6
In preparation for privatizing the object, use the accessor. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_util.c | 116 ++++++++++++++++++++++++--------------------- 1 file changed, 63 insertions(+), 53 deletions(-) diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index b23b6dd1d1..a10e4590f3 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -396,6 +396,7 @@ storageBackendCreateRaw(virConnectPtr conn ATTRIBUTE_UNUSED, virStorageVolDefPtr inputvol, unsigned int flags) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); int ret = -1; int fd = -1; int operation_flags; @@ -431,7 +432,7 @@ storageBackendCreateRaw(virConnectPtr conn ATTRIBUTE_UNUSED, } operation_flags = VIR_FILE_OPEN_FORCE_MODE | VIR_FILE_OPEN_FORCE_OWNER; - if (pool->def->type == VIR_STORAGE_POOL_NETFS) + if (def->type == VIR_STORAGE_POOL_NETFS) operation_flags |= VIR_FILE_OPEN_FORK; if (vol->target.perms->mode != (mode_t) -1) @@ -597,6 +598,7 @@ virStorageBackendCreateExecCommand(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, virCommandPtr cmd) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); struct stat st; gid_t gid; uid_t uid; @@ -606,7 +608,7 @@ virStorageBackendCreateExecCommand(virStoragePoolObjPtr pool, bool filecreated = false; int ret = -1; - if ((pool->def->type == VIR_STORAGE_POOL_NETFS) + if ((def->type == VIR_STORAGE_POOL_NETFS) && (((geteuid() == 0) && (vol->target.perms->uid != (uid_t) -1) && (vol->target.perms->uid != 0)) @@ -1029,6 +1031,7 @@ storageBackendCreateQemuImgSetBacking(virStoragePoolObjPtr pool, virStorageVolDefPtr inputvol, struct _virStorageBackendQemuImgInfo *info) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); int accessRetCode = -1; char *absolutePath = NULL; @@ -1071,7 +1074,7 @@ storageBackendCreateQemuImgSetBacking(virStoragePoolObjPtr pool, * validation. */ if ('/' != *(info->backingPath) && - virAsprintf(&absolutePath, "%s/%s", pool->def->target.path, + virAsprintf(&absolutePath, "%s/%s", def->target.path, info->backingPath) < 0) return -1; accessRetCode = access(absolutePath ? absolutePath : @@ -1921,7 +1924,7 @@ virStorageBackendPoolPathIsStable(const char *path) /* * Given a volume path directly in /dev/XXX, iterate over the - * entries in the directory pool->def->target.path and find the + * entries in the directory def->target.path and find the * first symlink pointing to the volume path. * * If, the target.path is /dev/, then return the original volume @@ -1940,6 +1943,7 @@ virStorageBackendStablePath(virStoragePoolObjPtr pool, const char *devpath, bool loop) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); DIR *dh; struct dirent *dent; char *stablepath; @@ -1948,8 +1952,8 @@ virStorageBackendStablePath(virStoragePoolObjPtr pool, int direrr; /* Logical pools are under /dev but already have stable paths */ - if (pool->def->type == VIR_STORAGE_POOL_LOGICAL || - !virStorageBackendPoolPathIsStable(pool->def->target.path)) + if (def->type == VIR_STORAGE_POOL_LOGICAL || + !virStorageBackendPoolPathIsStable(def->target.path)) goto ret_strdup; /* We loop here because /dev/disk/by-{id,path} may not have existed @@ -1957,7 +1961,7 @@ virStorageBackendStablePath(virStoragePoolObjPtr pool, * get created. */ reopen: - if (virDirOpenQuiet(&dh, pool->def->target.path) < 0) { + if (virDirOpenQuiet(&dh, def->target.path) < 0) { opentries++; if (loop && errno == ENOENT && opentries < 50) { usleep(100 * 1000); @@ -1965,7 +1969,7 @@ virStorageBackendStablePath(virStoragePoolObjPtr pool, } virReportSystemError(errno, _("cannot read dir '%s'"), - pool->def->target.path); + def->target.path); return NULL; } @@ -1981,8 +1985,7 @@ virStorageBackendStablePath(virStoragePoolObjPtr pool, retry: while ((direrr = virDirRead(dh, &dent, NULL)) > 0) { if (virAsprintf(&stablepath, "%s/%s", - pool->def->target.path, - dent->d_name) < 0) { + def->target.path, dent->d_name) < 0) { VIR_DIR_CLOSE(dh); return NULL; } @@ -2020,6 +2023,7 @@ createFileDir(virConnectPtr conn ATTRIBUTE_UNUSED, virStorageVolDefPtr inputvol, unsigned int flags) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); int err; virCheckFlags(0, -1); @@ -2044,7 +2048,7 @@ createFileDir(virConnectPtr conn ATTRIBUTE_UNUSED, vol->target.perms->mode), vol->target.perms->uid, vol->target.perms->gid, - (pool->def->type == VIR_STORAGE_POOL_NETFS + (def->type == VIR_STORAGE_POOL_NETFS ? VIR_DIR_CREATE_AS_UID : 0))) < 0) { return -1; } @@ -2064,6 +2068,8 @@ virStorageBackendVolCreateLocal(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool, virStorageVolDefPtr vol) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); + if (vol->target.format == VIR_STORAGE_FILE_DIR) vol->type = VIR_STORAGE_VOL_DIR; else if (vol->target.format == VIR_STORAGE_FILE_PLOOP) @@ -2081,8 +2087,7 @@ virStorageBackendVolCreateLocal(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 (virFileExists(vol->target.path)) { @@ -2768,6 +2773,7 @@ virStorageBackendVolWipeLocal(virConnectPtr conn ATTRIBUTE_UNUSED, int virStorageBackendBuildLocal(virStoragePoolObjPtr pool) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); int ret = -1; char *parent = NULL; char *p = NULL; @@ -2775,12 +2781,12 @@ virStorageBackendBuildLocal(virStoragePoolObjPtr pool) bool needs_create_as_uid; unsigned int dir_create_flags; - if (VIR_STRDUP(parent, pool->def->target.path) < 0) + if (VIR_STRDUP(parent, def->target.path) < 0) goto cleanup; if (!(p = strrchr(parent, '/'))) { virReportError(VIR_ERR_INVALID_ARG, _("path '%s' is not absolute"), - pool->def->target.path); + def->target.path); goto cleanup; } @@ -2796,11 +2802,11 @@ virStorageBackendBuildLocal(virStoragePoolObjPtr pool) } dir_create_flags = VIR_DIR_CREATE_ALLOW_EXIST; - needs_create_as_uid = (pool->def->type == VIR_STORAGE_POOL_NETFS); - mode = pool->def->target.perms.mode; + needs_create_as_uid = (def->type == VIR_STORAGE_POOL_NETFS); + mode = def->target.perms.mode; if (mode == (mode_t) -1 && - (needs_create_as_uid || !virFileExists(pool->def->target.path))) + (needs_create_as_uid || !virFileExists(def->target.path))) mode = VIR_STORAGE_DEFAULT_POOL_PERM_MODE; if (needs_create_as_uid) dir_create_flags |= VIR_DIR_CREATE_AS_UID; @@ -2808,10 +2814,10 @@ virStorageBackendBuildLocal(virStoragePoolObjPtr pool) /* Now create the final dir in the path with the uid/gid/mode * requested in the config. If the dir already exists, just set * the perms. */ - if (virDirCreate(pool->def->target.path, + if (virDirCreate(def->target.path, mode, - pool->def->target.perms.uid, - pool->def->target.perms.gid, + def->target.perms.uid, + def->target.perms.gid, dir_create_flags) < 0) goto cleanup; @@ -2836,14 +2842,16 @@ virStorageBackendDeleteLocal(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool, unsigned int flags) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); + virCheckFlags(0, -1); /* XXX delete all vols first ? */ - if (rmdir(pool->def->target.path) < 0) { + if (rmdir(def->target.path) < 0) { virReportSystemError(errno, _("failed to remove pool '%s'"), - pool->def->target.path); + def->target.path); return -1; } @@ -3575,6 +3583,7 @@ int virStorageBackendRefreshLocal(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); DIR *dir; struct dirent *ent; struct statvfs sb; @@ -3584,15 +3593,15 @@ virStorageBackendRefreshLocal(virConnectPtr conn ATTRIBUTE_UNUSED, int direrr; int fd = -1, ret = -1; - if (virDirOpen(&dir, pool->def->target.path) < 0) + if (virDirOpen(&dir, def->target.path) < 0) goto cleanup; - while ((direrr = virDirRead(dir, &ent, pool->def->target.path)) > 0) { + while ((direrr = virDirRead(dir, &ent, def->target.path)) > 0) { int err; if (virStringHasControlChars(ent->d_name)) { VIR_WARN("Ignoring file with control characters under '%s'", - pool->def->target.path); + def->target.path); continue; } @@ -3604,8 +3613,7 @@ virStorageBackendRefreshLocal(virConnectPtr conn ATTRIBUTE_UNUSED, vol->type = VIR_STORAGE_VOL_FILE; if (virAsprintf(&vol->target.path, "%s/%s", - pool->def->target.path, - vol->name) < 0) + def->target.path, vol->name) < 0) goto cleanup; if (VIR_STRDUP(vol->key, vol->target.path) < 0) @@ -3633,17 +3641,17 @@ virStorageBackendRefreshLocal(virConnectPtr conn ATTRIBUTE_UNUSED, if (VIR_ALLOC(target)) goto cleanup; - if ((fd = open(pool->def->target.path, O_RDONLY)) < 0) { + if ((fd = open(def->target.path, O_RDONLY)) < 0) { virReportSystemError(errno, _("cannot open path '%s'"), - pool->def->target.path); + def->target.path); goto cleanup; } if (fstat(fd, &statbuf) < 0) { virReportSystemError(errno, _("cannot stat path '%s'"), - pool->def->target.path); + def->target.path); goto cleanup; } @@ -3651,24 +3659,24 @@ virStorageBackendRefreshLocal(virConnectPtr conn ATTRIBUTE_UNUSED, goto cleanup; /* VolTargetInfoFD doesn't update capacity correctly for the pool case */ - if (statvfs(pool->def->target.path, &sb) < 0) { + if (statvfs(def->target.path, &sb) < 0) { virReportSystemError(errno, _("cannot statvfs path '%s'"), - pool->def->target.path); + def->target.path); goto cleanup; } - pool->def->capacity = ((unsigned long long)sb.f_frsize * - (unsigned long long)sb.f_blocks); - pool->def->available = ((unsigned long long)sb.f_bfree * - (unsigned long long)sb.f_frsize); - pool->def->allocation = pool->def->capacity - pool->def->available; + def->capacity = ((unsigned long long)sb.f_frsize * + (unsigned long long)sb.f_blocks); + def->available = ((unsigned long long)sb.f_bfree * + (unsigned long long)sb.f_frsize); + def->allocation = def->capacity - def->available; - pool->def->target.perms.mode = target->perms->mode; - pool->def->target.perms.uid = target->perms->uid; - pool->def->target.perms.gid = target->perms->gid; - VIR_FREE(pool->def->target.perms.label); - if (VIR_STRDUP(pool->def->target.perms.label, target->perms->label) < 0) + def->target.perms.mode = target->perms->mode; + def->target.perms.uid = target->perms->uid; + def->target.perms.gid = target->perms->gid; + VIR_FREE(def->target.perms.label); + if (VIR_STRDUP(def->target.perms.label, target->perms->label) < 0) goto cleanup; ret = 0; @@ -3738,6 +3746,7 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool, uint32_t lun, const char *dev) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); virStorageVolDefPtr vol = NULL; char *devpath = NULL; int retval = -1; @@ -3749,12 +3758,12 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool, * path to the device if the virDirRead loop to search the * target pool path for our devpath had failed. */ - if (!virStorageBackendPoolPathIsStable(pool->def->target.path) && - !(STREQ(pool->def->target.path, "/dev") || - STREQ(pool->def->target.path, "/dev/"))) { + if (!virStorageBackendPoolPathIsStable(def->target.path) && + !(STREQ(def->target.path, "/dev") || + STREQ(def->target.path, "/dev/"))) { virReportError(VIR_ERR_INVALID_ARG, _("unable to use target path '%s' for dev '%s'"), - NULLSTR(pool->def->target.path), dev); + NULLSTR(def->target.path), dev); goto cleanup; } @@ -3788,11 +3797,11 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool, goto cleanup; if (STREQ(devpath, vol->target.path) && - !(STREQ(pool->def->target.path, "/dev") || - STREQ(pool->def->target.path, "/dev/"))) { + !(STREQ(def->target.path, "/dev") || + STREQ(def->target.path, "/dev/"))) { VIR_DEBUG("No stable path found for '%s' in '%s'", - devpath, pool->def->target.path); + devpath, def->target.path); retval = -2; goto cleanup; @@ -3807,8 +3816,8 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool, if (!(vol->key = virStorageBackendSCSISerial(vol->target.path))) goto cleanup; - pool->def->capacity += vol->target.capacity; - pool->def->allocation += vol->target.allocation; + def->capacity += vol->target.capacity; + def->allocation += vol->target.allocation; if (virStoragePoolObjAddVol(pool, vol) < 0) goto cleanup; @@ -4085,6 +4094,7 @@ int virStorageBackendSCSIFindLUs(virStoragePoolObjPtr pool, uint32_t scanhost) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); int retval = 0; uint32_t bus, target, lun; const char *device_path = "/sys/bus/scsi/devices"; @@ -4126,7 +4136,7 @@ virStorageBackendSCSIFindLUs(virStoragePoolObjPtr pool, if (retval < 0) return -1; - VIR_DEBUG("Found %d LUs for pool %s", found, pool->def->name); + VIR_DEBUG("Found %d LUs for pool %s", found, def->name); return found; } -- 2.13.6
In preparation for privatizing the object, use the accessor. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_backend_disk.c | 98 +++++++++++++++++++++----------------- 1 file changed, 55 insertions(+), 43 deletions(-) diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c index a0f94512e3..44c135d807 100644 --- a/src/storage/storage_backend_disk.c +++ b/src/storage/storage_backend_disk.c @@ -59,6 +59,7 @@ virStorageBackendDiskMakeDataVol(virStoragePoolObjPtr pool, char **const groups, virStorageVolDefPtr vol) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); char *tmp, *devpath, *partname; /* Prepended path will be same for all partitions, so we can @@ -114,8 +115,7 @@ virStorageBackendDiskMakeDataVol(virStoragePoolObjPtr pool, * our deletion will fail because the name we generated is wrong. * Check for our conditions and see if the generated name is the * same as StablePath returns and has the 'p' in it */ - if (pool->def->source.devices[0].part_separator == - VIR_TRISTATE_BOOL_YES && + if (def->source.devices[0].part_separator == VIR_TRISTATE_BOOL_YES && !virIsDevMapperDevice(vol->target.path) && STREQ(groups[0], vol->target.path) && (tmp = strrchr(groups[0], 'p'))) { @@ -158,7 +158,7 @@ virStorageBackendDiskMakeDataVol(virStoragePoolObjPtr pool, } if (VIR_STRDUP(vol->source.extents[0].path, - pool->def->source.devices[0].path) < 0) + def->source.devices[0].path) < 0) return -1; } @@ -212,9 +212,9 @@ virStorageBackendDiskMakeDataVol(virStoragePoolObjPtr pool, } if (STRNEQ(groups[2], "metadata")) - pool->def->allocation += vol->target.allocation; - if (vol->source.extents[0].end > pool->def->capacity) - pool->def->capacity = vol->source.extents[0].end; + def->allocation += vol->target.allocation; + if (vol->source.extents[0].end > def->capacity) + def->capacity = vol->source.extents[0].end; return 0; } @@ -223,7 +223,8 @@ static int virStorageBackendDiskMakeFreeExtent(virStoragePoolObjPtr pool, char **const groups) { - virStoragePoolSourceDevicePtr dev = &pool->def->source.devices[0]; + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); + virStoragePoolSourceDevicePtr dev = &def->source.devices[0]; if (VIR_REALLOC_N(dev->freeExtents, dev->nfreeExtent + 1) < 0) @@ -253,11 +254,10 @@ virStorageBackendDiskMakeFreeExtent(virStoragePoolObjPtr pool, if (dev->freeExtents[dev->nfreeExtent].start == 0) dev->freeExtents[dev->nfreeExtent].start = SECTOR_SIZE; - pool->def->available += - (dev->freeExtents[dev->nfreeExtent].end - - dev->freeExtents[dev->nfreeExtent].start); - if (dev->freeExtents[dev->nfreeExtent].end > pool->def->capacity) - pool->def->capacity = dev->freeExtents[dev->nfreeExtent].end; + def->available += (dev->freeExtents[dev->nfreeExtent].end - + dev->freeExtents[dev->nfreeExtent].start); + if (dev->freeExtents[dev->nfreeExtent].end > def->capacity) + def->capacity = dev->freeExtents[dev->nfreeExtent].end; dev->nfreeExtent++; @@ -339,6 +339,7 @@ virStorageBackendDiskReadPartitions(virStoragePoolObjPtr pool, * */ + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); char *parthelper_path; virCommandPtr cmd; struct virStorageBackendDiskPoolVolData cbdata = { @@ -353,7 +354,7 @@ virStorageBackendDiskReadPartitions(virStoragePoolObjPtr pool, return -1; cmd = virCommandNewArgList(parthelper_path, - pool->def->source.devices[0].path, + def->source.devices[0].path, NULL); /* Check for the presence of the part_separator='yes'. Pass this @@ -362,16 +363,15 @@ virStorageBackendDiskReadPartitions(virStoragePoolObjPtr pool, * the generated device name for a source device which ends with * a non-numeric value (e.g. mpatha would generate mpathap#). */ - if (pool->def->source.devices[0].part_separator == - VIR_TRISTATE_BOOL_YES) + if (def->source.devices[0].part_separator == VIR_TRISTATE_BOOL_YES) virCommandAddArg(cmd, "-p"); /* If a volume is passed, virStorageBackendDiskMakeVol only updates the * pool allocation for that single volume. */ if (!vol) - pool->def->allocation = 0; - pool->def->capacity = pool->def->available = 0; + def->allocation = 0; + def->capacity = def->available = 0; ret = virCommandRunNul(cmd, 6, @@ -388,7 +388,8 @@ virStorageBackendDiskMakePoolGeometry(size_t ntok ATTRIBUTE_UNUSED, void *data) { virStoragePoolObjPtr pool = data; - virStoragePoolSourceDevicePtr device = &(pool->def->source.devices[0]); + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); + virStoragePoolSourceDevicePtr device = &(def->source.devices[0]); if (virStrToLong_i(groups[0], NULL, 0, &device->geometry.cylinders) < 0 || virStrToLong_i(groups[1], NULL, 0, &device->geometry.heads) < 0 || virStrToLong_i(groups[2], NULL, 0, &device->geometry.sectors) < 0) { @@ -403,6 +404,7 @@ virStorageBackendDiskMakePoolGeometry(size_t ntok ATTRIBUTE_UNUSED, static int virStorageBackendDiskReadGeometry(virStoragePoolObjPtr pool) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); char *parthelper_path; virCommandPtr cmd; int ret; @@ -413,9 +415,9 @@ virStorageBackendDiskReadGeometry(virStoragePoolObjPtr pool) return -1; cmd = virCommandNewArgList(parthelper_path, - pool->def->source.devices[0].path, - "-g", - NULL); + def->source.devices[0].path, + "-g", + NULL); ret = virCommandRunNul(cmd, 3, @@ -430,15 +432,17 @@ static int virStorageBackendDiskRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool) { - VIR_FREE(pool->def->source.devices[0].freeExtents); - pool->def->source.devices[0].nfreeExtent = 0; + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); + + VIR_FREE(def->source.devices[0].freeExtents); + def->source.devices[0].nfreeExtent = 0; virWaitForDevices(); - if (!virFileExists(pool->def->source.devices[0].path)) { + if (!virFileExists(def->source.devices[0].path)) { virReportError(VIR_ERR_INVALID_ARG, _("device path '%s' doesn't exist"), - pool->def->source.devices[0].path); + def->source.devices[0].path); return -1; } @@ -453,8 +457,9 @@ static int virStorageBackendDiskStartPool(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); const char *format; - const char *path = pool->def->source.devices[0].path; + const char *path = def->source.devices[0].path; virWaitForDevices(); @@ -464,9 +469,9 @@ virStorageBackendDiskStartPool(virConnectPtr conn ATTRIBUTE_UNUSED, return -1; } - if (pool->def->source.format == VIR_STORAGE_POOL_DISK_UNKNOWN) - pool->def->source.format = VIR_STORAGE_POOL_DISK_DOS; - format = virStoragePoolFormatDiskTypeToString(pool->def->source.format); + if (def->source.format == VIR_STORAGE_POOL_DISK_UNKNOWN) + def->source.format = VIR_STORAGE_POOL_DISK_DOS; + format = virStoragePoolFormatDiskTypeToString(def->source.format); if (!virStorageBackendDeviceIsEmpty(path, format, false)) return -1; @@ -482,7 +487,8 @@ virStorageBackendDiskBuildPool(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool, unsigned int flags) { - int format = pool->def->source.format; + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); + int format = def->source.format; const char *fmt; bool ok_to_mklabel = false; int ret = -1; @@ -499,26 +505,26 @@ virStorageBackendDiskBuildPool(virConnectPtr conn ATTRIBUTE_UNUSED, if (flags & VIR_STORAGE_POOL_BUILD_OVERWRITE) { ok_to_mklabel = true; } else { - if (virStorageBackendDeviceIsEmpty(pool->def->source.devices[0].path, + if (virStorageBackendDeviceIsEmpty(def->source.devices[0].path, fmt, true)) ok_to_mklabel = true; } if (ok_to_mklabel) { - if (virStorageBackendZeroPartitionTable(pool->def->source.devices[0].path, + if (virStorageBackendZeroPartitionTable(def->source.devices[0].path, 1024 * 1024) < 0) goto error; /* eg parted /dev/sda mklabel --script msdos */ if (format == VIR_STORAGE_POOL_DISK_UNKNOWN) - format = pool->def->source.format = VIR_STORAGE_POOL_DISK_DOS; + format = def->source.format = VIR_STORAGE_POOL_DISK_DOS; if (format == VIR_STORAGE_POOL_DISK_DOS) fmt = "msdos"; else fmt = virStoragePoolFormatDiskTypeToString(format); cmd = virCommandNewArgList(PARTED, - pool->def->source.devices[0].path, + def->source.devices[0].path, "mklabel", "--script", fmt, @@ -557,9 +563,10 @@ virStorageVolNumOfPartTypes(virStorageVolDefPtr def, static int virStorageBackendDiskPartTypeToCreate(virStoragePoolObjPtr pool) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); struct virStorageVolNumData data = { .count = 0 }; - if (pool->def->source.format == VIR_STORAGE_POOL_DISK_DOS) { + if (def->source.format == VIR_STORAGE_POOL_DISK_DOS) { /* count primary and extended partitions, can't be more than 3 to create a new primary partition */ if (virStoragePoolObjForEachVolume(pool, virStorageVolNumOfPartTypes, @@ -578,7 +585,9 @@ virStorageBackendDiskPartFormat(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, char** partFormat) { - if (pool->def->source.format == VIR_STORAGE_POOL_DISK_DOS) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); + + if (def->source.format == VIR_STORAGE_POOL_DISK_DOS) { const char *partedFormat; partedFormat = virStoragePartedFsTypeToString(vol->target.format); if (partedFormat == NULL) { @@ -652,7 +661,8 @@ virStorageBackendDiskPartBoundaries(virStoragePoolObjPtr pool, unsigned long long smallestSize = 0; unsigned long long extraBytes = 0; unsigned long long alignedAllocation = allocation; - virStoragePoolSourceDevicePtr dev = &pool->def->source.devices[0]; + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); + virStoragePoolSourceDevicePtr dev = &def->source.devices[0]; unsigned long long cylinderSize = (unsigned long long)dev->geometry.heads * dev->geometry.sectors * SECTOR_SIZE; @@ -670,7 +680,7 @@ virStorageBackendDiskPartBoundaries(virStoragePoolObjPtr pool, dev->freeExtents[i].start; unsigned long long neededSize = allocation; - if (pool->def->source.format == VIR_STORAGE_POOL_DISK_DOS) { + if (def->source.format == VIR_STORAGE_POOL_DISK_DOS) { /* align to cylinder boundary */ neededSize += extraBytes; if ((*start % cylinderSize) > extraBytes) { @@ -718,7 +728,7 @@ virStorageBackendDiskPartBoundaries(virStoragePoolObjPtr pool, } *end = *start + alignedAllocation; - if (pool->def->source.format == VIR_STORAGE_POOL_DISK_DOS) { + if (def->source.format == VIR_STORAGE_POOL_DISK_DOS) { /* adjust our allocation if start is not at a cylinder boundary */ *end -= (*start % cylinderSize); } @@ -764,7 +774,8 @@ virStorageBackendDiskDeleteVol(virConnectPtr conn, char *part_num = NULL; char *devpath = NULL; char *dev_name; - char *src_path = pool->def->source.devices[0].path; + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); + char *src_path = def->source.devices[0].path; char *srcname = last_component(src_path); virCommandPtr cmd = NULL; bool isDevMapperDevice; @@ -854,8 +865,9 @@ virStorageBackendDiskCreateVol(virConnectPtr conn, int res = -1; char *partFormat = NULL; unsigned long long startOffset = 0, endOffset = 0; + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); virCommandPtr cmd = virCommandNewArgList(PARTED, - pool->def->source.devices[0].path, + def->source.devices[0].path, "mkpart", "--script", NULL); @@ -887,8 +899,8 @@ virStorageBackendDiskCreateVol(virConnectPtr conn, virWaitForDevices(); /* Blow away free extent info, as we're about to re-populate it */ - VIR_FREE(pool->def->source.devices[0].freeExtents); - pool->def->source.devices[0].nfreeExtent = 0; + VIR_FREE(def->source.devices[0].freeExtents); + def->source.devices[0].nfreeExtent = 0; /* Specifying a target path is meaningless */ VIR_FREE(vol->target.path); -- 2.13.6
In preparation for privatizing the object, use the accessor. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_backend_logical.c | 63 ++++++++++++++++++++--------------- 1 file changed, 37 insertions(+), 26 deletions(-) diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 7bfe211c2d..0ad357729b 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -53,10 +53,11 @@ virStorageBackendLogicalSetActive(virStoragePoolObjPtr pool, int on) { int ret; + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); virCommandPtr cmd = virCommandNewArgList(VGCHANGE, on ? "-aly" : "-aln", - pool->def->source.name, + def->source.name, NULL); ret = virCommandRun(cmd, NULL); @@ -266,6 +267,7 @@ virStorageBackendLogicalMakeVol(char **const groups, { struct virStorageBackendLogicalPoolVolData *data = opaque; virStoragePoolObjPtr pool = data->pool; + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); virStorageVolDefPtr vol = NULL; bool is_new_vol = false; int ret = -1; @@ -309,7 +311,7 @@ virStorageBackendLogicalMakeVol(char **const groups, if (vol->target.path == NULL) { if (virAsprintf(&vol->target.path, "%s/%s", - pool->def->target.path, vol->name) < 0) + def->target.path, vol->name) < 0) goto cleanup; } @@ -334,7 +336,7 @@ virStorageBackendLogicalMakeVol(char **const groups, goto cleanup; if (virAsprintf(&vol->target.backingStore->path, "%s/%s", - pool->def->target.path, groups[1]) < 0) + def->target.path, groups[1]) < 0) goto cleanup; vol->target.backingStore->format = VIR_STORAGE_POOL_LOGICAL_LVM2; @@ -433,6 +435,7 @@ virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool, }; int ret = -1; virCommandPtr cmd; + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); struct virStorageBackendLogicalPoolVolData cbdata = { .pool = pool, .vol = vol, @@ -446,7 +449,7 @@ virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool, "--nosuffix", "--options", "lv_name,origin,uuid,devices,segtype,stripes,seg_size,vg_extent_size,size,lv_attr", - pool->def->source.name, + def->source.name, NULL); if (virCommandRunRegex(cmd, 1, @@ -469,11 +472,13 @@ virStorageBackendLogicalRefreshPoolFunc(char **const groups, void *data) { virStoragePoolObjPtr pool = data; - if (virStrToLong_ull(groups[0], NULL, 10, &pool->def->capacity) < 0) + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); + + if (virStrToLong_ull(groups[0], NULL, 10, &def->capacity) < 0) return -1; - if (virStrToLong_ull(groups[1], NULL, 10, &pool->def->available) < 0) + if (virStrToLong_ull(groups[1], NULL, 10, &def->available) < 0) return -1; - pool->def->allocation = pool->def->capacity - pool->def->available; + def->allocation = def->capacity - def->available; return 0; } @@ -631,6 +636,7 @@ virStorageBackendLogicalFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSED, static bool virStorageBackendLogicalMatchPoolSource(virStoragePoolObjPtr pool) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); virStoragePoolSourceList sourceList; virStoragePoolSource *thisSource = NULL; size_t i, j; @@ -646,14 +652,14 @@ virStorageBackendLogicalMatchPoolSource(virStoragePoolObjPtr pool) /* Search the pvs output for this pool's source.name */ for (i = 0; i < sourceList.nsources; i++) { thisSource = &sourceList.sources[i]; - if (STREQ(thisSource->name, pool->def->source.name)) + if (STREQ(thisSource->name, def->source.name)) break; } if (i == sourceList.nsources) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("cannot find logical volume group name '%s'"), - pool->def->source.name); + def->source.name); goto cleanup; } @@ -661,7 +667,7 @@ virStorageBackendLogicalMatchPoolSource(virStoragePoolObjPtr pool) * they match as well; otherwise, matching can only occur on the * pool's name. */ - if (!pool->def->source.ndevice) { + if (!def->source.ndevice) { ret = true; goto cleanup; } @@ -669,9 +675,9 @@ virStorageBackendLogicalMatchPoolSource(virStoragePoolObjPtr pool) /* Let's make sure the pool's device(s) match what the pvs output has * for volume group devices. */ - for (i = 0; i < pool->def->source.ndevice; i++) { + for (i = 0; i < def->source.ndevice; i++) { for (j = 0; j < thisSource->ndevice; j++) { - if (STREQ(pool->def->source.devices[i].path, + if (STREQ(def->source.devices[i].path, thisSource->devices[j].path)) matchcount++; } @@ -683,7 +689,7 @@ virStorageBackendLogicalMatchPoolSource(virStoragePoolObjPtr pool) if (matchcount == 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("cannot find any matching source devices for logical " - "volume group '%s'"), pool->def->source.name); + "volume group '%s'"), def->source.name); goto cleanup; } @@ -692,7 +698,7 @@ virStorageBackendLogicalMatchPoolSource(virStoragePoolObjPtr pool) * to 'add' to or 'remove' from the volume group outside of libvirt's * knowledge. Rather than fail on that, provide a warning and move on. */ - if (matchcount != pool->def->source.ndevice) + if (matchcount != def->source.ndevice) VIR_WARN("pool device list count doesn't match pvs device list count"); ret = true; @@ -710,10 +716,12 @@ static int virStorageBackendLogicalCheckPool(virStoragePoolObjPtr pool, bool *isActive) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); + /* If we can find the target.path as well as ensure that the * pool's def source */ - *isActive = virFileExists(pool->def->target.path) && + *isActive = virFileExists(def->target.path) && virStorageBackendLogicalMatchPoolSource(pool); return 0; } @@ -738,6 +746,7 @@ virStorageBackendLogicalBuildPool(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool, unsigned int flags) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); virCommandPtr vgcmd = NULL; int ret = -1; size_t i = 0; @@ -749,10 +758,10 @@ virStorageBackendLogicalBuildPool(virConnectPtr conn ATTRIBUTE_UNUSED, VIR_STORAGE_POOL_BUILD_NO_OVERWRITE, cleanup); - vgcmd = virCommandNewArgList(VGCREATE, pool->def->source.name, NULL); + vgcmd = virCommandNewArgList(VGCREATE, def->source.name, NULL); - for (i = 0; i < pool->def->source.ndevice; i++) { - const char *path = pool->def->source.devices[i].path; + for (i = 0; i < def->source.ndevice; i++) { + const char *path = def->source.devices[i].path; /* The blkid FS and Part probing code doesn't know "lvm2" (this * pool's only format type), but it does know "LVM2_member", so @@ -782,7 +791,7 @@ virStorageBackendLogicalBuildPool(virConnectPtr conn ATTRIBUTE_UNUSED, if (ret < 0) { size_t j; for (j = 0; j < i; j++) - virStorageBackendLogicalRemoveDevice(pool->def->source.devices[j].path); + virStorageBackendLogicalRemoveDevice(def->source.devices[j].path); } return ret; } @@ -806,6 +815,7 @@ virStorageBackendLogicalRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, int vars[] = { 2 }; + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); virCommandPtr cmd = NULL; int ret = -1; @@ -822,7 +832,7 @@ virStorageBackendLogicalRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, "--unbuffered", "--nosuffix", "--options", "vg_size,vg_free", - pool->def->source.name, + def->source.name, NULL); /* Now get basic volgrp metadata */ @@ -865,6 +875,7 @@ virStorageBackendLogicalDeletePool(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool, unsigned int flags) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); virCommandPtr cmd = NULL; size_t i; int ret = -1; @@ -873,14 +884,14 @@ virStorageBackendLogicalDeletePool(virConnectPtr conn ATTRIBUTE_UNUSED, /* first remove the volume group */ cmd = virCommandNewArgList(VGREMOVE, - "-f", pool->def->source.name, + "-f", def->source.name, NULL); if (virCommandRun(cmd, NULL) < 0) goto cleanup; /* now remove the pv devices and clear them out */ - for (i = 0; i < pool->def->source.ndevice; i++) - virStorageBackendLogicalRemoveDevice(pool->def->source.devices[i].path); + for (i = 0; i < def->source.ndevice; i++) + virStorageBackendLogicalRemoveDevice(def->source.devices[i].path); ret = 0; @@ -931,6 +942,7 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn, virStorageVolDefPtr vol) { int fd = -1; + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); virCommandPtr cmd = NULL; virErrorPtr err; struct stat sb; @@ -947,8 +959,7 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn, 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; cmd = virCommandNewArgList(LVCREATE, @@ -968,7 +979,7 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn, if (vol->target.backingStore) virCommandAddArgList(cmd, "-s", vol->target.backingStore->path, NULL); else - virCommandAddArg(cmd, pool->def->source.name); + virCommandAddArg(cmd, def->source.name); if (virCommandRun(cmd, NULL) < 0) goto error; -- 2.13.6
In preparation for privatizing the object, use the accessor. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_backend_sheepdog.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/src/storage/storage_backend_sheepdog.c b/src/storage/storage_backend_sheepdog.c index e72dcda9c8..3d9c341a11 100644 --- a/src/storage/storage_backend_sheepdog.c +++ b/src/storage/storage_backend_sheepdog.c @@ -96,13 +96,14 @@ void virStorageBackendSheepdogAddHostArg(virCommandPtr cmd, virStoragePoolObjPtr pool) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); const char *address = "localhost"; int port = 7000; - if (pool->def->source.nhost > 0) { - if (pool->def->source.hosts[0].name != NULL) - address = pool->def->source.hosts[0].name; - if (pool->def->source.hosts[0].port) - port = pool->def->source.hosts[0].port; + if (def->source.nhost > 0) { + if (def->source.hosts[0].name != NULL) + address = def->source.hosts[0].name; + if (def->source.hosts[0].port) + port = def->source.hosts[0].port; } virCommandAddArg(cmd, "-a"); virCommandAddArgFormat(cmd, "%s", address); @@ -202,7 +203,8 @@ virStorageBackendSheepdogRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, if (virCommandRun(cmd, NULL) < 0) goto cleanup; - if (virStorageBackendSheepdogParseNodeInfo(pool->def, output) < 0) + if (virStorageBackendSheepdogParseNodeInfo(virStoragePoolObjGetDef(pool), + output) < 0) goto cleanup; ret = virStorageBackendSheepdogRefreshAllVol(conn, pool); @@ -236,6 +238,8 @@ virStorageBackendSheepdogCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool, virStorageVolDefPtr vol) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); + if (vol->target.encryption != NULL) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("storage pool does not support encrypted " @@ -247,7 +251,7 @@ virStorageBackendSheepdogCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED, 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; VIR_FREE(vol->target.path); @@ -356,8 +360,9 @@ virStorageBackendSheepdogRefreshVol(virConnectPtr conn ATTRIBUTE_UNUSED, { int ret; char *output = NULL; - + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); virCommandPtr cmd = virCommandNewArgList(SHEEPDOGCLI, "vdi", "list", vol->name, "-r", NULL); + virStorageBackendSheepdogAddHostArg(cmd, pool); virCommandSetOutputBuffer(cmd, &output); ret = virCommandRun(cmd, NULL); @@ -372,7 +377,7 @@ virStorageBackendSheepdogRefreshVol(virConnectPtr conn ATTRIBUTE_UNUSED, 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; VIR_FREE(vol->target.path); -- 2.13.6
In preparation for privatizing the object, use the accessor. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_backend_fs.c | 90 ++++++++++++++++++++++------------------ 1 file changed, 50 insertions(+), 40 deletions(-) diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 847dbc9e02..f54759983c 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -222,25 +222,27 @@ virStorageBackendFileSystemNetFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSE static int virStorageBackendFileSystemIsValid(virStoragePoolObjPtr pool) { - if (pool->def->type == VIR_STORAGE_POOL_NETFS) { - if (pool->def->source.nhost != 1) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); + + if (def->type == VIR_STORAGE_POOL_NETFS) { + 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.dir == NULL) { + if (def->source.dir == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing source path")); return -1; } } else { - if (pool->def->source.ndevice != 1) { - if (pool->def->source.ndevice == 0) + if (def->source.ndevice != 1) { + if (def->source.ndevice == 0) virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing source device")); else @@ -264,22 +266,23 @@ virStorageBackendFileSystemIsValid(virStoragePoolObjPtr pool) static char * virStorageBackendFileSystemGetPoolSource(virStoragePoolObjPtr pool) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); char *src = NULL; - if (pool->def->type == VIR_STORAGE_POOL_NETFS) { - if (pool->def->source.format == VIR_STORAGE_POOL_NETFS_CIFS) { + if (def->type == VIR_STORAGE_POOL_NETFS) { + if (def->source.format == VIR_STORAGE_POOL_NETFS_CIFS) { if (virAsprintf(&src, "//%s/%s", - pool->def->source.hosts[0].name, - pool->def->source.dir) < 0) + def->source.hosts[0].name, + def->source.dir) < 0) return NULL; } else { if (virAsprintf(&src, "%s:%s", - pool->def->source.hosts[0].name, - pool->def->source.dir) < 0) + def->source.hosts[0].name, + def->source.dir) < 0) return NULL; } } else { - if (VIR_STRDUP(src, pool->def->source.devices[0].path) < 0) + if (VIR_STRDUP(src, def->source.devices[0].path) < 0) return NULL; } return src; @@ -297,6 +300,7 @@ static int virStorageBackendFileSystemIsMounted(virStoragePoolObjPtr pool) { int ret = -1; + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); char *src = NULL; FILE *mtab; struct mntent ent; @@ -317,8 +321,7 @@ virStorageBackendFileSystemIsMounted(virStoragePoolObjPtr pool) /* compare both mount destinations and sources to be sure the mounted * FS pool is really the one we're looking for */ - if ((rc1 = virFileComparePaths(ent.mnt_dir, - pool->def->target.path)) < 0 || + if ((rc1 = virFileComparePaths(ent.mnt_dir, def->target.path)) < 0 || (rc2 = virFileComparePaths(ent.mnt_fsname, src)) < 0) goto cleanup; @@ -349,16 +352,17 @@ virStorageBackendFileSystemIsMounted(virStoragePoolObjPtr pool) static int virStorageBackendFileSystemMount(virStoragePoolObjPtr pool) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); char *src = NULL; /* 'mount -t auto' doesn't seem to auto determine nfs (or cifs), * while plain 'mount' does. We have to craft separate argvs to * accommodate this */ - bool netauto = (pool->def->type == VIR_STORAGE_POOL_NETFS && - pool->def->source.format == VIR_STORAGE_POOL_NETFS_AUTO); - bool glusterfs = (pool->def->type == VIR_STORAGE_POOL_NETFS && - pool->def->source.format == VIR_STORAGE_POOL_NETFS_GLUSTERFS); - bool cifsfs = (pool->def->type == VIR_STORAGE_POOL_NETFS && - pool->def->source.format == VIR_STORAGE_POOL_NETFS_CIFS); + bool netauto = (def->type == VIR_STORAGE_POOL_NETFS && + def->source.format == VIR_STORAGE_POOL_NETFS_AUTO); + bool glusterfs = (def->type == VIR_STORAGE_POOL_NETFS && + def->source.format == VIR_STORAGE_POOL_NETFS_GLUSTERFS); + bool cifsfs = (def->type == VIR_STORAGE_POOL_NETFS && + def->source.format == VIR_STORAGE_POOL_NETFS_CIFS); virCommandPtr cmd = NULL; int ret = -1; int rc; @@ -371,7 +375,7 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr pool) /* Short-circuit if already mounted */ if (rc == 1) { - VIR_INFO("Target '%s' is already mounted", pool->def->target.path); + VIR_INFO("Target '%s' is already mounted", def->target.path); return 0; } @@ -381,34 +385,34 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr pool) if (netauto) cmd = virCommandNewArgList(MOUNT, src, - pool->def->target.path, + def->target.path, NULL); else if (glusterfs) cmd = virCommandNewArgList(MOUNT, "-t", - virStoragePoolFormatFileSystemNetTypeToString(pool->def->source.format), + virStoragePoolFormatFileSystemNetTypeToString(def->source.format), src, "-o", "direct-io-mode=1", - pool->def->target.path, + def->target.path, NULL); else if (cifsfs) cmd = virCommandNewArgList(MOUNT, "-t", - virStoragePoolFormatFileSystemNetTypeToString(pool->def->source.format), + virStoragePoolFormatFileSystemNetTypeToString(def->source.format), src, - pool->def->target.path, + def->target.path, "-o", "guest", NULL); else cmd = virCommandNewArgList(MOUNT, "-t", - (pool->def->type == VIR_STORAGE_POOL_FS ? - virStoragePoolFormatFileSystemTypeToString(pool->def->source.format) : - virStoragePoolFormatFileSystemNetTypeToString(pool->def->source.format)), + (def->type == VIR_STORAGE_POOL_FS ? + virStoragePoolFormatFileSystemTypeToString(def->source.format) : + virStoragePoolFormatFileSystemNetTypeToString(def->source.format)), src, - pool->def->target.path, + def->target.path, NULL); if (virCommandRun(cmd, NULL) < 0) @@ -435,7 +439,9 @@ static int virStorageBackendFileSystemStart(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool) { - if (pool->def->type != VIR_STORAGE_POOL_DIR && + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); + + if (def->type != VIR_STORAGE_POOL_DIR && virStorageBackendFileSystemMount(pool) < 0) return -1; @@ -459,6 +465,7 @@ static int virStorageBackendFileSystemStop(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); virCommandPtr cmd = NULL; int ret = -1; int rc; @@ -470,7 +477,7 @@ virStorageBackendFileSystemStop(virConnectPtr conn ATTRIBUTE_UNUSED, if ((rc = virStorageBackendFileSystemIsMounted(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; @@ -486,8 +493,10 @@ static int virStorageBackendFileSystemCheck(virStoragePoolObjPtr pool, bool *isActive) { - if (pool->def->type == VIR_STORAGE_POOL_DIR) { - *isActive = virFileExists(pool->def->target.path); + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); + + if (def->type == VIR_STORAGE_POOL_DIR) { + *isActive = virFileExists(def->target.path); #if WITH_STORAGE_FS } else { int ret; @@ -561,25 +570,26 @@ static int virStorageBackendMakeFileSystem(virStoragePoolObjPtr pool, unsigned int flags) { + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); const char *device = NULL, *format = NULL; bool ok_to_mkfs = false; int ret = -1; - if (pool->def->source.devices == NULL) { + if (def->source.devices == NULL) { virReportError(VIR_ERR_OPERATION_INVALID, _("No source device specified when formatting pool '%s'"), - pool->def->name); + def->name); goto error; } - device = pool->def->source.devices[0].path; - format = virStoragePoolFormatFileSystemTypeToString(pool->def->source.format); + device = def->source.devices[0].path; + format = virStoragePoolFormatFileSystemTypeToString(def->source.format); VIR_DEBUG("source device: '%s' format: '%s'", device, format); if (!virFileExists(device)) { virReportError(VIR_ERR_OPERATION_INVALID, _("Source device does not exist when formatting pool '%s'"), - pool->def->name); + def->name); goto error; } -- 2.13.6
In preparation for privatizing the object, use the accessor. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_backend_gluster.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index 05e7bff638..eac771b42f 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -74,8 +74,9 @@ static virStorageBackendGlusterStatePtr virStorageBackendGlusterOpen(virStoragePoolObjPtr pool) { virStorageBackendGlusterStatePtr ret = NULL; - const char *name = pool->def->source.name; - const char *dir = pool->def->source.dir; + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); + const char *name = def->source.name; + const char *dir = def->source.dir; bool trailing_slash = true; /* Volume name must not contain '/'; optional path allows use of a @@ -112,11 +113,11 @@ virStorageBackendGlusterOpen(virStoragePoolObjPtr pool) goto error; if (VIR_STRDUP(ret->uri->scheme, "gluster") < 0) goto error; - if (VIR_STRDUP(ret->uri->server, pool->def->source.hosts[0].name) < 0) + if (VIR_STRDUP(ret->uri->server, def->source.hosts[0].name) < 0) goto error; if (virAsprintf(&ret->uri->path, "/%s%s", ret->volname, ret->dir) < 0) goto error; - ret->uri->port = pool->def->source.hosts[0].port; + ret->uri->port = def->source.hosts[0].port; /* Actually connect to glfs */ if (!(ret->vol = glfs_new(ret->volname))) { @@ -343,6 +344,7 @@ virStorageBackendGlusterRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool) { int ret = -1; + virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); virStorageBackendGlusterStatePtr state = NULL; struct { struct dirent ent; @@ -401,11 +403,11 @@ virStorageBackendGlusterRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, goto cleanup; } - pool->def->capacity = ((unsigned long long)sb.f_frsize * - (unsigned long long)sb.f_blocks); - pool->def->available = ((unsigned long long)sb.f_bfree * - (unsigned long long)sb.f_frsize); - pool->def->allocation = pool->def->capacity - pool->def->available; + def->capacity = ((unsigned long long)sb.f_frsize * + (unsigned long long)sb.f_blocks); + def->available = ((unsigned long long)sb.f_bfree * + (unsigned long long)sb.f_frsize); + def->allocation = def->capacity - def->available; ret = 0; cleanup: -- 2.13.6
On 10/06/2017 01:23 PM, 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 series is the first half of the storage backend changes to use the virStoragePoolObjGetDef accessor.
The next/last series will be the next half, plus the change to move virStoragePoolObj and virStorageVolDefList into virstoragepoolobj.c.
After that adjustments to virstoragepoolobj similar to other drivers and perhaps even generating a virStorageVolObjPtr - e.g. a hash table of volume defs rather than a linked list of defs.
John Ferlan (6): storage: Use virStoragePoolObjGetDef accessor for storage_util storage: Use virStoragePoolObjGetDef accessor for Disk backend storage: Use virStoragePoolObjGetDef accessor for Logical backend storage: Use virStoragePoolObjGetDef accessor for Sheepdog backend storage: Use virStoragePoolObjGetDef accessor for FS backend storage: Use virStoragePoolObjGetDef accessor for Gluster backend
src/storage/storage_backend_disk.c | 98 ++++++++++++++++------------ src/storage/storage_backend_fs.c | 90 +++++++++++++------------ src/storage/storage_backend_gluster.c | 20 +++--- src/storage/storage_backend_logical.c | 63 ++++++++++-------- src/storage/storage_backend_sheepdog.c | 23 ++++--- src/storage/storage_util.c | 116 ++++++++++++++++++--------------- 6 files changed, 230 insertions(+), 180 deletions(-)
ACK Michal
participants (2)
-
John Ferlan -
Michal Privoznik