Construct the command in multiple steps instead of using a sentinel in the args array. Signed-off-by: Ján Tomko <jtomko(a)redhat.com&gt; --- src/openvz/openvz_driver.c | 96 +++++++++++++++++--------------------- 1 file changed, 42 insertions(+), 54 deletions(-) diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 1a189dbbe7..0a08c63b1b 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -481,30 +481,11 @@ static char *openvzDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) { } -/* - * Convenient helper to target a command line argv - * and fill in an empty slot with the supplied - * key value. This lets us declare the argv on the - * stack and just splice in the domain name after - */ -#define PROGRAM_SENTINEL ((char *)0x1) -static void openvzSetProgramSentinal(const char **prog, const char *key) -{ - const char **tmp = prog; - while (tmp && *tmp) { - if (*tmp == PROGRAM_SENTINEL) { - *tmp = key; - break; - } - tmp++; - } -} - static int openvzDomainSuspend(virDomainPtr dom) { + g_autoptr(virCommand) cmd = virCommandNewArgList(VZCTL, "--quiet", "chkpnt", NULL); struct openvz_driver *driver = dom->conn->privateData; virDomainObjPtr vm; - const char *prog[] = {VZCTL, "--quiet", "chkpnt", PROGRAM_SENTINEL, "--suspend", NULL}; int ret = -1; if (!(vm = openvzDomObjFromDomain(driver, dom->uuid))) @@ -514,8 +495,8 @@ static int openvzDomainSuspend(virDomainPtr dom) goto cleanup; if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_PAUSED) { - openvzSetProgramSentinal(prog, vm->def->name); - if (virRun(prog, NULL) < 0) + virCommandAddArgList(cmd, vm->def->name, "--suspend", NULL); + if (virCommandRun(cmd, NULL) < 0) goto cleanup; virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, VIR_DOMAIN_PAUSED_USER); } @@ -529,9 +510,9 @@ static int openvzDomainSuspend(virDomainPtr dom) static int openvzDomainResume(virDomainPtr dom) { + g_autoptr(virCommand) cmd = virCommandNewArgList(VZCTL, "--quiet", "chkpnt", NULL); struct openvz_driver *driver = dom->conn->privateData; virDomainObjPtr vm; - const char *prog[] = {VZCTL, "--quiet", "chkpnt", PROGRAM_SENTINEL, "--resume", NULL}; int ret = -1; if (!(vm = openvzDomObjFromDomain(driver, dom->uuid))) @@ -541,8 +522,8 @@ static int openvzDomainResume(virDomainPtr dom) goto cleanup; if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_PAUSED) { - openvzSetProgramSentinal(prog, vm->def->name); - if (virRun(prog, NULL) < 0) + virCommandAddArgList(cmd, vm->def->name, "--resume", NULL); + if (virCommandRun(cmd, NULL) < 0) goto cleanup; virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_UNPAUSED); } @@ -558,9 +539,9 @@ static int openvzDomainShutdownFlags(virDomainPtr dom, unsigned int flags) { + g_autoptr(virCommand) cmd = virCommandNewArgList(VZCTL, "--quiet", "stop", NULL); struct openvz_driver *driver = dom->conn->privateData; virDomainObjPtr vm; - const char *prog[] = {VZCTL, "--quiet", "stop", PROGRAM_SENTINEL, NULL}; int ret = -1; int status; @@ -572,14 +553,15 @@ openvzDomainShutdownFlags(virDomainPtr dom, if (openvzGetVEStatus(vm, &status, NULL) == -1) goto cleanup; - openvzSetProgramSentinal(prog, vm->def->name); if (status != VIR_DOMAIN_RUNNING) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("domain is not in running state")); goto cleanup; } - if (virRun(prog, NULL) < 0) + virCommandAddArg(cmd, vm->def->name); + + if (virCommandRun(cmd, NULL) < 0) goto cleanup; vm->def->id = -1; @@ -613,9 +595,9 @@ openvzDomainDestroyFlags(virDomainPtr dom, unsigned int flags) static int openvzDomainReboot(virDomainPtr dom, unsigned int flags) { + g_autoptr(virCommand) cmd = virCommandNewArgList(VZCTL, "--quiet", "restart", NULL); struct openvz_driver *driver = dom->conn->privateData; virDomainObjPtr vm; - const char *prog[] = {VZCTL, "--quiet", "restart", PROGRAM_SENTINEL, NULL}; int ret = -1; int status; @@ -627,15 +609,16 @@ static int openvzDomainReboot(virDomainPtr dom, if (openvzGetVEStatus(vm, &status, NULL) == -1) goto cleanup; - openvzSetProgramSentinal(prog, vm->def->name); if (status != VIR_DOMAIN_RUNNING) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("domain is not in running state")); goto cleanup; } - if (virRun(prog, NULL) < 0) + virCommandAddArg(cmd, vm->def->name); + if (virCommandRun(cmd, NULL) < 0) goto cleanup; + ret = 0; virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_BOOTED); @@ -935,11 +918,11 @@ static virDomainPtr openvzDomainCreateXML(virConnectPtr conn, const char *xml, unsigned int flags) { + g_autoptr(virCommand) cmd = virCommandNewArgList(VZCTL, "--quiet", "start", NULL); struct openvz_driver *driver = conn->privateData; virDomainDefPtr vmdef = NULL; virDomainObjPtr vm = NULL; virDomainPtr dom = NULL; - const char *progstart[] = {VZCTL, "--quiet", "start", PROGRAM_SENTINEL, NULL}; unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE; virCheckFlags(VIR_DOMAIN_START_VALIDATE, NULL); @@ -986,9 +969,9 @@ openvzDomainCreateXML(virConnectPtr conn, const char *xml, if (openvzDomainSetNetworkConfig(conn, vm->def) < 0) goto cleanup; - openvzSetProgramSentinal(progstart, vm->def->name); + virCommandAddArg(cmd, vm->def->name); - if (virRun(progstart, NULL) < 0) + if (virCommandRun(cmd, NULL) < 0) goto cleanup; vm->pid = strtoI(vm->def->name); @@ -1016,9 +999,9 @@ openvzDomainCreateXML(virConnectPtr conn, const char *xml, static int openvzDomainCreateWithFlags(virDomainPtr dom, unsigned int flags) { + g_autoptr(virCommand) cmd = virCommandNewArgList(VZCTL, "--quiet", "start", NULL); struct openvz_driver *driver = dom->conn->privateData; virDomainObjPtr vm; - const char *prog[] = {VZCTL, "--quiet", "start", PROGRAM_SENTINEL, NULL }; int ret = -1; int status; @@ -1043,8 +1026,9 @@ openvzDomainCreateWithFlags(virDomainPtr dom, unsigned int flags) goto cleanup; } - openvzSetProgramSentinal(prog, vm->def->name); - if (virRun(prog, NULL) < 0) + virCommandAddArg(cmd, vm->def->name); + + if (virCommandRun(cmd, NULL) < 0) goto cleanup; vm->pid = strtoI(vm->def->name); @@ -1068,9 +1052,9 @@ static int openvzDomainUndefineFlags(virDomainPtr dom, unsigned int flags) { + g_autoptr(virCommand) cmd = virCommandNewArgList(VZCTL, "--quiet", "start", "destroy", NULL); struct openvz_driver *driver = dom->conn->privateData; virDomainObjPtr vm; - const char *prog[] = { VZCTL, "--quiet", "destroy", PROGRAM_SENTINEL, NULL }; int ret = -1; int status; @@ -1083,8 +1067,9 @@ openvzDomainUndefineFlags(virDomainPtr dom, if (openvzGetVEStatus(vm, &status, NULL) == -1) goto cleanup; - openvzSetProgramSentinal(prog, vm->def->name); - if (virRun(prog, NULL) < 0) + virCommandAddArg(cmd, vm->def->name); + + if (virCommandRun(cmd, NULL) < 0) goto cleanup; if (virDomainObjIsActive(vm)) @@ -1110,17 +1095,19 @@ openvzDomainSetAutostart(virDomainPtr dom, int autostart) { struct openvz_driver *driver = dom->conn->privateData; virDomainObjPtr vm; - const char *prog[] = { VZCTL, "--quiet", "set", PROGRAM_SENTINEL, - "--onboot", autostart ? "yes" : "no", - "--save", NULL }; + g_autoptr(virCommand) cmd = virCommandNewArgList(VZCTL, "--quiet", NULL); int ret = -1; if (!(vm = openvzDomObjFromDomain(driver, dom->uuid))) return -1; - openvzSetProgramSentinal(prog, vm->def->name); - if (virRun(prog, NULL) < 0) + virCommandAddArgList(cmd, "set", vm->def->name, + "--onboot", autostart ? "yes" : "no", + "--save", NULL); + + if (virCommandRun(cmd, NULL) < 0) goto cleanup; + ret = 0; cleanup: @@ -1191,9 +1178,8 @@ static int openvzDomainSetVcpusInternal(virDomainObjPtr vm, unsigned int nvcpus, virDomainXMLOptionPtr xmlopt) { + g_autoptr(virCommand) cmd = virCommandNewArgList(VZCTL, "--quiet", NULL); char str_vcpus[32]; - const char *prog[] = { VZCTL, "--quiet", "set", PROGRAM_SENTINEL, - "--cpus", str_vcpus, "--save", NULL }; unsigned int pcpus; pcpus = virHostCPUGetCount(); if (pcpus > 0 && pcpus < nvcpus) @@ -1201,8 +1187,10 @@ static int openvzDomainSetVcpusInternal(virDomainObjPtr vm, g_snprintf(str_vcpus, sizeof(str_vcpus), "%d", nvcpus); - openvzSetProgramSentinal(prog, vm->def->name); - if (virRun(prog, NULL) < 0) + virCommandAddArgList(cmd, "set", vm->def->name, + "--cpus", str_vcpus, "--save", NULL); + + if (virCommandRun(cmd, NULL) < 0) return -1; if (virDomainDefSetVcpusMax(vm->def, nvcpus, xmlopt) < 0) @@ -1544,16 +1532,16 @@ static int openvzDomainSetMemoryInternal(virDomainObjPtr vm, unsigned long long mem) { + g_autoptr(virCommand) cmd = virCommandNewArgList(VZCTL, "--quiet", NULL); char str_mem[16]; - const char *prog[] = { VZCTL, "--quiet", "set", PROGRAM_SENTINEL, - "--kmemsize", str_mem, "--save", NULL - }; /* memory has to be changed its format from kbyte to byte */ g_snprintf(str_mem, sizeof(str_mem), "%llu", mem * 1024); - openvzSetProgramSentinal(prog, vm->def->name); - if (virRun(prog, NULL) < 0) + virCommandAddArgList(cmd, "set", vm->def->name, + "--kmemsize", str_mem, "--save", NULL); + + if (virCommandRun(cmd, NULL) < 0) return -1; return 0; -- 2.25.1

