Re: [PATCH v2] Fix exec migration on Windows (w32+w64).

[Daniel P . Berrangé]

On Tue, Jan 31, 2023 at 02:01:07PM +0400, Marc-André Lureau wrote:
> Hi
> 
> On Tue, Jan 17, 2023 at 9:07 PM John Berberian, Jr <jeb.study@gmail.com> 
> wrote:
> >
> > Apologies for the late response, I was traveling most of yesterday.
> >
> > On 1/16/23 4:22 AM, Daniel P. Berrangé wrote:
> > > When we introduce a new QAPI format for migration args though, I've
> > > suggested we drop support for passing exec via shell, and require an
> > > explicit argv[] array:
> > >
> > >    https://lists.gnu.org/archive/html/qemu-devel/2023-01/msg01434.html
> > >
> > > For Windows since we don't have back compat to worry about, we
> > > can avoid passing via cmd.exe from the start.
> >
> > I think we should keep the behavior the same on all platforms. If such a
> > change is to occur, it should happen at the same time on Windows and
> > Unix-like systems. Platform-dependent ifdefs should be used to overcome
> > platform-specific differences (e.g. the location of the shell), rather
> > than give one platform entirely different functionality - otherwise we
> > introduce needless confusion when someone accustomed to Linux tries to
> > use an exec migration on Windows and it doesn't work the same way at all.
> 
> I agree with Daniel, we should make the migrate/exec command take an
> argv[] (not run through the shell) and deprecate support for "exec:.."
> in QMP. The "exec:..." form support could later be moved to HMP...

Note, I was *not* arguing in favour of deprecating 'exec:' support,
only that we should prefer argv[] to bypass use of shell, because
shell introducing massive scope for unintended consquences possibly
with security implications.

> Tbh, allowing fork/exec from QEMU is not a great thing in the first
> place (although with GSpawn using posix_spawn on modern systems, that
> should help.. and win32 has a different API).
> 
> Instead, QMP/HMP clients could handle consumer process creation, and
> passing FDs via 'getfd,' and using the migrate 'fd:fdname' form (that
> is not really possible on win32 today, but I am adding support for
> importing sockets in a series on the list. This should do the job now
> that win32 supports unix sockets. We could also add support for pipes
> for older windows, and other kind of handles too). I admit this is not
> as convenient as the current "exec:cmdline" form... I don't know
> whether we have enough motivation to push those changes... I see it
> fitting with the goal to make HMP a human-friendly QMP client though.

We could make the same argument against supporting the other
migration protocols too, because all can be handled by merely
passing in a pre-opened FD from outside. That is not very
friendly though, even for QMP clients. I think it is sensible
to have an exec: protocol in general as long as we bypass shell.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|