[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2] Fix exec migration on Windows (w32+w64).
From: |
Daniel P . Berrangé |
Subject: |
Re: [PATCH v2] Fix exec migration on Windows (w32+w64). |
Date: |
Mon, 16 Jan 2023 09:22:15 +0000 |
User-agent: |
Mutt/2.2.9 (2022-11-12) |
On Mon, Jan 16, 2023 at 11:17:08AM +0400, Marc-André Lureau wrote:
> Hi
>
> On Mon, Jan 16, 2023 at 5:35 AM John Berberian, Jr <jeb.study@gmail.com>
> wrote:
> >
> > * Use cmd instead of /bin/sh on Windows.
> >
> > * Try to auto-detect cmd.exe's path, but default to a hard-coded path.
> >
> > Note that this will require that gspawn-win[32|64]-helper.exe and
> > gspawn-win[32|64]-helper-console.exe are included in the Windows binary
> > distributions (cc: Stefan Weil).
> >
> > Signed-off-by: "John Berberian, Jr" <jeb.study@gmail.com>
> > ---
> > Whoops, forgot a header. Here's a revised patch.
> >
> > migration/exec.c | 24 ++++++++++++++++++++++++
> > 1 file changed, 24 insertions(+)
> >
> > diff --git a/migration/exec.c b/migration/exec.c
> > index 375d2e1b54..38604d73a6 100644
> > --- a/migration/exec.c
> > +++ b/migration/exec.c
> > @@ -23,12 +23,31 @@
> > #include "migration.h"
> > #include "io/channel-command.h"
> > #include "trace.h"
> > +#include "qemu/cutils.h"
> >
> > +#ifdef WIN32
> > +const char *exec_get_cmd_path(void);
> > +const char *exec_get_cmd_path(void)
> > +{
> > + g_autofree char *detected_path = g_new(char, MAX_PATH);
> > + if (GetSystemDirectoryA(detected_path, MAX_PATH) == 0) {
> > + warn_report("Could not detect cmd.exe path, using default.");
> > + return "C:\\Windows\\System32\\cmd.exe";
> > + }
> > + pstrcat(detected_path, MAX_PATH, "\\cmd.exe");
> > + return g_steal_pointer(&detected_path);
> > +}
> > +#endif
> >
> > void exec_start_outgoing_migration(MigrationState *s, const char *command,
> > Error **errp)
> > {
> > QIOChannel *ioc;
> > +
> > +#ifdef WIN32
> > + const char *argv[] = { exec_get_cmd_path(), "/c", command, NULL };
> > +#else
> > const char *argv[] = { "/bin/sh", "-c", command, NULL };
> > +#endif
>
> It may be a better idea to use g_shell_parse_argv() instead.
For non-Windows that would not be compatible though. eg if someone is
currently using shell redirection or pipelined commands:
migrate "exec:dd of=/foo 1>/dev/null 2>&1"
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.
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 :|