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

["Denis V. Lunev" <den@openvz.org>]

On 09/07/15 04:30, Michael Roth wrote:
> 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 <okrishtal@virtuozzo.com>
>>>>>>
>>>>>> 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.

This would be problematic on Windows. It is difficult to redirect
to Windows names like \\.Global\org.qemu.guest.agent.0
AFAIK only DOS names are allowed and specific call to associate
Win32 name with Dos name and disassociate them looks
overkill.

This means that ability to pass descriptors would be necessary.
There are the following options
- open without handle_stdin etc create pipes inside and returns
    them as a result
- open with handles just passed them to forked process

This seems fine to me.

Den