From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38821) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZD0fg-0007Mr-2k for qemu-devel@nongnu.org; Wed, 08 Jul 2015 21:30:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZD0fc-0004hY-5v for qemu-devel@nongnu.org; Wed, 08 Jul 2015 21:30:39 -0400 Received: from e18.ny.us.ibm.com ([129.33.205.208]:56735) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZD0fb-0004hR-W2 for qemu-devel@nongnu.org; Wed, 08 Jul 2015 21:30:36 -0400 Received: from /spool/local by e18.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 8 Jul 2015 21:30:35 -0400 Received: from b01cxnp22034.gho.pok.ibm.com (b01cxnp22034.gho.pok.ibm.com [9.57.198.24]) by d01dlp02.pok.ibm.com (Postfix) with ESMTP id 9EC376E8045 for ; Wed, 8 Jul 2015 21:22:20 -0400 (EDT) Received: from d01av05.pok.ibm.com (d01av05.pok.ibm.com [9.56.224.195]) by b01cxnp22034.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id t691UXWP62914666 for ; Thu, 9 Jul 2015 01:30:33 GMT Received: from d01av05.pok.ibm.com (localhost [127.0.0.1]) by d01av05.pok.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id t691UXmE002403 for ; Wed, 8 Jul 2015 21:30:33 -0400 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Michael Roth In-Reply-To: <559DA897.2030301@openvz.org> 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> <559DA897.2030301@openvz.org> Message-ID: <20150709013029.23206.48963@loki> Date: Wed, 08 Jul 2015 20:30:29 -0500 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: "Denis V. Lunev" Cc: Olga Krishtal , qemu-devel@nongnu.org Quoting Denis V. Lunev (2015-07-08 17:47:51) > 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 optio= ns > >>>> * 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/ipconf= ig.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/ipconf= ig.exe', > >>> 'timeout':5000}} > >>> {"error": {"class": "GenericError", "desc": "CreateProcessW() failed: > >>> The parameter is incorrect. (error: 57)"}} > >>> {'execute':'guest-exec','arguments':{'path':'/Windows/System32/ipconf= ig.exe', > >>> 'timeout':5000}} > >>> {"error": {"class": "GenericError", "desc": "CreateProcessW() failed: > >>> The parameter is incorrect. (error: 57)"}} > >>> > >>> {'execute':'guest-exec','arguments':{'path':'/Windows/System32/ipconf= ig.exe', > >>> 'timeout':5000}} > >>> {"return": {"pid": 1836}} > >> I'll check this later during office time. Something definitely went wr= ong. > >> > >>> 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 thi= s in > >>> it's current state. > >>> > >>> I have concerns over the schema as well. I think last time we discuss= ed > >>> 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 handl= es > >>> 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 mechani= cs > >> with pipes is quite inconvenient for end-users of the feature for exam= ple > >> 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 t= hrough > >> this API to fork/exec the child using this serial as a stdio in/out. I= n 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 wri= te > >> 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 (tes= t) > 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. With even the bare-minimum API we could still do something like: cmd: '/bash/sh' arg0: '-c' arg1: 'sh virt-serialpath' From=20the qga perspective I think we'd just end up with empty stdio for the life of both the parent 'sh' and child 'sh'. As a general use case this is probably a bit worse, since we're farming work out to a specific shell implementation instead of figuring out a platform-agnostic API for doing it. But if we consider this to be a niche use case then taking this approach gives us a little more flexibility for simplifying the API. But I'm not really against being able to supply stdio/stdout/stderr through the API. The main thing is that, by default, guest-exec should just supply you with a set of pipes automatically. This one thing let's us drop guest-pipe-open completely. And it's just like with a normal shell: you get stdio by default, and can redirect as needed. You don't have to do /dev/stdout unless you're explicitly wanting to redirect somewhere other than the defaults. > = > Den