[libvirt] [PATCH v3 1/6] virCommand: Introduce virCommandDoAsyncIO
Peter Krempa
pkrempa at redhat.com
Mon Feb 4 15:47:53 UTC 2013
On 02/04/13 15:56, Michal Privoznik wrote:
> Currently, if we want to feed stdin, or catch stdout or stderr of a
> virCommand we have to use virCommandRun(). When using virCommandRunAsync()
> we have to register FD handles by hand. This may lead to code duplication.
> Hence, introduce an internal API, which does this automatically within
> virCommandRunAsync(). The intended usage looks like this:
>
> virCommandPtr cmd = virCommandNew*(...);
> char *buf = NULL;
>
> ...
>
> virCommandSetOutputBuffer(cmd, &buf);
> virCommandDoAsyncIO(cmd);
>
> if (virCommandRunAsync(cmd, NULL) < 0)
> goto cleanup;
>
> ...
>
> if (virCommandWait(cmd, NULL) < 0)
> goto cleanup;
>
> /* @buf now contains @cmd's stdout */
> VIR_DEBUG("STDOUT: %s", NULLSTR(buf));
>
> ...
>
> cleanup:
> VIR_FREE(buf);
> virCommandFree(cmd);
>
> Note, that both stdout and stderr buffers may change until virCommandWait()
> returns.
> ---
> src/libvirt_private.syms | 1 +
> src/util/vircommand.c | 308 ++++++++++++++++++++++++++++++++++++++++++++---
> src/util/vircommand.h | 1 +
> 3 files changed, 293 insertions(+), 17 deletions(-)
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index c589236..99e20c0 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -143,6 +143,7 @@ virCommandAddEnvString;
> virCommandAllowCap;
> virCommandClearCaps;
> virCommandDaemonize;
> +virCommandDoAsyncIO;
> virCommandExec;
> virCommandFree;
> virCommandHandshakeNotify;
> diff --git a/src/util/vircommand.c b/src/util/vircommand.c
> index 8566d1a..270e8a2 100644
> --- a/src/util/vircommand.c
> +++ b/src/util/vircommand.c
> @@ -47,11 +47,12 @@
>
> /* Flags for virExecWithHook */
> enum {
> - VIR_EXEC_NONE = 0,
> - VIR_EXEC_NONBLOCK = (1 << 0),
> - VIR_EXEC_DAEMON = (1 << 1),
> + VIR_EXEC_NONE = 0,
> + VIR_EXEC_NONBLOCK = (1 << 0),
> + VIR_EXEC_DAEMON = (1 << 1),
> VIR_EXEC_CLEAR_CAPS = (1 << 2),
> - VIR_EXEC_RUN_SYNC = (1 << 3),
> + VIR_EXEC_RUN_SYNC = (1 << 3),
> + VIR_EXEC_ASYNC_IO = (1 << 4),
> };
>
> struct _virCommand {
> @@ -84,6 +85,11 @@ struct _virCommand {
> int *outfdptr;
> int *errfdptr;
>
> + size_t inbufOffset;
> + int inWatch;
> + int outWatch;
> + int errWatch;
> +
> bool handshake;
> int handshakeWait[2];
> int handshakeNotify[2];
> @@ -779,6 +785,7 @@ virCommandNewArgs(const char *const*args)
> cmd->handshakeNotify[1] = -1;
>
> cmd->infd = cmd->outfd = cmd->errfd = -1;
> + cmd->inWatch = cmd->outWatch = cmd->errWatch = -1;
> cmd->pid = -1;
>
> virCommandAddArgSet(cmd, args);
> @@ -1394,8 +1401,8 @@ virCommandSetWorkingDirectory(virCommandPtr cmd, const char *pwd)
> * @cmd: the command to modify
> * @inbuf: string to feed to stdin
> *
> - * Feed the child's stdin from a string buffer. This requires the use
> - * of virCommandRun().
Instead of getting rid of this sentence, you should add the second
condition when this will work: when asyncIO is requested.
> + * Feed the child's stdin from a string buffer.
> + * The buffer is forgotten after each @cmd run.
> */
> void
> virCommandSetInputBuffer(virCommandPtr cmd, const char *inbuf)
> @@ -1423,8 +1430,8 @@ virCommandSetInputBuffer(virCommandPtr cmd, const char *inbuf)
> * Capture the child's stdout to a string buffer. *outbuf is
> * guaranteed to be allocated after successful virCommandRun or
> * virCommandWait, and is best-effort allocated after failed
> - * virCommandRun; caller is responsible for freeing *outbuf.
> - * This requires the use of virCommandRun.
> + * virCommandRun or virCommandRunAsync; caller is responsible for
> + * freeing *outbuf. The buffer is forgotten after each @cmd run.
> */
> void
> virCommandSetOutputBuffer(virCommandPtr cmd, char **outbuf)
> @@ -1452,11 +1459,11 @@ virCommandSetOutputBuffer(virCommandPtr cmd, char **outbuf)
> * Capture the child's stderr to a string buffer. *errbuf is
> * guaranteed to be allocated after successful virCommandRun or
> * virCommandWait, and is best-effort allocated after failed
> - * virCommandRun; caller is responsible for freeing *errbuf.
> - * This requires the use of virCommandRun. It is possible to
> - * pass the same pointer as for virCommandSetOutputBuffer(), in
> - * which case the child process will interleave all output into
> - * a single string.
> + * virCommandRun or virCommandRunAsync; caller is responsible for
> + * freeing *errbuf. It is possible to pass the same pointer as
> + * for virCommandSetOutputBuffer(), in which case the child
> + * process will interleave all output into a single string. The
> + * buffer is forgotten after each @cmd run.
> */
> void
> virCommandSetErrorBuffer(virCommandPtr cmd, char **errbuf)
> @@ -2122,6 +2129,183 @@ virCommandHook(void *data)
> }
>
>
> +static void
> +virCommandHandleReadWrite(int watch, int fd, int events, void *opaque)
> +{
> + virCommandPtr cmd = (virCommandPtr) opaque;
> + char ***bufptr = NULL;
> + char buf[1024];
> + ssize_t nread, nwritten;
> + size_t len = 0;
> + int *watchPtr = NULL;
> + bool eof = false;
> + int tmpfd, *fdptr = NULL, **fdptrptr = NULL;
> +
> + VIR_DEBUG("watch=%d fd=%d events=%d", watch, fd, events);
> + errno = 0;
> +
> + if (watch == cmd->inWatch) {
> + watchPtr = &cmd->inWatch;
> + fdptr = &cmd->infd;
> +
> + if (events & VIR_EVENT_HANDLE_WRITABLE) {
> + len = strlen(cmd->inbuf);
> +
> + while (true) {
> + nwritten = write(fd, cmd->inbuf + cmd->inbufOffset,
> + len - cmd->inbufOffset);
> + if (nwritten < 0) {
> + if (errno != EAGAIN && errno != EINTR) {
> + virReportSystemError(errno,
> + _("Unable to write command's "
> + "input to FD %d"),
> + fd);
> + eof = true;
> + }
> + break;
> + }
> +
> + if (nwritten == 0) {
> + eof = true;
> + break;
> + }
> +
> + cmd->inbufOffset += nwritten;
> + if (cmd->inbufOffset == len) {
> + tmpfd = cmd->infd;
> + if (VIR_CLOSE(cmd->infd) < 0)
> + VIR_DEBUG("ignoring failed close on fd %d", tmpfd);
Hm, use rather VIR_FORCE_CLOSE if you don't care that it failed. It
should report the debug info for you. [1]
> + eof = true;
> + break;
> + }
> + }
> +
> + }
> + } else {
> + if (watch == cmd->outWatch) {
> + watchPtr = &cmd->outWatch;
> + bufptr = &cmd->outbuf;
> + fdptr = &cmd->outfd;
> + fdptrptr = &cmd->outfdptr;
> + } else {
> + watchPtr = &cmd->errWatch;
> + bufptr = &cmd->errbuf;
> + fdptr = &cmd->errfd;
> + fdptrptr = &cmd->errfdptr;
> + }
> +
> + if (events & VIR_EVENT_HANDLE_READABLE) {
> + if (**bufptr)
> + len = strlen(**bufptr);
> +
> + while (true) {
> + nread = read(fd, buf, sizeof(buf));
> + if (nread < 0) {
> + if (errno != EAGAIN && errno != EINTR) {
> + virReportSystemError(errno,
> + _("unable to read command's "
> + "output from FD %d"),
> + fd);
> + eof = true;
> + }
> + break;
> + }
> +
> + if (nread == 0) {
> + eof = true;
> + break;
> + }
> +
> + if (VIR_REALLOC_N(**bufptr, len + nread + 1) < 0) {
> + virReportOOMError();
> + break;
> + }
> +
> + memcpy(**bufptr + len, buf, nread);
> + (**bufptr)[len + nread] = '\0';
> + }
> +
> + }
> + }
> +
> + if (eof || (events & VIR_EVENT_HANDLE_HANGUP) ||
> + (events & VIR_EVENT_HANDLE_ERROR)) {
> + virEventRemoveHandle(watch);
> +
> + *watchPtr = -1;
> + VIR_FORCE_CLOSE(*fdptr);
[1] ah, you already did it here where I complained the last time.
> + if (bufptr)
> + *bufptr = NULL;
> + if (fdptrptr)
> + *fdptrptr = NULL;
> + }
> +}
> +
> +
> +static int
> +virCommandRegisterEventLoop(virCommandPtr cmd)
> +{
> + int ret = -1;
> +
> + if (cmd->inbuf &&
> + (cmd->inWatch = virEventAddHandle(cmd->infd,
> + VIR_EVENT_HANDLE_WRITABLE |
> + VIR_EVENT_HANDLE_HANGUP |
> + VIR_EVENT_HANDLE_ERROR,
> + virCommandHandleReadWrite,
> + cmd, NULL)) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Unable to register infd %d in the event loop"),
> + cmd->infd);
> + goto cleanup;
> + }
> +
> + if (cmd->outbuf && cmd->outfdptr == &cmd->outfd &&
> + (cmd->outWatch = virEventAddHandle(cmd->outfd,
> + VIR_EVENT_HANDLE_READABLE |
> + VIR_EVENT_HANDLE_HANGUP |
> + VIR_EVENT_HANDLE_ERROR,
> + virCommandHandleReadWrite,
> + cmd, NULL)) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Unable to register outfd %d in the event loop"),
> + cmd->outfd);
> +
> + if (cmd->inWatch != -1) {
> + virEventRemoveHandle(cmd->inWatch);
> + cmd->inWatch = -1;
> + }
> + goto cleanup;
> + }
> +
> + if (cmd->errbuf && cmd->errfdptr == &cmd->errfd &&
> + (cmd->errWatch = virEventAddHandle(cmd->errfd,
> + VIR_EVENT_HANDLE_READABLE |
> + VIR_EVENT_HANDLE_HANGUP |
> + VIR_EVENT_HANDLE_ERROR,
> + virCommandHandleReadWrite,
> + cmd, NULL)) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Unable to register errfd %d in the event loop"),
> + cmd->errfd);
> + if (cmd->inWatch != -1) {
> + virEventRemoveHandle(cmd->inWatch);
> + cmd->inWatch = -1;
> + }
> + if (cmd->outWatch != -1) {
> + virEventRemoveHandle(cmd->outWatch);
> + cmd->outWatch = -1;
> + }
Hm, okay.
> + goto cleanup;
> + }
> +
> + ret = 0;
> +
> +cleanup:
> + return ret;
> +}
> +
> +
> /**
> * virCommandRunAsync:
> * @cmd: command to start
> @@ -2149,6 +2333,7 @@ virCommandRunAsync(virCommandPtr cmd, pid_t *pid)
> char *str;
> int i;
> bool synchronous = false;
> + int infd[2] = {-1, -1};
>
> if (!cmd || cmd->has_error == ENOMEM) {
> virReportOOMError();
>
ACK with the two nits fixed. ACKs on the unchanged patches from the last
time still stand.
Peter
More information about the libvir-list
mailing list