On 05/13/2011 02:10 PM, Cole Robinson wrote: ... Yep, that does the trick for auto-cleanup (I'm liking virCommand more and more as we convert things to use it). ACK. -- Eric Blake eblake(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

On 05/13/2011 02:10 PM, Cole Robinson wrote: Oops, this now frees names[] on success. You need to gate this for loop on whether rc >= 0, now that the success path falls through to the out label. ACK with that nit fixed. -- Eric Blake eblake(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

On 05/17/2011 08:24 AM, Cole Robinson wrote: Phooey, we got it backwards. On success, we want names[0] to be valid. On failure, we want names[0] to be NULL. So the condition needs to be if (rc < 0) { free names }. I'm pushing this followup under the trivial rule: diff --git i/src/openvz/openvz_driver.c w/src/openvz/openvz_driver.c index cefba16..ae951a2 100644 --- i/src/openvz/openvz_driver.c +++ w/src/openvz/openvz_driver.c @@ -1492,7 +1492,7 @@ static int openvzListDefinedDomains(virConnectPtr conn ATTRIBUTE_UNUSED, out: VIR_FORCE_CLOSE(outfd); virCommandFree(cmd); - if (rc >= 0) { + if (rc < 0) { for ( ; got >= 0 ; got--) VIR_FREE(names[got]); } -- Eric Blake eblake(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

v2: Have virCommand cleanup intermediate process for us Signed-off-by: Cole Robinson <crobinso(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 35 +++++++++++------------------------ src/qemu/qemu_process.c | 2 +- 2 files changed, 12 insertions(+), 25 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index fff41e0..a826b6d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3238,11 +3238,9 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, int ret = -1; virDomainEventPtr event; int intermediatefd = -1; - pid_t intermediate_pid = -1; - int childstat; + virCommandPtr cmd = NULL; if (header->version == 2) { - const char *intermediate_argv[3] = { NULL, "-dc", NULL }; const char *prog = qemudSaveCompressionTypeToString(header->compressed); if (prog == NULL) { qemuReportError(VIR_ERR_OPERATION_FAILED, @@ -3252,14 +3250,17 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, } if (header->compressed != QEMUD_SAVE_FORMAT_RAW) { - intermediate_argv[0] = prog; + cmd = virCommandNewArgList(prog, "-dc", NULL); intermediatefd = *fd; *fd = -1; - if (virExec(intermediate_argv, NULL, NULL, - &intermediate_pid, intermediatefd, fd, NULL, 0) < 0) { + + virCommandSetInputFD(cmd, intermediatefd); + virCommandSetOutputFD(cmd, fd); + + if (virCommandRunAsync(cmd, NULL) < 0) { qemuReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to start decompression binary %s"), - intermediate_argv[0]); + prog); *fd = intermediatefd; goto out; } @@ -3270,23 +3271,8 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, ret = qemuProcessStart(conn, driver, vm, "stdio", true, *fd, path, VIR_VM_OP_RESTORE); - if (intermediate_pid != -1) { - if (ret < 0) { - /* if there was an error setting up qemu, the intermediate - * process will wait forever to write to stdout, so we - * must manually kill it. - */ - VIR_FORCE_CLOSE(intermediatefd); - VIR_FORCE_CLOSE(*fd); - kill(intermediate_pid, SIGTERM); - } - - /* Wait for intermediate process to exit */ - while (waitpid(intermediate_pid, &childstat, 0) == -1 && - errno == EINTR) { - /* empty */ - } - } + if (cmd && virCommandWait(cmd, NULL) < 0) + goto out; VIR_FORCE_CLOSE(intermediatefd); if (VIR_CLOSE(*fd) < 0) { @@ -3324,6 +3310,7 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, ret = 0; out: + virCommandFree(cmd); if (virSecurityManagerRestoreSavedStateLabel(driver->securityManager, vm, path) < 0) VIR_WARN("failed to restore save state label on %s", path); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f745aea..96c01f7 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2275,7 +2275,7 @@ int qemuProcessStart(virConnectPtr conn, * because the child no longer exists. */ - /* The virExec process that launches the daemon failed. Pending on + /* The virCommand process that launches the daemon failed. Pending on * when it failed (we can't determine for sure), there may be * extra info in the domain log (if the hook failed for example). * -- 1.7.4.4

v2: Simplify command creation Add a missing virCommandFree Use virCommand auto-cleanup for async process Signed-off-by: Cole Robinson <crobinso(a)redhat.com&gt; --- src/storage/storage_backend.c | 91 +++++++++------------------------------- 1 files changed, 21 insertions(+), 70 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index e05679d..a78793a 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1404,8 +1404,7 @@ virStorageBackendRunProgRegex(virStoragePoolObjPtr pool, void *data, int *outexit) { - int fd = -1, exitstatus, err, failed = 1; - pid_t child = 0; + int fd = -1, err, ret = -1; FILE *list = NULL; regex_t *reg; regmatch_t *vars = NULL; @@ -1413,6 +1412,7 @@ 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) { @@ -1449,10 +1449,9 @@ virStorageBackendRunProgRegex(virStoragePoolObjPtr pool, goto cleanup; } - - /* Run the program and capture its output */ - if (virExec(prog, NULL, NULL, - &child, -1, &fd, NULL, VIR_EXEC_NONE) < 0) { + cmd = virCommandNewArgs(prog); + virCommandSetOutputFD(cmd, &fd); + if (virCommandRunAsync(cmd, NULL) < 0) { goto cleanup; } @@ -1501,9 +1500,8 @@ virStorageBackendRunProgRegex(virStoragePoolObjPtr pool, } } - failed = 0; - - cleanup: + ret = virCommandWait(cmd, outexit); +cleanup: if (groups) { for (j = 0 ; j < totgroups ; j++) VIR_FREE(groups[j]); @@ -1515,33 +1513,12 @@ virStorageBackendRunProgRegex(virStoragePoolObjPtr pool, regfree(&reg[i]); VIR_FREE(reg); + virCommandFree(cmd); VIR_FORCE_FCLOSE(list); VIR_FORCE_CLOSE(fd); - while ((err = waitpid(child, &exitstatus, 0) == -1) && errno == EINTR); - - /* Don't bother checking exit status if we already failed */ - if (failed) - return -1; - - if (err == -1) { - virReportSystemError(errno, - _("failed to wait for command '%s'"), - prog[0]); - return -1; - } else { - if (WIFEXITED(exitstatus)) { - if (outexit != NULL) - *outexit = WEXITSTATUS(exitstatus); - } else { - virStorageReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("command did not exit cleanly")); - return -1; - } - } - - return 0; + return ret; } /* @@ -1563,13 +1540,12 @@ virStorageBackendRunProgNul(virStoragePoolObjPtr pool, void *data) { size_t n_tok = 0; - int fd = -1, exitstatus; - pid_t child = 0; + int fd = -1; FILE *fp = NULL; char **v; - int err = -1; - int w_err; + int ret = -1; int i; + virCommandPtr cmd = NULL; if (n_columns == 0) return -1; @@ -1581,9 +1557,9 @@ virStorageBackendRunProgNul(virStoragePoolObjPtr pool, for (i = 0; i < n_columns; i++) v[i] = NULL; - /* Run the program and capture its output */ - if (virExec(prog, NULL, NULL, - &child, -1, &fd, NULL, VIR_EXEC_NONE) < 0) { + cmd = virCommandNewArgs(prog); + virCommandSetOutputFD(cmd, &fd); + if (virCommandRunAsync(cmd, NULL) < 0) { goto cleanup; } @@ -1618,48 +1594,23 @@ virStorageBackendRunProgNul(virStoragePoolObjPtr pool, } } - if (feof (fp)) - err = 0; - else + if (feof (fp) < 0) { virReportSystemError(errno, _("read error on pipe to '%s'"), prog[0]); + goto cleanup; + } + ret = virCommandWait(cmd, NULL); cleanup: for (i = 0; i < n_columns; i++) VIR_FREE(v[i]); VIR_FREE(v); + virCommandFree(cmd); VIR_FORCE_FCLOSE(fp); VIR_FORCE_CLOSE(fd); - while ((w_err = waitpid (child, &exitstatus, 0) == -1) && errno == EINTR) - /* empty */ ; - - /* Don't bother checking exit status if we already failed */ - if (err < 0) - return -1; - - if (w_err == -1) { - virReportSystemError(errno, - _("failed to wait for command '%s'"), - prog[0]); - return -1; - } else { - if (WIFEXITED(exitstatus)) { - if (WEXITSTATUS(exitstatus) != 0) { - virStorageReportError(VIR_ERR_INTERNAL_ERROR, - _("non-zero exit status from command %d"), - WEXITSTATUS(exitstatus)); - return -1; - } - } else { - virStorageReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("command did not exit cleanly")); - return -1; - } - } - - return 0; + return ret; } #else /* WIN32 */ -- 1.7.4.4

Some clients overwrite the error from the regex helper, or do half-baked error reporting with the exitstatus. Signed-off-by: Cole Robinson <crobinso(a)redhat.com&gt; --- src/storage/storage_backend.c | 11 +++++------ src/storage/storage_backend.h | 3 +-- src/storage/storage_backend_fs.c | 5 ++--- src/storage/storage_backend_iscsi.c | 16 ++-------------- src/storage/storage_backend_logical.c | 30 +++++------------------------- 5 files changed, 15 insertions(+), 50 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index a78793a..61d14b1 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -1401,8 +1401,7 @@ virStorageBackendRunProgRegex(virStoragePoolObjPtr pool, const char **regex, int *nvars, virStorageBackendListVolRegexFunc func, - void *data, - int *outexit) + void *data) { int fd = -1, err, ret = -1; FILE *list = NULL; @@ -1500,7 +1499,7 @@ virStorageBackendRunProgRegex(virStoragePoolObjPtr pool, } } - ret = virCommandWait(cmd, outexit); + ret = virCommandWait(cmd, NULL); cleanup: if (groups) { for (j = 0 ; j < totgroups ; j++) @@ -1623,10 +1622,10 @@ virStorageBackendRunProgRegex(virConnectPtr conn, const char **regex ATTRIBUTE_UNUSED, int *nvars ATTRIBUTE_UNUSED, virStorageBackendListVolRegexFunc func ATTRIBUTE_UNUSED, - void *data ATTRIBUTE_UNUSED, - int *outexit ATTRIBUTE_UNUSED) + void *data ATTRIBUTE_UNUSED) { - virStorageReportError(VIR_ERR_INTERNAL_ERROR, _("%s not implemented on Win32"), __FUNCTION__); + virStorageReportError(VIR_ERR_INTERNAL_ERROR, + _("%s not implemented on Win32"), __FUNCTION__); return -1; } diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index 65cbd7e..fcfbed0 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -135,8 +135,7 @@ int virStorageBackendRunProgRegex(virStoragePoolObjPtr pool, const char **regex, int *nvars, virStorageBackendListVolRegexFunc func, - void *data, - int *exitstatus); + void *data); int virStorageBackendRunProgNul(virStoragePoolObjPtr pool, const char **prog, diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 0a6b074..b8d4d63 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -233,7 +233,6 @@ virStorageBackendFileSystemNetFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSE }; const char *prog[] = { SHOWMOUNT, "--no-headers", "--exports", NULL, NULL }; virStoragePoolSourcePtr source = NULL; - int exitstatus; char *retval = NULL; unsigned int i; @@ -246,8 +245,8 @@ virStorageBackendFileSystemNetFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSE prog[3] = source->host.name; if (virStorageBackendRunProgRegex(NULL, prog, 1, regexes, vars, - virStorageBackendFileSystemNetFindPoolSourcesFunc, - &state, &exitstatus) < 0) + virStorageBackendFileSystemNetFindPoolSourcesFunc, + &state) < 0) goto cleanup; retval = virStoragePoolSourceListFormat(&state.list); diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c index bdd58cd..15b5862 100644 --- a/src/storage/storage_backend_iscsi.c +++ b/src/storage/storage_backend_iscsi.c @@ -160,8 +160,7 @@ virStorageBackendISCSISession(virStoragePoolObjPtr pool, regexes, vars, virStorageBackendISCSIExtractSession, - &session, - NULL) < 0) + &session) < 0) return NULL; if (session == NULL && @@ -503,7 +502,6 @@ virStorageBackendISCSIScanTargets(const char *portal, }; struct virStorageBackendISCSITargetList list; int i; - int exitstatus; memset(&list, 0, sizeof(list)); @@ -513,17 +511,7 @@ virStorageBackendISCSIScanTargets(const char *portal, regexes, vars, virStorageBackendISCSIGetTargets, - &list, - &exitstatus) < 0) { - virStorageReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("iscsiadm command failed")); - return -1; - } - - if (exitstatus != 0) { - virStorageReportError(VIR_ERR_INTERNAL_ERROR, - _("iscsiadm sendtargets command failed with exitstatus %d"), - exitstatus); + &list) < 0) { return -1; } diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index a1ff35b..7d5adf9 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -205,25 +205,13 @@ virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool, pool->def->source.name, NULL }; - int exitstatus; - if (virStorageBackendRunProgRegex(pool, prog, 1, regexes, vars, virStorageBackendLogicalMakeVol, - vol, - &exitstatus) < 0) { - virStorageReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("lvs command failed")); - return -1; - } - - if (exitstatus != 0) { - virStorageReportError(VIR_ERR_INTERNAL_ERROR, - _("lvs command failed with exitstatus %d"), - exitstatus); + vol) < 0) { return -1; } @@ -321,7 +309,6 @@ virStorageBackendLogicalFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSED, }; const char *const prog[] = { PVS, "--noheadings", "-o", "pv_name,vg_name", NULL }; const char *const scanprog[] = { VGSCAN, NULL }; - int exitstatus; char *retval = NULL; virStoragePoolSourceList sourceList; int i; @@ -331,7 +318,7 @@ 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, &exitstatus) < 0) { + if (virRun(scanprog, NULL) < 0) { VIR_WARN("Failure when running vgscan to refresh physical volumes"); } @@ -339,8 +326,8 @@ virStorageBackendLogicalFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSED, sourceList.type = VIR_STORAGE_POOL_LOGICAL; if (virStorageBackendRunProgRegex(NULL, prog, 1, regexes, vars, - virStorageBackendLogicalFindPoolSourcesFunc, - &sourceList, &exitstatus) < 0) + virStorageBackendLogicalFindPoolSourcesFunc, + &sourceList) < 0) return NULL; retval = virStoragePoolSourceListFormat(&sourceList); @@ -489,7 +476,6 @@ virStorageBackendLogicalRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, "--nosuffix", "--options", "vg_size,vg_free", pool->def->source.name, NULL }; - int exitstatus; virFileWaitForDevices(); @@ -506,13 +492,7 @@ virStorageBackendLogicalRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, regexes, vars, virStorageBackendLogicalRefreshPoolFunc, - NULL, - &exitstatus) < 0) { - virStoragePoolObjClearVols(pool); - return -1; - } - - if (exitstatus != 0) { + NULL) < 0) { virStoragePoolObjClearVols(pool); return -1; } -- 1.7.4.4