Catch the individual usage not removed in previous commits. Signed-off-by: Ján Tomko <jtomko(a)redhat.com&gt; --- src/lxc/lxc_driver.c | 5 +++-- src/qemu/qemu_domain.c | 16 ++++++++-------- src/security/security_apparmor.c | 11 +++-------- src/util/virnetdev.c | 24 ++++++++---------------- 4 files changed, 22 insertions(+), 34 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 851894c459..61dd35360a 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1423,10 +1423,11 @@ lxcDomainDestroy(virDomainPtr dom) static int lxcCheckNetNsSupport(void) { - const char *argv[] = {"ip", "link", "set", "lo", "netns", "-1", NULL}; + g_autoptr(virCommand) cmd = virCommandNewArgList("ip", "link", "set", "lo", + "netns", "-1", NULL); int ip_rc; - if (virRun(argv, &ip_rc) < 0 || ip_rc == 255) + if (virCommandRun(cmd, &ip_rc) < 0 || ip_rc == 255) return 0; if (virProcessNamespaceAvailable(VIR_PROCESS_NAMESPACE_NET) < 0) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 98ffd23a71..3d075bca26 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -7553,20 +7553,20 @@ qemuDomainSnapshotForEachQcow2Raw(virQEMUDriverPtr driver, bool try_all, int ndisks) { - const char *qemuimgarg[] = { NULL, "snapshot", NULL, NULL, NULL, NULL }; + const char *qemuimgbin; size_t i; bool skipped = false; - qemuimgarg[0] = qemuFindQemuImgBinary(driver); - if (qemuimgarg[0] == NULL) { + qemuimgbin = qemuFindQemuImgBinary(driver); + if (qemuimgbin == NULL) { /* qemuFindQemuImgBinary set the error */ return -1; } - qemuimgarg[2] = op; - qemuimgarg[3] = name; - for (i = 0; i < ndisks; i++) { + g_autoptr(virCommand) cmd = virCommandNewArgList(qemuimgbin, "snapshot", + op, name, NULL); + /* FIXME: we also need to handle LVM here */ if (def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_DISK) { int format = virDomainDiskGetFormat(def->disks[i]); @@ -7593,9 +7593,9 @@ qemuDomainSnapshotForEachQcow2Raw(virQEMUDriverPtr driver, return -1; } - qemuimgarg[4] = virDomainDiskGetSource(def->disks[i]); + virCommandAddArg(cmd, virDomainDiskGetSource(def->disks[i])); - if (virRun(qemuimgarg, NULL) < 0) { + if (virCommandRun(cmd, NULL) < 0) { if (try_all) { VIR_WARN("skipping snapshot action on %s", def->disks[i]->dst); diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index ca02631f7f..55eb110522 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -203,15 +203,10 @@ load_profile(virSecurityManagerPtr mgr G_GNUC_UNUSED, static int remove_profile(const char *profile) { - int rc = -1; - const char * const argv[] = { - VIRT_AA_HELPER, "-D", "-u", profile, NULL - }; + g_autoptr(virCommand) cmd = virCommandArgList(VIRT_AA_HELPER, "-D", "-u", + profile, NULL); - if (virRun(argv, NULL) == 0) - rc = 0; - - return rc; + return virCommandRun(cmd, NULL); } static char * diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index dea0bccd57..1d024b8b97 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -508,6 +508,7 @@ int virNetDevSetNamespace(const char *ifname, pid_t pidInNs) g_autofree char *pid = NULL; g_autofree char *phy = NULL; g_autofree char *phy_path = NULL; + g_autoptr(virCommand) cmd = NULL; int len; pid = g_strdup_printf("%lld", (long long) pidInNs); @@ -518,28 +519,19 @@ int virNetDevSetNamespace(const char *ifname, pid_t pidInNs) if ((len = virFileReadAllQuiet(phy_path, 1024, &phy)) <= 0) { /* Not a wireless device. */ - const char *argv[] = { - "ip", "link", "set", ifname, "netns", NULL, NULL - }; - - argv[5] = pid; - if (virRun(argv, NULL) < 0) - return -1; - + cmd = virCommandNewArgList("ip", "link", + "set", ifname, "netns", pid, NULL); } else { - const char *argv[] = { - "iw", "phy", NULL, "set", "netns", NULL, NULL - }; - /* Remove a line break. */ phy[len - 1] = '\0'; - argv[2] = phy; - argv[5] = pid; - if (virRun(argv, NULL) < 0) - return -1; + cmd = virCommandNewArgList("iw", "phy", phy, + "set", "netns", pid, NULL); } + if (virCommandRun(cmd, NULL) < 0) + return -1; + return 0; } -- 2.25.1

virCommand is now used everywhere. Signed-off-by: Ján Tomko <jtomko(a)redhat.com&gt; Suggested-by: Sebastian Mitterle <smitterl(a)redhat.com&gt; --- src/util/virprocess.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/virprocess.c b/src/util/virprocess.c index b126c3c9af..afb1f9b79f 100644 --- a/src/util/virprocess.c +++ b/src/util/virprocess.c @@ -129,7 +129,7 @@ VIR_ENUM_IMPL(virProcessSchedPolicy, * @status: child exit status to translate * * Translate an exit status into a malloc'd string. Generic helper - * for virCommandRun(), virCommandWait(), virRun(), and virProcessWait() + * for virCommandRun(), virCommandWait() and virProcessWait() * status argument, as well as raw waitpid(). */ char * @@ -1190,7 +1190,7 @@ virProcessRunInForkHelper(int errfd, * particular no mutexes should be used in @cb, unless steps were * taken before forking to guarantee a predictable state. @cb * must not exec any external binaries, instead - * virCommand/virExec should be used for that purpose. + * virCommand should be used for that purpose. * * On return, the returned value is either -1 with error message * reported if something went bad in the parent, if child has -- 2.25.1