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.st...@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 :|
