[libvirt] [PATCH v2 11/32] 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; Reviewed-by: Erik Skultety <eskultet(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 | 59 ++++++--------- 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 | 67 +++++++--------- tests/storagepoolxml2argvtest.c | 7 +- tests/storagevolxml2argvtest.c | 6 +- tests/virstoragetest.c | 6 +- 12 files changed, 156 insertions(+), 312 deletions(-) diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c index 061c494b7d..4fb38178b2 100644 --- a/src/storage/storage_backend_disk.c +++ b/src/storage/storage_backend_disk.c @@ -356,12 +356,12 @@ virStorageBackendDiskReadPartitions(virStoragePoolObjPtr pool, virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); char *parthelper_path; - virCommandPtr cmd; struct virStorageBackendDiskPoolVolData cbdata = { .pool = pool, .vol = vol, }; int ret; + VIR_AUTOPTR(virCommand) cmd = NULL; if (!(parthelper_path = virFileFindResource("libvirt_parthelper", abs_topbuilddir "/src", @@ -392,7 +392,6 @@ virStorageBackendDiskReadPartitions(virStoragePoolObjPtr pool, 6, virStorageBackendDiskMakeVol, &cbdata); - virCommandFree(cmd); VIR_FREE(parthelper_path); return ret; } @@ -421,8 +420,8 @@ virStorageBackendDiskReadGeometry(virStoragePoolObjPtr pool) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); char *parthelper_path; - virCommandPtr cmd; int ret; + VIR_AUTOPTR(virCommand) cmd = NULL; if (!(parthelper_path = virFileFindResource("libvirt_parthelper", abs_topbuilddir "/src", @@ -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,9 +774,9 @@ virStorageBackendDiskDeleteVol(virStoragePoolObjPtr pool, virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); char *src_path = def->source.devices[0].path; char *srcname = last_component(src_path); - virCommandPtr cmd = NULL; bool isDevMapperDevice; int rc = -1; + VIR_AUTOPTR(virCommand) cmd = NULL; virCheckFlags(0, -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 420601a303..0436c25af0 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,9 +300,9 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr pool) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); char *src = NULL; - virCommandPtr cmd = NULL; int ret = -1; int rc; + VIR_AUTOPTR(virCommand) cmd = NULL; if (virStorageBackendFileSystemIsValid(pool) < 0) return -1; @@ -334,7 +325,6 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr pool) ret = 0; cleanup: - virCommandFree(cmd); VIR_FREE(src); return ret; } @@ -376,9 +366,8 @@ static int virStorageBackendFileSystemStop(virStoragePoolObjPtr pool) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - virCommandPtr cmd = NULL; - int ret = -1; int rc; + VIR_AUTOPTR(virCommand) cmd = NULL; if (virStorageBackendFileSystemIsValid(pool) < 0) return -1; @@ -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..9ebc560a46 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,13 +415,12 @@ virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool, int vars[] = { VIR_STORAGE_VOL_LOGICAL_REGEX_COUNT }; - int ret = -1; - virCommandPtr cmd; virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); struct virStorageBackendLogicalPoolVolData cbdata = { .pool = pool, .vol = vol, }; + VIR_AUTOPTR(virCommand) cmd = NULL; cmd = virCommandNewArgList(LVS, "--separator", "#", @@ -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,9 +707,9 @@ virStorageBackendLogicalBuildPool(virStoragePoolObjPtr pool, unsigned int flags) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - virCommandPtr vgcmd = NULL; int ret = -1; size_t i = 0; + VIR_AUTOPTR(virCommand) vgcmd = NULL; virCheckFlags(VIR_STORAGE_POOL_BUILD_OVERWRITE | VIR_STORAGE_POOL_BUILD_NO_OVERWRITE, ret); @@ -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,8 +773,8 @@ virStorageBackendLogicalRefreshPool(virStoragePoolObjPtr pool) 2 }; virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - virCommandPtr cmd = NULL; int ret = -1; + VIR_AUTOPTR(virCommand) cmd = NULL; 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; size_t i; - int ret = -1; + VIR_AUTOPTR(virCommand) cmd = NULL; 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 9ab318bb4d..73dcfb2f40 100644 --- a/src/storage/storage_backend_sheepdog.c +++ b/src/storage/storage_backend_sheepdog.c @@ -141,8 +141,9 @@ virStorageBackendSheepdogRefreshAllVol(virStoragePoolObjPtr pool) size_t i; VIR_AUTOPTR(virString) lines = NULL; VIR_AUTOPTR(virString) cells = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; - 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,13 @@ 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 +250,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); } @@ -341,33 +332,30 @@ static int virStorageBackendSheepdogRefreshVol(virStoragePoolObjPtr pool, virStorageVolDefPtr vol) { - 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; + if (virCommandRun(cmd, NULL) < 0) + return -1; - if ((ret = virStorageBackendSheepdogParseVdiList(vol, output)) < 0) - goto cleanup; + if (virStorageBackendSheepdogParseVdiList(vol, output) < 0) + return -1; 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 +365,14 @@ 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..8c5becd4c4 100644 --- a/src/storage/storage_backend_vstorage.c +++ b/src/storage/storage_backend_vstorage.c @@ -39,10 +39,10 @@ virStorageBackendVzPoolStart(virStoragePoolObjPtr pool) { int ret = -1; virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - virCommandPtr cmd = NULL; char *grp_name = NULL; char *usr_name = NULL; char *mode = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; /* Check the permissions */ if (def->target.perms.mode == (mode_t)-1) @@ -75,7 +75,6 @@ virStorageBackendVzPoolStart(virStoragePoolObjPtr pool) ret = 0; cleanup: - virCommandFree(cmd); VIR_FREE(mode); VIR_FREE(grp_name); VIR_FREE(usr_name); @@ -125,22 +124,15 @@ static int virStorageBackendVzPoolStop(virStoragePoolObjPtr pool) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - virCommandPtr cmd = NULL; - int ret = -1; int rc; + VIR_AUTOPTR(virCommand) cmd = NULL; /* Short-circuit if already unmounted */ if ((rc = virStorageBackendVzIsMounted(pool)) != 1) 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 4235b48c14..7d1a3dd2cd 100644 --- a/src/storage/storage_backend_zfs.c +++ b/src/storage/storage_backend_zfs.c @@ -51,9 +51,9 @@ VIR_LOG_INIT("storage.storage_backend_zfs"); static int virStorageBackendZFSVolModeNeeded(void) { - virCommandPtr cmd = NULL; int ret = -1, exit_code = -1; char *error = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; /* 'zfs get' without arguments prints out * usage information to stderr, including @@ -77,7 +77,6 @@ virStorageBackendZFSVolModeNeeded(void) ret = 0; cleanup: - virCommandFree(cmd); VIR_FREE(error); return ret; } @@ -179,10 +178,10 @@ virStorageBackendZFSFindVols(virStoragePoolObjPtr pool, virStorageVolDefPtr vol) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - virCommandPtr cmd = NULL; char *volumes_list = NULL; size_t i; VIR_AUTOPTR(virString) lines = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; /** * $ zfs list -Hp -t volume -o name,volsize -r test @@ -218,7 +217,6 @@ virStorageBackendZFSFindVols(virStoragePoolObjPtr pool, } cleanup: - virCommandFree(cmd); VIR_FREE(volumes_list); return 0; @@ -228,9 +226,9 @@ static int virStorageBackendZFSRefreshPool(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - virCommandPtr cmd = NULL; char *zpool_props = NULL; size_t i; + VIR_AUTOPTR(virCommand) cmd = NULL; VIR_AUTOPTR(virString) lines = NULL; VIR_AUTOPTR(virString) tokens = NULL; @@ -292,7 +290,6 @@ virStorageBackendZFSRefreshPool(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED) goto cleanup; cleanup: - virCommandFree(cmd); VIR_FREE(zpool_props); return 0; @@ -303,9 +300,9 @@ virStorageBackendZFSCreateVol(virStoragePoolObjPtr pool, virStorageVolDefPtr vol) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - virCommandPtr cmd = NULL; int ret = -1; int volmode_needed = -1; + VIR_AUTOPTR(virCommand) cmd = NULL; if (vol->target.encryption != NULL) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -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; size_t i; - int ret = -1; + VIR_AUTOPTR(virCommand) cmd = NULL; 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 c684f44475..a71a16add1 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -2207,9 +2207,9 @@ static int virStorageBackendPloopRestoreDesc(char *path) { int ret = -1; - virCommandPtr cmd = NULL; char *refresh_tool = NULL; char *desc = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; if (virAsprintf(&desc, "%s/DiskDescriptor.xml", path) < 0) return ret; @@ -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 1770d21f33..fa364941c5 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -617,9 +617,9 @@ storageBackendCreatePloop(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, unsigned int flags) { int ret = -1; - virCommandPtr cmd = NULL; char *create_tool = NULL; bool created = false; + VIR_AUTOPTR(virCommand) cmd = NULL; virCheckFlags(0, -1); @@ -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,8 +689,8 @@ storagePloopResize(virStorageVolDefPtr vol, unsigned long long capacity) { int ret = -1; - virCommandPtr cmd = NULL; char *resize_tool = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; resize_tool = virFindFileInPath("ploop"); if (!resize_tool) { @@ -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,11 +2317,11 @@ storageBackendResizeQemuImg(virStoragePoolObjPtr pool, { int ret = -1; char *img_tool = NULL; - virCommandPtr cmd = NULL; const char *type; char *secretPath = NULL; char *secretAlias = NULL; virStorageEncryptionPtr enc = vol->target.encryption; + VIR_AUTOPTR(virCommand) cmd = NULL; if (enc && (enc->format == VIR_STORAGE_ENCRYPTION_FORMAT_QCOW || enc->format == VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT) && @@ -2395,7 +2388,6 @@ storageBackendResizeQemuImg(virStoragePoolObjPtr pool, VIR_FREE(secretPath); } VIR_FREE(secretAlias); - virCommandFree(cmd); return ret; } @@ -2449,10 +2441,10 @@ virStorageBackendVolResizeLocal(virStoragePoolObjPtr pool, static int storageBackendPloopHasSnapshots(char *path) { - virCommandPtr cmd = NULL; char *output = NULL; char *snap_tool = NULL; int ret = -1; + VIR_AUTOPTR(virCommand) cmd = NULL; snap_tool = virFindFileInPath("ploop"); if (!snap_tool) { @@ -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,10 +2763,10 @@ static int storageBackendVolWipePloop(virStorageVolDefPtr vol, unsigned int algorithm) { - virCommandPtr cmd = NULL; char *target_path = NULL; char *disk_desc = NULL; char *create_tool = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; int ret = -1; @@ -2820,7 +2810,6 @@ storageBackendVolWipePloop(virStorageVolDefPtr vol, VIR_FREE(disk_desc); VIR_FREE(target_path); VIR_FREE(create_tool); - virCommandFree(cmd); return ret; } @@ -3034,8 +3023,8 @@ virStorageBackendFindGlusterPoolSources(const char *host, { char *glusterpath = NULL; char *outbuf = NULL; - virCommandPtr cmd = NULL; int rc; + VIR_AUTOPTR(virCommand) cmd = NULL; 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); char *output = NULL; char *error = NULL; char *start, *end; int ret = VIR_STORAGE_PARTED_ERROR; + VIR_AUTOPTR(virCommand) cmd = NULL; + 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 fc26c2f22e..828e95d5d3 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 - ); int ret = -1; + VIR_AUTOPTR(virCommand) cmd = NULL; + 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, @@ -1521,24 +1515,24 @@ virStorageFileGetNPIVKey(const char *path, char **key) { int status; - VIR_AUTOFREE(char *) outbuf = NULL; const char *serial; const char *port; - virCommandPtr cmd = virCommandNewArgList("/lib/udev/scsi_id", - "--replace-whitespace", - "--whitelisted", - "--export", - "--device", path, - NULL - ); - int ret = -2; - + VIR_AUTOFREE(char *) outbuf = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; + + 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 e7f42dc0f8..c9ba9b3fde 100644 --- a/tests/storagepoolxml2argvtest.c +++ b/tests/storagepoolxml2argvtest.c @@ -22,12 +22,12 @@ testCompareXMLToArgvFiles(bool shouldFail, const char *poolxml, const char *cmdline) { - VIR_AUTOFREE(char *) actualCmdline = NULL; - VIR_AUTOFREE(char *) src = NULL; int ret = -1; - virCommandPtr cmd = NULL; virStoragePoolDefPtr def = NULL; virStoragePoolObjPtr pool = NULL; + VIR_AUTOFREE(char *) actualCmdline = NULL; + VIR_AUTOFREE(char *) src = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; if (!(def = virStoragePoolDefParseFile(poolxml))) goto cleanup; @@ -83,7 +83,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 edff8d8477..d4ac02b31d 100644 --- a/tests/storagevolxml2argvtest.c +++ b/tests/storagevolxml2argvtest.c @@ -45,14 +45,12 @@ testCompareXMLToArgvFiles(bool shouldFail, char *actualCmdline = NULL; virStorageVolEncryptConvertStep convertStep = VIR_STORAGE_VOL_ENCRYPT_NONE; int ret = -1; - - virCommandPtr cmd = NULL; - virStoragePoolDefPtr def = NULL; virStoragePoolObjPtr obj = NULL; VIR_AUTOPTR(virStorageVolDef) vol = NULL; VIR_AUTOPTR(virStorageVolDef) inputvol = NULL; VIR_AUTOPTR(virStoragePoolDef) inputpool = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; if (!(def = virStoragePoolDefParseFile(poolxml))) goto cleanup; @@ -90,6 +88,7 @@ testCompareXMLToArgvFiles(bool shouldFail, convertStep = VIR_STORAGE_VOL_ENCRYPT_CREATE; do { + virCommandFree(cmd); cmd = virStorageBackendCreateQemuImgCmdFromVol(obj, vol, inputvol, flags, create_tool, @@ -139,7 +138,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..7272df7e33 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -128,9 +128,9 @@ static int testPrepImages(void) { int ret = EXIT_FAILURE; - virCommandPtr cmd = NULL; char *buf = NULL; bool compat = false; + VIR_AUTOPTR(virCommand) cmd = NULL; qemuimg = virFindFileInPath("qemu-img"); if (!qemuimg) @@ -246,7 +246,6 @@ testPrepImages(void) ret = 0; cleanup: VIR_FREE(buf); - virCommandFree(cmd); if (ret) testCleanupImages(); return ret; @@ -713,7 +712,6 @@ static int mymain(void) { int ret; - virCommandPtr cmd = NULL; struct testChainData data; struct testLookupData data2; struct testPathCanonicalizeData data3; @@ -722,6 +720,7 @@ mymain(void) virStorageSourcePtr chain = NULL; virStorageSourcePtr chain2; /* short for chain->backingStore */ virStorageSourcePtr chain3; /* short for chain2->backingStore */ + VIR_AUTOPTR(virCommand) cmd = NULL; if (storageRegisterAll() < 0) return EXIT_FAILURE; @@ -1604,7 +1603,6 @@ mymain(void) /* Final cleanup */ virStorageSourceFree(chain); testCleanupImages(); - virCommandFree(cmd); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.20.1