From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54922) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZCy8O-00068b-Ls for qemu-devel@nongnu.org; Wed, 08 Jul 2015 18:48:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZCy8L-0000M5-CQ for qemu-devel@nongnu.org; Wed, 08 Jul 2015 18:48:08 -0400 Received: from relay.parallels.com ([195.214.232.42]:48794) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZCy8K-0000Le-Vk for qemu-devel@nongnu.org; Wed, 08 Jul 2015 18:48:05 -0400 Message-ID: <559DA897.2030301@openvz.org> Date: Thu, 9 Jul 2015 01:47:51 +0300 From: "Denis V. Lunev" MIME-Version: 1.0 References: <1435659923-2211-1-git-send-email-den@openvz.org> <1435659923-2211-7-git-send-email-den@openvz.org> <20150707013127.17393.82688@loki> <559B8870.30704@openvz.org> <20150708220219.23206.20088@loki> In-Reply-To: <20150708220219.23206.20088@loki> Content-Type: multipart/mixed; boundary="------------050105040303010300090909" Subject: Re: [Qemu-devel] [PATCH 06/10] qga: guest exec functionality for Windows guests List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Michael Roth Cc: Olga Krishtal , qemu-devel@nongnu.org --------------050105040303010300090909 Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit On 09/07/15 01:02, Michael Roth wrote: > Quoting Denis V. Lunev (2015-07-07 03:06:08) >> On 07/07/15 04:31, Michael Roth wrote: >>> Quoting Denis V. Lunev (2015-06-30 05:25:19) >>>> From: Olga Krishtal >>>> >>>> Child process' stdin/stdout/stderr can be associated >>>> with handles for communication via read/write interfaces. >>>> >>>> The workflow should be something like this: >>>> * Open an anonymous pipe through guest-pipe-open >>>> * Execute a binary or a script in the guest. Arbitrary arguments and >>>> environment to a new child process could be passed through options >>>> * Read/pass information from/to executed process using >>>> guest-file-read/write >>>> * Collect the status of a child process >>> Have you seen anything like this in your testing? >>> >>> {'execute':'guest-exec','arguments':{'path':'/Windows/System32/ipconfig.exe', >>> 'timeout':5000}} >>> {"return": {"pid": 588}} >>> {'execute':'guest-exec-status','arguments':{'pid':588}} >>> {"return": {"exit": 0, "handle-stdout": -1, "handle-stderr": -1, >>> "handle-stdin": -1, "signal": -1}} >>> {'execute':'guest-exec-status','arguments':{'pid':588}} >>> {"error": {"class": "GenericError", "desc": "Invalid parameter 'pid'"}} >>> >>> {'execute':'guest-exec','arguments':{'path':'/Windows/System32/ipconfig.exe', >>> 'timeout':5000}} >>> {"error": {"class": "GenericError", "desc": "CreateProcessW() failed: >>> The parameter is incorrect. (error: 57)"}} >>> {'execute':'guest-exec','arguments':{'path':'/Windows/System32/ipconfig.exe', >>> 'timeout':5000}} >>> {"error": {"class": "GenericError", "desc": "CreateProcessW() failed: >>> The parameter is incorrect. (error: 57)"}} >>> >>> {'execute':'guest-exec','arguments':{'path':'/Windows/System32/ipconfig.exe', >>> 'timeout':5000}} >>> {"return": {"pid": 1836}} >> I'll check this later during office time. Something definitely went wrong. >> >>> The guest-exec-status failures are expected since the first call reaps >>> everything, but the CreateProcessW() failures are not. Will look into it >>> more this evening, but it doesn't look like I'll be able to apply this in >>> it's current state. >>> >>> I have concerns over the schema as well. I think last time we discussed >>> it we both seemed to agree that guest-file-open was unwieldy and >>> unnecessary. We should just let guest-exec return a set of file handles >>> instead of having users do all the plumbing. >> no, the discussion was a bit different AFAIR. First of all, you have >> proposed >> to use unified code to perform exec. On the other hand current mechanics >> with pipes is quite inconvenient for end-users of the feature for example >> for interactive shell in the guest. >> >> We have used very simple approach for our application: pipes are not >> used, the application creates VirtIO serial channel and forces guest through >> this API to fork/exec the child using this serial as a stdio in/out. In this >> case we do receive a convenient API for shell processing. >> >> This means that this flexibility with direct specification of the file >> descriptors is necessary. > But if I'm understanding things correctly, you're simply *not* using the > guest-pipe-* interfaces in this case, and it's just a matter of having > the option of not overriding the child's stdio? We wouldn't > necessarilly lose that flexibility if we dropped guest-pipe-*, > specifying whether we want to wire qemu-ga into stdin/stdout could > still be done via options to guest-exec. > > Do you have an example of the sort of invocation you're doing? > >> There are two solutions from my point of view: >> - keep current API, it is suitable for us >> - switch to "pipe only" mechanics for guest exec, i.e. the command >> will work like "ssh" with one descriptor for read and one for write >> created automatically, but in this case we do need either a way >> to connect Unix socket in host with file descriptor in guest or >> make possibility to send events from QGA to client using QMP >> >>> I'm really sorry for chiming in right before hard freeze, very poor >>> timing/planning on my part. >> :( can we somehow schedule this better next time? This functionality >> is mandatory for us and we can not afford to drop it or forget about >> it for long. There was no pressure in winter but now I am on a hard >> pressure. Thus can we at least agree on API terms and come to an >> agreement? > Yes, absolutely. Let's get the API down early and hopefully we can > get it all merged early this time. > I have attached entire C program, which allows to obtain interactive (test) shell in guest. actually we perform "{\"execute\": \"guest-file-open\", \"arguments\":{\"path\":\"%s\", \"mode\":\"r+b\"}}" and after that "{\"execute\":\"guest-exec\", \"arguments\":{\"path\":\"/bin/sh\"," "\"handle_stdin\":%u,\"handle_stdout\":%u,\"handle_stderr\":%u }}", ctx.guest_io_handle, ctx.guest_io_handle, ctx.guest_io_handle); with the handle obtained above. Den --------------050105040303010300090909 Content-Type: text/x-csrc; name="qemu-sh.c" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="qemu-sh.c" /* A simple shell on top of qemu-ga. */ #include #include #include #include #include #include #include #include #include #include static struct { const char *host_ga_socket; const char *host_io_socket; const char *guest_io_socket; } conf; static struct { int gafd; int iofd; int guest_io_handle; int guest_sh_pid; } ctx; static void ctx_cleanup(void); static void usage(void) { printf("Usage: qemu-sh HOST_GA_SOCKET HOST_IO_SOCKET GUEST_IO_SOCKET\n"); } static void dbg(const char *msg) { printf("qemu-sh: debug: %s\n", msg); } static void warn(const char *msg) { fprintf(stderr, "qemu-sh: warning: %s\n", msg); } static void error(const char *msg) { fprintf(stderr, "qemu-sh: error: %s\n", msg); ctx_cleanup(); exit(1); } static void prepare_ga(const char *host_ga_socket) { struct sockaddr_un adr; adr.sun_family = AF_UNIX; if (strlen(host_ga_socket) >= sizeof(adr.sun_path)) { error("too large path to unix socket"); } strcpy(adr.sun_path, host_ga_socket); ctx.gafd = socket(AF_UNIX, SOCK_STREAM, 0); if (ctx.gafd == -1) { error("socket() GA socket on host"); } dbg("connecting to GA..."); if (connect(ctx.gafd, (struct sockaddr *)&adr, sizeof(struct sockaddr_un)) != 0) { error("connect() to GA"); } dbg("connected to GA"); } static void prepare_io(const char *host_io_socket) { struct sockaddr_un adr; adr.sun_family = AF_UNIX; if (strlen(host_io_socket) >= sizeof(adr.sun_path)) { error("too large path to unix socket"); } strcpy(adr.sun_path, host_io_socket); ctx.iofd = socket(AF_UNIX, SOCK_STREAM, 0); if (ctx.iofd == -1) { error("socket() IO socket on host"); } dbg("connecting to IO socket..."); if (connect(ctx.iofd, (struct sockaddr *)&adr, sizeof(struct sockaddr_un)) != 0) { error("connect() to GA"); } dbg("connected to IO socket"); { int n = 1; ioctl(ctx.iofd, FIONBIO, &n); } } static void prepare_guest_io(const char *guest_io_socket) { char *cmd; int cmdlen, handle; char buf[4096]; size_t nbuf = 0, r; cmdlen = asprintf(&cmd, "{\"execute\": \"guest-file-open\", \"arguments\":{\"path\":\"%s\", \"mode\":\"r+b\"}}", guest_io_socket); if (cmdlen == -1) { error("asprintf()"); } dbg("writing to GA..."); if (write(ctx.gafd, cmd, cmdlen) != cmdlen) { error("write() to GA"); } free(cmd); /*{"return": 1000}*/ for (;;) { dbg("reading from GA..."); r = read(ctx.gafd, buf + nbuf, sizeof(buf) - nbuf - 1); if (r == -1 || r == 0) { error("read() from GA"); } nbuf += r; buf[nbuf] = '\0'; if (!strncmp(buf, "{\"error", sizeof("{\"error") - 1)) { error(buf); } if (sscanf(buf, "{\"return\": %u}", &handle) != EOF) { break; } if (nbuf == sizeof(buf) - 1) { error("too large response from GA"); } } ctx.guest_io_handle = handle; dbg("got guest_io_handle"); } static void close_guest_io(void) { char *cmd; int cmdlen, handle; char buf[4096]; size_t nbuf = 0, r; cmdlen = asprintf(&cmd, "{\"execute\": \"guest-file-close\", \"arguments\":{\"handle\":%u}}", ctx.guest_io_handle); if (cmdlen == -1) { warn("asprintf()"); return; } dbg("writing to GA..."); if (write(ctx.gafd, cmd, cmdlen) != cmdlen) { warn("write() to GA"); return; } free(cmd); for (;;) { dbg("reading from GA..."); r = read(ctx.gafd, buf + nbuf, sizeof(buf) - nbuf - 1); if (r == -1 || r == 0) { warn("read() from GA"); return; } nbuf += r; buf[nbuf] = '\0'; if (!strncmp(buf, "{\"error", sizeof("{\"error") - 1)) { warn(buf); return; } break; } ctx.guest_io_handle = -1; dbg("closed guest_io_handle"); } static void close_guest_sh(void) { char *cmd; int cmdlen, handle; char buf[4096]; size_t nbuf = 0, r; cmdlen = asprintf(&cmd, "{\"execute\": \"guest-exec-status\", \"arguments\":{\"pid\":%u}}", ctx.guest_sh_pid); if (cmdlen == -1) { warn("asprintf()"); return; } dbg("writing to GA..."); if (write(ctx.gafd, cmd, cmdlen) != cmdlen) { warn("write() to GA"); return; } free(cmd); for (;;) { dbg("reading from GA..."); r = read(ctx.gafd, buf + nbuf, sizeof(buf) - nbuf - 1); if (r == -1 || r == 0) { warn("read() from GA"); return; } nbuf += r; buf[nbuf] = '\0'; if (!strncmp(buf, "{\"error", sizeof("{\"error") - 1)) { warn(buf); return; } break; } ctx.guest_sh_pid = 0; dbg("closed guest_sh_pid"); } static void exec_shell() { char *cmd; int cmdlen, pid; char buf[4096]; size_t nbuf = 0, r; cmdlen = asprintf(&cmd, "{\"execute\":\"guest-exec\", \"arguments\":{\"path\":\"/bin/sh\"," "\"handle_stdin\":%u,\"handle_stdout\":%u,\"handle_stderr\":%u }}", ctx.guest_io_handle, ctx.guest_io_handle, ctx.guest_io_handle); if (cmdlen == -1) { error("asprintf()"); } dbg("writing to GA..."); if (write(ctx.gafd, cmd, cmdlen) != cmdlen) { error("write() to GA"); } free(cmd); /*{"return": 2395}*/ for (;;) { dbg("reading from GA..."); r = read(ctx.gafd, buf + nbuf, sizeof(buf) - nbuf - 1); if (r == -1 || r == 0) { error("read() from GA"); } nbuf += r; buf[nbuf] = '\0'; if (!strncmp(buf, "{\"error", sizeof("{\"error") - 1)) { error(buf); } if (sscanf(buf, "{\"return\": %u}", &pid) != EOF) { break; } if (nbuf == sizeof(buf) - 1) { error("too large response from GA"); } } ctx.guest_sh_pid = pid; dbg("got guest_sh_pid"); } static void intr_handler(int signo) { dbg("signal caught"); } static void ctx_cleanup(void) { if (ctx.guest_io_handle != 0) { close_guest_io(); } if (ctx.guest_sh_pid != 0) { close_guest_sh(); } if (ctx.iofd != -1) { close(ctx.iofd); ctx.iofd = -1; } if (ctx.gafd != -1) { close(ctx.gafd); ctx.gafd = -1; } } static void ev_loop(void) { char buf[4096]; size_t r; fd_set rset; for (;;) { FD_ZERO(&rset); FD_SET(STDIN_FILENO, &rset); FD_SET(ctx.iofd, &rset); r = select(ctx.iofd + 1, &rset, NULL, NULL, NULL); if (r == -1) { if (errno == EINTR) break; error("select()"); } if (FD_ISSET(ctx.iofd, &rset)) { r = read(ctx.iofd, buf, sizeof(buf)); if (r == -1 && (errno == EAGAIN || errno == EWOULDBLOCK)) continue; if (r == -1) error("read() from socket"); write(STDOUT_FILENO, buf, r); } if (FD_ISSET(STDIN_FILENO, &rset)) { r = read(STDIN_FILENO, buf, sizeof(buf)); if (r == -1) error("read() from stdin"); write(ctx.iofd, buf, r); } } } int main(int argc, char **argv, char **envp) { if (argc != 4) { usage(); return 0; } ctx.gafd = -1; ctx.iofd = -1; ctx.guest_io_handle = -1; conf.host_ga_socket = argv[1]; conf.host_io_socket = argv[2]; conf.guest_io_socket = argv[3]; { struct sigaction sa = {0}; sa.sa_handler = &intr_handler; if (sigaction(SIGINT, &sa, NULL) != 0) { error("sigaction()"); } } prepare_ga(conf.host_ga_socket); prepare_io(conf.host_io_socket); prepare_guest_io(conf.guest_io_socket); exec_shell(); dbg("Shell's started. Press Ctrl+C to exit."); ev_loop(); ctx_cleanup(); return 0; } --------------050105040303010300090909--