[libvirt] [PATCH] Remove all use of virRun in storage code

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; To make it easier to dynamically change the command line ARGV, switch all storage code over to use virCommandPtr APIs for running programs Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/storage/storage_backend.c | 15 +-- src/storage/storage_backend.h | 5 +- src/storage/storage_backend_disk.c | 93 +++++++-------- src/storage/storage_backend_fs.c | 134 +++++++++----------- src/storage/storage_backend_iscsi.c | 107 +++++++++------- src/storage/storage_backend_logical.c | 225 ++++++++++++++++++--------------- 6 files changed, 294 insertions(+), 285 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index e2e9b51..e0ff717 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -57,7 +57,6 @@ #include "storage_backend.h" #include "logging.h" #include "virfile.h" -#include "command.h" #if WITH_STORAGE_LVM # include "storage_backend_logical.h" @@ -1418,7 +1417,7 @@ virStorageBackendStablePath(virStoragePoolObjPtr pool, */ int virStorageBackendRunProgRegex(virStoragePoolObjPtr pool, - const char *const*prog, + virCommandPtr cmd, int nregex, const char **regex, int *nvars, @@ -1433,7 +1432,6 @@ virStorageBackendRunProgRegex(virStoragePoolObjPtr pool, int maxReg = 0, i, j; int totgroups = 0, ngroup = 0, maxvars = 0; char **groups; - virCommandPtr cmd = NULL; /* Compile all regular expressions */ if (VIR_ALLOC_N(reg, nregex) < 0) { @@ -1470,7 +1468,6 @@ virStorageBackendRunProgRegex(virStoragePoolObjPtr pool, goto cleanup; } - cmd = virCommandNewArgs(prog); virCommandSetOutputFD(cmd, &fd); if (virCommandRunAsync(cmd, NULL) < 0) { goto cleanup; @@ -1541,7 +1538,6 @@ cleanup: regfree(&reg[i]); VIR_FREE(reg); - virCommandFree(cmd); VIR_FORCE_FCLOSE(list); VIR_FORCE_CLOSE(fd); @@ -1562,7 +1558,7 @@ cleanup: */ int virStorageBackendRunProgNul(virStoragePoolObjPtr pool, - const char **prog, + virCommandPtr cmd, size_t n_columns, virStorageBackendListVolNulFunc func, void *data) @@ -1573,7 +1569,6 @@ virStorageBackendRunProgNul(virStoragePoolObjPtr pool, char **v; int ret = -1; int i; - virCommandPtr cmd = NULL; if (n_columns == 0) return -1; @@ -1585,7 +1580,6 @@ virStorageBackendRunProgNul(virStoragePoolObjPtr pool, for (i = 0; i < n_columns; i++) v[i] = NULL; - cmd = virCommandNewArgs(prog); virCommandSetOutputFD(cmd, &fd); if (virCommandRunAsync(cmd, NULL) < 0) { goto cleanup; @@ -1623,8 +1617,8 @@ virStorageBackendRunProgNul(virStoragePoolObjPtr pool, } if (feof (fp) < 0) { - virReportSystemError(errno, - _("read error on pipe to '%s'"), prog[0]); + virReportSystemError(errno, "%s", + _("read error on pipe")); goto cleanup; } @@ -1633,7 +1627,6 @@ virStorageBackendRunProgNul(virStoragePoolObjPtr pool, for (i = 0; i < n_columns; i++) VIR_FREE(v[i]); VIR_FREE(v); - virCommandFree(cmd); VIR_FORCE_FCLOSE(fp); VIR_FORCE_CLOSE(fd); diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index a37bf7c..4c371fb 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -27,6 +27,7 @@ # include <stdint.h> # include "internal.h" # include "storage_conf.h" +# include "command.h" typedef char * (*virStorageBackendFindPoolSources)(virConnectPtr conn, const char *srcSpec, unsigned int flags); typedef int (*virStorageBackendCheckPool)(virConnectPtr conn, virStoragePoolObjPtr pool, bool *active); @@ -141,7 +142,7 @@ typedef int (*virStorageBackendListVolNulFunc)(virStoragePoolObjPtr pool, void *data); int virStorageBackendRunProgRegex(virStoragePoolObjPtr pool, - const char *const*prog, + virCommandPtr cmd, int nregex, const char **regex, int *nvars, @@ -149,7 +150,7 @@ int virStorageBackendRunProgRegex(virStoragePoolObjPtr pool, void *data, const char *cmd_to_ignore); int virStorageBackendRunProgNul(virStoragePoolObjPtr pool, - const char **prog, + virCommandPtr cmd, size_t n_columns, virStorageBackendListVolNulFunc func, void *data); diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c index 995ad2f..ecc51fd 100644 --- a/src/storage/storage_backend_disk.c +++ b/src/storage/storage_backend_disk.c @@ -269,17 +269,20 @@ virStorageBackendDiskReadPartitions(virStoragePoolObjPtr pool, * - normal metadata 100027630080 100030242304 2612736 * */ - const char *prog[] = { - PARTHELPER, pool->def->source.devices[0].path, NULL, - }; + virCommandPtr cmd = virCommandNewArgList(PARTHELPER, + pool->def->source.devices[0].path, + NULL); + int ret; pool->def->allocation = pool->def->capacity = pool->def->available = 0; - return virStorageBackendRunProgNul(pool, - prog, - 6, - virStorageBackendDiskMakeVol, - vol); + ret = virStorageBackendRunProgNul(pool, + cmd, + 6, + virStorageBackendDiskMakeVol, + vol); + virCommandFree(cmd); + return ret; } static int @@ -299,15 +302,19 @@ virStorageBackendDiskMakePoolGeometry(virStoragePoolObjPtr pool, static int virStorageBackendDiskReadGeometry(virStoragePoolObjPtr pool) { - const char *prog[] = { - PARTHELPER, pool->def->source.devices[0].path, "-g", NULL, - }; - - return virStorageBackendRunProgNul(pool, - prog, - 3, - virStorageBackendDiskMakePoolGeometry, - NULL); + virCommandPtr cmd = virCommandNewArgList(PARTHELPER, + pool->def->source.devices[0].path, + "-g", + NULL); + int ret; + + ret = virStorageBackendRunProgNul(pool, + cmd, + 3, + virStorageBackendDiskMakePoolGeometry, + NULL); + virCommandFree(cmd); + return ret; } static int @@ -379,15 +386,13 @@ virStorageBackendDiskBuildPool(virConnectPtr conn ATTRIBUTE_UNUSED, bool ok_to_mklabel = false; int ret = -1; /* eg parted /dev/sda mklabel msdos */ - const char *prog[] = { - PARTED, - pool->def->source.devices[0].path, - "mklabel", - "--script", - ((pool->def->source.format == VIR_STORAGE_POOL_DISK_DOS) ? "msdos" : - virStoragePoolFormatDiskTypeToString(pool->def->source.format)), - NULL, - }; + virCommandPtr cmd = virCommandNewArgList(PARTED, + pool->def->source.devices[0].path, + "mklabel", + "--script", + ((pool->def->source.format == VIR_STORAGE_POOL_DISK_DOS) ? "msdos" : + virStoragePoolFormatDiskTypeToString(pool->def->source.format)), + NULL); virCheckFlags(VIR_STORAGE_POOL_BUILD_OVERWRITE | VIR_STORAGE_POOL_BUILD_NO_OVERWRITE, ret); @@ -419,9 +424,10 @@ virStorageBackendDiskBuildPool(virConnectPtr conn ATTRIBUTE_UNUSED, } if (ok_to_mklabel) - ret = virRun(prog, NULL); + ret = virCommandRun(cmd, NULL); error: + virCommandFree(cmd); return ret; } @@ -628,20 +634,13 @@ virStorageBackendDiskCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED, virStorageVolDefPtr vol) { int res = -1; - char *start = NULL; - char *end = NULL; char *partFormat; unsigned long long startOffset = 0, endOffset = 0; - const char *cmdargv[] = { - PARTED, - pool->def->source.devices[0].path, - "mkpart", - "--script", - NULL /*partFormat*/, - NULL /*start*/, - NULL /*end*/, - NULL - }; + virCommandPtr cmd = virCommandNewArgList(PARTED, + pool->def->source.devices[0].path, + "mkpart", + "--script", + NULL); if (vol->target.encryption != NULL) { virStorageReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -653,7 +652,7 @@ virStorageBackendDiskCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED, if (virStorageBackendDiskPartFormat(pool, vol, &partFormat) != 0) { return -1; } - cmdargv[4] = partFormat; + virCommandAddArg(cmd, partFormat); if (virStorageBackendDiskPartBoundries(pool, &startOffset, &endOffset, @@ -661,15 +660,10 @@ virStorageBackendDiskCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED, goto cleanup; } - if (virAsprintf(&start, "%lluB", startOffset) < 0 || - virAsprintf(&end, "%lluB", endOffset) < 0) { - virReportOOMError(); - goto cleanup; - } - cmdargv[5] = start; - cmdargv[6] = end; + virCommandAddArgFormat(cmd, "%lluB", startOffset); + virCommandAddArgFormat(cmd, "%lluB", endOffset); - if (virRun(cmdargv, NULL) < 0) + if (virCommandRun(cmd, NULL) < 0) goto cleanup; /* wait for device node to show up */ @@ -690,8 +684,7 @@ virStorageBackendDiskCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED, cleanup: VIR_FREE(partFormat); - VIR_FREE(start); - VIR_FREE(end); + virCommandFree(cmd); return res; } diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 4894994..b6ca858 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -251,10 +251,10 @@ virStorageBackendFileSystemNetFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSE .sources = NULL } }; - const char *prog[] = { SHOWMOUNT, "--no-headers", "--exports", NULL, NULL }; virStoragePoolSourcePtr source = NULL; char *retval = NULL; unsigned int i; + virCommandPtr cmd = NULL; virCheckFlags(0, NULL); @@ -270,9 +270,14 @@ virStorageBackendFileSystemNetFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSE } state.host = source->hosts[0].name; - prog[3] = source->hosts[0].name; - if (virStorageBackendRunProgRegex(NULL, prog, 1, regexes, vars, + cmd = virCommandNewArgList(SHOWMOUNT, + "--no-headers", + "--exports", + source->hosts[0].name, + NULL); + + if (virStorageBackendRunProgRegex(NULL, cmd, 1, regexes, vars, virStorageBackendFileSystemNetFindPoolSourcesFunc, &state, NULL) < 0) goto cleanup; @@ -289,7 +294,7 @@ virStorageBackendFileSystemNetFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSE VIR_FREE(state.list.sources); virStoragePoolSourceFree(source); - + virCommandFree(cmd); return retval; } @@ -337,63 +342,16 @@ virStorageBackendFileSystemIsMounted(virStoragePoolObjPtr pool) { */ static int virStorageBackendFileSystemMount(virStoragePoolObjPtr pool) { - char *src; - const char **mntargv; - + char *src = NULL; /* 'mount -t auto' doesn't seem to auto determine nfs (or cifs), * while plain 'mount' does. We have to craft separate argvs to * accommodate this */ - int netauto = (pool->def->type == VIR_STORAGE_POOL_NETFS && - pool->def->source.format == VIR_STORAGE_POOL_NETFS_AUTO); - int glusterfs = (pool->def->type == VIR_STORAGE_POOL_NETFS && - pool->def->source.format == VIR_STORAGE_POOL_NETFS_GLUSTERFS); - - int source_index; - - const char *netfs_auto_argv[] = { - MOUNT, - NULL, /* source path */ - pool->def->target.path, - NULL, - }; - - const char *fs_argv[] = { - MOUNT, - "-t", - pool->def->type == VIR_STORAGE_POOL_FS ? - virStoragePoolFormatFileSystemTypeToString(pool->def->source.format) : - virStoragePoolFormatFileSystemNetTypeToString(pool->def->source.format), - NULL, /* Fill in shortly - careful not to add extra fields - before this */ - pool->def->target.path, - NULL, - }; - - const char *glusterfs_argv[] = { - MOUNT, - "-t", - pool->def->type == VIR_STORAGE_POOL_FS ? - virStoragePoolFormatFileSystemTypeToString(pool->def->source.format) : - virStoragePoolFormatFileSystemNetTypeToString(pool->def->source.format), - NULL, - "-o", - "direct-io-mode=1", - pool->def->target.path, - NULL, - }; - - if (netauto) { - mntargv = netfs_auto_argv; - source_index = 1; - } else if (glusterfs) { - mntargv = glusterfs_argv; - source_index = 3; - } else { - mntargv = fs_argv; - source_index = 3; - } - - int ret; + bool netauto = (pool->def->type == VIR_STORAGE_POOL_NETFS && + pool->def->source.format == VIR_STORAGE_POOL_NETFS_AUTO); + bool glusterfs = (pool->def->type == VIR_STORAGE_POOL_NETFS && + pool->def->source.format == VIR_STORAGE_POOL_NETFS_GLUSTERFS); + virCommandPtr cmd = NULL; + int ret = -1; if (pool->def->type == VIR_STORAGE_POOL_NETFS) { if (pool->def->source.nhost != 1) { @@ -441,14 +399,41 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr pool) { return -1; } } - mntargv[source_index] = src; - if (virRun(mntargv, NULL) < 0) { - VIR_FREE(src); - return -1; - } + if (netauto) + cmd = virCommandNewArgList(MOUNT, + src, + pool->def->target.path, + NULL); + else if (glusterfs) + cmd = virCommandNewArgList( MOUNT, + "-t", + (pool->def->type == VIR_STORAGE_POOL_FS ? + virStoragePoolFormatFileSystemTypeToString(pool->def->source.format) : + virStoragePoolFormatFileSystemNetTypeToString(pool->def->source.format)), + src, + "-o", + "direct-io-mode=1", + pool->def->target.path, + NULL); + else + cmd = virCommandNewArgList(MOUNT, + "-t", + (pool->def->type == VIR_STORAGE_POOL_FS ? + virStoragePoolFormatFileSystemTypeToString(pool->def->source.format) : + virStoragePoolFormatFileSystemNetTypeToString(pool->def->source.format)), + src, + pool->def->target.path, + NULL); + + if (virCommandRun(cmd, NULL) < 0) + goto cleanup; + + ret = 0; +cleanup: + virCommandFree(cmd); VIR_FREE(src); - return 0; + return ret; } /** @@ -462,8 +447,8 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr pool) { */ static int virStorageBackendFileSystemUnmount(virStoragePoolObjPtr pool) { - const char *mntargv[3]; - int ret; + virCommandPtr cmd = NULL; + int ret = -1; if (pool->def->type == VIR_STORAGE_POOL_NETFS) { if (pool->def->source.nhost != 1) { @@ -497,14 +482,17 @@ virStorageBackendFileSystemUnmount(virStoragePoolObjPtr pool) { return 0; } - mntargv[0] = UMOUNT; - mntargv[1] = pool->def->target.path; - mntargv[2] = NULL; + cmd = virCommandNewArgList(UMOUNT, + pool->def->target.path, + NULL); - if (virRun(mntargv, NULL) < 0) { - return -1; - } - return 0; + if (virCommandRun(cmd, NULL) < 0) + goto cleanup; + + ret = 0; +cleanup: + virCommandFree(cmd); + return ret; } #endif /* WITH_STORAGE_FS */ diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c index e286c84..88de9fd 100644 --- a/src/storage/storage_backend_iscsi.c +++ b/src/storage/storage_backend_iscsi.c @@ -151,31 +151,32 @@ virStorageBackendISCSISession(virStoragePoolObjPtr pool, int vars[] = { 2, }; - const char *const prog[] = { - ISCSIADM, "--mode", "session", NULL - }; char *session = NULL; + virCommandPtr cmd = virCommandNewArgList(ISCSIADM, "--mode", "session", NULL); + /* Note that we ignore the exitstatus. Older versions of iscsiadm tools * returned an exit status of > 0, even if they succeeded. We will just * rely on whether session got filled in properly. */ if (virStorageBackendRunProgRegex(pool, - prog, + cmd, 1, regexes, vars, virStorageBackendISCSIExtractSession, &session, NULL) < 0) - return NULL; + goto cleanup; if (session == NULL && !probe) { virStorageReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot find session")); - return NULL; + goto cleanup; } +cleanup: + virCommandFree(cmd); return session; } @@ -279,42 +280,52 @@ virStorageBackendCreateIfaceIQN(const char *initiatoriqn, char **ifacename) { int ret = -1, exitstatus = -1; - char temp_ifacename[32]; - const char *const cmdargv1[] = { - ISCSIADM, "--mode", "iface", "--interface", - temp_ifacename, "--op", "new", NULL - }; - const char *const cmdargv2[] = { - ISCSIADM, "--mode", "iface", "--interface", temp_ifacename, - "--op", "update", "--name", "iface.initiatorname", "--value", - initiatoriqn, NULL - }; + char *temp_ifacename; + virCommandPtr cmd = NULL; - snprintf(temp_ifacename, sizeof(temp_ifacename), "libvirt-iface-%08llx", - (unsigned long long)virRandomBits(30)); + if (virAsprintf(&temp_ifacename, + "libvirt-iface-%08llx", + (unsigned long long)virRandomBits(30)) < 0) { + virReportOOMError(); + return -1; + } VIR_DEBUG("Attempting to create interface '%s' with IQN '%s'", - &temp_ifacename[0], initiatoriqn); + temp_ifacename, initiatoriqn); + cmd = virCommandNewArgList(ISCSIADM, + "--mode", "iface", + "--interface", temp_ifacename, + "--op", "new", + NULL); /* Note that we ignore the exitstatus. Older versions of iscsiadm * tools returned an exit status of > 0, even if they succeeded. * We will just rely on whether the interface got created * properly. */ - if (virRun(cmdargv1, &exitstatus) < 0) { + if (virCommandRun(cmd, &exitstatus) < 0) { virStorageReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to run command '%s' to create new iscsi interface"), - cmdargv1[0]); - goto out; + ISCSIADM); + goto cleanup; } + virCommandFree(cmd); + cmd = virCommandNewArgList(ISCSIADM, + "--mode", "iface", + "--interface", temp_ifacename, + "--op", "update", + "--name", "iface.initiatorname", + "--value", + initiatoriqn, + NULL); /* Note that we ignore the exitstatus. Older versions of iscsiadm tools * returned an exit status of > 0, even if they succeeded. We will just * rely on whether iface file got updated properly. */ - if (virRun(cmdargv2, &exitstatus) < 0) { + if (virCommandRun(cmd, &exitstatus) < 0) { virStorageReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to run command '%s' to update iscsi interface with IQN '%s'"), - cmdargv2[0], initiatoriqn); - goto out; + ISCSIADM, initiatoriqn); + goto cleanup; } /* Check again to make sure the interface was created. */ @@ -322,7 +333,7 @@ virStorageBackendCreateIfaceIQN(const char *initiatoriqn, VIR_DEBUG("Failed to find interface '%s' with IQN '%s' " "after attempting to create it", &temp_ifacename[0], initiatoriqn); - goto out; + goto cleanup; } else { VIR_DEBUG("Interface '%s' with IQN '%s' was created successfully", *ifacename, initiatoriqn); @@ -330,7 +341,9 @@ virStorageBackendCreateIfaceIQN(const char *initiatoriqn, ret = 0; -out: +cleanup: + virCommandFree(cmd); + VIR_FREE(temp_ifacename); if (ret != 0) VIR_FREE(*ifacename); return ret; @@ -426,14 +439,14 @@ static int virStorageBackendISCSIRescanLUNs(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, const char *session) { - const char *const cmdargv[] = { - ISCSIADM, "--mode", "session", "-r", session, "-R", NULL, - }; - - if (virRun(cmdargv, NULL) < 0) - return -1; - - return 0; + virCommandPtr cmd = virCommandNewArgList(ISCSIADM, + "--mode", "session", + "-r", session, + "-R", + NULL); + int ret = virCommandRun(cmd, NULL); + virCommandFree(cmd); + return ret; } struct virStorageBackendISCSITargetList { @@ -501,24 +514,25 @@ virStorageBackendISCSIScanTargets(const char *portal, "^\\s*(\\S+)\\s+(\\S+)\\s*$" }; int vars[] = { 2 }; - const char *const cmdsendtarget[] = { - ISCSIADM, "--mode", "discovery", "--type", "sendtargets", - "--portal", portal, NULL - }; struct virStorageBackendISCSITargetList list; - int i; + size_t i; + int ret = -1; + virCommandPtr cmd = virCommandNewArgList(ISCSIADM, + "--mode", "discovery", + "--type", "sendtargets", + "--portal", portal, + NULL); memset(&list, 0, sizeof(list)); if (virStorageBackendRunProgRegex(NULL, /* No pool for callback */ - cmdsendtarget, + cmd, 1, regexes, vars, virStorageBackendISCSIGetTargets, - &list, NULL) < 0) { - return -1; - } + &list, NULL) < 0) + goto cleanup; for (i = 0 ; i < list.ntargets ; i++) { /* We have to ignore failure, because we can't undo @@ -542,7 +556,10 @@ virStorageBackendISCSIScanTargets(const char *portal, VIR_FREE(list.targets); } - return 0; + ret = 0; +cleanup: + virCommandFree(cmd); + return ret; } diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 9a91dd9..9fe769b 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -48,17 +48,16 @@ static int virStorageBackendLogicalSetActive(virStoragePoolObjPtr pool, int on) { - const char *cmdargv[4]; - - cmdargv[0] = VGCHANGE; - cmdargv[1] = on ? "-aly" : "-aln"; - cmdargv[2] = pool->def->source.name; - cmdargv[3] = NULL; - - if (virRun(cmdargv, NULL) < 0) - return -1; - - return 0; + int ret; + virCommandPtr cmd = + virCommandNewArgList(VGCHANGE, + on ? "-aly" : "-aln", + pool->def->source.name, + NULL); + + ret = virCommandRun(cmd, NULL); + virCommandFree(cmd); + return ret; } @@ -296,24 +295,31 @@ virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool, int vars[] = { 8 }; - const char *prog[] = { - LVS, "--separator", "#", "--noheadings", "--units", "b", - "--unbuffered", "--nosuffix", "--options", - "lv_name,origin,uuid,devices,segtype,stripes,seg_size,vg_extent_size", - pool->def->source.name, NULL - }; - + int ret = -1; + virCommandPtr cmd; + + cmd = virCommandNewArgList(LVS, + "--separator", "#", + "--noheadings", + "--units", "b", + "--unbuffered", + "--nosuffix", + "--options", "lv_name,origin,uuid,devices,segtype,stripes,seg_size,vg_extent_size", + pool->def->source.name, + NULL); if (virStorageBackendRunProgRegex(pool, - prog, + cmd, 1, regexes, vars, virStorageBackendLogicalMakeVol, - vol, "lvs") < 0) { - return -1; - } + vol, "lvs") < 0) + goto cleanup; - return 0; + ret = 0; +cleanup: + virCommandFree(cmd); + return ret; } static int @@ -405,8 +411,7 @@ virStorageBackendLogicalFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSED, int vars[] = { 2 }; - const char *const prog[] = { PVS, "--noheadings", "-o", "pv_name,vg_name", NULL }; - const char *const scanprog[] = { VGSCAN, NULL }; + virCommandPtr cmd; char *retval = NULL; virStoragePoolSourceList sourceList; int i; @@ -418,17 +423,25 @@ virStorageBackendLogicalFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSED, * that might be hanging around, so if this fails for some reason, the * worst that happens is that scanning doesn't pick everything up */ - if (virRun(scanprog, NULL) < 0) { + cmd = virCommandNew(VGSCAN); + if (virCommandRun(cmd, NULL) < 0) VIR_WARN("Failure when running vgscan to refresh physical volumes"); - } + virCommandFree(cmd); memset(&sourceList, 0, sizeof(sourceList)); sourceList.type = VIR_STORAGE_POOL_LOGICAL; - if (virStorageBackendRunProgRegex(NULL, prog, 1, regexes, vars, - virStorageBackendLogicalFindPoolSourcesFunc, - &sourceList, "pvs") < 0) + cmd = virCommandNewArgList(PVS, + "--noheadings", + "-o", "pv_name,vg_name", + NULL); + if (virStorageBackendRunProgRegex(NULL, cmd, 1, regexes, vars, + virStorageBackendLogicalFindPoolSourcesFunc, + &sourceList, "pvs") < 0) { + virCommandFree(cmd); return NULL; + } + virCommandFree(cmd); retval = virStoragePoolSourceListFormat(&sourceList); if (retval == NULL) { @@ -483,26 +496,20 @@ virStorageBackendLogicalBuildPool(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool, unsigned int flags) { - const char **vgargv; - const char *pvargv[3]; - int n = 0, i, fd; + virCommandPtr vgcmd; + int fd; char zeros[PV_BLANK_SECTOR_SIZE]; + int ret = -1; + size_t i; virCheckFlags(0, -1); memset(zeros, 0, sizeof(zeros)); - if (VIR_ALLOC_N(vgargv, 3 + pool->def->source.ndevice) < 0) { - virReportOOMError(); - return -1; - } + vgcmd = virCommandNewArgList(VGCREATE, pool->def->source.name, NULL); - vgargv[n++] = VGCREATE; - vgargv[n++] = pool->def->source.name; - - pvargv[0] = PVCREATE; - pvargv[2] = NULL; for (i = 0 ; i < pool->def->source.ndevice ; i++) { + virCommandPtr pvcmd; /* * LVM requires that the first sector is blanked if using * a whole disk as a PV. So we just blank them out regardless @@ -539,25 +546,27 @@ virStorageBackendLogicalBuildPool(virConnectPtr conn ATTRIBUTE_UNUSED, * Initialize the physical volume because vgcreate is not * clever enough todo this for us :-( */ - vgargv[n++] = pool->def->source.devices[i].path; - pvargv[1] = pool->def->source.devices[i].path; - if (virRun(pvargv, NULL) < 0) + pvcmd = virCommandNewArgList(PVCREATE, + pool->def->source.devices[i].path, + NULL); + if (virCommandRun(pvcmd, NULL) < 0) { + virCommandFree(pvcmd); goto cleanup; - } + } + virCommandFree(pvcmd); - vgargv[n] = NULL; + virCommandAddArg(vgcmd, pool->def->source.devices[i].path); + } /* Now create the volume group itself */ - if (virRun(vgargv, NULL) < 0) + if (virCommandRun(vgcmd, NULL) < 0) goto cleanup; - VIR_FREE(vgargv); - - return 0; + ret = 0; - cleanup: - VIR_FREE(vgargv); - return -1; +cleanup: + virCommandFree(vgcmd); + return ret; } @@ -579,33 +588,42 @@ virStorageBackendLogicalRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, int vars[] = { 2 }; - const char *prog[] = { - VGS, "--separator", ":", "--noheadings", "--units", "b", "--unbuffered", - "--nosuffix", "--options", "vg_size,vg_free", - pool->def->source.name, NULL - }; + virCommandPtr cmd = NULL; + int ret = -1; virFileWaitForDevices(); /* Get list of all logical volumes */ - if (virStorageBackendLogicalFindLVs(pool, NULL) < 0) { - virStoragePoolObjClearVols(pool); - return -1; - } + if (virStorageBackendLogicalFindLVs(pool, NULL) < 0) + goto cleanup; + + cmd = virCommandNewArgList(VGS, + "--separator", ":", + "--noheadings", + "--units", "b", + "--unbuffered", + "--nosuffix", + "--options", "vg_size,vg_free", + pool->def->source.name, + NULL); /* Now get basic volgrp metadata */ if (virStorageBackendRunProgRegex(pool, - prog, + cmd, 1, regexes, vars, virStorageBackendLogicalRefreshPoolFunc, - NULL, "vgs") < 0) { - virStoragePoolObjClearVols(pool); - return -1; - } + NULL, "vgs") < 0) + goto cleanup; - return 0; + ret = 0; + +cleanup: + virCommandFree(cmd); + if (ret < 0) + virStoragePoolObjClearVols(pool); + return ret; } /* @@ -628,31 +646,37 @@ virStorageBackendLogicalDeletePool(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool, unsigned int flags) { - const char *cmdargv[] = { - VGREMOVE, "-f", pool->def->source.name, NULL - }; - const char *pvargv[3]; - int i, error; + virCommandPtr cmd = NULL; + size_t i; + int ret = -1; virCheckFlags(0, -1); /* first remove the volume group */ - if (virRun(cmdargv, NULL) < 0) - return -1; + cmd = virCommandNewArgList(VGREMOVE, + "-f", pool->def->source.name, + NULL); + if (virCommandRun(cmd, NULL) < 0) + goto cleanup; + virCommandFree(cmd); /* now remove the pv devices and clear them out */ - error = 0; - pvargv[0] = PVREMOVE; - pvargv[2] = NULL; + ret = 0; for (i = 0 ; i < pool->def->source.ndevice ; i++) { - pvargv[1] = pool->def->source.devices[i].path; - if (virRun(pvargv, NULL) < 0) { - error = -1; + cmd = virCommandNewArgList(PVREMOVE, + pool->def->source.devices[i].path, + NULL); + if (virCommandRun(cmd, NULL) < 0) { + ret = -1; break; } + virCommandFree(cmd); + cmd = NULL; } - return error; +cleanup: + virCommandFree(cmd); + return ret; } @@ -669,16 +693,7 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn, virStorageVolDefPtr vol) { int fdret, fd = -1; - char size[100]; - const char *cmdargvnew[] = { - LVCREATE, "--name", vol->name, "-L", size, - pool->def->source.name, NULL - }; - const char *cmdargvsnap[] = { - LVCREATE, "--name", vol->name, "-L", size, - "-s", vol->backingStore.path, NULL - }; - const char **cmdargv = cmdargvnew; + virCommandPtr cmd = NULL; if (vol->target.encryption != NULL) { virStorageReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -687,15 +702,6 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn, return -1; } - if (vol->backingStore.path) { - cmdargv = cmdargvsnap; - } - - snprintf(size, sizeof(size)-1, "%lluK", VIR_DIV_UP(vol->capacity, 1024)); - size[sizeof(size)-1] = '\0'; - - vol->type = VIR_STORAGE_VOL_BLOCK; - if (vol->target.path != NULL) { /* A target path passed to CreateVol has no meaning */ VIR_FREE(vol->target.path); @@ -708,8 +714,18 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn, return -1; } - if (virRun(cmdargv, NULL) < 0) - return -1; + cmd = virCommandNewArgList(LVCREATE, + "--name", vol->name, + NULL); + virCommandAddArg(cmd, "-L"); + virCommandAddArgFormat(cmd, "%lluK", VIR_DIV_UP(vol->capacity, 1024)); + if (vol->backingStore.path) + virCommandAddArgPair(cmd, "-s", vol->backingStore.path); + else + virCommandAddArg(cmd, pool->def->source.name); + + if (virCommandRun(cmd, NULL) < 0) + goto cleanup; if ((fdret = virStorageBackendVolOpen(vol->target.path)) < 0) goto cleanup; @@ -752,6 +768,7 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn, cleanup: VIR_FORCE_CLOSE(fd); virStorageBackendLogicalDeleteVol(conn, pool, vol, 0); + virCommandFree(cmd); return -1; } -- 1.7.7.6