[libvirt] [PATCH v3 4/8] storage: Use VIR_AUTOCLOSE
John Ferlan
jferlan at redhat.com
Tue Feb 12 14:19:02 UTC 2019
Modify code to use the VIR_AUTOCLOSE logic cleaning up any
now unnecessary goto paths.
Signed-off-by: John Ferlan <jferlan at redhat.com>
Reviewed-by: Erik Skultety <eskultet at redhat.com>
---
src/storage/storage_backend_logical.c | 3 +-
src/storage/storage_backend_scsi.c | 13 +--
src/storage/storage_file_fs.c | 15 +--
src/storage/storage_util.c | 143 +++++++++-----------------
src/util/virstoragefile.c | 39 +++----
5 files changed, 74 insertions(+), 139 deletions(-)
diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c
index c61d03519f..f153d23aec 100644
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -911,10 +911,10 @@ static int
virStorageBackendLogicalCreateVol(virStoragePoolObjPtr pool,
virStorageVolDefPtr vol)
{
- int fd = -1;
virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
virErrorPtr err;
struct stat sb;
+ VIR_AUTOCLOSE fd = -1;
vol->type = VIR_STORAGE_VOL_BLOCK;
@@ -971,7 +971,6 @@ virStorageBackendLogicalCreateVol(virStoragePoolObjPtr pool,
error:
err = virSaveLastError();
- VIR_FORCE_CLOSE(fd);
virStorageBackendLogicalDeleteVol(pool, vol, 0);
virSetError(err);
virFreeError(err);
diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c
index c1470fa1d5..1c27fc74f6 100644
--- a/src/storage/storage_backend_scsi.c
+++ b/src/storage/storage_backend_scsi.c
@@ -55,8 +55,7 @@ struct _virStoragePoolFCRefreshInfo {
static int
virStorageBackendSCSITriggerRescan(uint32_t host)
{
- int fd = -1;
- int retval = -1;
+ VIR_AUTOCLOSE fd = -1;
VIR_AUTOFREE(char *) path = NULL;
VIR_DEBUG("Triggering rescan of host %d", host);
@@ -73,7 +72,7 @@ virStorageBackendSCSITriggerRescan(uint32_t host)
virReportSystemError(errno,
_("Could not open '%s' to trigger host scan"),
path);
- goto cleanup;
+ return -1;
}
if (safewrite(fd,
@@ -82,15 +81,11 @@ virStorageBackendSCSITriggerRescan(uint32_t host)
virReportSystemError(errno,
_("Write to '%s' to trigger host scan failed"),
path);
- goto cleanup;
+ return -1;
}
- retval = 0;
-
- cleanup:
- VIR_FORCE_CLOSE(fd);
VIR_DEBUG("Rescan of host %d complete", host);
- return retval;
+ return 0;
}
/**
diff --git a/src/storage/storage_file_fs.c b/src/storage/storage_file_fs.c
index 3b6ed6e34d..8817970f44 100644
--- a/src/storage/storage_file_fs.c
+++ b/src/storage/storage_file_fs.c
@@ -83,8 +83,8 @@ virStorageFileBackendFileInit(virStorageSourcePtr src)
static int
virStorageFileBackendFileCreate(virStorageSourcePtr src)
{
- int fd = -1;
mode_t mode = S_IRUSR;
+ VIR_AUTOCLOSE fd = -1;
if (!src->readonly)
mode |= S_IWUSR;
@@ -95,7 +95,6 @@ virStorageFileBackendFileCreate(virStorageSourcePtr src)
return -1;
}
- VIR_FORCE_CLOSE(fd);
return 0;
}
@@ -121,8 +120,8 @@ virStorageFileBackendFileRead(virStorageSourcePtr src,
size_t len,
char **buf)
{
- int fd = -1;
ssize_t ret = -1;
+ VIR_AUTOCLOSE fd = -1;
if ((fd = virFileOpenAs(src->path, O_RDONLY, 0,
src->drv->uid, src->drv->gid, 0)) < 0) {
@@ -134,19 +133,15 @@ virStorageFileBackendFileRead(virStorageSourcePtr src,
if (offset > 0) {
if (lseek(fd, offset, SEEK_SET) == (off_t) -1) {
virReportSystemError(errno, _("cannot seek into '%s'"), src->path);
- goto cleanup;
+ return -1;
}
}
if ((ret = virFileReadHeaderFD(fd, len, buf)) < 0) {
- virReportSystemError(errno,
- _("cannot read header '%s'"), src->path);
- goto cleanup;
+ virReportSystemError(errno, _("cannot read header '%s'"), src->path);
+ return -1;
}
- cleanup:
- VIR_FORCE_CLOSE(fd);
-
return ret;
}
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index df3dfeb319..60a42a2828 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -130,7 +130,6 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol,
bool want_sparse,
bool reflink_copy)
{
- int inputfd = -1;
int amtread = -1;
int ret = 0;
size_t rbytes = READ_BLOCK_SIZE_DEFAULT;
@@ -139,13 +138,14 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol,
struct stat st;
VIR_AUTOFREE(char *) zerobuf = NULL;
VIR_AUTOFREE(char *) buf = NULL;
+ VIR_AUTOCLOSE inputfd = -1;
if ((inputfd = open(inputvol->target.path, O_RDONLY)) < 0) {
ret = -errno;
virReportSystemError(errno,
_("could not open input path '%s'"),
inputvol->target.path);
- goto cleanup;
+ return ret;
}
#ifdef __linux__
@@ -157,15 +157,11 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol,
if (wbytes < WRITE_BLOCK_SIZE_DEFAULT)
wbytes = WRITE_BLOCK_SIZE_DEFAULT;
- if (VIR_ALLOC_N(zerobuf, wbytes) < 0) {
- ret = -errno;
- goto cleanup;
- }
+ if (VIR_ALLOC_N(zerobuf, wbytes) < 0)
+ return -errno;
- if (VIR_ALLOC_N(buf, rbytes) < 0) {
- ret = -errno;
- goto cleanup;
- }
+ if (VIR_ALLOC_N(buf, rbytes) < 0)
+ return -errno;
if (reflink_copy) {
if (reflinkCloneFile(fd, inputfd) < 0) {
@@ -173,10 +169,10 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol,
virReportSystemError(errno,
_("failed to clone files from '%s'"),
inputvol->target.path);
- goto cleanup;
+ return ret;
} else {
VIR_DEBUG("btrfs clone finished.");
- goto cleanup;
+ return 0;
}
}
@@ -191,7 +187,7 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol,
virReportSystemError(errno,
_("failed reading from file '%s'"),
inputvol->target.path);
- goto cleanup;
+ return ret;
}
*total -= amtread;
@@ -208,14 +204,14 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol,
virReportSystemError(errno,
_("cannot extend file '%s'"),
vol->target.path);
- goto cleanup;
+ return ret;
}
} else if (safewrite(fd, buf+offset, interval) < 0) {
ret = -errno;
virReportSystemError(errno,
_("failed writing to file '%s'"),
vol->target.path);
- goto cleanup;
+ return ret;
}
} while ((amtleft -= interval) > 0);
@@ -225,23 +221,18 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol,
ret = -errno;
virReportSystemError(errno, _("cannot sync data to file '%s'"),
vol->target.path);
- goto cleanup;
+ return ret;
}
-
if (VIR_CLOSE(inputfd) < 0) {
ret = -errno;
virReportSystemError(errno,
_("cannot close file '%s'"),
inputvol->target.path);
- goto cleanup;
+ return ret;
}
- inputfd = -1;
-
- cleanup:
- VIR_FORCE_CLOSE(inputfd);
- return ret;
+ return 0;
}
static int
@@ -250,14 +241,13 @@ storageBackendCreateBlockFrom(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED,
virStorageVolDefPtr inputvol,
unsigned int flags)
{
- int fd = -1;
- int ret = -1;
unsigned long long remain;
struct stat st;
gid_t gid;
uid_t uid;
mode_t mode;
bool reflink_copy = false;
+ VIR_AUTOCLOSE fd = -1;
virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA |
VIR_STORAGE_VOL_CREATE_REFLINK,
@@ -267,7 +257,7 @@ storageBackendCreateBlockFrom(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED,
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("metadata preallocation is not supported for block "
"volumes"));
- goto cleanup;
+ return -1;
}
if (flags & VIR_STORAGE_VOL_CREATE_REFLINK)
@@ -277,7 +267,7 @@ storageBackendCreateBlockFrom(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED,
virReportSystemError(errno,
_("cannot create path '%s'"),
vol->target.path);
- goto cleanup;
+ return -1;
}
remain = vol->target.capacity;
@@ -285,13 +275,13 @@ storageBackendCreateBlockFrom(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED,
if (inputvol) {
if (virStorageBackendCopyToFD(vol, inputvol, fd, &remain,
false, reflink_copy) < 0)
- goto cleanup;
+ return -1;
}
if (fstat(fd, &st) == -1) {
virReportSystemError(errno, _("stat of '%s' failed"),
vol->target.path);
- goto cleanup;
+ return -1;
}
uid = (vol->target.perms->uid != st.st_uid) ? vol->target.perms->uid
: (uid_t)-1;
@@ -303,7 +293,7 @@ storageBackendCreateBlockFrom(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED,
_("cannot chown '%s' to (%u, %u)"),
vol->target.path, (unsigned int)uid,
(unsigned int)gid);
- goto cleanup;
+ return -1;
}
mode = (vol->target.perms->mode == (mode_t)-1 ?
@@ -312,21 +302,16 @@ storageBackendCreateBlockFrom(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED,
virReportSystemError(errno,
_("cannot set mode of '%s' to %04o"),
vol->target.path, mode);
- goto cleanup;
+ return -1;
}
if (VIR_CLOSE(fd) < 0) {
virReportSystemError(errno,
_("cannot close file '%s'"),
vol->target.path);
- goto cleanup;
+ return -1;
}
- fd = -1;
- ret = 0;
- cleanup:
- VIR_FORCE_CLOSE(fd);
-
- return ret;
+ return 0;
}
static int
@@ -419,11 +404,11 @@ storageBackendCreateRaw(virStoragePoolObjPtr pool,
{
virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
int ret = -1;
- int fd = -1;
int operation_flags;
bool reflink_copy = false;
mode_t open_mode = VIR_STORAGE_DEFAULT_VOL_PERM_MODE;
bool created = false;
+ VIR_AUTOCLOSE fd = -1;
virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA |
VIR_STORAGE_VOL_CREATE_REFLINK,
@@ -501,7 +486,6 @@ storageBackendCreateRaw(virStoragePoolObjPtr pool,
ignore_value(virFileRemove(vol->target.path,
vol->target.perms->uid,
vol->target.perms->gid));
- VIR_FORCE_CLOSE(fd);
return ret;
}
@@ -542,7 +526,7 @@ virStorageBackendCreateExecCommand(virStoragePoolObjPtr pool,
* re-open the file and attempt to force the mode change.
*/
if (mode != (st.st_mode & S_IRWXUGO)) {
- int fd = -1;
+ VIR_AUTOCLOSE fd = -1;
int flags = VIR_FILE_OPEN_FORK | VIR_FILE_OPEN_FORCE_MODE;
if ((fd = virFileOpenAs(vol->target.path, O_RDWR, mode,
@@ -550,7 +534,6 @@ virStorageBackendCreateExecCommand(virStoragePoolObjPtr pool,
vol->target.perms->gid,
flags)) >= 0) {
/* Success - means we're good */
- VIR_FORCE_CLOSE(fd);
ret = 0;
goto cleanup;
}
@@ -1227,10 +1210,10 @@ storageBackendCreateQemuImgSecretPath(virStoragePoolObjPtr pool,
{
virStorageEncryptionPtr enc = vol->target.encryption;
char *secretPath = NULL;
- int fd = -1;
uint8_t *secret = NULL;
size_t secretlen = 0;
virConnectPtr conn = NULL;
+ VIR_AUTOCLOSE fd = -1;
if (!enc) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -1268,7 +1251,6 @@ storageBackendCreateQemuImgSecretPath(virStoragePoolObjPtr pool,
_("failed to write secret file"));
goto error;
}
- VIR_FORCE_CLOSE(fd);
if ((vol->target.perms->uid != (uid_t)-1) &&
(vol->target.perms->gid != (gid_t)-1)) {
@@ -1283,7 +1265,6 @@ storageBackendCreateQemuImgSecretPath(virStoragePoolObjPtr pool,
cleanup:
virObjectUnref(conn);
VIR_DISPOSE_N(secret, secretlen);
- VIR_FORCE_CLOSE(fd);
return secretPath;
@@ -1754,19 +1735,18 @@ storageBackendUpdateVolTargetInfo(virStorageVolType voltype,
unsigned int openflags,
unsigned int readflags)
{
- int ret = -1;
int rc;
- int fd = -1;
struct stat sb;
ssize_t len = VIR_STORAGE_MAX_HEADER;
VIR_AUTOFREE(char *) buf = NULL;
+ VIR_AUTOCLOSE fd = -1;
if ((rc = virStorageBackendVolOpen(target->path, &sb, openflags)) < 0)
return rc;
fd = rc;
if ((virStorageBackendUpdateVolTargetInfoFD(target, fd, &sb)) < 0)
- goto cleanup;
+ return -1;
if ((voltype == VIR_STORAGE_VOL_FILE || voltype == VIR_STORAGE_VOL_BLOCK) &&
target->format != VIR_STORAGE_FILE_NONE) {
@@ -1774,49 +1754,39 @@ storageBackendUpdateVolTargetInfo(virStorageVolType voltype,
if (storageBackendIsPloopDir(target->path)) {
if ((storageBackendRedoPloopUpdate(target, &sb, &fd,
openflags)) < 0)
- goto cleanup;
+ return -1;
target->format = VIR_STORAGE_FILE_PLOOP;
} else {
- ret = 0;
- goto cleanup;
+ return 0;
}
}
if (lseek(fd, 0, SEEK_SET) == (off_t)-1) {
virReportSystemError(errno, _("cannot seek to start of '%s'"), target->path);
- goto cleanup;
+ return -1;
}
if ((len = virFileReadHeaderFD(fd, len, &buf)) < 0) {
if (readflags & VIR_STORAGE_VOL_READ_NOERROR) {
VIR_WARN("ignoring failed header read for '%s'",
target->path);
- ret = -2;
+ return -2;
} else {
virReportSystemError(errno,
_("cannot read header '%s'"),
target->path);
+ return -1;
}
- goto cleanup;
}
if (virStorageSourceUpdateCapacity(target, buf, len, false) < 0)
- goto cleanup;
- }
-
- if (withBlockVolFormat) {
- if ((rc = virStorageBackendDetectBlockVolFormatFD(target, fd,
- readflags)) < 0) {
- ret = rc;
- goto cleanup;
- }
+ return -1;
}
- ret = 0;
+ if (withBlockVolFormat)
+ return virStorageBackendDetectBlockVolFormatFD(target, fd, readflags);
- cleanup:
- VIR_FORCE_CLOSE(fd);
- return ret;
+ return 0;
}
/*
@@ -2622,9 +2592,9 @@ storageBackendVolWipeLocalFile(const char *path,
unsigned long long allocation,
bool zero_end)
{
- int ret = -1, fd = -1;
const char *alg_char = NULL;
struct stat st;
+ VIR_AUTOCLOSE fd = -1;
VIR_AUTOPTR(virCommand) cmd = NULL;
fd = open(path, O_RDWR);
@@ -2632,14 +2602,14 @@ storageBackendVolWipeLocalFile(const char *path,
virReportSystemError(errno,
_("Failed to open storage volume with path '%s'"),
path);
- goto cleanup;
+ return -1;
}
if (fstat(fd, &st) == -1) {
virReportSystemError(errno,
_("Failed to stat storage volume with path '%s'"),
path);
- goto cleanup;
+ return -1;
}
switch ((virStorageVolWipeAlgorithm) algorithm) {
@@ -2673,12 +2643,12 @@ storageBackendVolWipeLocalFile(const char *path,
case VIR_STORAGE_VOL_WIPE_ALG_TRIM:
virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
_("'trim' algorithm not supported"));
- goto cleanup;
+ return -1;
case VIR_STORAGE_VOL_WIPE_ALG_LAST:
virReportError(VIR_ERR_INVALID_ARG,
_("unsupported algorithm %d"),
algorithm);
- goto cleanup;
+ return -1;
}
VIR_DEBUG("Wiping file '%s' with algorithm '%s'", path, alg_char);
@@ -2687,24 +2657,14 @@ storageBackendVolWipeLocalFile(const char *path,
cmd = virCommandNew(SCRUB);
virCommandAddArgList(cmd, "-f", "-p", alg_char, path, NULL);
- if (virCommandRun(cmd, NULL) < 0)
- goto cleanup;
-
- ret = 0;
- } else {
- if (S_ISREG(st.st_mode) && st.st_blocks < (st.st_size / DEV_BSIZE)) {
- ret = storageBackendVolZeroSparseFileLocal(path, st.st_size, fd);
- } else {
- ret = storageBackendWipeLocal(path, fd, allocation, st.st_blksize,
- zero_end);
- }
- if (ret < 0)
- goto cleanup;
+ return virCommandRun(cmd, NULL);
}
- cleanup:
- VIR_FORCE_CLOSE(fd);
- return ret;
+ if (S_ISREG(st.st_mode) && st.st_blocks < (st.st_size / DEV_BSIZE))
+ return storageBackendVolZeroSparseFileLocal(path, st.st_size, fd);
+
+ return storageBackendWipeLocal(path, fd, allocation, st.st_blksize,
+ zero_end);
}
@@ -3397,11 +3357,11 @@ storageBackendProbeTarget(virStorageSourcePtr target,
virStorageEncryptionPtr *encryption)
{
int backingStoreFormat;
- int fd = -1;
int ret = -1;
int rc;
virStorageSourcePtr meta = NULL;
struct stat sb;
+ VIR_AUTOCLOSE fd = -1;
if (encryption)
*encryption = NULL;
@@ -3502,7 +3462,6 @@ storageBackendProbeTarget(virStorageSourcePtr target,
}
cleanup:
- VIR_FORCE_CLOSE(fd);
virStorageSourceFree(meta);
return ret;
}
@@ -3574,8 +3533,9 @@ virStorageBackendRefreshLocal(virStoragePoolObjPtr pool)
struct stat statbuf;
virStorageSourcePtr target = NULL;
int direrr;
- int fd = -1, ret = -1;
+ int ret = -1;
VIR_AUTOPTR(virStorageVolDef) vol = NULL;
+ VIR_AUTOCLOSE fd = -1;
if (virDirOpen(&dir, def->target.path) < 0)
goto cleanup;
@@ -3666,7 +3626,6 @@ virStorageBackendRefreshLocal(virStoragePoolObjPtr pool)
ret = 0;
cleanup:
VIR_DIR_CLOSE(dir);
- VIR_FORCE_CLOSE(fd);
virStorageSourceFree(target);
if (ret < 0)
virStoragePoolObjClearVols(pool);
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index b08070b782..5a8e5667f5 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1080,10 +1080,9 @@ virStorageFileGetMetadataInternal(virStorageSourcePtr meta,
int
virStorageFileProbeFormat(const char *path, uid_t uid, gid_t gid)
{
- int fd;
- int ret = -1;
struct stat sb;
ssize_t len = VIR_STORAGE_MAX_HEADER;
+ VIR_AUTOCLOSE fd = -1;
VIR_AUTOFREE(char *) header = NULL;
if ((fd = virFileOpenAs(path, O_RDONLY, 0, uid, gid, 0)) < 0) {
@@ -1093,31 +1092,24 @@ virStorageFileProbeFormat(const char *path, uid_t uid, gid_t gid)
if (fstat(fd, &sb) < 0) {
virReportSystemError(errno, _("cannot stat file '%s'"), path);
- goto cleanup;
+ return -1;
}
/* No header to probe for directories */
- if (S_ISDIR(sb.st_mode)) {
- ret = VIR_STORAGE_FILE_DIR;
- goto cleanup;
- }
+ if (S_ISDIR(sb.st_mode))
+ return VIR_STORAGE_FILE_DIR;
if (lseek(fd, 0, SEEK_SET) == (off_t)-1) {
virReportSystemError(errno, _("cannot set to start of '%s'"), path);
- goto cleanup;
+ return -1;
}
if ((len = virFileReadHeaderFD(fd, len, &header)) < 0) {
virReportSystemError(errno, _("cannot read header '%s'"), path);
- goto cleanup;
+ return -1;
}
- ret = virStorageFileProbeFormatFromBuf(path, header, len);
-
- cleanup:
- VIR_FORCE_CLOSE(fd);
-
- return ret;
+ return virStorageFileProbeFormatFromBuf(path, header, len);
}
@@ -1312,13 +1304,12 @@ virStorageFileResize(const char *path,
unsigned long long capacity,
bool pre_allocate)
{
- int fd = -1;
- int ret = -1;
int rc;
+ VIR_AUTOCLOSE fd = -1;
if ((fd = open(path, O_RDWR)) < 0) {
virReportSystemError(errno, _("Unable to open '%s'"), path);
- goto cleanup;
+ return -1;
}
if (pre_allocate) {
@@ -1331,26 +1322,22 @@ virStorageFileResize(const char *path,
_("Failed to pre-allocate space for "
"file '%s'"), path);
}
- goto cleanup;
+ return -1;
}
}
if (ftruncate(fd, capacity) < 0) {
virReportSystemError(errno,
_("Failed to truncate file '%s'"), path);
- goto cleanup;
+ return -1;
}
if (VIR_CLOSE(fd) < 0) {
virReportSystemError(errno, _("Unable to save '%s'"), path);
- goto cleanup;
+ return -1;
}
- ret = 0;
-
- cleanup:
- VIR_FORCE_CLOSE(fd);
- return ret;
+ return 0;
}
--
2.20.1
More information about the libvir-list
mailing list