[libvirt PATCH 0/5] Remove virRun in favor of virCommand

Ján Tomko (5): openvz: switch from virRun to virCommand vmware: use virCommand instead of virRun Remove all usage of virRun util: remove references to virRun/virExec util: remove virRun src/libvirt_private.syms | 1 - src/lxc/lxc_driver.c | 5 +- src/openvz/openvz_driver.c | 96 ++++++++++++++------------------ src/qemu/qemu_domain.c | 16 +++--- src/security/security_apparmor.c | 11 +--- src/util/vircommand.c | 35 ------------ src/util/vircommand.h | 2 - src/util/virnetdev.c | 24 +++----- src/util/virprocess.c | 4 +- src/vmware/vmware_conf.c | 9 ++- src/vmware/vmware_driver.c | 69 ++++++++++------------- 11 files changed, 98 insertions(+), 174 deletions(-) -- 2.25.1

Construct the command in multiple steps instead of using a sentinel in the args array. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- 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

Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/vmware/vmware_conf.c | 9 +++-- src/vmware/vmware_driver.c | 69 ++++++++++++++++---------------------- 2 files changed, 32 insertions(+), 46 deletions(-) diff --git a/src/vmware/vmware_conf.c b/src/vmware/vmware_conf.c index fd62bb96f7..8bea9c3b12 100644 --- a/src/vmware/vmware_conf.c +++ b/src/vmware/vmware_conf.c @@ -436,8 +436,7 @@ vmwareVmxPath(virDomainDefPtr vmdef, char **vmxPath) int vmwareMoveFile(char *srcFile, char *dstFile) { - const char *cmdmv[] = - { "mv", PROGRAM_SENTINEL, PROGRAM_SENTINEL, NULL }; + g_autoptr(virCommand) cmd = NULL; if (!virFileExists(srcFile)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("file %s does not exist"), @@ -448,9 +447,9 @@ vmwareMoveFile(char *srcFile, char *dstFile) if (STREQ(srcFile, dstFile)) return 0; - vmwareSetSentinal(cmdmv, srcFile); - vmwareSetSentinal(cmdmv, dstFile); - if (virRun(cmdmv, NULL) < 0) { + cmd = virCommandNewArgList("mv", srcFile, dstFile, NULL); + + if (virCommandRun(cmd, NULL) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("failed to move file to %s "), dstFile); return -1; diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c index d5dd6e4f5e..5b4057a8f6 100644 --- a/src/vmware/vmware_driver.c +++ b/src/vmware/vmware_driver.c @@ -347,15 +347,12 @@ vmwareStopVM(struct vmware_driver *driver, virDomainObjPtr vm, virDomainShutoffReason reason) { - const char *cmd[] = { - driver->vmrun, "-T", PROGRAM_SENTINEL, "stop", - PROGRAM_SENTINEL, "soft", NULL - }; + g_autoptr(virCommand) cmd = virCommandNew(driver->vmrun); - vmwareSetSentinal(cmd, vmwareDriverTypeToString(driver->type)); - vmwareSetSentinal(cmd, ((vmwareDomainPtr) vm->privateData)->vmxPath); + virCommandAddArgList(cmd, "-T", vmwareDriverTypeToString(driver->type), + "stop", ((vmwareDomainPtr) vm->privateData)->vmxPath, "soft", NULL); - if (virRun(cmd, NULL) < 0) + if (virCommandRun(cmd, NULL) < 0) return -1; vm->def->id = -1; @@ -367,26 +364,22 @@ vmwareStopVM(struct vmware_driver *driver, static int vmwareStartVM(struct vmware_driver *driver, virDomainObjPtr vm) { - const char *cmd[] = { - driver->vmrun, "-T", PROGRAM_SENTINEL, "start", - PROGRAM_SENTINEL, PROGRAM_SENTINEL, NULL - }; + g_autoptr(virCommand) cmd = virCommandNew(driver->vmrun); const char *vmxPath = ((vmwareDomainPtr) vm->privateData)->vmxPath; + virCommandAddArgList(cmd, "-T", vmwareDriverTypeToString(driver->type), + "start", vmxPath, NULL); + if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_SHUTOFF) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not in shutoff state")); return -1; } - vmwareSetSentinal(cmd, vmwareDriverTypeToString(driver->type)); - vmwareSetSentinal(cmd, vmxPath); if (!((vmwareDomainPtr) vm->privateData)->gui) - vmwareSetSentinal(cmd, NOGUI); - else - vmwareSetSentinal(cmd, NULL); + virCommandAddArg(cmd, NOGUI); - if (virRun(cmd, NULL) < 0) + if (virCommandRun(cmd, NULL) < 0) return -1; if ((vm->def->id = vmwareExtractPid(vmxPath)) < 0) { @@ -543,12 +536,9 @@ static int vmwareDomainSuspend(virDomainPtr dom) { struct vmware_driver *driver = dom->conn->privateData; + g_autoptr(virCommand) cmd = virCommandNew(driver->vmrun); virDomainObjPtr vm; - const char *cmd[] = { - driver->vmrun, "-T", PROGRAM_SENTINEL, "pause", - PROGRAM_SENTINEL, NULL - }; int ret = -1; if (driver->type == VMWARE_DRIVER_PLAYER) { @@ -561,15 +551,17 @@ vmwareDomainSuspend(virDomainPtr dom) if (!(vm = vmwareDomObjFromDomain(driver, dom->uuid))) return -1; - vmwareSetSentinal(cmd, vmwareDriverTypeToString(driver->type)); - vmwareSetSentinal(cmd, ((vmwareDomainPtr) vm->privateData)->vmxPath); if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_RUNNING) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("domain is not in running state")); goto cleanup; } - if (virRun(cmd, NULL) < 0) + virCommandAddArgList(cmd, "-T", vmwareDriverTypeToString(driver->type), + "pause", ((vmwareDomainPtr) vm->privateData)->vmxPath, + NULL); + + if (virCommandRun(cmd, NULL) < 0) goto cleanup; virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, VIR_DOMAIN_PAUSED_USER); @@ -584,12 +576,9 @@ static int vmwareDomainResume(virDomainPtr dom) { struct vmware_driver *driver = dom->conn->privateData; + g_autoptr(virCommand) cmd = virCommandNew(driver->vmrun); virDomainObjPtr vm; - const char *cmd[] = { - driver->vmrun, "-T", PROGRAM_SENTINEL, "unpause", PROGRAM_SENTINEL, - NULL - }; int ret = -1; if (driver->type == VMWARE_DRIVER_PLAYER) { @@ -602,15 +591,17 @@ vmwareDomainResume(virDomainPtr dom) if (!(vm = vmwareDomObjFromDomain(driver, dom->uuid))) return -1; - vmwareSetSentinal(cmd, vmwareDriverTypeToString(driver->type)); - vmwareSetSentinal(cmd, ((vmwareDomainPtr) vm->privateData)->vmxPath); if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_PAUSED) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("domain is not in suspend state")); goto cleanup; } - if (virRun(cmd, NULL) < 0) + virCommandAddArgList(cmd, "-T", vmwareDriverTypeToString(driver->type), + "unpause", ((vmwareDomainPtr) vm->privateData)->vmxPath, + NULL); + + if (virCommandRun(cmd, NULL) < 0) goto cleanup; virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_UNPAUSED); @@ -625,12 +616,8 @@ static int vmwareDomainReboot(virDomainPtr dom, unsigned int flags) { struct vmware_driver *driver = dom->conn->privateData; - const char * vmxPath = NULL; + g_autoptr(virCommand) cmd = virCommandNew(driver->vmrun); virDomainObjPtr vm; - const char *cmd[] = { - driver->vmrun, "-T", PROGRAM_SENTINEL, - "reset", PROGRAM_SENTINEL, "soft", NULL - }; int ret = -1; virCheckFlags(0, -1); @@ -638,10 +625,6 @@ vmwareDomainReboot(virDomainPtr dom, unsigned int flags) if (!(vm = vmwareDomObjFromDomain(driver, dom->uuid))) return -1; - vmxPath = ((vmwareDomainPtr) vm->privateData)->vmxPath; - vmwareSetSentinal(cmd, vmwareDriverTypeToString(driver->type)); - vmwareSetSentinal(cmd, vmxPath); - if (vmwareUpdateVMStatus(driver, vm) < 0) goto cleanup; @@ -651,7 +634,11 @@ vmwareDomainReboot(virDomainPtr dom, unsigned int flags) goto cleanup; } - if (virRun(cmd, NULL) < 0) + virCommandAddArgList(cmd, "-T", vmwareDriverTypeToString(driver->type), + "reset", ((vmwareDomainPtr) vm->privateData)->vmxPath, + "soft", NULL); + + if (virCommandRun(cmd, NULL) < 0) goto cleanup; ret = 0; -- 2.25.1

Catch the individual usage not removed in previous commits. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- 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

On 4/22/20 6:52 PM, Ján Tomko wrote:
Catch the individual usage not removed in previous commits.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- 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/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);
s/virCommandArgList/virCommandNewArgList/ Michal

virCommand is now used everywhere. Signed-off-by: Ján Tomko <jtomko@redhat.com> Suggested-by: Sebastian Mitterle <smitterl@redhat.com> --- 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

Everything is using virCommand now. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/libvirt_private.syms | 1 - src/util/vircommand.c | 35 ----------------------------------- src/util/vircommand.h | 2 -- 3 files changed, 38 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 28610a837e..7f497898eb 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1874,7 +1874,6 @@ virCommandToString; virCommandWait; virCommandWriteArgLog; virFork; -virRun; # util/virconf.h diff --git a/src/util/vircommand.c b/src/util/vircommand.c index b84fb40948..20f196104f 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -893,43 +893,8 @@ virExec(virCommandPtr cmd) } -/** - * virRun: - * @argv NULL terminated argv to run - * @status optional variable to return exit status in - * - * Run a command without using the shell. - * - * If status is NULL, then return 0 if the command run and - * exited with 0 status; Otherwise return -1 - * - * If status is not-NULL, then return 0 if the command ran. - * The status variable is filled with the command exit status - * and should be checked by caller for success. Return -1 - * only if the command could not be run. - */ -int -virRun(const char *const*argv, int *status) -{ - g_autoptr(virCommand) cmd = virCommandNewArgs(argv); - - return virCommandRun(cmd, status); -} - #else /* WIN32 */ -int -virRun(const char *const *argv G_GNUC_UNUSED, - int *status) -{ - if (status) - *status = ENOTSUP; - else - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("virRun is not implemented for WIN32")); - return -1; -} - pid_t virFork(void) { diff --git a/src/util/vircommand.h b/src/util/vircommand.h index 9086f9a90c..e2be5bcf1c 100644 --- a/src/util/vircommand.h +++ b/src/util/vircommand.h @@ -34,8 +34,6 @@ typedef int (*virExecHook)(void *data); pid_t virFork(void) G_GNUC_WARN_UNUSED_RESULT; -int virRun(const char *const*argv, int *status) G_GNUC_WARN_UNUSED_RESULT; - virCommandPtr virCommandNew(const char *binary) ATTRIBUTE_NONNULL(1); virCommandPtr virCommandNewArgs(const char *const*args) ATTRIBUTE_NONNULL(1); -- 2.25.1

On 4/22/20 6:52 PM, Ján Tomko wrote:
Ján Tomko (5): openvz: switch from virRun to virCommand vmware: use virCommand instead of virRun Remove all usage of virRun util: remove references to virRun/virExec util: remove virRun
src/libvirt_private.syms | 1 - src/lxc/lxc_driver.c | 5 +- src/openvz/openvz_driver.c | 96 ++++++++++++++------------------ src/qemu/qemu_domain.c | 16 +++--- src/security/security_apparmor.c | 11 +--- src/util/vircommand.c | 35 ------------ src/util/vircommand.h | 2 - src/util/virnetdev.c | 24 +++----- src/util/virprocess.c | 4 +- src/vmware/vmware_conf.c | 9 ++- src/vmware/vmware_driver.c | 69 ++++++++++------------- 11 files changed, 98 insertions(+), 174 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Ján Tomko
-
Michal Privoznik