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

[Michael Roth]

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.