Let's make use of the auto __cleanup capabilities. This also allows
for the cleanup of some goto paths.
Signed-off-by: John Ferlan <jferlan@redhat.com>
---
src/storage/storage_backend.c | 9 +--
src/storage/storage_backend_disk.c | 62 ++++++-----------
src/storage/storage_backend_fs.c | 17 ++---
src/storage/storage_backend_gluster.c | 30 +++-----
src/storage/storage_backend_iscsi.c | 73 +++++++-------------
src/storage/storage_backend_iscsi_direct.c | 36 ++++------
src/storage/storage_backend_logical.c | 35 +++-------
src/storage/storage_backend_mpath.c | 18 ++---
src/storage/storage_backend_rbd.c | 35 +++-------
src/storage/storage_backend_scsi.c | 79 ++++++++--------------
src/storage/storage_backend_sheepdog.c | 27 +++-----
src/storage/storage_backend_vstorage.c | 25 +++----
src/storage/storage_backend_zfs.c | 15 ++--
src/storage/storage_file_gluster.c | 16 ++---
14 files changed, 158 insertions(+), 319 deletions(-)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index a54c338cf0..5c8275e978 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -87,8 +87,7 @@ virStorageDriverLoadBackendModule(const char *name,
const char *regfunc,
bool forceload)
{
- char *modfile = NULL;
- int ret;
+ VIR_AUTOFREE(char *) modfile = NULL;
if (!(modfile = virFileFindResourceFull(name,
"libvirt_storage_backend_",
@@ -98,11 +97,7 @@ virStorageDriverLoadBackendModule(const char *name,
"LIBVIRT_STORAGE_BACKEND_DIR")))
return -1;
- ret = virModuleLoad(modfile, regfunc, forceload);
-
- VIR_FREE(modfile);
-
- return ret;
+ return virModuleLoad(modfile, regfunc, forceload);
}
diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c
index 4fb38178b2..4103f2d039 100644
--- a/src/storage/storage_backend_disk.c
+++ b/src/storage/storage_backend_disk.c
@@ -56,8 +56,9 @@ virStorageBackendDiskMakeDataVol(virStoragePoolObjPtr pool,
virStorageVolDefPtr vol)
{
virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
- char *tmp, *devpath, *partname;
+ char *tmp, *partname;
bool addVol = false;
+ VIR_AUTOFREE(char *) devpath = NULL;
/* Prepended path will be same for all partitions, so we can
* strip the path to form a reasonable pool-unique name
@@ -89,7 +90,6 @@ virStorageBackendDiskMakeDataVol(virStoragePoolObjPtr pool,
* way of doing this...
*/
vol->target.path = virStorageBackendStablePath(pool, devpath, true);
- VIR_FREE(devpath);
if (vol->target.path == NULL)
goto error;
}
@@ -355,12 +355,11 @@ virStorageBackendDiskReadPartitions(virStoragePoolObjPtr pool,
*/
virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
- char *parthelper_path;
struct virStorageBackendDiskPoolVolData cbdata = {
.pool = pool,
.vol = vol,
};
- int ret;
+ VIR_AUTOFREE(char *) parthelper_path = NULL;
VIR_AUTOPTR(virCommand) cmd = NULL;
if (!(parthelper_path = virFileFindResource("libvirt_parthelper",
@@ -388,12 +387,7 @@ virStorageBackendDiskReadPartitions(virStoragePoolObjPtr pool,
def->allocation = 0;
def->capacity = def->available = 0;
- ret = virCommandRunNul(cmd,
- 6,
- virStorageBackendDiskMakeVol,
- &cbdata);
- VIR_FREE(parthelper_path);
- return ret;
+ return virCommandRunNul(cmd, 6, virStorageBackendDiskMakeVol, &cbdata);
}
static int
@@ -419,8 +413,7 @@ static int
virStorageBackendDiskReadGeometry(virStoragePoolObjPtr pool)
{
virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
- char *parthelper_path;
- int ret;
+ VIR_AUTOFREE(char *) parthelper_path = NULL;
VIR_AUTOPTR(virCommand) cmd = NULL;
if (!(parthelper_path = virFileFindResource("libvirt_parthelper",
@@ -433,12 +426,8 @@ virStorageBackendDiskReadGeometry(virStoragePoolObjPtr pool)
"-g",
NULL);
- ret = virCommandRunNul(cmd,
- 3,
- virStorageBackendDiskMakePoolGeometry,
- pool);
- VIR_FREE(parthelper_path);
- return ret;
+ return virCommandRunNul(cmd, 3, virStorageBackendDiskMakePoolGeometry,
+ pool);
}
static int
@@ -769,13 +758,12 @@ virStorageBackendDiskDeleteVol(virStoragePoolObjPtr pool,
unsigned int flags)
{
char *part_num = NULL;
- char *devpath = NULL;
char *dev_name;
virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
char *src_path = def->source.devices[0].path;
char *srcname = last_component(src_path);
bool isDevMapperDevice;
- int rc = -1;
+ VIR_AUTOFREE(char *) devpath = NULL;
VIR_AUTOPTR(virCommand) cmd = NULL;
virCheckFlags(0, -1);
@@ -799,7 +787,7 @@ virStorageBackendDiskDeleteVol(virStoragePoolObjPtr pool,
virReportSystemError(errno,
_("Couldn't read volume target path '%s'"),
vol->target.path);
- goto cleanup;
+ return -1;
}
dev_name = last_component(devpath);
}
@@ -810,7 +798,7 @@ virStorageBackendDiskDeleteVol(virStoragePoolObjPtr pool,
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Volume path '%s' did not start with parent "
"pool source device name."), dev_name);
- goto cleanup;
+ return -1;
}
part_num = dev_name + strlen(srcname);
@@ -824,7 +812,7 @@ virStorageBackendDiskDeleteVol(virStoragePoolObjPtr pool,
virReportError(VIR_ERR_INTERNAL_ERROR,
_("cannot parse partition number from target "
"'%s'"), dev_name);
- goto cleanup;
+ return -1;
}
/* eg parted /dev/sda rm 2 or /dev/mapper/mpathc rm 2 */
@@ -835,7 +823,7 @@ virStorageBackendDiskDeleteVol(virStoragePoolObjPtr pool,
part_num,
NULL);
if (virCommandRun(cmd, NULL) < 0)
- goto cleanup;
+ return -1;
/* Refreshing the pool is the easiest option as LOGICAL and EXTENDED
* partition allocation/capacity management is handled within
@@ -844,12 +832,9 @@ virStorageBackendDiskDeleteVol(virStoragePoolObjPtr pool,
*/
virStoragePoolObjClearVols(pool);
if (virStorageBackendDiskRefreshPool(pool) < 0)
- goto cleanup;
+ return -1;
- rc = 0;
- cleanup:
- VIR_FREE(devpath);
- return rc;
+ return 0;
}
@@ -857,11 +842,10 @@ static int
virStorageBackendDiskCreateVol(virStoragePoolObjPtr pool,
virStorageVolDefPtr vol)
{
- int res = -1;
- char *partFormat = NULL;
unsigned long long startOffset = 0, endOffset = 0;
virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
virErrorPtr save_err;
+ VIR_AUTOFREE(char *)partFormat = NULL;
VIR_AUTOPTR(virCommand) cmd = NULL;
cmd = virCommandNewArgList(PARTED,
@@ -874,11 +858,11 @@ virStorageBackendDiskCreateVol(virStoragePoolObjPtr pool,
vol->target.encryption->format != VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("storage pool only supports LUKS encrypted volumes"));
- goto cleanup;
+ return -1;
}
if (virStorageBackendDiskPartFormat(pool, vol, &partFormat) != 0)
- goto cleanup;
+ return -1;
virCommandAddArg(cmd, partFormat);
/* If we're going to encrypt using LUKS, then we could need up to
@@ -888,13 +872,13 @@ virStorageBackendDiskCreateVol(virStoragePoolObjPtr pool,
if (virStorageBackendDiskPartBoundaries(pool, &startOffset, &endOffset,
vol->target.capacity) < 0)
- goto cleanup;
+ return -1;
virCommandAddArgFormat(cmd, "%lluB", startOffset);
virCommandAddArgFormat(cmd, "%lluB", endOffset);
if (virCommandRun(cmd, NULL) < 0)
- goto cleanup;
+ return -1;
/* wait for device node to show up */
virWaitForDevices();
@@ -918,11 +902,7 @@ virStorageBackendDiskCreateVol(virStoragePoolObjPtr pool,
goto error;
}
- res = 0;
-
- cleanup:
- VIR_FREE(partFormat);
- return res;
+ return 0;
error:
/* Best effort to remove the partition. Ignore any errors
@@ -932,7 +912,7 @@ virStorageBackendDiskCreateVol(virStoragePoolObjPtr pool,
ignore_value(virStorageBackendDiskDeleteVol(pool, vol, 0));
virSetError(save_err);
virFreeError(save_err);
- goto cleanup;
+ return -1;
}
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 0436c25af0..97148acebe 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -246,11 +246,11 @@ virStorageBackendFileSystemIsMounted(virStoragePoolObjPtr pool)
{
int ret = -1;
virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
- char *src = NULL;
FILE *mtab;
struct mntent ent;
char buf[1024];
int rc1, rc2;
+ VIR_AUTOFREE(char *) src = NULL;
if ((mtab = fopen(_PATH_MOUNTED, "r")) == NULL) {
virReportSystemError(errno,
@@ -282,7 +282,6 @@ virStorageBackendFileSystemIsMounted(virStoragePoolObjPtr pool)
cleanup:
VIR_FORCE_FCLOSE(mtab);
- VIR_FREE(src);
return ret;
}
@@ -299,9 +298,8 @@ static int
virStorageBackendFileSystemMount(virStoragePoolObjPtr pool)
{
virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
- char *src = NULL;
- int ret = -1;
int rc;
+ VIR_AUTOFREE(char *) src = NULL;
VIR_AUTOPTR(virCommand) cmd = NULL;
if (virStorageBackendFileSystemIsValid(pool) < 0)
@@ -320,13 +318,7 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr pool)
return -1;
cmd = virStorageBackendFileSystemMountCmd(MOUNT, def, src);
- if (virCommandRun(cmd, NULL) < 0)
- goto cleanup;
-
- ret = 0;
- cleanup:
- VIR_FREE(src);
- return ret;
+ return virCommandRun(cmd, NULL);
}
@@ -579,10 +571,10 @@ virStoragePoolDefFSNamespaceParse(xmlXPathContextPtr ctxt,
void **data)
{
virStoragePoolFSMountOptionsDefPtr cmdopts = NULL;
- xmlNodePtr *nodes = NULL;
int nnodes;
size_t i;
int ret = -1;
+ VIR_AUTOFREE(xmlNodePtr *)nodes = NULL;
if (xmlXPathRegisterNs(ctxt, BAD_CAST "fs",
BAD_CAST STORAGE_POOL_FS_NAMESPACE_HREF) < 0) {
@@ -617,7 +609,6 @@ virStoragePoolDefFSNamespaceParse(xmlXPathContextPtr ctxt,
ret = 0;
cleanup:
- VIR_FREE(nodes);
virStoragePoolDefFSNamespaceFree(cmdopts);
return ret;
}
diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c
index 5428bb92ba..1888314d95 100644
--- a/src/storage/storage_backend_gluster.c
+++ b/src/storage/storage_backend_gluster.c
@@ -127,11 +127,9 @@ virStorageBackendGlusterOpen(virStoragePoolObjPtr pool)
if (glfs_set_volfile_server(ret->vol, "tcp",
ret->uri->server, ret->uri->port) < 0 ||
glfs_init(ret->vol) < 0) {
- char *uri = virURIFormat(ret->uri);
-
- virReportSystemError(errno, _("failed to connect to %s"),
- NULLSTR(uri));
- VIR_FREE(uri);
+ VIR_AUTOFREE(char *) uri = NULL;
+ uri = virURIFormat(ret->uri);
+ virReportSystemError(errno, _("failed to connect to %s"), NULLSTR(uri));
goto error;
}
@@ -187,9 +185,8 @@ virStorageBackendGlusterSetMetadata(virStorageBackendGlusterStatePtr state,
virStorageVolDefPtr vol,
const char *name)
{
- int ret = -1;
- char *path = NULL;
char *tmp;
+ VIR_AUTOFREE(char *) path = NULL;
VIR_FREE(vol->key);
VIR_FREE(vol->target.path);
@@ -200,35 +197,31 @@ virStorageBackendGlusterSetMetadata(virStorageBackendGlusterStatePtr state,
if (name) {
VIR_FREE(vol->name);
if (VIR_STRDUP(vol->name, name) < 0)
- goto cleanup;
+ return -1;
}
if (virAsprintf(&path, "%s%s%s", state->volname, state->dir,
vol->name) < 0)
- goto cleanup;
+ return -1;
tmp = state->uri->path;
if (virAsprintf(&state->uri->path, "/%s", path) < 0) {
state->uri->path = tmp;
- goto cleanup;
+ return -1;
}
if (!(vol->target.path = virURIFormat(state->uri))) {
VIR_FREE(state->uri->path);
state->uri->path = tmp;
- goto cleanup;
+ return -1;
}
VIR_FREE(state->uri->path);
state->uri->path = tmp;
/* the path is unique enough to serve as a volume key */
if (VIR_STRDUP(vol->key, vol->target.path) < 0)
- goto cleanup;
-
- ret = 0;
+ return -1;
- cleanup:
- VIR_FREE(path);
- return ret;
+ return 0;
}
@@ -244,10 +237,10 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state,
int ret = -1;
glfs_fd_t *fd = NULL;
virStorageSourcePtr meta = NULL;
- char *header = NULL;
ssize_t len;
int backingFormat;
VIR_AUTOPTR(virStorageVolDef) vol = NULL;
+ VIR_AUTOFREE(char *) header = NULL;
*volptr = NULL;
@@ -333,7 +326,6 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state,
virStorageSourceFree(meta);
if (fd)
glfs_close(fd);
- VIR_FREE(header);
return ret;
}
diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c
index dc1a983b58..9314427cda 100644
--- a/src/storage/storage_backend_iscsi.c
+++ b/src/storage/storage_backend_iscsi.c
@@ -130,27 +130,20 @@ static int
virStorageBackendISCSIFindLUs(virStoragePoolObjPtr pool,
const char *session)
{
- char *sysfs_path;
- int retval = -1;
uint32_t host;
+ VIR_AUTOFREE(char *) sysfs_path = NULL;
if (virAsprintf(&sysfs_path,
"/sys/class/iscsi_session/session%s/device", session) < 0)
- goto cleanup;
+ return -1;
if (virStorageBackendISCSIGetHostNumber(sysfs_path, &host) < 0)
- goto cleanup;
+ return -1;
if (virStorageBackendSCSIFindLUs(pool, host) < 0)
- goto cleanup;
-
- retval = 0;
-
- cleanup:
-
- VIR_FREE(sysfs_path);
+ return -1;
- return retval;
+ return 0;
}
@@ -167,7 +160,7 @@ virStorageBackendISCSIFindPoolSources(const char *srcSpec,
.nsources = 0,
.sources = NULL
};
- char *portal = NULL;
+ VIR_AUTOFREE(char *) portal = NULL;
VIR_AUTOPTR(virStoragePoolSource) source = NULL;
virCheckFlags(0, NULL);
@@ -223,10 +216,10 @@ virStorageBackendISCSIFindPoolSources(const char *srcSpec,
}
VIR_FREE(list.sources);
}
+ /* NB: Not virString -like, managed by VIR_APPEND_ELEMENT */
for (i = 0; i < ntargets; i++)
VIR_FREE(targets[i]);
VIR_FREE(targets);
- VIR_FREE(portal);
return ret;
}
@@ -235,8 +228,8 @@ virStorageBackendISCSICheckPool(virStoragePoolObjPtr pool,
bool *isActive)
{
virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
- char *session = NULL;
int ret = -1;
+ VIR_AUTOFREE(char *) session = NULL;
*isActive = false;
@@ -259,10 +252,8 @@ virStorageBackendISCSICheckPool(virStoragePoolObjPtr pool,
return -1;
}
- if ((session = virStorageBackendISCSISession(pool, true)) != NULL) {
+ if ((session = virStorageBackendISCSISession(pool, true)))
*isActive = true;
- VIR_FREE(session);
- }
ret = 0;
return ret;
@@ -330,9 +321,8 @@ static int
virStorageBackendISCSIStartPool(virStoragePoolObjPtr pool)
{
virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
- char *portal = NULL;
- char *session = NULL;
- int ret = -1;
+ VIR_AUTOFREE(char *) portal = NULL;
+ VIR_AUTOFREE(char *) session = NULL;
if (def->source.nhost != 1) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
@@ -355,50 +345,40 @@ virStorageBackendISCSIStartPool(virStoragePoolObjPtr pool)
if ((session = virStorageBackendISCSISession(pool, true)) == NULL) {
if ((portal = virStorageBackendISCSIPortal(&def->source)) == NULL)
- goto cleanup;
+ return -1;
/* Create a static node record for the IQN target. Must be done
* in order for login to the target */
if (virISCSINodeNew(portal, def->source.devices[0].path) < 0)
- goto cleanup;
+ return -1;
if (virStorageBackendISCSISetAuth(portal, &def->source) < 0)
- goto cleanup;
+ return -1;
if (virISCSIConnectionLogin(portal,
def->source.initiator.iqn,
def->source.devices[0].path) < 0)
- goto cleanup;
+ return -1;
}
- ret = 0;
-
- cleanup:
- VIR_FREE(portal);
- VIR_FREE(session);
- return ret;
+ return 0;
}
static int
virStorageBackendISCSIRefreshPool(virStoragePoolObjPtr pool)
{
virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
- char *session = NULL;
+ VIR_AUTOFREE(char *) session = NULL;
def->allocation = def->capacity = def->available = 0;
if ((session = virStorageBackendISCSISession(pool, false)) == NULL)
- goto cleanup;
+ return -1;
if (virISCSIRescanLUNs(session) < 0)
- goto cleanup;
+ return -1;
if (virStorageBackendISCSIFindLUs(pool, session) < 0)
- goto cleanup;
- VIR_FREE(session);
+ return -1;
return 0;
-
- cleanup:
- VIR_FREE(session);
- return -1;
}
@@ -406,13 +386,11 @@ static int
virStorageBackendISCSIStopPool(virStoragePoolObjPtr pool)
{
virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
- char *portal;
- char *session;
- int ret = -1;
+ VIR_AUTOFREE(char *) portal = NULL;
+ VIR_AUTOFREE(char *) session = NULL;
if ((session = virStorageBackendISCSISession(pool, true)) == NULL)
return 0;
- VIR_FREE(session);
if ((portal = virStorageBackendISCSIPortal(&def->source)) == NULL)
return -1;
@@ -420,12 +398,9 @@ virStorageBackendISCSIStopPool(virStoragePoolObjPtr pool)
if (virISCSIConnectionLogout(portal,
def->source.initiator.iqn,
def->source.devices[0].path) < 0)
- goto cleanup;
- ret = 0;
+ return -1;
- cleanup:
- VIR_FREE(portal);
- return ret;
+ return 0;
}
virStorageBackend virStorageBackendISCSI = {
diff --git a/src/storage/storage_backend_iscsi_direct.c b/src/storage/storage_backend_iscsi_direct.c
index cf48c29cde..464732f7ab 100644
--- a/src/storage/storage_backend_iscsi_direct.c
+++ b/src/storage/storage_backend_iscsi_direct.c
@@ -421,15 +421,13 @@ virISCSIDirectUpdateTargets(struct iscsi_context *iscsi,
}
for (tmp_addr = addr; tmp_addr; tmp_addr = tmp_addr->next) {
- char *target = NULL;
+ VIR_AUTOFREE(char *) target = NULL;
if (VIR_STRDUP(target, tmp_addr->target_name) < 0)
goto cleanup;
- if (VIR_APPEND_ELEMENT(tmp_targets, tmp_ntargets, target) < 0) {
- VIR_FREE(target);
+ if (VIR_APPEND_ELEMENT(tmp_targets, tmp_ntargets, target) < 0)
goto cleanup;
- }
}
VIR_STEAL_PTR(*targets, tmp_targets);
@@ -489,7 +487,7 @@ virStorageBackendISCSIDirectFindPoolSources(const char *srcSpec,
.nsources = 0,
.sources = NULL
};
- char *portal = NULL;
+ VIR_AUTOFREE(char *) portal = NULL;
VIR_AUTOPTR(virStoragePoolSource) source = NULL;
virCheckFlags(0, NULL);
@@ -550,7 +548,6 @@ virStorageBackendISCSIDirectFindPoolSources(const char *srcSpec,
for (i = 0; i < ntargets; i++)
VIR_FREE(targets[i]);
VIR_FREE(targets);
- VIR_FREE(portal);
return ret;
}
@@ -560,7 +557,7 @@ virStorageBackendISCSIDirectSetConnection(virStoragePoolObjPtr pool,
{
virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
struct iscsi_context *iscsi = NULL;
- char *portal = NULL;
+ VIR_AUTOFREE(char *) portal = NULL;
if (!(iscsi = virISCSIDirectCreateContext(def->source.initiator.iqn)))
goto error;
@@ -577,7 +574,6 @@ virStorageBackendISCSIDirectSetConnection(virStoragePoolObjPtr pool,
VIR_STEAL_PTR(*portalRet, portal);
cleanup:
- VIR_FREE(portal);
return iscsi;
error:
@@ -590,19 +586,14 @@ static int
virStorageBackendISCSIDirectRefreshPool(virStoragePoolObjPtr pool)
{
struct iscsi_context *iscsi = NULL;
- char *portal = NULL;
int ret = -1;
- if (!(iscsi = virStorageBackendISCSIDirectSetConnection(pool, &portal)))
- goto cleanup;
- if (virISCSIDirectReportLuns(pool, iscsi, portal) < 0)
- goto disconect;
+ VIR_AUTOFREE(char *) portal = NULL;
- ret = 0;
- disconect:
+ if (!(iscsi = virStorageBackendISCSIDirectSetConnection(pool, &portal)))
+ return -1;
+ ret = virISCSIDirectReportLuns(pool, iscsi, portal);
virISCSIDirectDisconnect(iscsi);
iscsi_destroy_context(iscsi);
- cleanup:
- VIR_FREE(portal);
return ret;
}
@@ -638,7 +629,7 @@ virStorageBackendISCSIDirectVolWipeZero(virStorageVolDefPtr vol,
struct scsi_task *task = NULL;
int lun = 0;
int ret = -1;
- unsigned char *data;
+ VIR_AUTOFREE(unsigned char *) data = NULL;
if (virStorageBackendISCSIDirectGetLun(vol, &lun) < 0)
return ret;
@@ -655,22 +646,19 @@ virStorageBackendISCSIDirectVolWipeZero(virStorageVolDefPtr vol,
if (!(task = iscsi_write10_sync(iscsi, lun, lba, data,
block_size * BLOCK_PER_PACKET,
block_size, 0, 0, 0, 0, 0)))
- goto cleanup;
+ return -1;
scsi_free_scsi_task(task);
lba += BLOCK_PER_PACKET;
} else {
if (!(task = iscsi_write10_sync(iscsi, lun, lba, data, block_size,
block_size, 0, 0, 0, 0, 0)))
- goto cleanup;
+ return -1;
scsi_free_scsi_task(task);
lba++;
}
}
- ret = 0;
- cleanup:
- VIR_FREE(data);
- return ret;
+ return 0;
}
static int
diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c
index 9ebc560a46..c61d03519f 100644
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -117,14 +117,14 @@ virStorageBackendLogicalParseVolExtents(virStorageVolDefPtr vol,
{
int nextents, ret = -1;
const char *regex_unit = "(\\S+)\\((\\S+)\\)";
- char *regex = NULL;
- regex_t *reg = NULL;
- regmatch_t *vars = NULL;
char *p = NULL;
size_t i;
int err, nvars;
unsigned long long offset, size, length;
virStorageVolSourceExtent extent;
+ VIR_AUTOFREE(char *) regex = NULL;
+ VIR_AUTOFREE(regex_t *) reg = NULL;
+ VIR_AUTOFREE(regmatch_t *) vars = NULL;
memset(&extent, 0, sizeof(extent));
@@ -202,7 +202,7 @@ virStorageBackendLogicalParseVolExtents(virStorageVolDefPtr vol,
for (i = 0; i < nextents; i++) {
size_t j;
int len;
- char *offset_str = NULL;
+ VIR_AUTOFREE(char *) offset_str = NULL;
j = (i * 2) + 1;
len = vars[j].rm_eo - vars[j].rm_so;
@@ -219,10 +219,8 @@ virStorageBackendLogicalParseVolExtents(virStorageVolDefPtr vol,
if (virStrToLong_ull(offset_str, NULL, 10, &offset) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("malformed volume extent offset value"));
- VIR_FREE(offset_str);
goto cleanup;
}
- VIR_FREE(offset_str);
extent.start = offset * size;
extent.end = (offset * size) + length;
@@ -234,9 +232,6 @@ virStorageBackendLogicalParseVolExtents(virStorageVolDefPtr vol,
ret = 0;
cleanup:
- VIR_FREE(regex);
- VIR_FREE(reg);
- VIR_FREE(vars);
VIR_FREE(extent.path);
return ret;
}
@@ -459,15 +454,15 @@ virStorageBackendLogicalFindPoolSourcesFunc(char **const groups,
void *data)
{
virStoragePoolSourceListPtr sourceList = data;
- char *pvname = NULL;
- char *vgname = NULL;
size_t i;
virStoragePoolSourceDevicePtr dev;
virStoragePoolSource *thisSource;
+ VIR_AUTOFREE(char *) pvname = NULL;
+ VIR_AUTOFREE(char *) vgname = NULL;
if (VIR_STRDUP(pvname, groups[0]) < 0 ||
VIR_STRDUP(vgname, groups[1]) < 0)
- goto error;
+ return -1;
thisSource = NULL;
for (i = 0; i < sourceList->nsources; i++) {
@@ -479,30 +474,22 @@ virStorageBackendLogicalFindPoolSourcesFunc(char **const groups,
if (thisSource == NULL) {
if (!(thisSource = virStoragePoolSourceListNewSource(sourceList)))
- goto error;
+ return -1;
- thisSource->name = vgname;
+ VIR_STEAL_PTR(thisSource->name, vgname);
}
- else
- VIR_FREE(vgname);
if (VIR_REALLOC_N(thisSource->devices, thisSource->ndevice + 1) != 0)
- goto error;
+ return -1;
dev = &thisSource->devices[thisSource->ndevice];
thisSource->ndevice++;
thisSource->format = VIR_STORAGE_POOL_LOGICAL_LVM2;
memset(dev, 0, sizeof(*dev));
- dev->path = pvname;
+ VIR_STEAL_PTR(dev->path, pvname);
return 0;
-
- error:
- VIR_FREE(pvname);
- VIR_FREE(vgname);
-
- return -1;
}
/*
diff --git a/src/storage/storage_backend_mpath.c b/src/storage/storage_backend_mpath.c
index 423f945fbc..b78eb726b2 100644
--- a/src/storage/storage_backend_mpath.c
+++ b/src/storage/storage_backend_mpath.c
@@ -153,33 +153,32 @@ static int
virStorageBackendCreateVols(virStoragePoolObjPtr pool,
struct dm_names *names)
{
- int retval = -1, is_mpath = 0;
- char *map_device = NULL;
+ int is_mpath = 0;
uint32_t minor = -1;
uint32_t next;
+ VIR_AUTOFREE(char *) map_device = NULL;
do {
is_mpath = virStorageBackendIsMultipath(names->name);
if (is_mpath < 0)
- goto out;
+ return -1;
if (is_mpath == 1) {
if (virAsprintf(&map_device, "mapper/%s", names->name) < 0)
- goto out;
+ return -1;
if (virStorageBackendGetMinorNumber(names->name, &minor) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Failed to get %s minor number"),
names->name);
- goto out;
+ return -1;
}
if (virStorageBackendMpathNewVol(pool, minor, map_device) < 0)
- goto out;
+ return -1;
- VIR_FREE(map_device);
}
/* Given the way libdevmapper returns its data, I don't see
@@ -191,10 +190,7 @@ virStorageBackendCreateVols(virStoragePoolObjPtr pool,
} while (next);
- retval = 0;
- out:
- VIR_FREE(map_device);
- return retval;
+ return 0;
}
diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c
index ece04f0f2d..2b7af1db23 100644
--- a/src/storage/storage_backend_rbd.c
+++ b/src/storage/storage_backend_rbd.c
@@ -86,10 +86,10 @@ virStoragePoolDefRBDNamespaceParse(xmlXPathContextPtr ctxt,
void **data)
{
virStoragePoolRBDConfigOptionsDefPtr cmdopts = NULL;
- xmlNodePtr *nodes = NULL;
int nnodes;
size_t i;
int ret = -1;
+ VIR_AUTOFREE(xmlNodePtr *)nodes = NULL;
if (xmlXPathRegisterNs(ctxt, BAD_CAST "rbd",
BAD_CAST STORAGE_POOL_RBD_NAMESPACE_HREF) < 0) {
@@ -145,7 +145,6 @@ virStoragePoolDefRBDNamespaceParse(xmlXPathContextPtr ctxt,
ret = 0;
cleanup:
- VIR_FREE(nodes);
virStoragePoolDefRBDNamespaceFree(cmdopts);
return ret;
}
@@ -213,12 +212,12 @@ virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr,
char *rados_key = NULL;
virBuffer mon_host = VIR_BUFFER_INITIALIZER;
size_t i;
- char *mon_buff = NULL;
const char *client_mount_timeout = "30";
const char *mon_op_timeout = "30";
const char *osd_op_timeout = "30";
const char *rbd_default_format = "2";
virConnectPtr conn = NULL;
+ VIR_AUTOFREE(char *) mon_buff = NULL;
if (authdef) {
VIR_DEBUG("Using cephx authorization, username: %s", authdef->username);
@@ -348,7 +347,6 @@ virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr,
virObjectUnref(conn);
virBufferFreeAndReset(&mon_host);
- VIR_FREE(mon_buff);
return ret;
}
@@ -574,11 +572,12 @@ virStorageBackendRBDRefreshPool(virStoragePoolObjPtr pool)
int ret = -1;
int len = -1;
int r = 0;
- char *name, *names = NULL;
+ char *name;
virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
virStorageBackendRBDStatePtr ptr = NULL;
struct rados_cluster_stat_t clusterstat;
struct rados_pool_stat_t poolstat;
+ VIR_AUTOFREE(char *) names = NULL;
if (!(ptr = virStorageBackendRBDNewState(pool)))
goto cleanup;
@@ -662,7 +661,6 @@ virStorageBackendRBDRefreshPool(virStoragePoolObjPtr pool)
ret = 0;
cleanup:
- VIR_FREE(names);
virStorageBackendRBDFreeState(&ptr);
return ret;
}
@@ -677,8 +675,8 @@ virStorageBackendRBDCleanupSnapshots(rados_ioctx_t ioctx,
int max_snaps = 128;
int snap_count, protected;
size_t i;
- rbd_snap_info_t *snaps = NULL;
rbd_image_t image = NULL;
+ VIR_AUTOFREE(rbd_snap_info_t *) snaps = NULL;
if ((r = rbd_open(ioctx, vol->name, &image, NULL)) < 0) {
virReportSystemError(-r, _("failed to open the RBD image '%s'"),
@@ -737,8 +735,6 @@ virStorageBackendRBDCleanupSnapshots(rados_ioctx_t ioctx,
if (snaps)
rbd_snap_list_end(snaps);
- VIR_FREE(snaps);
-
if (image)
rbd_close(image);
@@ -949,8 +945,8 @@ virStorageBackendRBDSnapshotFindNoDiff(rbd_image_t image,
int max_snaps = 128;
size_t i;
int diff;
- rbd_snap_info_t *snaps = NULL;
rbd_image_info_t info;
+ VIR_AUTOFREE(rbd_snap_info_t *) snaps = NULL;
if ((r = rbd_stat(image, &info, sizeof(info))) < 0) {
virReportSystemError(-r, _("failed to stat the RBD image %s"),
@@ -1023,8 +1019,6 @@ virStorageBackendRBDSnapshotFindNoDiff(rbd_image_t image,
if (snaps)
rbd_snap_list_end(snaps);
- VIR_FREE(snaps);
-
return ret;
}
@@ -1098,8 +1092,8 @@ virStorageBackendRBDCloneImage(rados_ioctx_t io,
uint64_t stripe_count;
uint64_t stripe_unit;
virBuffer snapname = VIR_BUFFER_INITIALIZER;
- char *snapname_buff = NULL;
rbd_image_t image = NULL;
+ VIR_AUTOFREE(char *) snapname_buff = NULL;
if ((r = rbd_open(io, origvol, &image, NULL)) < 0) {
virReportSystemError(-r, _("failed to open the RBD image %s"),
@@ -1170,7 +1164,6 @@ virStorageBackendRBDCloneImage(rados_ioctx_t io,
cleanup:
virBufferFreeAndReset(&snapname);
- VIR_FREE(snapname_buff);
if (image)
rbd_close(image);
@@ -1271,13 +1264,12 @@ virStorageBackendRBDVolWipeZero(rbd_image_t image,
uint64_t stripe_count)
{
int r = -1;
- int ret = -1;
unsigned long long offset = 0;
unsigned long long length;
- char *writebuf;
+ VIR_AUTOFREE(char *) writebuf = NULL;
if (VIR_ALLOC_N(writebuf, info->obj_size * stripe_count) < 0)
- goto cleanup;
+ return -1;
while (offset < info->size) {
length = MIN((info->size - offset), (info->obj_size * stripe_count));
@@ -1286,7 +1278,7 @@ virStorageBackendRBDVolWipeZero(rbd_image_t image,
virReportSystemError(-r, _("writing %llu bytes failed on "
"RBD image %s at offset %llu"),
length, imgname, offset);
- goto cleanup;
+ return -1;
}
VIR_DEBUG("Wrote %llu bytes to RBD image %s at offset %llu",
@@ -1295,12 +1287,7 @@ virStorageBackendRBDVolWipeZero(rbd_image_t image,
offset += length;
}
- ret = 0;
-
- cleanup:
- VIR_FREE(writebuf);
-
- return ret;
+ return 0;
}
static int
diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c
index 14f01f9ec0..7460349c81 100644
--- a/src/storage/storage_backend_scsi.c
+++ b/src/storage/storage_backend_scsi.c
@@ -56,16 +56,14 @@ static int
virStorageBackendSCSITriggerRescan(uint32_t host)
{
int fd = -1;
- int retval = 0;
- char *path;
+ int retval = -1;
+ VIR_AUTOFREE(char *) path = NULL;
VIR_DEBUG("Triggering rescan of host %d", host);
if (virAsprintf(&path, "%s/host%u/scan",
- LINUX_SYSFS_SCSI_HOST_PREFIX, host) < 0) {
- retval = -1;
- goto out;
- }
+ LINUX_SYSFS_SCSI_HOST_PREFIX, host) < 0)
+ return -1;
VIR_DEBUG("Scan trigger path is '%s'", path);
@@ -75,8 +73,7 @@ virStorageBackendSCSITriggerRescan(uint32_t host)
virReportSystemError(errno,
_("Could not open '%s' to trigger host scan"),
path);
- retval = -1;
- goto free_path;
+ goto cleanup;
}
if (safewrite(fd,
@@ -86,13 +83,12 @@ virStorageBackendSCSITriggerRescan(uint32_t host)
virReportSystemError(errno,
_("Write to '%s' to trigger host scan failed"),
path);
- retval = -1;
}
+ retval = 0;
+
+ cleanup:
VIR_FORCE_CLOSE(fd);
- free_path:
- VIR_FREE(path);
- out:
VIR_DEBUG("Rescan of host %d complete", host);
return retval;
}
@@ -178,7 +174,6 @@ static char *
getAdapterName(virStorageAdapterPtr adapter)
{
char *name = NULL;
- char *parentaddr = NULL;
if (adapter->type == VIR_STORAGE_ADAPTER_TYPE_SCSI_HOST) {
virStorageAdapterSCSIHostPtr scsi_host = &adapter->data.scsi_host;
@@ -192,7 +187,7 @@ getAdapterName(virStorageAdapterPtr adapter)
addr->slot,
addr->function,
unique_id)))
- goto cleanup;
+ return NULL;
} else {
ignore_value(VIR_STRDUP(name, scsi_host->name));
}
@@ -206,8 +201,6 @@ getAdapterName(virStorageAdapterPtr adapter)
}
}
- cleanup:
- VIR_FREE(parentaddr);
return name;
}
@@ -248,10 +241,10 @@ checkParent(const char *name,
const char *parent_name)
{
unsigned int host_num;
- char *scsi_host_name = NULL;
- char *vhba_parent = NULL;
bool retval = false;
virConnectPtr conn = NULL;
+ VIR_AUTOFREE(char *) scsi_host_name = NULL;
+ VIR_AUTOFREE(char *) vhba_parent = NULL;
VIR_DEBUG("name=%s, parent_name=%s", name, parent_name);
@@ -291,8 +284,6 @@ checkParent(const char *name,
cleanup:
virObjectUnref(conn);
- VIR_FREE(vhba_parent);
- VIR_FREE(scsi_host_name);
return retval;
}
@@ -302,10 +293,9 @@ createVport(virStoragePoolDefPtr def,
const char *configFile,
virStorageAdapterFCHostPtr fchost)
{
- char *name = NULL;
virStoragePoolFCRefreshInfoPtr cbdata = NULL;
virThread thread;
- int ret = -1;
+ VIR_AUTOFREE(char *) name = NULL;
VIR_DEBUG("configFile='%s' parent='%s', wwnn='%s' wwpn='%s'",
NULLSTR(configFile), NULLSTR(fchost->parent),
@@ -317,14 +307,14 @@ createVport(virStoragePoolDefPtr def,
*/
if ((name = virVHBAGetHostByWWN(NULL, fchost->wwnn, fchost->wwpn))) {
if (!(checkName(name)))
- goto cleanup;
+ return -1;
/* If a parent was provided, let's make sure the 'name' we've
* retrieved has the same parent. If not this will cause failure. */
if (!fchost->parent || checkParent(name, fchost->parent))
- ret = 0;
+ return 0;
- goto cleanup;
+ return -1;
}
/* Since we're creating the vHBA, then we need to manage removing it
@@ -336,12 +326,12 @@ createVport(virStoragePoolDefPtr def,
fchost->managed = VIR_TRISTATE_BOOL_YES;
if (configFile) {
if (virStoragePoolSaveConfig(configFile, def) < 0)
- goto cleanup;
+ return -1;
}
}
if (!(name = virNodeDeviceCreateVport(fchost)))
- goto cleanup;
+ return -1;
/* Creating our own VPORT didn't leave enough time to find any LUN's,
* so, let's create a thread whose job it is to call the FindLU's with
@@ -360,11 +350,7 @@ createVport(virStoragePoolDefPtr def,
}
}
- ret = 0;
-
- cleanup:
- VIR_FREE(name);
- return ret;
+ return 0;
}
@@ -373,10 +359,9 @@ virStorageBackendSCSICheckPool(virStoragePoolObjPtr pool,
bool *isActive)
{
virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
- char *path = NULL;
- char *name = NULL;
unsigned int host;
- int ret = -1;
+ VIR_AUTOFREE(char *) path = NULL;
+ VIR_AUTOFREE(char *) name = NULL;
*isActive = false;
@@ -394,28 +379,23 @@ virStorageBackendSCSICheckPool(virStoragePoolObjPtr pool,
}
if (virSCSIHostGetNumber(name, &host) < 0)
- goto cleanup;
+ return -1;
if (virAsprintf(&path, "%s/host%d",
LINUX_SYSFS_SCSI_HOST_PREFIX, host) < 0)
- goto cleanup;
+ return -1;
*isActive = virFileExists(path);
- ret = 0;
- cleanup:
- VIR_FREE(path);
- VIR_FREE(name);
- return ret;
+ return 0;
}
static int
virStorageBackendSCSIRefreshPool(virStoragePoolObjPtr pool)
{
virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
- char *name = NULL;
unsigned int host;
- int ret = -1;
+ VIR_AUTOFREE(char *) name = NULL;
def->allocation = def->capacity = def->available = 0;
@@ -423,20 +403,17 @@ virStorageBackendSCSIRefreshPool(virStoragePoolObjPtr pool)
return -1;
if (virSCSIHostGetNumber(name, &host) < 0)
- goto out;
+ return -1;
VIR_DEBUG("Scanning host%u", host);
if (virStorageBackendSCSITriggerRescan(host) < 0)
- goto out;
+ return -1;
if (virStorageBackendSCSIFindLUs(pool, host) < 0)
- goto out;
+ return -1;
- ret = 0;
- out:
- VIR_FREE(name);
- return ret;
+ return 0;
}
diff --git a/src/storage/storage_backend_sheepdog.c b/src/storage/storage_backend_sheepdog.c
index 73dcfb2f40..d3f126da8d 100644
--- a/src/storage/storage_backend_sheepdog.c
+++ b/src/storage/storage_backend_sheepdog.c
@@ -136,9 +136,8 @@ virStorageBackendSheepdogAddVolume(virStoragePoolObjPtr pool, const char *diskIn
static int
virStorageBackendSheepdogRefreshAllVol(virStoragePoolObjPtr pool)
{
- int ret = -1;
- char *output = NULL;
size_t i;
+ VIR_AUTOFREE(char *) output = NULL;
VIR_AUTOPTR(virString) lines = NULL;
VIR_AUTOPTR(virString) cells = NULL;
VIR_AUTOPTR(virCommand) cmd = NULL;
@@ -147,11 +146,11 @@ virStorageBackendSheepdogRefreshAllVol(virStoragePoolObjPtr pool)
virStorageBackendSheepdogAddHostArg(cmd, pool);
virCommandSetOutputBuffer(cmd, &output);
if (virCommandRun(cmd, NULL) < 0)
- goto cleanup;
+ return -1;
lines = virStringSplit(output, "\n", 0);
if (lines == NULL)
- goto cleanup;
+ return -1;
for (i = 0; lines[i]; i++) {
const char *line = lines[i];
@@ -163,42 +162,34 @@ virStorageBackendSheepdogRefreshAllVol(virStoragePoolObjPtr pool)
if (cells != NULL &&
virStringListLength((const char * const *)cells) > 2) {
if (virStorageBackendSheepdogAddVolume(pool, cells[1]) < 0)
- goto cleanup;
+ return -1;
}
virStringListFree(cells);
cells = NULL;
}
- ret = 0;
-
- cleanup:
- VIR_FREE(output);
- return ret;
+ return 0;
}
static int
virStorageBackendSheepdogRefreshPool(virStoragePoolObjPtr pool)
{
- int ret = -1;
- char *output = NULL;
+ VIR_AUTOFREE(char *) output = NULL;
VIR_AUTOPTR(virCommand) cmd = NULL;
cmd = virCommandNewArgList(SHEEPDOGCLI, "node", "info", "-r", NULL);
virStorageBackendSheepdogAddHostArg(cmd, pool);
virCommandSetOutputBuffer(cmd, &output);
if (virCommandRun(cmd, NULL) < 0)
- goto cleanup;
+ return -1;
if (virStorageBackendSheepdogParseNodeInfo(virStoragePoolObjGetDef(pool),
output) < 0)
- goto cleanup;
+ return -1;
- ret = virStorageBackendSheepdogRefreshAllVol(pool);
- cleanup:
- VIR_FREE(output);
- return ret;
+ return virStorageBackendSheepdogRefreshAllVol(pool);
}
diff --git a/src/storage/storage_backend_vstorage.c b/src/storage/storage_backend_vstorage.c
index 8c5becd4c4..df157a48b1 100644
--- a/src/storage/storage_backend_vstorage.c
+++ b/src/storage/storage_backend_vstorage.c
@@ -39,9 +39,9 @@ virStorageBackendVzPoolStart(virStoragePoolObjPtr pool)
{
int ret = -1;
virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
- char *grp_name = NULL;
- char *usr_name = NULL;
- char *mode = NULL;
+ VIR_AUTOFREE(char *) grp_name = NULL;
+ VIR_AUTOFREE(char *) usr_name = NULL;
+ VIR_AUTOFREE(char *) mode = NULL;
VIR_AUTOPTR(virCommand) cmd = NULL;
/* Check the permissions */
@@ -55,13 +55,13 @@ virStorageBackendVzPoolStart(virStoragePoolObjPtr pool)
/* Convert ids to names because vstorage uses names */
if (!(grp_name = virGetGroupName(def->target.perms.gid)))
- goto cleanup;
+ return -1;
if (!(usr_name = virGetUserName(def->target.perms.uid)))
- goto cleanup;
+ return -1;
if (virAsprintf(&mode, "%o", def->target.perms.mode) < 0)
- goto cleanup;
+ return -1;
cmd = virCommandNewArgList(VSTORAGE_MOUNT,
"-c", def->source.name,
@@ -70,15 +70,7 @@ virStorageBackendVzPoolStart(virStoragePoolObjPtr pool)
"-g", grp_name, "-u", usr_name,
NULL);
- if (virCommandRun(cmd, NULL) < 0)
- goto cleanup;
- ret = 0;
-
- cleanup:
- VIR_FREE(mode);
- VIR_FREE(grp_name);
- VIR_FREE(usr_name);
- return ret;
+ return virCommandRun(cmd, NULL);
}
@@ -90,7 +82,7 @@ virStorageBackendVzIsMounted(virStoragePoolObjPtr pool)
FILE *mtab;
struct mntent ent;
char buf[1024];
- char *cluster = NULL;
+ VIR_AUTOFREE(char *) cluster = NULL;
if (virAsprintf(&cluster, "vstorage://%s", def->source.name) < 0)
return -1;
@@ -115,7 +107,6 @@ virStorageBackendVzIsMounted(virStoragePoolObjPtr pool)
cleanup:
VIR_FORCE_FCLOSE(mtab);
- VIR_FREE(cluster);
return ret;
}
diff --git a/src/storage/storage_backend_zfs.c b/src/storage/storage_backend_zfs.c
index 7d1a3dd2cd..7ffdff638e 100644
--- a/src/storage/storage_backend_zfs.c
+++ b/src/storage/storage_backend_zfs.c
@@ -52,7 +52,7 @@ static int
virStorageBackendZFSVolModeNeeded(void)
{
int ret = -1, exit_code = -1;
- char *error = NULL;
+ VIR_AUTOFREE(char *) error = NULL;
VIR_AUTOPTR(virCommand) cmd = NULL;
/* 'zfs get' without arguments prints out
@@ -77,7 +77,6 @@ virStorageBackendZFSVolModeNeeded(void)
ret = 0;
cleanup:
- VIR_FREE(error);
return ret;
}
@@ -86,13 +85,12 @@ virStorageBackendZFSCheckPool(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED,
bool *isActive)
{
virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
- char *devpath;
+ VIR_AUTOFREE(char *) devpath = NULL;
if (virAsprintf(&devpath, "/dev/zvol/%s",
def->source.name) < 0)
return -1;
*isActive = virFileIsDir(devpath);
- VIR_FREE(devpath);
return 0;
}
@@ -178,10 +176,10 @@ virStorageBackendZFSFindVols(virStoragePoolObjPtr pool,
virStorageVolDefPtr vol)
{
virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
- char *volumes_list = NULL;
size_t i;
VIR_AUTOPTR(virString) lines = NULL;
VIR_AUTOPTR(virCommand) cmd = NULL;
+ VIR_AUTOFREE(char *) volumes_list = NULL;
/**
* $ zfs list -Hp -t volume -o name,volsize -r test
@@ -203,10 +201,10 @@ virStorageBackendZFSFindVols(virStoragePoolObjPtr pool,
NULL);
virCommandSetOutputBuffer(cmd, &volumes_list);
if (virCommandRun(cmd, NULL) < 0)
- goto cleanup;
+ return -1;
if (!(lines = virStringSplit(volumes_list, "\n", 0)))
- goto cleanup;
+ return -1;
for (i = 0; lines[i]; i++) {
if (STREQ(lines[i], ""))
@@ -216,9 +214,6 @@ virStorageBackendZFSFindVols(virStoragePoolObjPtr pool,
continue;
}
- cleanup:
- VIR_FREE(volumes_list);
-
return 0;
}
diff --git a/src/storage/storage_file_gluster.c b/src/storage/storage_file_gluster.c
index f8bbde8cfe..7c2189d297 100644
--- a/src/storage/storage_file_gluster.c
+++ b/src/storage/storage_file_gluster.c
@@ -258,10 +258,10 @@ virStorageFileBackendGlusterReadlinkCallback(const char *path,
void *data)
{
virStorageFileBackendGlusterPrivPtr priv = data;
- char *buf = NULL;
size_t bufsiz = 0;
ssize_t ret;
struct stat st;
+ VIR_AUTOFREE(char *) buf = NULL;
*linkpath = NULL;
@@ -277,13 +277,13 @@ virStorageFileBackendGlusterReadlinkCallback(const char *path,
realloc:
if (VIR_EXPAND_N(buf, bufsiz, 256) < 0)
- goto error;
+ return -1;
if ((ret = glfs_readlink(priv->vol, path, buf, bufsiz)) < 0) {
virReportSystemError(errno,
_("failed to read link of gluster file '%s'"),
path);
- goto error;
+ return -1;
}
if (ret == bufsiz)
@@ -291,13 +291,9 @@ virStorageFileBackendGlusterReadlinkCallback(const char *path,
buf[ret] = '\0';
- *linkpath = buf;
+ VIR_STEAL_PTR(*linkpath, buf);
return 0;
-
- error:
- VIR_FREE(buf);
- return -1;
}
@@ -305,7 +301,7 @@ static const char *
virStorageFileBackendGlusterGetUniqueIdentifier(virStorageSourcePtr src)
{
virStorageFileBackendGlusterPrivPtr priv = src->drv->priv;
- char *filePath = NULL;
+ VIR_AUTOFREE(char *) filePath = NULL;
if (priv->canonpath)
return priv->canonpath;
@@ -321,8 +317,6 @@ virStorageFileBackendGlusterGetUniqueIdentifier(virStorageSourcePtr src)
src->volume,
filePath));
- VIR_FREE(filePath);
-
return priv->canonpath;
}
--
2.20.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Fri, Feb 08, 2019 at 01:37:06PM -0500, John Ferlan wrote:
> Let's make use of the auto __cleanup capabilities. This also allows
> for the cleanup of some goto paths.
>
> Signed-off-by: John Ferlan <jferlan@redhat.com>
> ---
> @@ -459,15 +454,15 @@ virStorageBackendLogicalFindPoolSourcesFunc(char **const groups,
> void *data)
> {
> virStoragePoolSourceListPtr sourceList = data;
> - char *pvname = NULL;
> - char *vgname = NULL;
> size_t i;
> virStoragePoolSourceDevicePtr dev;
> virStoragePoolSource *thisSource;
> + VIR_AUTOFREE(char *) pvname = NULL;
> + VIR_AUTOFREE(char *) vgname = NULL;
>
> if (VIR_STRDUP(pvname, groups[0]) < 0 ||
> VIR_STRDUP(vgname, groups[1]) < 0)
> - goto error;
> + return -1;
>
> thisSource = NULL;
> for (i = 0; i < sourceList->nsources; i++) {
> @@ -479,30 +474,22 @@ virStorageBackendLogicalFindPoolSourcesFunc(char **const groups,
>
> if (thisSource == NULL) {
> if (!(thisSource = virStoragePoolSourceListNewSource(sourceList)))
> - goto error;
> + return -1;
>
> - thisSource->name = vgname;
> + VIR_STEAL_PTR(thisSource->name, vgname);
> }
> - else
> - VIR_FREE(vgname);
>
> if (VIR_REALLOC_N(thisSource->devices, thisSource->ndevice + 1) != 0)
> - goto error;
> + return -1;
>
> dev = &thisSource->devices[thisSource->ndevice];
> thisSource->ndevice++;
> thisSource->format = VIR_STORAGE_POOL_LOGICAL_LVM2;
>
> memset(dev, 0, sizeof(*dev));
> - dev->path = pvname;
> + VIR_STEAL_PTR(dev->path, pvname);
I still don't see why ^this needs to stay and can't be separated into a
preceding patch.
>
> return 0;
> -
> - error:
> - VIR_FREE(pvname);
> - VIR_FREE(vgname);
> -
> - return -1;
> }
...
>
> diff --git a/src/storage/storage_file_gluster.c b/src/storage/storage_file_gluster.c
> index f8bbde8cfe..7c2189d297 100644
> --- a/src/storage/storage_file_gluster.c
> +++ b/src/storage/storage_file_gluster.c
> @@ -258,10 +258,10 @@ virStorageFileBackendGlusterReadlinkCallback(const char *path,
> void *data)
> {
> virStorageFileBackendGlusterPrivPtr priv = data;
> - char *buf = NULL;
> size_t bufsiz = 0;
> ssize_t ret;
> struct stat st;
> + VIR_AUTOFREE(char *) buf = NULL;
>
> *linkpath = NULL;
>
> @@ -277,13 +277,13 @@ virStorageFileBackendGlusterReadlinkCallback(const char *path,
>
> realloc:
> if (VIR_EXPAND_N(buf, bufsiz, 256) < 0)
> - goto error;
> + return -1;
>
> if ((ret = glfs_readlink(priv->vol, path, buf, bufsiz)) < 0) {
> virReportSystemError(errno,
> _("failed to read link of gluster file '%s'"),
> path);
> - goto error;
> + return -1;
> }
>
> if (ret == bufsiz)
> @@ -291,13 +291,9 @@ virStorageFileBackendGlusterReadlinkCallback(const char *path,
>
> buf[ret] = '\0';
>
> - *linkpath = buf;
> + VIR_STEAL_PTR(*linkpath, buf);
^This VIR_STEAL_PTR can also be separated, it doesn't depend on the
VIR_AUTOFREE stuff, it's a simple 1 change hunk.
Erik
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 2/11/19 5:39 AM, Erik Skultety wrote:
> On Fri, Feb 08, 2019 at 01:37:06PM -0500, John Ferlan wrote:
>> Let's make use of the auto __cleanup capabilities. This also allows
>> for the cleanup of some goto paths.
>>
>> Signed-off-by: John Ferlan <jferlan@redhat.com>
>> ---
>
>> @@ -459,15 +454,15 @@ virStorageBackendLogicalFindPoolSourcesFunc(char **const groups,
>> void *data)
>> {
>> virStoragePoolSourceListPtr sourceList = data;
>> - char *pvname = NULL;
>> - char *vgname = NULL;
>> size_t i;
>> virStoragePoolSourceDevicePtr dev;
>> virStoragePoolSource *thisSource;
>> + VIR_AUTOFREE(char *) pvname = NULL;
>> + VIR_AUTOFREE(char *) vgname = NULL;
>>
>> if (VIR_STRDUP(pvname, groups[0]) < 0 ||
>> VIR_STRDUP(vgname, groups[1]) < 0)
>> - goto error;
>> + return -1;
>>
>> thisSource = NULL;
>> for (i = 0; i < sourceList->nsources; i++) {
>> @@ -479,30 +474,22 @@ virStorageBackendLogicalFindPoolSourcesFunc(char **const groups,
>>
>> if (thisSource == NULL) {
>> if (!(thisSource = virStoragePoolSourceListNewSource(sourceList)))
>> - goto error;
>> + return -1;
>>
>> - thisSource->name = vgname;
>> + VIR_STEAL_PTR(thisSource->name, vgname);
>> }
>> - else
>> - VIR_FREE(vgname);
>>
>> if (VIR_REALLOC_N(thisSource->devices, thisSource->ndevice + 1) != 0)
>> - goto error;
>> + return -1;
>>
>> dev = &thisSource->devices[thisSource->ndevice];
>> thisSource->ndevice++;
>> thisSource->format = VIR_STORAGE_POOL_LOGICAL_LVM2;
>>
>> memset(dev, 0, sizeof(*dev));
>> - dev->path = pvname;
>> + VIR_STEAL_PTR(dev->path, pvname);
>
> I still don't see why ^this needs to stay and can't be separated into a
> preceding patch.
>
Because in my view/mind - previously there was no problem for pvname in
the success path; however, with this change and without a VIR_STEAL_PTR
there would be a problem.
Moving the VIR_STEAL_PTR to a/the previous patch for this line is a noop
since we never fall through to the err: label.
I suppose if you insist I can move it as it really doesn't matter that
much to me.
>>
>> return 0;
>> -
>> - error:
>> - VIR_FREE(pvname);
>> - VIR_FREE(vgname);
>> -
>> - return -1;
>> }
>
> ...
>
>>
>> diff --git a/src/storage/storage_file_gluster.c b/src/storage/storage_file_gluster.c
>> index f8bbde8cfe..7c2189d297 100644
>> --- a/src/storage/storage_file_gluster.c
>> +++ b/src/storage/storage_file_gluster.c
>> @@ -258,10 +258,10 @@ virStorageFileBackendGlusterReadlinkCallback(const char *path,
>> void *data)
>> {
>> virStorageFileBackendGlusterPrivPtr priv = data;
>> - char *buf = NULL;
>> size_t bufsiz = 0;
>> ssize_t ret;
>> struct stat st;
>> + VIR_AUTOFREE(char *) buf = NULL;
>>
>> *linkpath = NULL;
>>
>> @@ -277,13 +277,13 @@ virStorageFileBackendGlusterReadlinkCallback(const char *path,
>>
>> realloc:
>> if (VIR_EXPAND_N(buf, bufsiz, 256) < 0)
>> - goto error;
>> + return -1;
>>
>> if ((ret = glfs_readlink(priv->vol, path, buf, bufsiz)) < 0) {
>> virReportSystemError(errno,
>> _("failed to read link of gluster file '%s'"),
>> path);
>> - goto error;
>> + return -1;
>> }
>>
>> if (ret == bufsiz)
>> @@ -291,13 +291,9 @@ virStorageFileBackendGlusterReadlinkCallback(const char *path,
>>
>> buf[ret] = '\0';
>>
>> - *linkpath = buf;
>> + VIR_STEAL_PTR(*linkpath, buf);
>
> ^This VIR_STEAL_PTR can also be separated, it doesn't depend on the
> VIR_AUTOFREE stuff, it's a simple 1 change hunk.
>
Same reasoning here.
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Fri, Feb 08, 2019 at 01:37:06PM -0500, John Ferlan wrote:
>Let's make use of the auto __cleanup capabilities. This also allows
>for the cleanup of some goto paths.
>
>Signed-off-by: John Ferlan <jferlan@redhat.com>
>---
> src/storage/storage_backend.c | 9 +--
> src/storage/storage_backend_disk.c | 62 ++++++-----------
> src/storage/storage_backend_fs.c | 17 ++---
> src/storage/storage_backend_gluster.c | 30 +++-----
> src/storage/storage_backend_iscsi.c | 73 +++++++-------------
> src/storage/storage_backend_iscsi_direct.c | 36 ++++------
> src/storage/storage_backend_logical.c | 35 +++-------
> src/storage/storage_backend_mpath.c | 18 ++---
> src/storage/storage_backend_rbd.c | 35 +++-------
> src/storage/storage_backend_scsi.c | 79 ++++++++--------------
> src/storage/storage_backend_sheepdog.c | 27 +++-----
> src/storage/storage_backend_vstorage.c | 25 +++----
> src/storage/storage_backend_zfs.c | 15 ++--
> src/storage/storage_file_gluster.c | 16 ++---
> 14 files changed, 158 insertions(+), 319 deletions(-)
>
>@@ -223,10 +216,10 @@ virStorageBackendISCSIFindPoolSources(const char *srcSpec,
> }
> VIR_FREE(list.sources);
> }
>+ /* NB: Not virString -like, managed by VIR_APPEND_ELEMENT */
I don't see the point of this comment.
> for (i = 0; i < ntargets; i++)
> VIR_FREE(targets[i]);
> VIR_FREE(targets);
>- VIR_FREE(portal);
> return ret;
> }
>
>diff --git a/src/storage/storage_backend_mpath.c b/src/storage/storage_backend_mpath.c
>index 423f945fbc..b78eb726b2 100644
>--- a/src/storage/storage_backend_mpath.c
>+++ b/src/storage/storage_backend_mpath.c
>@@ -153,33 +153,32 @@ static int
> virStorageBackendCreateVols(virStoragePoolObjPtr pool,
> struct dm_names *names)
> {
>- int retval = -1, is_mpath = 0;
>- char *map_device = NULL;
>+ int is_mpath = 0;
> uint32_t minor = -1;
> uint32_t next;
>+ VIR_AUTOFREE(char *) map_device = NULL;
>
> do {
> is_mpath = virStorageBackendIsMultipath(names->name);
>
> if (is_mpath < 0)
>- goto out;
>+ return -1;
>
> if (is_mpath == 1) {
>
> if (virAsprintf(&map_device, "mapper/%s", names->name) < 0)
>- goto out;
>+ return -1;
>
> if (virStorageBackendGetMinorNumber(names->name, &minor) < 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("Failed to get %s minor number"),
> names->name);
>- goto out;
>+ return -1;
> }
>
> if (virStorageBackendMpathNewVol(pool, minor, map_device) < 0)
>- goto out;
>+ return -1;
>
>- VIR_FREE(map_device);
This is called in a loop, one VIR_FREE has to stay.
> }
>
> /* Given the way libdevmapper returns its data, I don't see
>diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c
>index 14f01f9ec0..7460349c81 100644
>--- a/src/storage/storage_backend_scsi.c
>+++ b/src/storage/storage_backend_scsi.c
>@@ -56,16 +56,14 @@ static int
> virStorageBackendSCSITriggerRescan(uint32_t host)
> {
> int fd = -1;
>- int retval = 0;
>- char *path;
>+ int retval = -1;
This inverts the logic of the function
>+ VIR_AUTOFREE(char *) path = NULL;
>
> VIR_DEBUG("Triggering rescan of host %d", host);
>
> if (virAsprintf(&path, "%s/host%u/scan",
>- LINUX_SYSFS_SCSI_HOST_PREFIX, host) < 0) {
>- retval = -1;
>- goto out;
>- }
>+ LINUX_SYSFS_SCSI_HOST_PREFIX, host) < 0)
>+ return -1;
>
> VIR_DEBUG("Scan trigger path is '%s'", path);
>
>@@ -75,8 +73,7 @@ virStorageBackendSCSITriggerRescan(uint32_t host)
> virReportSystemError(errno,
> _("Could not open '%s' to trigger host scan"),
> path);
>- retval = -1;
>- goto free_path;
>+ goto cleanup;
Unrelated rename. (There's no jump to 'cleanup' with fd != -1)
> }
>
> if (safewrite(fd,
>@@ -86,13 +83,12 @@ virStorageBackendSCSITriggerRescan(uint32_t host)
> virReportSystemError(errno,
> _("Write to '%s' to trigger host scan failed"),
> path);
>- retval = -1;
Before, this returned -1, now it will return 0.
> }
>
>+ retval = 0;
>+
>+ cleanup:
> VIR_FORCE_CLOSE(fd);
>- free_path:
>- VIR_FREE(path);
>- out:
> VIR_DEBUG("Rescan of host %d complete", host);
> return retval;
> }
>diff --git a/src/storage/storage_file_gluster.c b/src/storage/storage_file_gluster.c
>index f8bbde8cfe..7c2189d297 100644
>--- a/src/storage/storage_file_gluster.c
>+++ b/src/storage/storage_file_gluster.c
>@@ -258,10 +258,10 @@ virStorageFileBackendGlusterReadlinkCallback(const char *path,
> void *data)
> {
> virStorageFileBackendGlusterPrivPtr priv = data;
>- char *buf = NULL;
> size_t bufsiz = 0;
> ssize_t ret;
> struct stat st;
>+ VIR_AUTOFREE(char *) buf = NULL;
>
> *linkpath = NULL;
>
>@@ -277,13 +277,13 @@ virStorageFileBackendGlusterReadlinkCallback(const char *path,
>
> realloc:
> if (VIR_EXPAND_N(buf, bufsiz, 256) < 0)
>- goto error;
>+ return -1;
>
> if ((ret = glfs_readlink(priv->vol, path, buf, bufsiz)) < 0) {
> virReportSystemError(errno,
> _("failed to read link of gluster file '%s'"),
> path);
>- goto error;
>+ return -1;
> }
>
> if (ret == bufsiz)
>@@ -291,13 +291,9 @@ virStorageFileBackendGlusterReadlinkCallback(const char *path,
>
> buf[ret] = '\0';
>
>- *linkpath = buf;
>+ VIR_STEAL_PTR(*linkpath, buf);
>
This can also be separated into joining the success/error code paths and
actually switching to VIR_AUTOFREE.
Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 2/11/19 7:52 AM, Ján Tomko wrote:
> On Fri, Feb 08, 2019 at 01:37:06PM -0500, John Ferlan wrote:
>> Let's make use of the auto __cleanup capabilities. This also allows
>> for the cleanup of some goto paths.
>>
>> Signed-off-by: John Ferlan <jferlan@redhat.com>
>> ---
>> src/storage/storage_backend.c | 9 +--
>> src/storage/storage_backend_disk.c | 62 ++++++-----------
>> src/storage/storage_backend_fs.c | 17 ++---
>> src/storage/storage_backend_gluster.c | 30 +++-----
>> src/storage/storage_backend_iscsi.c | 73 +++++++-------------
>> src/storage/storage_backend_iscsi_direct.c | 36 ++++------
>> src/storage/storage_backend_logical.c | 35 +++-------
>> src/storage/storage_backend_mpath.c | 18 ++---
>> src/storage/storage_backend_rbd.c | 35 +++-------
>> src/storage/storage_backend_scsi.c | 79 ++++++++--------------
>> src/storage/storage_backend_sheepdog.c | 27 +++-----
>> src/storage/storage_backend_vstorage.c | 25 +++----
>> src/storage/storage_backend_zfs.c | 15 ++--
>> src/storage/storage_file_gluster.c | 16 ++---
>> 14 files changed, 158 insertions(+), 319 deletions(-)
>>
[...]
>> diff --git a/src/storage/storage_backend_scsi.c
>> b/src/storage/storage_backend_scsi.c
>> index 14f01f9ec0..7460349c81 100644
>> --- a/src/storage/storage_backend_scsi.c
>> +++ b/src/storage/storage_backend_scsi.c
>> @@ -56,16 +56,14 @@ static int
>> virStorageBackendSCSITriggerRescan(uint32_t host)
>> {
>> int fd = -1;
>> - int retval = 0;
>> - char *path;
>> + int retval = -1;
>
> This inverts the logic of the function
>
>> + VIR_AUTOFREE(char *) path = NULL;
>>
>> VIR_DEBUG("Triggering rescan of host %d", host);
>>
>> if (virAsprintf(&path, "%s/host%u/scan",
>> - LINUX_SYSFS_SCSI_HOST_PREFIX, host) < 0) {
>> - retval = -1;
>> - goto out;
>> - }
>> + LINUX_SYSFS_SCSI_HOST_PREFIX, host) < 0)
>> + return -1;
>>
>> VIR_DEBUG("Scan trigger path is '%s'", path);
>>
>> @@ -75,8 +73,7 @@ virStorageBackendSCSITriggerRescan(uint32_t host)
>> virReportSystemError(errno,
>> _("Could not open '%s' to trigger host
>> scan"),
>> path);
>
>> - retval = -1;
>> - goto free_path;
>> + goto cleanup;
>
> Unrelated rename. (There's no jump to 'cleanup' with fd != -1)
>
>> }
>>
>> if (safewrite(fd,
>> @@ -86,13 +83,12 @@ virStorageBackendSCSITriggerRescan(uint32_t host)
>> virReportSystemError(errno,
>> _("Write to '%s' to trigger host scan
>> failed"),
>> path);
>> - retval = -1;
>
> Before, this returned -1, now it will return 0.
>
>> }
>>
>> + retval = 0;
>> +
>> + cleanup:
>> VIR_FORCE_CLOSE(fd);
>> - free_path:
>> - VIR_FREE(path);
>> - out:
>> VIR_DEBUG("Rescan of host %d complete", host);
>> return retval;
>> }
>
So two pre-patches are attached with any luck...
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Feb 11, 2019 at 10:14:56PM -0500, John Ferlan wrote:
>
>
>On 2/11/19 7:52 AM, Ján Tomko wrote:
>> On Fri, Feb 08, 2019 at 01:37:06PM -0500, John Ferlan wrote:
>>> Let's make use of the auto __cleanup capabilities. This also allows
>>> for the cleanup of some goto paths.
>>>
>>> Signed-off-by: John Ferlan <jferlan@redhat.com>
>>> ---
>>> src/storage/storage_backend.c | 9 +--
>>> src/storage/storage_backend_disk.c | 62 ++++++-----------
>>> src/storage/storage_backend_fs.c | 17 ++---
>>> src/storage/storage_backend_gluster.c | 30 +++-----
>>> src/storage/storage_backend_iscsi.c | 73 +++++++-------------
>>> src/storage/storage_backend_iscsi_direct.c | 36 ++++------
>>> src/storage/storage_backend_logical.c | 35 +++-------
>>> src/storage/storage_backend_mpath.c | 18 ++---
>>> src/storage/storage_backend_rbd.c | 35 +++-------
>>> src/storage/storage_backend_scsi.c | 79 ++++++++--------------
>>> src/storage/storage_backend_sheepdog.c | 27 +++-----
>>> src/storage/storage_backend_vstorage.c | 25 +++----
>>> src/storage/storage_backend_zfs.c | 15 ++--
>>> src/storage/storage_file_gluster.c | 16 ++---
>>> 14 files changed, 158 insertions(+), 319 deletions(-)
>>>
>
>[...]
>
>>> diff --git a/src/storage/storage_backend_scsi.c
>>> b/src/storage/storage_backend_scsi.c
>>> index 14f01f9ec0..7460349c81 100644
>>> --- a/src/storage/storage_backend_scsi.c
>>> +++ b/src/storage/storage_backend_scsi.c
>>> @@ -56,16 +56,14 @@ static int
>>> virStorageBackendSCSITriggerRescan(uint32_t host)
>>> {
>>> int fd = -1;
>>> - int retval = 0;
>>> - char *path;
>>> + int retval = -1;
>>
>> This inverts the logic of the function
>>
>>> + VIR_AUTOFREE(char *) path = NULL;
>>>
>>> VIR_DEBUG("Triggering rescan of host %d", host);
>>>
>>> if (virAsprintf(&path, "%s/host%u/scan",
>>> - LINUX_SYSFS_SCSI_HOST_PREFIX, host) < 0) {
>>> - retval = -1;
>>> - goto out;
>>> - }
>>> + LINUX_SYSFS_SCSI_HOST_PREFIX, host) < 0)
>>> + return -1;
>>>
>>> VIR_DEBUG("Scan trigger path is '%s'", path);
>>>
>>> @@ -75,8 +73,7 @@ virStorageBackendSCSITriggerRescan(uint32_t host)
>>> virReportSystemError(errno,
>>> _("Could not open '%s' to trigger host
>>> scan"),
>>> path);
>>
>>> - retval = -1;
>>> - goto free_path;
>>> + goto cleanup;
>>
>> Unrelated rename. (There's no jump to 'cleanup' with fd != -1)
>>
>>> }
>>>
>>> if (safewrite(fd,
>>> @@ -86,13 +83,12 @@ virStorageBackendSCSITriggerRescan(uint32_t host)
>>> virReportSystemError(errno,
>>> _("Write to '%s' to trigger host scan
>>> failed"),
>>> path);
>>> - retval = -1;
>>
>> Before, this returned -1, now it will return 0.
>>
>>> }
>>>
>>> + retval = 0;
>>> +
>>> + cleanup:
>>> VIR_FORCE_CLOSE(fd);
>>> - free_path:
>>> - VIR_FREE(path);
>>> - out:
>>> VIR_DEBUG("Rescan of host %d complete", host);
>>> return retval;
>>> }
>>
>
>So two pre-patches are attached with any luck...
>
git send-email, please.
>John
>
>From 46e3b6fe94d68220c52e23e359d5b43a3f5cfbba Mon Sep 17 00:00:00 2001
>From: John Ferlan <jferlan@redhat.com>
>Date: Mon, 11 Feb 2019 21:48:53 -0500
>Subject: [PATCH 2/2] storage: Invert retval logic in
> virStorageBackendSCSITriggerRescan
>
>Rather than initialize to 0 and change to -1 on error, let's do the
>normal operation of initializing to -1 and set to 0 on success.
>
>Signed-off-by: John Ferlan <jferlan@redhat.com>
>---
> src/storage/storage_backend_scsi.c | 11 +++++------
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
>diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c
>index 85a177865f..591bcb04e2 100644
>--- a/src/storage/storage_backend_scsi.c
>+++ b/src/storage/storage_backend_scsi.c
>@@ -56,16 +56,14 @@ static int
> virStorageBackendSCSITriggerRescan(uint32_t host)
> {
> int fd = -1;
>- int retval = 0;
>+ int retval = -1;
> char *path = NULL;
>
> VIR_DEBUG("Triggering rescan of host %d", host);
>
> if (virAsprintf(&path, "%s/host%u/scan",
>- LINUX_SYSFS_SCSI_HOST_PREFIX, host) < 0) {
>- retval = -1;
>+ LINUX_SYSFS_SCSI_HOST_PREFIX, host) < 0)
> goto cleanup;
>- }
>
> VIR_DEBUG("Scan trigger path is '%s'", path);
>
>@@ -75,7 +73,6 @@ virStorageBackendSCSITriggerRescan(uint32_t host)
> virReportSystemError(errno,
> _("Could not open '%s' to trigger host scan"),
> path);
>- retval = -1;
> goto cleanup;
> }
>
>@@ -85,9 +82,11 @@ virStorageBackendSCSITriggerRescan(uint32_t host)
> virReportSystemError(errno,
> _("Write to '%s' to trigger host scan failed"),
> path);
>- retval = -1;
>+ goto cleanup;
> }
>
>+ retval = 0;
>+
> cleanup:
> VIR_FORCE_CLOSE(fd);
> VIR_FREE(path);
>--
>2.20.1
>
>From e7e2aa6a7fb00a1207e9a5a52596fb4ec3ffce8b Mon Sep 17 00:00:00 2001
>From: John Ferlan <jferlan@redhat.com>
>Date: Mon, 11 Feb 2019 21:46:28 -0500
>Subject: [PATCH 1/2] src: Fix label logic in
> virStorageBackendSCSITriggerRescan
>
>Let's initialize @path to NULL, then rather than use two labels
>free_path and out labels, let's use the cleanup: label to call
>VIR_FREE(path); and VIR_FORCE_CLOSE(fd);
>
>Signed-off-by: John Ferlan <jferlan@redhat.com>
>---
> src/storage/storage_backend_scsi.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2026 Red Hat, Inc.