[libvirt] [PATCH 06/15] storage: Use VIR_AUTOPTR(virCommand)

Let's make use of the auto __cleanup capabilities cleaning up any now unnecessary goto paths. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/storage/storage_backend_disk.c | 85 +++++++++------------ src/storage/storage_backend_fs.c | 39 +++------- src/storage/storage_backend_logical.c | 101 +++++++------------------ src/storage/storage_backend_sheepdog.c | 50 +++++------- src/storage/storage_backend_vstorage.c | 14 +--- src/storage/storage_backend_zfs.c | 47 +++--------- src/storage/storage_driver.c | 3 +- src/storage/storage_util.c | 34 +++------ src/util/virstoragefile.c | 63 +++++++-------- tests/storagepoolxml2argvtest.c | 3 +- tests/storagevolxml2argvtest.c | 6 +- tests/virstoragetest.c | 6 +- 12 files changed, 148 insertions(+), 303 deletions(-) diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c index 061c494b7d..230cf44b97 100644 --- a/src/storage/storage_backend_disk.c +++ b/src/storage/storage_backend_disk.c @@ -356,7 +356,7 @@ virStorageBackendDiskReadPartitions(virStoragePoolObjPtr pool, virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); char *parthelper_path; - virCommandPtr cmd; + VIR_AUTOPTR(virCommand) cmd = NULL; struct virStorageBackendDiskPoolVolData cbdata = { .pool = pool, .vol = vol, @@ -392,7 +392,6 @@ virStorageBackendDiskReadPartitions(virStoragePoolObjPtr pool, 6, virStorageBackendDiskMakeVol, &cbdata); - virCommandFree(cmd); VIR_FREE(parthelper_path); return ret; } @@ -421,7 +420,7 @@ virStorageBackendDiskReadGeometry(virStoragePoolObjPtr pool) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); char *parthelper_path; - virCommandPtr cmd; + VIR_AUTOPTR(virCommand) cmd = NULL; int ret; if (!(parthelper_path = virFileFindResource("libvirt_parthelper", @@ -438,7 +437,6 @@ virStorageBackendDiskReadGeometry(virStoragePoolObjPtr pool) 3, virStorageBackendDiskMakePoolGeometry, pool); - virCommandFree(cmd); VIR_FREE(parthelper_path); return ret; } @@ -502,51 +500,40 @@ virStorageBackendDiskBuildPool(virStoragePoolObjPtr pool, virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); int format = def->source.format; const char *fmt; - bool ok_to_mklabel = false; - int ret = -1; - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; virCheckFlags(VIR_STORAGE_POOL_BUILD_OVERWRITE | - VIR_STORAGE_POOL_BUILD_NO_OVERWRITE, ret); + VIR_STORAGE_POOL_BUILD_NO_OVERWRITE, -1); - VIR_EXCLUSIVE_FLAGS_GOTO(VIR_STORAGE_POOL_BUILD_OVERWRITE, - VIR_STORAGE_POOL_BUILD_NO_OVERWRITE, - error); + VIR_EXCLUSIVE_FLAGS_RET(VIR_STORAGE_POOL_BUILD_OVERWRITE, + VIR_STORAGE_POOL_BUILD_NO_OVERWRITE, + -1); fmt = virStoragePoolFormatDiskTypeToString(format); - if (flags & VIR_STORAGE_POOL_BUILD_OVERWRITE) { - ok_to_mklabel = true; - } else { - if (virStorageBackendDeviceIsEmpty(def->source.devices[0].path, - fmt, true)) - ok_to_mklabel = true; - } + if (!(flags & VIR_STORAGE_POOL_BUILD_OVERWRITE) && + !(virStorageBackendDeviceIsEmpty(def->source.devices[0].path, + fmt, true))) + return -1; - if (ok_to_mklabel) { - if (virStorageBackendZeroPartitionTable(def->source.devices[0].path, - 1024 * 1024) < 0) - goto error; + if (virStorageBackendZeroPartitionTable(def->source.devices[0].path, + 1024 * 1024) < 0) + return -1; - /* eg parted /dev/sda mklabel --script msdos */ - if (format == VIR_STORAGE_POOL_DISK_UNKNOWN) - 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, - def->source.devices[0].path, - "mklabel", - "--script", - fmt, - NULL); - ret = virCommandRun(cmd, NULL); - } + /* eg parted /dev/sda mklabel --script msdos */ + if (format == VIR_STORAGE_POOL_DISK_UNKNOWN) + format = def->source.format = VIR_STORAGE_POOL_DISK_DOS; + if (format == VIR_STORAGE_POOL_DISK_DOS) + fmt = "msdos"; + else + fmt = virStoragePoolFormatDiskTypeToString(format); - error: - virCommandFree(cmd); - return ret; + cmd = virCommandNewArgList(PARTED, + def->source.devices[0].path, + "mklabel", + "--script", + fmt, + NULL); + return virCommandRun(cmd, NULL); } @@ -787,7 +774,7 @@ virStorageBackendDiskDeleteVol(virStoragePoolObjPtr pool, virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); char *src_path = def->source.devices[0].path; char *srcname = last_component(src_path); - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; bool isDevMapperDevice; int rc = -1; @@ -862,7 +849,6 @@ virStorageBackendDiskDeleteVol(virStoragePoolObjPtr pool, rc = 0; cleanup: VIR_FREE(devpath); - virCommandFree(cmd); return rc; } @@ -876,11 +862,13 @@ virStorageBackendDiskCreateVol(virStoragePoolObjPtr pool, unsigned long long startOffset = 0, endOffset = 0; virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); virErrorPtr save_err; - virCommandPtr cmd = virCommandNewArgList(PARTED, - def->source.devices[0].path, - "mkpart", - "--script", - NULL); + VIR_AUTOPTR(virCommand) cmd = NULL; + + cmd = virCommandNewArgList(PARTED, + def->source.devices[0].path, + "mkpart", + "--script", + NULL); if (vol->target.encryption && vol->target.encryption->format != VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) { @@ -934,7 +922,6 @@ virStorageBackendDiskCreateVol(virStoragePoolObjPtr pool, cleanup: VIR_FREE(partFormat); - virCommandFree(cmd); return res; error: diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 06c76fde4f..7d05ceeeb8 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -95,8 +95,6 @@ virStorageBackendFileSystemNetFindPoolSourcesFunc(char **const groups, static int virStorageBackendFileSystemNetFindNFSPoolSources(virNetfsDiscoverState *state) { - int ret = -1; - /* * # showmount --no-headers -e HOSTNAME * /tmp * @@ -112,7 +110,7 @@ virStorageBackendFileSystemNetFindNFSPoolSources(virNetfsDiscoverState *state) 1 }; - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; cmd = virCommandNewArgList(SHOWMOUNT, "--no-headers", @@ -120,16 +118,9 @@ virStorageBackendFileSystemNetFindNFSPoolSources(virNetfsDiscoverState *state) state->host, NULL); - if (virCommandRunRegex(cmd, 1, regexes, vars, - virStorageBackendFileSystemNetFindPoolSourcesFunc, - state, NULL, NULL) < 0) - goto cleanup; - - ret = 0; - - cleanup: - virCommandFree(cmd); - return ret; + return virCommandRunRegex(cmd, 1, regexes, vars, + virStorageBackendFileSystemNetFindPoolSourcesFunc, + state, NULL, NULL); } @@ -309,7 +300,7 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr pool) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); char *src = NULL; - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; int ret = -1; int rc; @@ -334,7 +325,6 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr pool) ret = 0; cleanup: - virCommandFree(cmd); VIR_FREE(src); return ret; } @@ -376,8 +366,7 @@ static int virStorageBackendFileSystemStop(virStoragePoolObjPtr pool) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - virCommandPtr cmd = NULL; - int ret = -1; + VIR_AUTOPTR(virCommand) cmd = NULL; int rc; if (virStorageBackendFileSystemIsValid(pool) < 0) @@ -388,13 +377,7 @@ virStorageBackendFileSystemStop(virStoragePoolObjPtr pool) return rc; cmd = virCommandNewArgList(UMOUNT, def->target.path, NULL); - if (virCommandRun(cmd, NULL) < 0) - goto cleanup; - - ret = 0; - cleanup: - virCommandFree(cmd); - return ret; + return virCommandRun(cmd, NULL); } #endif /* WITH_STORAGE_FS */ @@ -432,8 +415,7 @@ static int virStorageBackendExecuteMKFS(const char *device, const char *format) { - int ret = 0; - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; cmd = virCommandNewArgList(MKFS, "-t", format, NULL); @@ -456,11 +438,10 @@ virStorageBackendExecuteMKFS(const char *device, _("Failed to make filesystem of " "type '%s' on device '%s'"), format, device); - ret = -1; + return -1; } - virCommandFree(cmd); - return ret; + return 0; } #else /* #ifdef MKFS */ static int diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index fd95bd0d48..e0ddc962b1 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -48,13 +48,11 @@ static int virStorageBackendLogicalSetActive(virStoragePoolObjPtr pool, bool on) { - int ret; virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - virCommandPtr cmd = virStorageBackendLogicalChangeCmd(VGCHANGE, def, on); + VIR_AUTOPTR(virCommand) cmd = NULL; - ret = virCommandRun(cmd, NULL); - virCommandFree(cmd); - return ret; + cmd = virStorageBackendLogicalChangeCmd(VGCHANGE, def, on); + return virCommandRun(cmd, NULL); } @@ -67,11 +65,11 @@ virStorageBackendLogicalSetActive(virStoragePoolObjPtr pool, static void virStorageBackendLogicalRemoveDevice(const char *path) { - virCommandPtr cmd = virCommandNewArgList(PVREMOVE, path, NULL); + VIR_AUTOPTR(virCommand) cmd = NULL; + cmd = virCommandNewArgList(PVREMOVE, path, NULL); if (virCommandRun(cmd, NULL) < 0) VIR_INFO("Failed to pvremove logical device '%s'", path); - virCommandFree(cmd); } @@ -85,8 +83,7 @@ virStorageBackendLogicalRemoveDevice(const char *path) static int virStorageBackendLogicalInitializeDevice(const char *path) { - int ret = -1; - virCommandPtr pvcmd = NULL; + VIR_AUTOPTR(virCommand) pvcmd = NULL; /* * LVM requires that the first sector is blanked if using @@ -101,15 +98,7 @@ virStorageBackendLogicalInitializeDevice(const char *path) * clever enough todo this for us :-( */ pvcmd = virCommandNewArgList(PVCREATE, path, NULL); - if (virCommandRun(pvcmd, NULL) < 0) - goto cleanup; - - ret = 0; - - cleanup: - virCommandFree(pvcmd); - - return ret; + return virCommandRun(pvcmd, NULL); } @@ -426,8 +415,7 @@ virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool, int vars[] = { VIR_STORAGE_VOL_LOGICAL_REGEX_COUNT }; - int ret = -1; - virCommandPtr cmd; + VIR_AUTOPTR(virCommand) cmd = NULL; virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); struct virStorageBackendLogicalPoolVolData cbdata = { .pool = pool, @@ -444,20 +432,9 @@ virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool, "lv_name,origin,uuid,devices,segtype,stripes,seg_size,vg_extent_size,size,lv_attr", def->source.name, NULL); - if (virCommandRunRegex(cmd, - 1, - regexes, - vars, - virStorageBackendLogicalMakeVol, - &cbdata, - "lvs", - NULL) < 0) - goto cleanup; - - ret = 0; - cleanup: - virCommandFree(cmd); - return ret; + return virCommandRunRegex(cmd, 1, regexes, vars, + virStorageBackendLogicalMakeVol, + &cbdata, "lvs", NULL); } static int @@ -550,8 +527,7 @@ virStorageBackendLogicalGetPoolSources(virStoragePoolSourceListPtr sourceList) int vars[] = { 2 }; - virCommandPtr cmd; - int ret = -1; + VIR_AUTOPTR(virCommand) cmd = NULL; /* * NOTE: ignoring errors here; this is just to "touch" any logical volumes @@ -567,15 +543,9 @@ virStorageBackendLogicalGetPoolSources(virStoragePoolSourceListPtr sourceList) "--noheadings", "-o", "pv_name,vg_name", NULL, NULL); - if (virCommandRunRegex(cmd, 1, regexes, vars, - virStorageBackendLogicalFindPoolSourcesFunc, - sourceList, "pvs", NULL) < 0) - goto cleanup; - ret = 0; - - cleanup: - virCommandFree(cmd); - return ret; + return virCommandRunRegex(cmd, 1, regexes, vars, + virStorageBackendLogicalFindPoolSourcesFunc, + sourceList, "pvs", NULL); } @@ -737,7 +707,7 @@ virStorageBackendLogicalBuildPool(virStoragePoolObjPtr pool, unsigned int flags) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - virCommandPtr vgcmd = NULL; + VIR_AUTOPTR(virCommand) vgcmd = NULL; int ret = -1; size_t i = 0; @@ -773,8 +743,6 @@ virStorageBackendLogicalBuildPool(virStoragePoolObjPtr pool, ret = 0; cleanup: - virCommandFree(vgcmd); - /* On any failure, run through the devices that had pvcreate run in * in order to run pvremove on the device; otherwise, subsequent build * will fail if a pvcreate had been run already. */ @@ -805,7 +773,7 @@ virStorageBackendLogicalRefreshPool(virStoragePoolObjPtr pool) 2 }; virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; int ret = -1; virWaitForDevices(); @@ -838,7 +806,6 @@ virStorageBackendLogicalRefreshPool(virStoragePoolObjPtr pool) ret = 0; cleanup: - virCommandFree(cmd); if (ret < 0) virStoragePoolObjClearVols(pool); return ret; @@ -863,9 +830,8 @@ virStorageBackendLogicalDeletePool(virStoragePoolObjPtr pool, unsigned int flags) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; size_t i; - int ret = -1; virCheckFlags(0, -1); @@ -874,17 +840,13 @@ virStorageBackendLogicalDeletePool(virStoragePoolObjPtr pool, "-f", def->source.name, NULL); if (virCommandRun(cmd, NULL) < 0) - goto cleanup; + return -1; /* now remove the pv devices and clear them out */ for (i = 0; i < def->source.ndevice; i++) virStorageBackendLogicalRemoveDevice(def->source.devices[i].path); - ret = 0; - - cleanup: - virCommandFree(cmd); - return ret; + return 0; } @@ -893,10 +855,8 @@ virStorageBackendLogicalDeleteVol(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, virStorageVolDefPtr vol, unsigned int flags) { - int ret = -1; - - virCommandPtr lvchange_cmd = NULL; - virCommandPtr lvremove_cmd = NULL; + VIR_AUTOPTR(virCommand) lvchange_cmd = NULL; + VIR_AUTOPTR(virCommand) lvremove_cmd = NULL; virCheckFlags(0, -1); @@ -907,18 +867,14 @@ virStorageBackendLogicalDeleteVol(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, if (virCommandRun(lvremove_cmd, NULL) < 0) { if (virCommandRun(lvchange_cmd, NULL) < 0) { - goto cleanup; + return -1; } else { if (virCommandRun(lvremove_cmd, NULL) < 0) - goto cleanup; + return -1; } } - ret = 0; - cleanup: - virCommandFree(lvchange_cmd); - virCommandFree(lvremove_cmd); - return ret; + return 0; } @@ -926,9 +882,8 @@ static int virStorageBackendLogicalLVCreate(virStorageVolDefPtr vol, virStoragePoolDefPtr def) { - int ret; unsigned long long capacity = vol->target.capacity; - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; if (vol->target.encryption && vol->target.encryption->format != VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) { @@ -961,9 +916,7 @@ virStorageBackendLogicalLVCreate(virStorageVolDefPtr vol, else virCommandAddArg(cmd, def->source.name); - ret = virCommandRun(cmd, NULL); - virCommandFree(cmd); - return ret; + return virCommandRun(cmd, NULL); } diff --git a/src/storage/storage_backend_sheepdog.c b/src/storage/storage_backend_sheepdog.c index 458dbca58b..d9de74f52e 100644 --- a/src/storage/storage_backend_sheepdog.c +++ b/src/storage/storage_backend_sheepdog.c @@ -140,9 +140,10 @@ virStorageBackendSheepdogRefreshAllVol(virStoragePoolObjPtr pool) char *output = NULL; VIR_AUTOPTR(virString) lines = NULL; VIR_AUTOPTR(virString) cells = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; size_t i; - virCommandPtr cmd = virCommandNewArgList(SHEEPDOGCLI, "vdi", "list", "-r", NULL); + cmd = virCommandNewArgList(SHEEPDOGCLI, "vdi", "list", "-r", NULL); virStorageBackendSheepdogAddHostArg(cmd, pool); virCommandSetOutputBuffer(cmd, &output); if (virCommandRun(cmd, NULL) < 0) @@ -172,7 +173,6 @@ virStorageBackendSheepdogRefreshAllVol(virStoragePoolObjPtr pool) ret = 0; cleanup: - virCommandFree(cmd); VIR_FREE(output); return ret; } @@ -183,7 +183,7 @@ virStorageBackendSheepdogRefreshPool(virStoragePoolObjPtr pool) { int ret = -1; char *output = NULL; - virCommandPtr cmd; + VIR_AUTOPTR(virCommand) cmd = NULL; cmd = virCommandNewArgList(SHEEPDOGCLI, "node", "info", "-r", NULL); virStorageBackendSheepdogAddHostArg(cmd, pool); @@ -197,7 +197,6 @@ virStorageBackendSheepdogRefreshPool(virStoragePoolObjPtr pool) ret = virStorageBackendSheepdogRefreshAllVol(pool); cleanup: - virCommandFree(cmd); VIR_FREE(output); return ret; } @@ -208,14 +207,12 @@ virStorageBackendSheepdogDeleteVol(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, unsigned int flags) { + VIR_AUTOPTR(virCommand) cmd = NULL; virCheckFlags(0, -1); - virCommandPtr cmd = virCommandNewArgList(SHEEPDOGCLI, "vdi", "delete", vol->name, NULL); + cmd = virCommandNewArgList(SHEEPDOGCLI, "vdi", "delete", vol->name, NULL); virStorageBackendSheepdogAddHostArg(cmd, pool); - int ret = virCommandRun(cmd, NULL); - - virCommandFree(cmd); - return ret; + return virCommandRun(cmd, NULL); } @@ -252,27 +249,20 @@ virStorageBackendSheepdogBuildVol(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, unsigned int flags) { - int ret = -1; - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; virCheckFlags(0, -1); if (!vol->target.capacity) { virReportError(VIR_ERR_NO_SUPPORT, "%s", _("volume capacity required for this pool")); - goto cleanup; + return -1; } cmd = virCommandNewArgList(SHEEPDOGCLI, "vdi", "create", vol->name, NULL); virCommandAddArgFormat(cmd, "%llu", vol->target.capacity); virStorageBackendSheepdogAddHostArg(cmd, pool); - if (virCommandRun(cmd, NULL) < 0) - goto cleanup; - - ret = 0; - cleanup: - virCommandFree(cmd); - return ret; + return virCommandRun(cmd, NULL); } @@ -344,30 +334,30 @@ virStorageBackendSheepdogRefreshVol(virStoragePoolObjPtr pool, int ret; char *output = NULL; virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - virCommandPtr cmd = virCommandNewArgList(SHEEPDOGCLI, "vdi", "list", vol->name, "-r", NULL); + VIR_AUTOPTR(virCommand) cmd = NULL; + cmd = virCommandNewArgList(SHEEPDOGCLI, "vdi", "list", vol->name, "-r", NULL); virStorageBackendSheepdogAddHostArg(cmd, pool); virCommandSetOutputBuffer(cmd, &output); ret = virCommandRun(cmd, NULL); if (ret < 0) - goto cleanup; + return ret; if ((ret = virStorageBackendSheepdogParseVdiList(vol, output)) < 0) - goto cleanup; + return ret; vol->type = VIR_STORAGE_VOL_NETWORK; VIR_FREE(vol->key); if (virAsprintf(&vol->key, "%s/%s", def->source.name, vol->name) < 0) - goto cleanup; + return -1; VIR_FREE(vol->target.path); ignore_value(VIR_STRDUP(vol->target.path, vol->name)); - cleanup: - virCommandFree(cmd); - return ret; + + return 0; } @@ -377,15 +367,13 @@ virStorageBackendSheepdogResizeVol(virStoragePoolObjPtr pool, unsigned long long capacity, unsigned int flags) { + VIR_AUTOPTR(virCommand) cmd = NULL; virCheckFlags(0, -1); - virCommandPtr cmd = virCommandNewArgList(SHEEPDOGCLI, "vdi", "resize", vol->name, NULL); + cmd = virCommandNewArgList(SHEEPDOGCLI, "vdi", "resize", vol->name, NULL); virCommandAddArgFormat(cmd, "%llu", capacity); virStorageBackendSheepdogAddHostArg(cmd, pool); - int ret = virCommandRun(cmd, NULL); - - virCommandFree(cmd); - return ret; + return virCommandRun(cmd, NULL); } diff --git a/src/storage/storage_backend_vstorage.c b/src/storage/storage_backend_vstorage.c index 0a9abd446b..e62234fd70 100644 --- a/src/storage/storage_backend_vstorage.c +++ b/src/storage/storage_backend_vstorage.c @@ -39,7 +39,7 @@ virStorageBackendVzPoolStart(virStoragePoolObjPtr pool) { int ret = -1; virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; char *grp_name = NULL; char *usr_name = NULL; char *mode = NULL; @@ -75,7 +75,6 @@ virStorageBackendVzPoolStart(virStoragePoolObjPtr pool) ret = 0; cleanup: - virCommandFree(cmd); VIR_FREE(mode); VIR_FREE(grp_name); VIR_FREE(usr_name); @@ -125,8 +124,7 @@ static int virStorageBackendVzPoolStop(virStoragePoolObjPtr pool) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - virCommandPtr cmd = NULL; - int ret = -1; + VIR_AUTOPTR(virCommand) cmd = NULL; int rc; /* Short-circuit if already unmounted */ @@ -134,13 +132,7 @@ virStorageBackendVzPoolStop(virStoragePoolObjPtr pool) return rc; cmd = virCommandNewArgList(UMOUNT, def->target.path, NULL); - if (virCommandRun(cmd, NULL) < 0) - goto cleanup; - - ret = 0; - cleanup: - virCommandFree(cmd); - return ret; + return virCommandRun(cmd, NULL); } diff --git a/src/storage/storage_backend_zfs.c b/src/storage/storage_backend_zfs.c index f728477c2a..55f8a138cf 100644 --- a/src/storage/storage_backend_zfs.c +++ b/src/storage/storage_backend_zfs.c @@ -51,7 +51,7 @@ VIR_LOG_INIT("storage.storage_backend_zfs"); static int virStorageBackendZFSVolModeNeeded(void) { - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; int ret = -1, exit_code = -1; char *error = NULL; @@ -77,7 +77,6 @@ virStorageBackendZFSVolModeNeeded(void) ret = 0; cleanup: - virCommandFree(cmd); VIR_FREE(error); return ret; } @@ -179,7 +178,7 @@ virStorageBackendZFSFindVols(virStoragePoolObjPtr pool, virStorageVolDefPtr vol) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; char *volumes_list = NULL; VIR_AUTOPTR(virString) lines = NULL; size_t i; @@ -218,7 +217,6 @@ virStorageBackendZFSFindVols(virStoragePoolObjPtr pool, } cleanup: - virCommandFree(cmd); VIR_FREE(volumes_list); return 0; @@ -228,8 +226,8 @@ static int virStorageBackendZFSRefreshPool(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - virCommandPtr cmd = NULL; char *zpool_props = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; VIR_AUTOPTR(virString) lines = NULL; VIR_AUTOPTR(virString) tokens = NULL; size_t i; @@ -292,7 +290,6 @@ virStorageBackendZFSRefreshPool(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED) goto cleanup; cleanup: - virCommandFree(cmd); VIR_FREE(zpool_props); return 0; @@ -303,7 +300,7 @@ virStorageBackendZFSCreateVol(virStoragePoolObjPtr pool, virStorageVolDefPtr vol) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; int ret = -1; int volmode_needed = -1; @@ -364,7 +361,6 @@ virStorageBackendZFSCreateVol(virStoragePoolObjPtr pool, ret = 0; cleanup: - virCommandFree(cmd); return ret; } @@ -374,9 +370,8 @@ virStorageBackendZFSDeleteVol(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, unsigned int flags) { - int ret = -1; virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - virCommandPtr destroy_cmd = NULL; + VIR_AUTOPTR(virCommand) destroy_cmd = NULL; virCheckFlags(0, -1); @@ -385,13 +380,7 @@ virStorageBackendZFSDeleteVol(virStoragePoolObjPtr pool, virCommandAddArgFormat(destroy_cmd, "%s/%s", def->source.name, vol->name); - if (virCommandRun(destroy_cmd, NULL) < 0) - goto cleanup; - - ret = 0; - cleanup: - virCommandFree(destroy_cmd); - return ret; + return virCommandRun(destroy_cmd, NULL); } static int @@ -399,9 +388,8 @@ virStorageBackendZFSBuildPool(virStoragePoolObjPtr pool, unsigned int flags) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; size_t i; - int ret = -1; virCheckFlags(0, -1); @@ -417,14 +405,7 @@ virStorageBackendZFSBuildPool(virStoragePoolObjPtr pool, for (i = 0; i < def->source.ndevice; i++) virCommandAddArg(cmd, def->source.devices[i].path); - if (virCommandRun(cmd, NULL) < 0) - goto cleanup; - - ret = 0; - - cleanup: - virCommandFree(cmd); - return ret; + return virCommandRun(cmd, NULL); } static int @@ -432,22 +413,14 @@ virStorageBackendZFSDeletePool(virStoragePoolObjPtr pool, unsigned int flags) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - virCommandPtr cmd = NULL; - int ret = -1; + VIR_AUTOPTR(virCommand) cmd = NULL; virCheckFlags(0, -1); cmd = virCommandNewArgList(ZPOOL, "destroy", def->source.name, NULL); - if (virCommandRun(cmd, NULL) < 0) - goto cleanup; - - ret = 0; - - cleanup: - virCommandFree(cmd); - return ret; + return virCommandRun(cmd, NULL); } virStorageBackend virStorageBackendZFS = { diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index dcdd52bbbf..7c377439dc 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -2207,7 +2207,7 @@ static int virStorageBackendPloopRestoreDesc(char *path) { int ret = -1; - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; char *refresh_tool = NULL; char *desc = NULL; @@ -2238,7 +2238,6 @@ virStorageBackendPloopRestoreDesc(char *path) cleanup: VIR_FREE(refresh_tool); - virCommandFree(cmd); VIR_FREE(desc); return ret; } diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index 158cf3adb6..b9dbd3048a 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -617,7 +617,7 @@ storageBackendCreatePloop(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, unsigned int flags) { int ret = -1; - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; char *create_tool = NULL; bool created = false; @@ -677,7 +677,6 @@ storageBackendCreatePloop(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, created = true; ret = virCommandRun(cmd, NULL); cleanup: - virCommandFree(cmd); VIR_FREE(create_tool); if (ret < 0 && created) virFileDeleteTree(vol->target.path); @@ -690,7 +689,7 @@ storagePloopResize(virStorageVolDefPtr vol, unsigned long long capacity) { int ret = -1; - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; char *resize_tool = NULL; resize_tool = virFindFileInPath("ploop"); @@ -705,7 +704,6 @@ storagePloopResize(virStorageVolDefPtr vol, virCommandAddArgFormat(cmd, "%s/DiskDescriptor.xml", vol->target.path); ret = virCommandRun(cmd, NULL); - virCommandFree(cmd); VIR_FREE(resize_tool); return ret; } @@ -1319,8 +1317,7 @@ storageBackendDoCreateQemuImg(virStoragePoolObjPtr pool, const char *inputSecretPath, virStorageVolEncryptConvertStep convertStep) { - int ret; - virCommandPtr cmd; + VIR_AUTOPTR(virCommand) cmd = NULL; cmd = virStorageBackendCreateQemuImgCmdFromVol(pool, vol, inputvol, flags, create_tool, @@ -1329,11 +1326,7 @@ storageBackendDoCreateQemuImg(virStoragePoolObjPtr pool, if (!cmd) return -1; - ret = virStorageBackendCreateExecCommand(pool, vol, cmd); - - virCommandFree(cmd); - - return ret; + return virStorageBackendCreateExecCommand(pool, vol, cmd); } @@ -2324,7 +2317,7 @@ storageBackendResizeQemuImg(virStoragePoolObjPtr pool, { int ret = -1; char *img_tool = NULL; - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; const char *type; char *secretPath = NULL; char *secretAlias = NULL; @@ -2395,7 +2388,6 @@ storageBackendResizeQemuImg(virStoragePoolObjPtr pool, VIR_FREE(secretPath); } VIR_FREE(secretAlias); - virCommandFree(cmd); return ret; } @@ -2449,7 +2441,7 @@ virStorageBackendVolResizeLocal(virStoragePoolObjPtr pool, static int storageBackendPloopHasSnapshots(char *path) { - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; char *output = NULL; char *snap_tool = NULL; int ret = -1; @@ -2477,7 +2469,6 @@ storageBackendPloopHasSnapshots(char *path) cleanup: VIR_FREE(output); - virCommandFree(cmd); return ret; } @@ -2685,7 +2676,7 @@ storageBackendVolWipeLocalFile(const char *path, int ret = -1, fd = -1; const char *alg_char = NULL; struct stat st; - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; fd = open(path, O_RDWR); if (fd == -1) { @@ -2763,7 +2754,6 @@ storageBackendVolWipeLocalFile(const char *path, } cleanup: - virCommandFree(cmd); VIR_FORCE_CLOSE(fd); return ret; } @@ -2773,7 +2763,7 @@ static int storageBackendVolWipePloop(virStorageVolDefPtr vol, unsigned int algorithm) { - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; char *target_path = NULL; char *disk_desc = NULL; char *create_tool = NULL; @@ -2820,7 +2810,6 @@ storageBackendVolWipePloop(virStorageVolDefPtr vol, VIR_FREE(disk_desc); VIR_FREE(target_path); VIR_FREE(create_tool); - virCommandFree(cmd); return ret; } @@ -3034,7 +3023,7 @@ virStorageBackendFindGlusterPoolSources(const char *host, { char *glusterpath = NULL; char *outbuf = NULL; - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; int rc; int ret = -1; @@ -3069,7 +3058,6 @@ virStorageBackendFindGlusterPoolSources(const char *host, cleanup: VIR_FREE(outbuf); - virCommandFree(cmd); VIR_FREE(glusterpath); return ret; } @@ -3309,12 +3297,13 @@ virStorageBackendPARTEDFindLabel(const char *device, const char *const args[] = { device, "print", "--script", NULL, }; - virCommandPtr cmd = virCommandNew(PARTED); + VIR_AUTOPTR(virCommand) cmd = NULL; char *output = NULL; char *error = NULL; char *start, *end; int ret = VIR_STORAGE_PARTED_ERROR; + cmd = virCommandNew(PARTED); virCommandAddArgSet(cmd, args); virCommandAddEnvString(cmd, "LC_ALL=C"); virCommandSetOutputBuffer(cmd, &output); @@ -3359,7 +3348,6 @@ virStorageBackendPARTEDFindLabel(const char *device, ret = VIR_STORAGE_PARTED_DIFFERENT; cleanup: - virCommandFree(cmd); VIR_FREE(output); VIR_FREE(error); return ret; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index cd2437a03e..2e5c64e034 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1376,13 +1376,14 @@ int virStorageFileGetLVMKey(const char *path, * 06UgP5-2rhb-w3Bo-3mdR-WeoL-pytO-SAa2ky */ int status; - virCommandPtr cmd = virCommandNewArgList(LVS, "--noheadings", - "--unbuffered", "--nosuffix", - "--options", "uuid", path, - NULL - ); + VIR_AUTOPTR(virCommand) cmd = NULL; int ret = -1; + cmd = virCommandNewArgList(LVS, "--noheadings", + "--unbuffered", "--nosuffix", + "--options", "uuid", path, + NULL + ); *key = NULL; /* Run the program and capture its output */ @@ -1417,8 +1418,6 @@ int virStorageFileGetLVMKey(const char *path, if (*key && STREQ(*key, "")) VIR_FREE(*key); - virCommandFree(cmd); - return ret; } #else @@ -1451,20 +1450,20 @@ virStorageFileGetSCSIKey(const char *path, bool ignoreError ATTRIBUTE_UNUSED) { int status; - virCommandPtr cmd = virCommandNewArgList("/lib/udev/scsi_id", - "--replace-whitespace", - "--whitelisted", - "--device", path, - NULL - ); - int ret = -2; - + VIR_AUTOPTR(virCommand) cmd = NULL; + + cmd = virCommandNewArgList("/lib/udev/scsi_id", + "--replace-whitespace", + "--whitelisted", + "--device", path, + NULL + ); *key = NULL; /* Run the program and capture its output */ virCommandSetOutputBuffer(cmd, key); if (virCommandRun(cmd, &status) < 0) - goto cleanup; + return -2; /* Explicitly check status == 0, rather than passing NULL * to virCommandRun because we don't want to raise an actual @@ -1476,15 +1475,10 @@ virStorageFileGetSCSIKey(const char *path, *nl = '\0'; } - ret = 0; - - cleanup: if (*key && STREQ(*key, "")) VIR_FREE(*key); - virCommandFree(cmd); - - return ret; + return 0; } #else int virStorageFileGetSCSIKey(const char *path, @@ -1522,23 +1516,23 @@ virStorageFileGetNPIVKey(const char *path, { int status; VIR_AUTOFREE(char *) outbuf = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; const char *serial; const char *port; - virCommandPtr cmd = virCommandNewArgList("/lib/udev/scsi_id", - "--replace-whitespace", - "--whitelisted", - "--export", - "--device", path, - NULL - ); - int ret = -2; + cmd = virCommandNewArgList("/lib/udev/scsi_id", + "--replace-whitespace", + "--whitelisted", + "--export", + "--device", path, + NULL + ); *key = NULL; /* Run the program and capture its output */ virCommandSetOutputBuffer(cmd, &outbuf); if (virCommandRun(cmd, &status) < 0) - goto cleanup; + return -2; /* Explicitly check status == 0, rather than passing NULL * to virCommandRun because we don't want to raise an actual @@ -1562,12 +1556,7 @@ virStorageFileGetNPIVKey(const char *path, ignore_value(virAsprintf(key, "%s_PORT%s", serial, port)); } - ret = 0; - - cleanup: - virCommandFree(cmd); - - return ret; + return 0; } #else int virStorageFileGetNPIVKey(const char *path ATTRIBUTE_UNUSED, diff --git a/tests/storagepoolxml2argvtest.c b/tests/storagepoolxml2argvtest.c index 2dd87ab731..116a75c3ea 100644 --- a/tests/storagepoolxml2argvtest.c +++ b/tests/storagepoolxml2argvtest.c @@ -25,9 +25,9 @@ testCompareXMLToArgvFiles(bool shouldFail, VIR_AUTOFREE(char *) actualCmdline = NULL; VIR_AUTOFREE(char *) src = NULL; VIR_AUTOPTR(virStoragePoolDef) def = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; int defType; int ret = -1; - virCommandPtr cmd = NULL; virStoragePoolObjPtr pool = NULL; virStoragePoolDefPtr objDef; @@ -88,7 +88,6 @@ testCompareXMLToArgvFiles(bool shouldFail, ret = 0; cleanup: - virCommandFree(cmd); VIR_FREE(actualCmdline); virStoragePoolObjEndAPI(&pool); if (shouldFail) { diff --git a/tests/storagevolxml2argvtest.c b/tests/storagevolxml2argvtest.c index 2acbf567ca..38bb2ae004 100644 --- a/tests/storagevolxml2argvtest.c +++ b/tests/storagevolxml2argvtest.c @@ -45,9 +45,7 @@ testCompareXMLToArgvFiles(bool shouldFail, char *actualCmdline = NULL; virStorageVolEncryptConvertStep convertStep = VIR_STORAGE_VOL_ENCRYPT_NONE; int ret = -1; - - virCommandPtr cmd = NULL; - + VIR_AUTOPTR(virCommand) cmd = NULL; VIR_AUTOPTR(virStorageVolDef) vol = NULL; VIR_AUTOPTR(virStorageVolDef) inputvol = NULL; VIR_AUTOPTR(virStoragePoolDef) inputpool = NULL; @@ -91,6 +89,7 @@ testCompareXMLToArgvFiles(bool shouldFail, convertStep = VIR_STORAGE_VOL_ENCRYPT_CREATE; do { + virCommandFree(cmd); cmd = virStorageBackendCreateQemuImgCmdFromVol(obj, vol, inputvol, flags, create_tool, @@ -140,7 +139,6 @@ testCompareXMLToArgvFiles(bool shouldFail, ret = 0; cleanup: - virCommandFree(cmd); VIR_FREE(actualCmdline); virStoragePoolObjEndAPI(&obj); return ret; diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index c7c40b16f8..c448d8b733 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -128,7 +128,7 @@ static int testPrepImages(void) { int ret = EXIT_FAILURE; - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; char *buf = NULL; bool compat = false; @@ -246,7 +246,6 @@ testPrepImages(void) ret = 0; cleanup: VIR_FREE(buf); - virCommandFree(cmd); if (ret) testCleanupImages(); return ret; @@ -713,7 +712,7 @@ static int mymain(void) { int ret; - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; struct testChainData data; struct testLookupData data2; struct testPathCanonicalizeData data3; @@ -1604,7 +1603,6 @@ mymain(void) /* Final cleanup */ virStorageSourceFree(chain); testCleanupImages(); - virCommandFree(cmd); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.20.1