Re: [Qemu-devel] [PATCH 06/10] qga: guest exec functionality for Windows guests

[Michael Roth]

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 <address@hidden>
> >>>>
> >>>> 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.

With even the bare-minimum API we could still do something like:

  cmd: '/bash/sh'
  arg0: '-c'
  arg1: 'sh <virt-serialpath >virt-serialpath'

From the 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

  <cmd> </dev/stdin >/dev/stdout

unless you're explicitly wanting to redirect somewhere other
than the defaults.

> 
> Den