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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list