Re: [libvirt] [PATCHv2 6/7] conf: drop redundant parameters during probe

Now that each virStorageSource can track allocation information, and given that we already have the information without extra syscalls, it's easier to just always populate the information directly into the struct than it is to sometimes pass the address of the struct members down the call chain. * src/storage/storage_backend.h (virStorageBackendUpdateVolInfo) (virStorageBackendUpdateVolTargetInfo) (virStorageBackendUpdateVolTargetInfoFD): Update signature. * src/storage/storage_backend.c (virStorageBackendUpdateVolInfo) (virStorageBackendUpdateVolTargetInfo) (virStorageBackendUpdateVolTargetInfoFD): Always populate struct members instead. * src/storage/storage_backend_disk.c (virStorageBackendDiskMakeDataVol): Update client. * src/storage/storage_backend_fs.c (virStorageBackendProbeTarget) (virStorageBackendFileSystemRefresh) (virStorageBackendFileSystemVolRefresh): Likewise. * src/storage/storage_backend_gluster.c (virStorageBackendGlusterRefreshVol): Likewise. * src/storage/storage_backend_logical.c (virStorageBackendLogicalMakeVol): Likewise. * src/storage/storage_backend_mpath.c (virStorageBackendMpathNewVol): Likewise. * src/storage/storage_backend_scsi.c (virStorageBackendSCSINewLun): Likewise. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- src/storage/storage_backend.c | 74 +++++++++++++---------------------- src/storage/storage_backend.h | 7 +--- src/storage/storage_backend_disk.c | 2 +- src/storage/storage_backend_fs.c | 16 +++----- src/storage/storage_backend_gluster.c | 4 +- src/storage/storage_backend_logical.c | 2 +- src/storage/storage_backend_mpath.c | 2 +- src/storage/storage_backend_scsi.c | 2 +- 8 files changed, 39 insertions(+), 70 deletions(-)

diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index c8cf50e..8fe3687 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1385,8 +1385,6 @@ virStorageBackendVolOpen(const char *path, struct stat *sb, int virStorageBackendUpdateVolTargetInfo(virStorageSourcePtr target, - unsigned long long *allocation, - unsigned long long *capacity, bool withBlockVolFormat, unsigned int openflags) { @@ -1397,11 +1395,7 @@ virStorageBackendUpdateVolTargetInfo(virStorageSourcePtr target, goto cleanup; fd = ret; - if ((ret = virStorageBackendUpdateVolTargetInfoFD(target, - fd, - &sb, - allocation, - capacity)) < 0) + if ((ret = virStorageBackendUpdateVolTargetInfoFD(target, fd, &sb)) < 0) goto cleanup; if (withBlockVolFormat) { @@ -1417,22 +1411,18 @@ virStorageBackendUpdateVolTargetInfo(virStorageSourcePtr target, int virStorageBackendUpdateVolInfo(virStorageVolDefPtr vol, - bool withCapacity, bool withBlockVolFormat, unsigned int openflags) { int ret; if ((ret = virStorageBackendUpdateVolTargetInfo(&vol->target, - &vol->target.allocation, - withCapacity ? &vol->target.capacity : NULL, withBlockVolFormat, openflags)) < 0) return ret; if (vol->backingStore.path && (ret = virStorageBackendUpdateVolTargetInfo(&vol->backingStore, - NULL, NULL, withBlockVolFormat, VIR_STORAGE_VOL_OPEN_DEFAULT)) < 0) return ret; @@ -1453,50 +1443,42 @@ virStorageBackendUpdateVolInfo(virStorageVolDefPtr vol, int virStorageBackendUpdateVolTargetInfoFD(virStorageSourcePtr target, int fd, - struct stat *sb, - unsigned long long *allocation, - unsigned long long *capacity) + struct stat *sb) { #if WITH_SELINUX security_context_t filecon = NULL; #endif - if (allocation) { - if (S_ISREG(sb->st_mode)) { + if (S_ISREG(sb->st_mode)) { #ifndef WIN32 - *allocation = (unsigned long long)sb->st_blocks * - (unsigned long long)DEV_BSIZE; + target->allocation = (unsigned long long)sb->st_blocks * + (unsigned long long)DEV_BSIZE; #else - *allocation = sb->st_size; + target->allocation = sb->st_size; #endif - /* Regular files may be sparse, so logical size (capacity) is not same - * as actual allocation above - */ - if (capacity) - *capacity = sb->st_size; - } else if (S_ISDIR(sb->st_mode)) { - *allocation = 0; - if (capacity) - *capacity = 0; - - } else if (fd >= 0) { - off_t end; - /* XXX this is POSIX compliant, but doesn't work for CHAR files, - * only BLOCK. There is a Linux specific ioctl() for getting - * size of both CHAR / BLOCK devices we should check for in - * configure - */ - end = lseek(fd, 0, SEEK_END); - if (end == (off_t)-1) { - virReportSystemError(errno, - _("cannot seek to end of file '%s'"), - target->path); - return -1; - } - *allocation = end; - if (capacity) - *capacity = end; + /* Regular files may be sparse, so logical size (capacity) is not same + * as actual allocation above + */ + target->capacity = sb->st_size; + } else if (S_ISDIR(sb->st_mode)) { + target->allocation = 0; + target->capacity = 0; + } else if (fd >= 0) { + off_t end; + /* XXX this is POSIX compliant, but doesn't work for CHAR files, + * only BLOCK. There is a Linux specific ioctl() for getting + * size of both CHAR / BLOCK devices we should check for in + * configure + */ + end = lseek(fd, 0, SEEK_END); + if (end == (off_t)-1) { + virReportSystemError(errno, + _("cannot seek to end of file '%s'"), + target->path); + return -1; } + target->allocation = end; + target->capacity = end; } if (!target->perms && VIR_ALLOC(target->perms) < 0) diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index 8442c13..89511f8 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -138,19 +138,14 @@ int virStorageBackendVolOpen(const char *path, struct stat *sb, ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); int virStorageBackendUpdateVolInfo(virStorageVolDefPtr vol, - bool withCapacity, bool withBlockVolFormat, unsigned int openflags); int virStorageBackendUpdateVolTargetInfo(virStorageSourcePtr target, - unsigned long long *allocation, - unsigned long long *capacity, bool withBlockVolFormat, unsigned int openflags); int virStorageBackendUpdateVolTargetInfoFD(virStorageSourcePtr target, int fd, - struct stat *sb, - unsigned long long *allocation, - unsigned long long *capacity); + struct stat *sb); char *virStorageBackendStablePath(virStoragePoolObjPtr pool, const char *devpath, diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c index 13336fc..9cebcca 100644 --- a/src/storage/storage_backend_disk.c +++ b/src/storage/storage_backend_disk.c @@ -113,7 +113,7 @@ virStorageBackendDiskMakeDataVol(virStoragePoolObjPtr pool, } /* Refresh allocation/capacity/perms */ - if (virStorageBackendUpdateVolInfo(vol, true, false, + if (virStorageBackendUpdateVolInfo(vol, false, VIR_STORAGE_VOL_OPEN_DEFAULT) < 0) return -1; diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index e8617cb..501fa8d 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -65,8 +65,6 @@ static int ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) virStorageBackendProbeTarget(virStorageSourcePtr target, char **backingStore, int *backingStoreFormat, - unsigned long long *allocation, - unsigned long long *capacity, virStorageEncryptionPtr *encryption) { int fd = -1; @@ -86,9 +84,7 @@ virStorageBackendProbeTarget(virStorageSourcePtr target, goto error; /* Take care to propagate ret, it is not always -1 */ fd = ret; - if ((ret = virStorageBackendUpdateVolTargetInfoFD(target, fd, &sb, - allocation, - capacity)) < 0) { + if ((ret = virStorageBackendUpdateVolTargetInfoFD(target, fd, &sb)) < 0) { goto error; } @@ -143,8 +139,8 @@ virStorageBackendProbeTarget(virStorageSourcePtr target, ret = 0; } - if (capacity && meta && meta->capacity) - *capacity = meta->capacity; + if (meta && meta->capacity) + target->capacity = meta->capacity; if (encryption && meta && meta->encrypted) { if (VIR_ALLOC(*encryption) < 0) @@ -880,8 +876,6 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn ATTRIBUTE_UNUSED, if ((ret = virStorageBackendProbeTarget(&vol->target, &backingStore, &backingStoreFormat, - &vol->target.allocation, - &vol->target.capacity, &vol->target.encryption)) < 0) { if (ret == -2) { /* Silently ignore non-regular files, @@ -909,7 +903,7 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn ATTRIBUTE_UNUSED, vol->backingStore.format = backingStoreFormat; if (virStorageBackendUpdateVolTargetInfo(&vol->backingStore, - NULL, NULL, false, + false, VIR_STORAGE_VOL_OPEN_DEFAULT) < 0) { /* The backing file is currently unavailable, the capacity, * allocation, owner, group and mode are unknown. Just log the @@ -1194,7 +1188,7 @@ virStorageBackendFileSystemVolRefresh(virConnectPtr conn, int ret; /* Refresh allocation / permissions info in case its changed */ - ret = virStorageBackendUpdateVolInfo(vol, false, false, + ret = virStorageBackendUpdateVolInfo(vol, false, VIR_STORAGE_VOL_FS_OPEN_FLAGS); if (ret < 0) return ret; diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index 6eed3ec..4aec44b 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -267,9 +267,7 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state, if (VIR_ALLOC(vol) < 0) goto cleanup; - if (virStorageBackendUpdateVolTargetInfoFD(&vol->target, -1, st, - &vol->target.allocation, - &vol->target.capacity) < 0) + if (virStorageBackendUpdateVolTargetInfoFD(&vol->target, -1, st) < 0) goto cleanup; if (virStorageBackendGlusterSetMetadata(state, vol, name) < 0) diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index a597e67..ed3a012 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -149,7 +149,7 @@ virStorageBackendLogicalMakeVol(char **const groups, if (!vol->key && VIR_STRDUP(vol->key, groups[2]) < 0) goto cleanup; - if (virStorageBackendUpdateVolInfo(vol, true, false, + if (virStorageBackendUpdateVolInfo(vol, false, VIR_STORAGE_VOL_OPEN_DEFAULT) < 0) goto cleanup; diff --git a/src/storage/storage_backend_mpath.c b/src/storage/storage_backend_mpath.c index 8c3b0df..f0ed189 100644 --- a/src/storage/storage_backend_mpath.c +++ b/src/storage/storage_backend_mpath.c @@ -60,7 +60,7 @@ virStorageBackendMpathNewVol(virStoragePoolObjPtr pool, if (virAsprintf(&vol->target.path, "/dev/%s", dev) < 0) goto cleanup; - if (virStorageBackendUpdateVolInfo(vol, true, true, + if (virStorageBackendUpdateVolInfo(vol, true, VIR_STORAGE_VOL_OPEN_DEFAULT) < 0) { goto cleanup; } diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index 6652a6a..cf546fb 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -199,7 +199,7 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool, goto free_vol; } - if (virStorageBackendUpdateVolInfo(vol, true, true, + if (virStorageBackendUpdateVolInfo(vol, true, VIR_STORAGE_VOL_OPEN_DEFAULT) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to update volume for '%s'"),