virRunWithHook is now unused, so we can drop it. Tested w/ raw + qcow2 volume creation and copying. v2: Use opaque data to skip hook second time around Simply command building Signed-off-by: Cole Robinson <crobinso(a)redhat.com&gt; --- src/libvirt_private.syms | 1 - src/storage/storage_backend.c | 162 ++++++++++++++++------------------------ src/util/util.c | 23 ++---- src/util/util.h | 3 - 4 files changed, 71 insertions(+), 118 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 21a65ad..8cf9c1a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -957,7 +957,6 @@ virPipeReadUntilEOF; virRandom; virRandomInitialize; virRun; -virRunWithHook; virSetBlocking; virSetCloseExec; virSetInherit; diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 61d14b1..76dca7b 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -523,8 +523,17 @@ cleanup: return ret; } +struct hookdata { + virStorageVolDefPtr vol; + bool skip; +}; + static int virStorageBuildSetUIDHook(void *data) { - virStorageVolDefPtr vol = data; + struct hookdata *tmp = data; + virStorageVolDefPtr vol = tmp->vol; + + if (tmp->skip) + return 0; if ((vol->target.perms.gid != -1) && (setgid(vol->target.perms.gid) != 0)) { @@ -545,11 +554,12 @@ static int virStorageBuildSetUIDHook(void *data) { static int virStorageBackendCreateExecCommand(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, - const char **cmdargv) { + virCommandPtr cmd) { struct stat st; gid_t gid; uid_t uid; int filecreated = 0; + struct hookdata data = {vol, false}; if ((pool->def->type == VIR_STORAGE_POOL_NETFS) && (((getuid() == 0) @@ -557,21 +567,25 @@ static int virStorageBackendCreateExecCommand(virStoragePoolObjPtr pool, && (vol->target.perms.uid != 0)) || ((vol->target.perms.gid != -1) && (vol->target.perms.gid != getgid())))) { - if (virRunWithHook(cmdargv, - virStorageBuildSetUIDHook, vol, NULL) == 0) { + + virCommandSetPreExecHook(cmd, virStorageBuildSetUIDHook, &data); + + if (virCommandRun(cmd, NULL) == 0) { /* command was successfully run, check if the file was created */ if (stat(vol->target.path, &st) >=0) filecreated = 1; } } + + data.skip = true; + if (!filecreated) { - if (virRun(cmdargv, NULL) < 0) { + if (virCommandRun(cmd, NULL) < 0) { return -1; } if (stat(vol->target.path, &st) < 0) { virReportSystemError(errno, - _("%s failed to create %s"), - cmdargv[0], vol->target.path); + _("failed to create %s"), vol->target.path); return -1; } } @@ -644,6 +658,8 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, char *size = NULL; char *create_tool; int imgformat = -1; + virCommandPtr cmd = NULL; + bool do_encryption = (vol->target.encryption != NULL); const char *type = virStorageFileFormatTypeToString(vol->target.format); const char *backingType = vol->backingStore.path ? @@ -716,7 +732,7 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, } } - if (vol->target.encryption != NULL) { + if (do_encryption) { virStorageEncryptionPtr enc; if (vol->target.format != VIR_STORAGE_FILE_QCOW && @@ -767,114 +783,66 @@ virStorageBackendCreateQemuImg(virConnectPtr conn, if (imgformat < 0) goto cleanup; + cmd = virCommandNew(create_tool); + if (inputvol) { - const char *imgargv[] = { - create_tool, - "convert", - "-f", inputType, - "-O", type, - inputPath, - vol->target.path, - NULL, - NULL, - NULL - }; - - if (vol->target.encryption != NULL) { + virCommandAddArgList(cmd, "convert", "-f", inputType, "-O", type, + inputPath, vol->target.path, NULL); + + if (do_encryption) { if (imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS) { - imgargv[8] = "-o"; - imgargv[9] = "encryption=on"; + virCommandAddArgList(cmd, "-o", "encryption=on", NULL); } else { - imgargv[8] = "-e"; + virCommandAddArg(cmd, "-e"); } } - ret = virStorageBackendCreateExecCommand(pool, vol, imgargv); } else if (vol->backingStore.path) { - const char *imgargv[] = { - create_tool, - "create", - "-f", type, - "-b", vol->backingStore.path, - NULL, - NULL, - NULL, - NULL, - NULL, - NULL - }; - - char *optflag = NULL; + virCommandAddArgList(cmd, "create", "-f", type, + "-b", vol->backingStore.path, NULL); + switch (imgformat) { case QEMU_IMG_BACKING_FORMAT_FLAG: - imgargv[6] = "-F"; - imgargv[7] = backingType; - imgargv[8] = vol->target.path; - imgargv[9] = size; - if (vol->target.encryption != NULL) - imgargv[10] = "-e"; + virCommandAddArgList(cmd, "-F", backingType, vol->target.path, + size, NULL); + + if (do_encryption) + virCommandAddArg(cmd, "-e"); break; case QEMU_IMG_BACKING_FORMAT_OPTIONS: - if (virAsprintf(&optflag, "backing_fmt=%s", backingType) < 0) { - virReportOOMError(); - goto cleanup; - } - - if (vol->target.encryption != NULL) { - char *tmp = NULL; - if (virAsprintf(&tmp, "%s,%s", optflag, "encryption=on") < 0) { - virReportOOMError(); - goto cleanup; - } - VIR_FREE(optflag); - optflag = tmp; - } - - imgargv[6] = "-o"; - imgargv[7] = optflag; - imgargv[8] = vol->target.path; - imgargv[9] = size; + virCommandAddArg(cmd, "-o"); + virCommandAddArgFormat(cmd, "backing_fmt=%s%s", backingType, + do_encryption ? ",encryption=on" : ""); + virCommandAddArgList(cmd, vol->target.path, size, NULL); + break; default: VIR_INFO("Unable to set backing store format for %s with %s", vol->target.path, create_tool); - imgargv[6] = vol->target.path; - imgargv[7] = size; - if (vol->target.encryption != NULL) - imgargv[8] = "-e"; - } - ret = virStorageBackendCreateExecCommand(pool, vol, imgargv); - VIR_FREE(optflag); + virCommandAddArgList(cmd, vol->target.path, size, NULL); + if (do_encryption) + virCommandAddArg(cmd, "-e"); + } } else { - /* The extra NULL field is for indicating encryption (-e). */ - const char *imgargv[] = { - create_tool, - "create", - "-f", type, - vol->target.path, - size, - NULL, - NULL, - NULL - }; - - if (vol->target.encryption != NULL) { + virCommandAddArgList(cmd, "create", "-f", type, + vol->target.path, size, NULL); + + if (do_encryption) { if (imgformat == QEMU_IMG_BACKING_FORMAT_OPTIONS) { - imgargv[6] = "-o"; - imgargv[7] = "encryption=on"; + virCommandAddArgList(cmd, "-o", "encryption=on", NULL); } else { - imgargv[6] = "-e"; + virCommandAddArg(cmd, "-e"); } } - - ret = virStorageBackendCreateExecCommand(pool, vol, imgargv); } - cleanup: + ret = virStorageBackendCreateExecCommand(pool, vol, cmd); +cleanup: VIR_FREE(size); VIR_FREE(create_tool); + virCommandFree(cmd); return ret; } @@ -892,7 +860,8 @@ virStorageBackendCreateQcowCreate(virConnectPtr conn ATTRIBUTE_UNUSED, { int ret; char *size; - const char *imgargv[4]; + char *create_tool; + virCommandPtr cmd; if (inputvol) { virStorageReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -926,13 +895,12 @@ virStorageBackendCreateQcowCreate(virConnectPtr conn ATTRIBUTE_UNUSED, return -1; } - imgargv[0] = virFindFileInPath("qcow-create"); - imgargv[1] = size; - imgargv[2] = vol->target.path; - imgargv[3] = NULL; + create_tool = virFindFileInPath("qcow-create"); + cmd = virCommandNewArgList(create_tool, size, vol->target.path, NULL); - ret = virStorageBackendCreateExecCommand(pool, vol, imgargv); - VIR_FREE(imgargv[0]); + ret = virStorageBackendCreateExecCommand(pool, vol, cmd); + virCommandFree(cmd); + VIR_FREE(create_tool); VIR_FREE(size); return ret; diff --git a/src/util/util.c b/src/util/util.c index f169e54..89fc82b 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -787,10 +787,8 @@ virExec(const char *const*argv, * only if the command could not be run. */ int -virRunWithHook(const char *const*argv, - virExecHook hook, - void *data, - int *status) { +virRun(const char *const*argv, int *status) +{ pid_t childpid; int exitstatus, execret, waitret; int ret = -1; @@ -807,7 +805,7 @@ virRunWithHook(const char *const*argv, if ((execret = virExecWithHook(argv, NULL, NULL, &childpid, -1, &outfd, &errfd, - VIR_EXEC_NONE, hook, data, NULL)) < 0) { + VIR_EXEC_NONE, NULL, NULL, NULL)) < 0) { ret = execret; goto error; } @@ -871,17 +869,14 @@ int virSetInherit(int fd ATTRIBUTE_UNUSED, bool inherit ATTRIBUTE_UNUSED) return -1; } -int -virRunWithHook(const char *const *argv ATTRIBUTE_UNUSED, - virExecHook hook ATTRIBUTE_UNUSED, - void *data ATTRIBUTE_UNUSED, - int *status) +virRun(const char *const *argv ATTRIBUTE_UNUSED, + int *status) { if (status) *status = ENOTSUP; else virUtilError(VIR_ERR_INTERNAL_ERROR, - "%s", _("virRunWithHook is not implemented for WIN32")); + "%s", _("virRun is not implemented for WIN32")); return -1; } @@ -1018,12 +1013,6 @@ error: return -1; } -int -virRun(const char *const*argv, - int *status) { - return virRunWithHook(argv, NULL, NULL, status); -} - /* Like gnulib's fread_file, but read no more than the specified maximum number of bytes. If the length of the input is <= max_len, and upon error while reading that data, it works just like fread_file. */ diff --git a/src/util/util.h b/src/util/util.h index 482294f..e2b8eb3 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -78,9 +78,6 @@ int virExec(const char *const*argv, int *errfd, int flags) ATTRIBUTE_RETURN_CHECK; int virRun(const char *const*argv, int *status) ATTRIBUTE_RETURN_CHECK; -int virRunWithHook(const char *const*argv, - virExecHook hook, void *data, - int *status) ATTRIBUTE_RETURN_CHECK; int virPipeReadUntilEOF(int outfd, int errfd, char **outbuf, char **errbuf); int virFork(pid_t *pid); -- 1.7.4.4

On 05/13/2011 06:13 PM, Eric Blake wrote: Thanks for the review, pushed with that fix. Also pushed 1, 4, and 5. - Cole

Signed-off-by: Cole Robinson <crobinso(a)redhat.com&gt; --- src/libvirt_private.syms | 1 - src/util/util.c | 35 ----------------------------------- src/util/util.h | 8 -------- 3 files changed, 0 insertions(+), 44 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 8cf9c1a..6066e16 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -914,7 +914,6 @@ virEnumFromString; virEnumToString; virEventAddHandle; virEventRemoveHandle; -virExec; virExecWithHook; virFileAbsPath; virFileDeletePid; diff --git a/src/util/util.c b/src/util/util.c index 89fc82b..7679834 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -752,26 +752,6 @@ virExecWithHook(const char *const*argv, return -1; } -/* - * See virExecWithHook for explanation of the arguments. - * - * Wrapper function for virExecWithHook, with a simpler set of parameters. - * Used to insulate the numerous callers from changes to virExecWithHook - * argument list. - */ -int -virExec(const char *const*argv, - const char *const*envp, - const fd_set *keepfd, - pid_t *retpid, - int infd, int *outfd, int *errfd, - int flags) -{ - return virExecWithHook(argv, envp, keepfd, retpid, - infd, outfd, errfd, - flags, NULL, NULL, NULL); -} - /** * @argv NULL terminated argv to run * @status optional variable to return exit status in @@ -881,21 +861,6 @@ virRun(const char *const *argv ATTRIBUTE_UNUSED, } int -virExec(const char *const*argv ATTRIBUTE_UNUSED, - const char *const*envp ATTRIBUTE_UNUSED, - const fd_set *keepfd ATTRIBUTE_UNUSED, - int *retpid ATTRIBUTE_UNUSED, - int infd ATTRIBUTE_UNUSED, - int *outfd ATTRIBUTE_UNUSED, - int *errfd ATTRIBUTE_UNUSED, - int flags ATTRIBUTE_UNUSED) -{ - virUtilError(VIR_ERR_INTERNAL_ERROR, - "%s", _("virExec is not implemented for WIN32")); - return -1; -} - -int virExecWithHook(const char *const*argv ATTRIBUTE_UNUSED, const char *const*envp ATTRIBUTE_UNUSED, const fd_set *keepfd ATTRIBUTE_UNUSED, diff --git a/src/util/util.h b/src/util/util.h index e2b8eb3..3e95cae 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -69,14 +69,6 @@ int virExecWithHook(const char *const*argv, virExecHook hook, void *data, char *pidfile) ATTRIBUTE_RETURN_CHECK; -int virExec(const char *const*argv, - const char *const*envp, - const fd_set *keepfd, - pid_t *retpid, - int infd, - int *outfd, - int *errfd, - int flags) ATTRIBUTE_RETURN_CHECK; int virRun(const char *const*argv, int *status) ATTRIBUTE_RETURN_CHECK; int virPipeReadUntilEOF(int outfd, int errfd, char **outbuf, char **errbuf); -- 1.7.4.4

v2: Simplify command building Handle command building failure Signed-off-by: Cole Robinson <crobinso(a)redhat.com&gt; --- src/util/util.c | 74 ++++-------------------------------------------------- 1 files changed, 6 insertions(+), 68 deletions(-) diff --git a/src/util/util.c b/src/util/util.c index 7679834..2fe3808 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -769,76 +769,14 @@ virExecWithHook(const char *const*argv, int virRun(const char *const*argv, int *status) { - pid_t childpid; - int exitstatus, execret, waitret; - int ret = -1; - int errfd = -1, outfd = -1; - char *outbuf = NULL; - char *errbuf = NULL; - char *argv_str = NULL; - - if ((argv_str = virArgvToString(argv)) == NULL) { - virReportOOMError(); - goto error; - } - VIR_DEBUG("%s", argv_str); - - if ((execret = virExecWithHook(argv, NULL, NULL, - &childpid, -1, &outfd, &errfd, - VIR_EXEC_NONE, NULL, NULL, NULL)) < 0) { - ret = execret; - goto error; - } - - if (virPipeReadUntilEOF(outfd, errfd, &outbuf, &errbuf) < 0) { - while (waitpid(childpid, &exitstatus, 0) == -1 && errno == EINTR) - ; - goto error; - } - - if (outbuf) - VIR_DEBUG("Command stdout: %s", outbuf); - if (errbuf) - VIR_DEBUG("Command stderr: %s", errbuf); - - while ((waitret = waitpid(childpid, &exitstatus, 0) == -1) && - errno == EINTR); - if (waitret == -1) { - virReportSystemError(errno, - _("cannot wait for '%s'"), - argv[0]); - goto error; - } - - if (status == NULL) { - errno = EINVAL; - if (exitstatus) { - char *str = virCommandTranslateStatus(exitstatus); - char *argvstr = virArgvToString(argv); - if (!argv_str) { - virReportOOMError(); - goto error; - } - - virUtilError(VIR_ERR_INTERNAL_ERROR, - _("'%s' exited unexpectedly: %s"), - argv_str, NULLSTR(str)); - - VIR_FREE(str); - VIR_FREE(argvstr); - goto error; - } - } else { - *status = exitstatus; - } + int ret; + virCommandPtr cmd = virCommandNewArgs(argv); - ret = 0; + if (!cmd) + return -1; - error: - VIR_FREE(outbuf); - VIR_FREE(errbuf); - VIR_FORCE_CLOSE(outfd); - VIR_FORCE_CLOSE(errfd); + ret = virCommandRun(cmd, status); + virCommandFree(cmd); return ret; } -- 1.7.4.4

Seems reasonable to have all command wrappers in the same place v2: Dont move SetInherit Signed-off-by: Cole Robinson <crobinso(a)redhat.com&gt; --- cfg.mk | 2 +- src/libvirt_private.syms | 5 +- src/lxc/veth.c | 2 +- src/nwfilter/nwfilter_ebiptables_driver.c | 1 + src/storage/storage_backend_fs.c | 2 +- src/storage/storage_backend_logical.c | 2 +- src/util/command.c | 568 +++++++++++++++++++++++++++++ src/util/command.h | 14 + src/util/ebtables.c | 2 +- src/util/pci.c | 2 +- src/util/util.c | 568 +---------------------------- src/util/util.h | 24 -- src/vmware/vmware_driver.c | 1 + 13 files changed, 602 insertions(+), 591 deletions(-) diff --git a/cfg.mk b/cfg.mk index 06b638b..db11d6e 100644 --- a/cfg.mk +++ b/cfg.mk @@ -621,7 +621,7 @@ exclude_file_name_regexp--sc_prohibit_doubled_word = ^po/ exclude_file_name_regexp--sc_prohibit_empty_lines_at_EOF = \ (^docs/api_extension/|^tests/qemuhelpdata/|\.(gif|ico|png)$$) -_src2=src/(util/util|libvirt|lxc/lxc_controller) +_src2=src/(util/command|libvirt|lxc/lxc_controller) exclude_file_name_regexp--sc_prohibit_fork_wrappers = \ (^docs|^($(_src2)|tests/testutils|daemon/libvirtd)\.c$$) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 6066e16..a3c8877 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -130,6 +130,8 @@ virCommandTransferFD; virCommandTranslateStatus; virCommandWait; virCommandWriteArgLog; +virFork; +virRun; # conf.h @@ -914,7 +916,6 @@ virEnumFromString; virEnumToString; virEventAddHandle; virEventRemoveHandle; -virExecWithHook; virFileAbsPath; virFileDeletePid; virFileExists; @@ -936,7 +937,6 @@ virFileStripSuffix; virFileWaitForDevices; virFileWriteStr; virFindFileInPath; -virFork; virFormatMacAddr; virGenerateMacAddr; virGetGroupID; @@ -955,7 +955,6 @@ virParseVersionString; virPipeReadUntilEOF; virRandom; virRandomInitialize; -virRun; virSetBlocking; virSetCloseExec; virSetInherit; diff --git a/src/lxc/veth.c b/src/lxc/veth.c index a00aa23..1a96f82 100644 --- a/src/lxc/veth.c +++ b/src/lxc/veth.c @@ -21,7 +21,7 @@ #include "internal.h" #include "logging.h" #include "memory.h" -#include "util.h" +#include "command.h" #include "virterror_internal.h" #define VIR_FROM_THIS VIR_FROM_LXC diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index 2ff392d..b5358ce 100644 --- a/src/nwfilter/nwfilter_ebiptables_driver.c +++ b/src/nwfilter/nwfilter_ebiptables_driver.c @@ -39,6 +39,7 @@ #include "nwfilter_gentech_driver.h" #include "nwfilter_ebiptables_driver.h" #include "files.h" +#include "command.h" #define VIR_FROM_THIS VIR_FROM_NWFILTER diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index b8d4d63..88f5b87 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -41,7 +41,7 @@ #include "storage_backend_fs.h" #include "storage_conf.h" #include "storage_file.h" -#include "util.h" +#include "command.h" #include "memory.h" #include "xml.h" #include "files.h" diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 7d5adf9..4de5442 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -34,7 +34,7 @@ #include "virterror_internal.h" #include "storage_backend_logical.h" #include "storage_conf.h" -#include "util.h" +#include "command.h" #include "memory.h" #include "logging.h" #include "files.h" diff --git a/src/util/command.c b/src/util/command.c index b488d55..3c1b3f2 100644 --- a/src/util/command.c +++ b/src/util/command.c @@ -26,6 +26,12 @@ #include <stdlib.h> #include <sys/stat.h> #include <sys/wait.h> +#include <sys/types.h> +#include <fcntl.h> + +#if HAVE_CAPNG +# include <cap-ng.h> +#endif #include "command.h" #include "memory.h" @@ -34,6 +40,7 @@ #include "logging.h" #include "files.h" #include "buf.h" +#include "ignore-value.h" #define VIR_FROM_THIS VIR_FROM_NONE @@ -41,6 +48,14 @@ virReportErrorHelper(VIR_FROM_NONE, code, __FILE__, \ __FUNCTION__, __LINE__, __VA_ARGS__) +/* Flags for virExecWithHook */ +enum { + VIR_EXEC_NONE = 0, + VIR_EXEC_NONBLOCK = (1 << 0), + VIR_EXEC_DAEMON = (1 << 1), + VIR_EXEC_CLEAR_CAPS = (1 << 2), +}; + enum { /* Internal-use extension beyond public VIR_EXEC_ flags */ VIR_EXEC_RUN_SYNC = 0x40000000, @@ -84,6 +99,559 @@ struct _virCommand { bool reap; }; +#ifndef WIN32 + +# if HAVE_CAPNG +static int virClearCapabilities(void) +{ + int ret; + + capng_clear(CAPNG_SELECT_BOTH); + + if ((ret = capng_apply(CAPNG_SELECT_BOTH)) < 0) { + virCommandError(VIR_ERR_INTERNAL_ERROR, + _("cannot clear process capabilities %d"), ret); + return -1; + } + + return 0; +} +# else +static int virClearCapabilities(void) +{ +// VIR_WARN0("libcap-ng support not compiled in, unable to clear capabilities"); + return 0; +} +# endif + + +/* virFork() - fork a new process while avoiding various race/deadlock + conditions + + @pid - a pointer to a pid_t that will receive the return value from + fork() + + on return from virFork(), if *pid < 0, the fork failed and there is + no new process. Otherwise, just like fork(), if *pid == 0, it is the + child process returning, and if *pid > 0, it is the parent. + + Even if *pid >= 0, if the return value from virFork() is < 0, it + indicates a failure that occurred in the parent or child process + after the fork. In this case, the child process should call + _exit(EXIT_FAILURE) after doing any additional error reporting. + + */ +int virFork(pid_t *pid) { +# ifdef HAVE_PTHREAD_SIGMASK + sigset_t oldmask, newmask; +# endif + struct sigaction sig_action; + int saved_errno, ret = -1; + + *pid = -1; + + /* + * Need to block signals now, so that child process can safely + * kill off caller's signal handlers without a race. + */ +# ifdef HAVE_PTHREAD_SIGMASK + sigfillset(&newmask); + if (pthread_sigmask(SIG_SETMASK, &newmask, &oldmask) != 0) { + saved_errno = errno; + virReportSystemError(errno, + "%s", _("cannot block signals")); + goto cleanup; + } +# endif + + /* Ensure we hold the logging lock, to protect child processes + * from deadlocking on another thread's inherited mutex state */ + virLogLock(); + + *pid = fork(); + saved_errno = errno; /* save for caller */ + + /* Unlock for both parent and child process */ + virLogUnlock(); + + if (*pid < 0) { +# ifdef HAVE_PTHREAD_SIGMASK + /* attempt to restore signal mask, but ignore failure, to + avoid obscuring the fork failure */ + ignore_value (pthread_sigmask(SIG_SETMASK, &oldmask, NULL)); +# endif + virReportSystemError(saved_errno, + "%s", _("cannot fork child process")); + goto cleanup; + } + + if (*pid) { + + /* parent process */ + +# ifdef HAVE_PTHREAD_SIGMASK + /* Restore our original signal mask now that the child is + safely running */ + if (pthread_sigmask(SIG_SETMASK, &oldmask, NULL) != 0) { + saved_errno = errno; /* save for caller */ + virReportSystemError(errno, "%s", _("cannot unblock signals")); + goto cleanup; + } +# endif + ret = 0; + + } else { + + /* child process */ + + int logprio; + int i; + + /* Remove any error callback so errors in child now + get sent to stderr where they stand a fighting chance + of being seen / logged */ + virSetErrorFunc(NULL, NULL); + virSetErrorLogPriorityFunc(NULL); + + /* Make sure any hook logging is sent to stderr, since child + * process may close the logfile FDs */ + logprio = virLogGetDefaultPriority(); + virLogReset(); + virLogSetDefaultPriority(logprio); + + /* Clear out all signal handlers from parent so nothing + unexpected can happen in our child once we unblock + signals */ + sig_action.sa_handler = SIG_DFL; + sig_action.sa_flags = 0; + sigemptyset(&sig_action.sa_mask); + + for (i = 1; i < NSIG; i++) { + /* Only possible errors are EFAULT or EINVAL + The former wont happen, the latter we + expect, so no need to check return value */ + + sigaction(i, &sig_action, NULL); + } + +# ifdef HAVE_PTHREAD_SIGMASK + /* Unmask all signals in child, since we've no idea + what the caller's done with their signal mask + and don't want to propagate that to children */ + sigemptyset(&newmask); + if (pthread_sigmask(SIG_SETMASK, &newmask, NULL) != 0) { + saved_errno = errno; /* save for caller */ + virReportSystemError(errno, "%s", _("cannot unblock signals")); + goto cleanup; + } +# endif + ret = 0; + } + +cleanup: + if (ret < 0) + errno = saved_errno; + return ret; +} + +/* + * @argv argv to exec + * @envp optional environment to use for exec + * @keepfd options fd_ret to keep open for child process + * @retpid optional pointer to store child process pid + * @infd optional file descriptor to use as child input, otherwise /dev/null + * @outfd optional pointer to communicate output fd behavior + * outfd == NULL : Use /dev/null + * *outfd == -1 : Use a new fd + * *outfd != -1 : Use *outfd + * @errfd optional pointer to communcate error fd behavior. See outfd + * @flags possible combination of the following: + * VIR_EXEC_NONE : Default function behavior + * VIR_EXEC_NONBLOCK : Set child process output fd's as non-blocking + * VIR_EXEC_DAEMON : Daemonize the child process + * @hook optional virExecHook function to call prior to exec + * @data data to pass to the hook function + * @pidfile path to use as pidfile for daemonized process (needs DAEMON flag) + */ +static int +virExecWithHook(const char *const*argv, + const char *const*envp, + const fd_set *keepfd, + pid_t *retpid, + int infd, int *outfd, int *errfd, + int flags, + virExecHook hook, + void *data, + char *pidfile) +{ + pid_t pid; + int null, i, openmax; + int pipeout[2] = {-1,-1}; + int pipeerr[2] = {-1,-1}; + int childout = -1; + int childerr = -1; + int tmpfd; + const char *binary = NULL; + int forkRet; + char *argv_str = NULL; + char *envp_str = NULL; + + if ((argv_str = virArgvToString(argv)) == NULL) { + virReportOOMError(); + return -1; + } + + if (envp) { + if ((envp_str = virArgvToString(envp)) == NULL) { + VIR_FREE(argv_str); + virReportOOMError(); + return -1; + } + VIR_DEBUG("%s %s", envp_str, argv_str); + VIR_FREE(envp_str); + } else { + VIR_DEBUG("%s", argv_str); + } + VIR_FREE(argv_str); + + if (argv[0][0] != '/') { + if (!(binary = virFindFileInPath(argv[0]))) { + virReportSystemError(ENOENT, + _("Cannot find '%s' in path"), + argv[0]); + return -1; + } + } else { + binary = argv[0]; + } + + if ((null = open("/dev/null", O_RDWR)) < 0) { + virReportSystemError(errno, + _("cannot open %s"), + "/dev/null"); + goto cleanup; + } + + if (outfd != NULL) { + if (*outfd == -1) { + if (pipe(pipeout) < 0) { + virReportSystemError(errno, + "%s", _("cannot create pipe")); + goto cleanup; + } + + if ((flags & VIR_EXEC_NONBLOCK) && + virSetNonBlock(pipeout[0]) == -1) { + virReportSystemError(errno, + "%s", _("Failed to set non-blocking file descriptor flag")); + goto cleanup; + } + + if (virSetCloseExec(pipeout[0]) == -1) { + virReportSystemError(errno, + "%s", _("Failed to set close-on-exec file descriptor flag")); + goto cleanup; + } + + childout = pipeout[1]; + } else { + childout = *outfd; + } + } else { + childout = null; + } + + if (errfd != NULL) { + if (*errfd == -1) { + if (pipe(pipeerr) < 0) { + virReportSystemError(errno, + "%s", _("Failed to create pipe")); + goto cleanup; + } + + if ((flags & VIR_EXEC_NONBLOCK) && + virSetNonBlock(pipeerr[0]) == -1) { + virReportSystemError(errno, + "%s", _("Failed to set non-blocking file descriptor flag")); + goto cleanup; + } + + if (virSetCloseExec(pipeerr[0]) == -1) { + virReportSystemError(errno, + "%s", _("Failed to set close-on-exec file descriptor flag")); + goto cleanup; + } + + childerr = pipeerr[1]; + } else { + childerr = *errfd; + } + } else { + childerr = null; + } + + forkRet = virFork(&pid); + + if (pid < 0) { + goto cleanup; + } + + if (pid) { /* parent */ + VIR_FORCE_CLOSE(null); + if (outfd && *outfd == -1) { + VIR_FORCE_CLOSE(pipeout[1]); + *outfd = pipeout[0]; + } + if (errfd && *errfd == -1) { + VIR_FORCE_CLOSE(pipeerr[1]); + *errfd = pipeerr[0]; + } + + if (forkRet < 0) { + goto cleanup; + } + + *retpid = pid; + + if (binary != argv[0]) + VIR_FREE(binary); + + return 0; + } + + /* child */ + + if (forkRet < 0) { + /* The fork was sucessful, but after that there was an error + * in the child (which was already logged). + */ + goto fork_error; + } + + openmax = sysconf (_SC_OPEN_MAX); + for (i = 3; i < openmax; i++) + if (i != infd && + i != null && + i != childout && + i != childerr && + (!keepfd || i >= FD_SETSIZE || !FD_ISSET(i, keepfd))) { + tmpfd = i; + VIR_FORCE_CLOSE(tmpfd); + } + + if (dup2(infd >= 0 ? infd : null, STDIN_FILENO) < 0) { + virReportSystemError(errno, + "%s", _("failed to setup stdin file handle")); + goto fork_error; + } + if (childout > 0 && + dup2(childout, STDOUT_FILENO) < 0) { + virReportSystemError(errno, + "%s", _("failed to setup stdout file handle")); + goto fork_error; + } + if (childerr > 0 && + dup2(childerr, STDERR_FILENO) < 0) { + virReportSystemError(errno, + "%s", _("failed to setup stderr file handle")); + goto fork_error; + } + + if (infd != STDIN_FILENO) + VIR_FORCE_CLOSE(infd); + VIR_FORCE_CLOSE(null); + if (childout > STDERR_FILENO) { + tmpfd = childout; /* preserve childout value */ + VIR_FORCE_CLOSE(tmpfd); + } + if (childerr > STDERR_FILENO && + childerr != childout) { + VIR_FORCE_CLOSE(childerr); + } + + /* Initialize full logging for a while */ + virLogSetFromEnv(); + + /* Daemonize as late as possible, so the parent process can detect + * the above errors with wait* */ + if (flags & VIR_EXEC_DAEMON) { + if (setsid() < 0) { + virReportSystemError(errno, + "%s", _("cannot become session leader")); + goto fork_error; + } + + if (chdir("/") < 0) { + virReportSystemError(errno, + "%s", _("cannot change to root directory")); + goto fork_error; + } + + pid = fork(); + if (pid < 0) { + virReportSystemError(errno, + "%s", _("cannot fork child process")); + goto fork_error; + } + + if (pid > 0) { + if (pidfile && virFileWritePidPath(pidfile,pid)) { + kill(pid, SIGTERM); + usleep(500*1000); + kill(pid, SIGTERM); + virReportSystemError(errno, + _("could not write pidfile %s for %d"), + pidfile, pid); + goto fork_error; + } + _exit(0); + } + } + + if (hook) { + /* virFork reset all signal handlers to the defaults. + * This is good for the child process, but our hook + * risks running something that generates SIGPIPE, + * so we need to temporarily block that again + */ + struct sigaction waxon, waxoff; + waxoff.sa_handler = SIG_IGN; + waxoff.sa_flags = 0; + sigemptyset(&waxoff.sa_mask); + memset(&waxon, 0, sizeof(waxon)); + if (sigaction(SIGPIPE, &waxoff, &waxon) < 0) { + virReportSystemError(errno, "%s", + _("Could not disable SIGPIPE")); + goto fork_error; + } + + if ((hook)(data) != 0) { + VIR_DEBUG("Hook function failed."); + goto fork_error; + } + + if (sigaction(SIGPIPE, &waxon, NULL) < 0) { + virReportSystemError(errno, "%s", + _("Could not re-enable SIGPIPE")); + goto fork_error; + } + } + + /* The steps above may need todo something privileged, so + * we delay clearing capabilities until the last minute */ + if ((flags & VIR_EXEC_CLEAR_CAPS) && + virClearCapabilities() < 0) + goto fork_error; + + /* Close logging again to ensure no FDs leak to child */ + virLogReset(); + + if (envp) + execve(binary, (char **) argv, (char**)envp); + else + execv(binary, (char **) argv); + + virReportSystemError(errno, + _("cannot execute binary %s"), + argv[0]); + + fork_error: + virDispatchError(NULL); + _exit(EXIT_FAILURE); + + cleanup: + /* This is cleanup of parent process only - child + should never jump here on error */ + + if (binary != argv[0]) + VIR_FREE(binary); + + /* NB we don't virCommandError() on any failures here + because the code which jumped hre already raised + an error condition which we must not overwrite */ + VIR_FORCE_CLOSE(pipeerr[0]); + VIR_FORCE_CLOSE(pipeerr[1]); + VIR_FORCE_CLOSE(pipeout[0]); + VIR_FORCE_CLOSE(pipeout[1]); + VIR_FORCE_CLOSE(null); + return -1; +} + +/** + * @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) +{ + int ret; + virCommandPtr cmd = virCommandNewArgs(argv); + + if (!cmd) + return -1; + + ret = virCommandRun(cmd, status); + virCommandFree(cmd); + return ret; +} + +#else /* WIN32 */ + +virRun(const char *const *argv ATTRIBUTE_UNUSED, + int *status) +{ + if (status) + *status = ENOTSUP; + else + virCommandError(VIR_ERR_INTERNAL_ERROR, + "%s", _("virRun is not implemented for WIN32")); + return -1; +} + +static int +virExecWithHook(const char *const*argv ATTRIBUTE_UNUSED, + const char *const*envp ATTRIBUTE_UNUSED, + const fd_set *keepfd ATTRIBUTE_UNUSED, + pid_t *retpid ATTRIBUTE_UNUSED, + int infd ATTRIBUTE_UNUSED, + int *outfd ATTRIBUTE_UNUSED, + int *errfd ATTRIBUTE_UNUSED, + int flags ATTRIBUTE_UNUSED, + virExecHook hook ATTRIBUTE_UNUSED, + void *data ATTRIBUTE_UNUSED, + char *pidfile ATTRIBUTE_UNUSED) +{ + /* XXX: Some day we can implement pieces of virCommand/virExec on + * top of _spawn() or CreateProcess(), but we can't implement + * everything, since mingw completely lacks fork(), so we cannot + * run hook code in the child. */ + virCommandError(VIR_ERR_INTERNAL_ERROR, + "%s", _("virExec is not implemented for WIN32")); + return -1; +} + +int +virFork(pid_t *pid) +{ + *pid = -1; + errno = ENOTSUP; + + return -1; +} + +#endif /* WIN32 */ + + /* * Create a new command for named binary */ diff --git a/src/util/command.h b/src/util/command.h index aa5136b..267cd73 100644 --- a/src/util/command.h +++ b/src/util/command.h @@ -29,6 +29,20 @@ typedef struct _virCommand virCommand; typedef virCommand *virCommandPtr; +/* This will execute in the context of the first child + * after fork() but before execve() */ +typedef int (*virExecHook)(void *data); + +/* + * Fork wrapper with extra error checking + */ +int virFork(pid_t *pid) ATTRIBUTE_RETURN_CHECK; + +/* + * Simple synchronous command wrapper + */ +int virRun(const char *const*argv, int *status) ATTRIBUTE_RETURN_CHECK; + /* * Create a new command for named binary */ diff --git a/src/util/ebtables.c b/src/util/ebtables.c index 27dce5d..d1afff0 100644 --- a/src/util/ebtables.c +++ b/src/util/ebtables.c @@ -41,7 +41,7 @@ #include "internal.h" #include "ebtables.h" -#include "util.h" +#include "command.h" #include "memory.h" #include "virterror_internal.h" #include "logging.h" diff --git a/src/util/pci.c b/src/util/pci.c index 9cc7b20..51c3988 100644 --- a/src/util/pci.c +++ b/src/util/pci.c @@ -35,7 +35,7 @@ #include "logging.h" #include "memory.h" -#include "util.h" +#include "command.h" #include "virterror_internal.h" #include "files.h" diff --git a/src/util/util.c b/src/util/util.c index 2fe3808..70b54c7 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -34,11 +34,11 @@ #include <errno.h> #include <poll.h> #include <time.h> -#include <sys/types.h> -#include <sys/stat.h> #include <sys/ioctl.h> #include <sys/wait.h> #include <sys/time.h> +#include <sys/stat.h> +#include <sys/types.h> #if HAVE_MMAP # include <sys/mman.h> #endif @@ -69,7 +69,6 @@ #include "virterror_internal.h" #include "logging.h" #include "event.h" -#include "ignore-value.h" #include "buf.h" #include "util.h" #include "memory.h" @@ -247,20 +246,6 @@ virArgvToString(const char *const *argv) return ret; } -int virSetBlocking(int fd, bool blocking) { - return set_nonblocking_flag (fd, !blocking); -} - -int virSetNonBlock(int fd) { - return virSetBlocking(fd, false); -} - - -int virSetCloseExec(int fd) -{ - return virSetInherit(fd, false); -} - #ifndef WIN32 int virSetInherit(int fd, bool inherit) { @@ -276,510 +261,6 @@ int virSetInherit(int fd, bool inherit) { return 0; } - -# if HAVE_CAPNG -static int virClearCapabilities(void) -{ - int ret; - - capng_clear(CAPNG_SELECT_BOTH); - - if ((ret = capng_apply(CAPNG_SELECT_BOTH)) < 0) { - virUtilError(VIR_ERR_INTERNAL_ERROR, - _("cannot clear process capabilities %d"), ret); - return -1; - } - - return 0; -} -# else -static int virClearCapabilities(void) -{ -// VIR_WARN("libcap-ng support not compiled in, unable to clear capabilities"); - return 0; -} -# endif - - -/* virFork() - fork a new process while avoiding various race/deadlock conditions - - @pid - a pointer to a pid_t that will receive the return value from - fork() - - on return from virFork(), if *pid < 0, the fork failed and there is - no new process. Otherwise, just like fork(), if *pid == 0, it is the - child process returning, and if *pid > 0, it is the parent. - - Even if *pid >= 0, if the return value from virFork() is < 0, it - indicates a failure that occurred in the parent or child process - after the fork. In this case, the child process should call - _exit(EXIT_FAILURE) after doing any additional error reporting. - - */ -int virFork(pid_t *pid) { -# ifdef HAVE_PTHREAD_SIGMASK - sigset_t oldmask, newmask; -# endif - struct sigaction sig_action; - int saved_errno, ret = -1; - - *pid = -1; - - /* - * Need to block signals now, so that child process can safely - * kill off caller's signal handlers without a race. - */ -# ifdef HAVE_PTHREAD_SIGMASK - sigfillset(&newmask); - if (pthread_sigmask(SIG_SETMASK, &newmask, &oldmask) != 0) { - saved_errno = errno; - virReportSystemError(errno, - "%s", _("cannot block signals")); - goto cleanup; - } -# endif - - /* Ensure we hold the logging lock, to protect child processes - * from deadlocking on another thread's inherited mutex state */ - virLogLock(); - - *pid = fork(); - saved_errno = errno; /* save for caller */ - - /* Unlock for both parent and child process */ - virLogUnlock(); - - if (*pid < 0) { -# ifdef HAVE_PTHREAD_SIGMASK - /* attempt to restore signal mask, but ignore failure, to - avoid obscuring the fork failure */ - ignore_value (pthread_sigmask(SIG_SETMASK, &oldmask, NULL)); -# endif - virReportSystemError(saved_errno, - "%s", _("cannot fork child process")); - goto cleanup; - } - - if (*pid) { - - /* parent process */ - -# ifdef HAVE_PTHREAD_SIGMASK - /* Restore our original signal mask now that the child is - safely running */ - if (pthread_sigmask(SIG_SETMASK, &oldmask, NULL) != 0) { - saved_errno = errno; /* save for caller */ - virReportSystemError(errno, "%s", _("cannot unblock signals")); - goto cleanup; - } -# endif - ret = 0; - - } else { - - /* child process */ - - int logprio; - int i; - - /* Remove any error callback so errors in child now - get sent to stderr where they stand a fighting chance - of being seen / logged */ - virSetErrorFunc(NULL, NULL); - virSetErrorLogPriorityFunc(NULL); - - /* Make sure any hook logging is sent to stderr, since child - * process may close the logfile FDs */ - logprio = virLogGetDefaultPriority(); - virLogReset(); - virLogSetDefaultPriority(logprio); - - /* Clear out all signal handlers from parent so nothing - unexpected can happen in our child once we unblock - signals */ - sig_action.sa_handler = SIG_DFL; - sig_action.sa_flags = 0; - sigemptyset(&sig_action.sa_mask); - - for (i = 1; i < NSIG; i++) { - /* Only possible errors are EFAULT or EINVAL - The former wont happen, the latter we - expect, so no need to check return value */ - - sigaction(i, &sig_action, NULL); - } - -# ifdef HAVE_PTHREAD_SIGMASK - /* Unmask all signals in child, since we've no idea - what the caller's done with their signal mask - and don't want to propagate that to children */ - sigemptyset(&newmask); - if (pthread_sigmask(SIG_SETMASK, &newmask, NULL) != 0) { - saved_errno = errno; /* save for caller */ - virReportSystemError(errno, "%s", _("cannot unblock signals")); - goto cleanup; - } -# endif - ret = 0; - } - -cleanup: - if (ret < 0) - errno = saved_errno; - return ret; -} - -/* - * @argv argv to exec - * @envp optional environment to use for exec - * @keepfd options fd_ret to keep open for child process - * @retpid optional pointer to store child process pid - * @infd optional file descriptor to use as child input, otherwise /dev/null - * @outfd optional pointer to communicate output fd behavior - * outfd == NULL : Use /dev/null - * *outfd == -1 : Use a new fd - * *outfd != -1 : Use *outfd - * @errfd optional pointer to communcate error fd behavior. See outfd - * @flags possible combination of the following: - * VIR_EXEC_NONE : Default function behavior - * VIR_EXEC_NONBLOCK : Set child process output fd's as non-blocking - * VIR_EXEC_DAEMON : Daemonize the child process - * @hook optional virExecHook function to call prior to exec - * @data data to pass to the hook function - * @pidfile path to use as pidfile for daemonized process (needs DAEMON flag) - */ -int -virExecWithHook(const char *const*argv, - const char *const*envp, - const fd_set *keepfd, - pid_t *retpid, - int infd, int *outfd, int *errfd, - int flags, - virExecHook hook, - void *data, - char *pidfile) -{ - pid_t pid; - int null, i, openmax; - int pipeout[2] = {-1,-1}; - int pipeerr[2] = {-1,-1}; - int childout = -1; - int childerr = -1; - int tmpfd; - const char *binary = NULL; - int forkRet; - char *argv_str = NULL; - char *envp_str = NULL; - - if ((argv_str = virArgvToString(argv)) == NULL) { - virReportOOMError(); - return -1; - } - - if (envp) { - if ((envp_str = virArgvToString(envp)) == NULL) { - VIR_FREE(argv_str); - virReportOOMError(); - return -1; - } - VIR_DEBUG("%s %s", envp_str, argv_str); - VIR_FREE(envp_str); - } else { - VIR_DEBUG("%s", argv_str); - } - VIR_FREE(argv_str); - - if (argv[0][0] != '/') { - if (!(binary = virFindFileInPath(argv[0]))) { - virReportSystemError(ENOENT, - _("Cannot find '%s' in path"), - argv[0]); - return -1; - } - } else { - binary = argv[0]; - } - - if ((null = open("/dev/null", O_RDWR)) < 0) { - virReportSystemError(errno, - _("cannot open %s"), - "/dev/null"); - goto cleanup; - } - - if (outfd != NULL) { - if (*outfd == -1) { - if (pipe(pipeout) < 0) { - virReportSystemError(errno, - "%s", _("cannot create pipe")); - goto cleanup; - } - - if ((flags & VIR_EXEC_NONBLOCK) && - virSetNonBlock(pipeout[0]) == -1) { - virReportSystemError(errno, - "%s", _("Failed to set non-blocking file descriptor flag")); - goto cleanup; - } - - if (virSetCloseExec(pipeout[0]) == -1) { - virReportSystemError(errno, - "%s", _("Failed to set close-on-exec file descriptor flag")); - goto cleanup; - } - - childout = pipeout[1]; - } else { - childout = *outfd; - } - } else { - childout = null; - } - - if (errfd != NULL) { - if (*errfd == -1) { - if (pipe(pipeerr) < 0) { - virReportSystemError(errno, - "%s", _("Failed to create pipe")); - goto cleanup; - } - - if ((flags & VIR_EXEC_NONBLOCK) && - virSetNonBlock(pipeerr[0]) == -1) { - virReportSystemError(errno, - "%s", _("Failed to set non-blocking file descriptor flag")); - goto cleanup; - } - - if (virSetCloseExec(pipeerr[0]) == -1) { - virReportSystemError(errno, - "%s", _("Failed to set close-on-exec file descriptor flag")); - goto cleanup; - } - - childerr = pipeerr[1]; - } else { - childerr = *errfd; - } - } else { - childerr = null; - } - - forkRet = virFork(&pid); - - if (pid < 0) { - goto cleanup; - } - - if (pid) { /* parent */ - VIR_FORCE_CLOSE(null); - if (outfd && *outfd == -1) { - VIR_FORCE_CLOSE(pipeout[1]); - *outfd = pipeout[0]; - } - if (errfd && *errfd == -1) { - VIR_FORCE_CLOSE(pipeerr[1]); - *errfd = pipeerr[0]; - } - - if (forkRet < 0) { - goto cleanup; - } - - *retpid = pid; - - if (binary != argv[0]) - VIR_FREE(binary); - - return 0; - } - - /* child */ - - if (forkRet < 0) { - /* The fork was sucessful, but after that there was an error - * in the child (which was already logged). - */ - goto fork_error; - } - - openmax = sysconf (_SC_OPEN_MAX); - for (i = 3; i < openmax; i++) - if (i != infd && - i != null && - i != childout && - i != childerr && - (!keepfd || i >= FD_SETSIZE || !FD_ISSET(i, keepfd))) { - tmpfd = i; - VIR_FORCE_CLOSE(tmpfd); - } - - if (dup2(infd >= 0 ? infd : null, STDIN_FILENO) < 0) { - virReportSystemError(errno, - "%s", _("failed to setup stdin file handle")); - goto fork_error; - } - if (childout > 0 && - dup2(childout, STDOUT_FILENO) < 0) { - virReportSystemError(errno, - "%s", _("failed to setup stdout file handle")); - goto fork_error; - } - if (childerr > 0 && - dup2(childerr, STDERR_FILENO) < 0) { - virReportSystemError(errno, - "%s", _("failed to setup stderr file handle")); - goto fork_error; - } - - if (infd != STDIN_FILENO) - VIR_FORCE_CLOSE(infd); - VIR_FORCE_CLOSE(null); - if (childout > STDERR_FILENO) { - tmpfd = childout; /* preserve childout value */ - VIR_FORCE_CLOSE(tmpfd); - } - if (childerr > STDERR_FILENO && - childerr != childout) { - VIR_FORCE_CLOSE(childerr); - } - - /* Initialize full logging for a while */ - virLogSetFromEnv(); - - /* Daemonize as late as possible, so the parent process can detect - * the above errors with wait* */ - if (flags & VIR_EXEC_DAEMON) { - if (setsid() < 0) { - virReportSystemError(errno, - "%s", _("cannot become session leader")); - goto fork_error; - } - - if (chdir("/") < 0) { - virReportSystemError(errno, - "%s", _("cannot change to root directory")); - goto fork_error; - } - - pid = fork(); - if (pid < 0) { - virReportSystemError(errno, - "%s", _("cannot fork child process")); - goto fork_error; - } - - if (pid > 0) { - if (pidfile && virFileWritePidPath(pidfile,pid)) { - kill(pid, SIGTERM); - usleep(500*1000); - kill(pid, SIGTERM); - virReportSystemError(errno, - _("could not write pidfile %s for %d"), - pidfile, pid); - goto fork_error; - } - _exit(0); - } - } - - if (hook) { - /* virFork reset all signal handlers to the defaults. - * This is good for the child process, but our hook - * risks running something that generates SIGPIPE, - * so we need to temporarily block that again - */ - struct sigaction waxon, waxoff; - waxoff.sa_handler = SIG_IGN; - waxoff.sa_flags = 0; - sigemptyset(&waxoff.sa_mask); - memset(&waxon, 0, sizeof(waxon)); - if (sigaction(SIGPIPE, &waxoff, &waxon) < 0) { - virReportSystemError(errno, "%s", - _("Could not disable SIGPIPE")); - goto fork_error; - } - - if ((hook)(data) != 0) { - VIR_DEBUG("Hook function failed."); - goto fork_error; - } - - if (sigaction(SIGPIPE, &waxon, NULL) < 0) { - virReportSystemError(errno, "%s", - _("Could not re-enable SIGPIPE")); - goto fork_error; - } - } - - /* The steps above may need todo something privileged, so - * we delay clearing capabilities until the last minute */ - if ((flags & VIR_EXEC_CLEAR_CAPS) && - virClearCapabilities() < 0) - goto fork_error; - - /* Close logging again to ensure no FDs leak to child */ - virLogReset(); - - if (envp) - execve(binary, (char **) argv, (char**)envp); - else - execv(binary, (char **) argv); - - virReportSystemError(errno, - _("cannot execute binary %s"), - argv[0]); - - fork_error: - virDispatchError(NULL); - _exit(EXIT_FAILURE); - - cleanup: - /* This is cleanup of parent process only - child - should never jump here on error */ - - if (binary != argv[0]) - VIR_FREE(binary); - - /* NB we don't virUtilError() on any failures here - because the code which jumped hre already raised - an error condition which we must not overwrite */ - VIR_FORCE_CLOSE(pipeerr[0]); - VIR_FORCE_CLOSE(pipeerr[1]); - VIR_FORCE_CLOSE(pipeout[0]); - VIR_FORCE_CLOSE(pipeout[1]); - VIR_FORCE_CLOSE(null); - return -1; -} - -/** - * @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) -{ - int ret; - virCommandPtr cmd = virCommandNewArgs(argv); - - if (!cmd) - return -1; - - ret = virCommandRun(cmd, status); - virCommandFree(cmd); - return ret; -} - #else /* WIN32 */ int virSetInherit(int fd ATTRIBUTE_UNUSED, bool inherit ATTRIBUTE_UNUSED) @@ -787,50 +268,21 @@ int virSetInherit(int fd ATTRIBUTE_UNUSED, bool inherit ATTRIBUTE_UNUSED) return -1; } -virRun(const char *const *argv ATTRIBUTE_UNUSED, - int *status) -{ - if (status) - *status = ENOTSUP; - else - virUtilError(VIR_ERR_INTERNAL_ERROR, - "%s", _("virRun is not implemented for WIN32")); - return -1; +#endif /* WIN32 */ + +int virSetBlocking(int fd, bool blocking) { + return set_nonblocking_flag (fd, !blocking); } -int -virExecWithHook(const char *const*argv ATTRIBUTE_UNUSED, - const char *const*envp ATTRIBUTE_UNUSED, - const fd_set *keepfd ATTRIBUTE_UNUSED, - pid_t *retpid ATTRIBUTE_UNUSED, - int infd ATTRIBUTE_UNUSED, - int *outfd ATTRIBUTE_UNUSED, - int *errfd ATTRIBUTE_UNUSED, - int flags ATTRIBUTE_UNUSED, - virExecHook hook ATTRIBUTE_UNUSED, - void *data ATTRIBUTE_UNUSED, - char *pidfile ATTRIBUTE_UNUSED) -{ - /* XXX: Some day we can implement pieces of virCommand/virExec on - * top of _spawn() or CreateProcess(), but we can't implement - * everything, since mingw completely lacks fork(), so we cannot - * run hook code in the child. */ - virUtilError(VIR_ERR_INTERNAL_ERROR, - "%s", _("virExec is not implemented for WIN32")); - return -1; +int virSetNonBlock(int fd) { + return virSetBlocking(fd, false); } -int -virFork(pid_t *pid) +int virSetCloseExec(int fd) { - *pid = -1; - errno = ENOTSUP; - - return -1; + return virSetInherit(fd, false); } -#endif /* WIN32 */ - int virPipeReadUntilEOF(int outfd, int errfd, char **outbuf, char **errbuf) { diff --git a/src/util/util.h b/src/util/util.h index 3e95cae..68a8431 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -42,37 +42,13 @@ ssize_t safewrite(int fd, const void *buf, size_t count) int safezero(int fd, int flags, off_t offset, off_t len) ATTRIBUTE_RETURN_CHECK; -enum { - VIR_EXEC_NONE = 0, - VIR_EXEC_NONBLOCK = (1 << 0), - VIR_EXEC_DAEMON = (1 << 1), - VIR_EXEC_CLEAR_CAPS = (1 << 2), -}; - int virSetBlocking(int fd, bool blocking) ATTRIBUTE_RETURN_CHECK; int virSetNonBlock(int fd) ATTRIBUTE_RETURN_CHECK; int virSetInherit(int fd, bool inherit) ATTRIBUTE_RETURN_CHECK; int virSetCloseExec(int fd) ATTRIBUTE_RETURN_CHECK; -/* This will execute in the context of the first child - * after fork() but before execve() */ -typedef int (*virExecHook)(void *data); - -int virExecWithHook(const char *const*argv, - const char *const*envp, - const fd_set *keepfd, - int *retpid, - int infd, - int *outfd, - int *errfd, - int flags, - virExecHook hook, - void *data, - char *pidfile) ATTRIBUTE_RETURN_CHECK; -int virRun(const char *const*argv, int *status) ATTRIBUTE_RETURN_CHECK; int virPipeReadUntilEOF(int outfd, int errfd, char **outbuf, char **errbuf); -int virFork(pid_t *pid); int virSetUIDGID(uid_t uid, gid_t gid); diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c index b920f79..34f8ff6 100644 --- a/src/vmware/vmware_driver.c +++ b/src/vmware/vmware_driver.c @@ -29,6 +29,7 @@ #include "files.h" #include "memory.h" #include "uuid.h" +#include "command.h" #include "vmx.h" #include "vmware_conf.h" #include "vmware_driver.h" -- 1.7.4.4