On Fri, May 12, 2023 at 02:32:37PM +0000, Het Gala wrote: > Exec transport backend for 'migrate'/'migrate-incoming' QAPIs accept > new wire protocol of MigrateAddress struct. > > It is achived by parsing 'uri' string and storing migration parameters > required for exec connection into strList struct. > > Suggested-by: Aravind Retnakaran <aravind.retnaka...@nutanix.com> > Signed-off-by: Het Gala <het.g...@nutanix.com> > --- > migration/exec.c | 58 ++++++++++++++++++++++++++++++++----------- > migration/exec.h | 4 +-- > migration/migration.c | 10 +++----- > 3 files changed, 50 insertions(+), 22 deletions(-) > > diff --git a/migration/exec.c b/migration/exec.c > index c4a3293246..210f4e9400 100644 > --- a/migration/exec.c > +++ b/migration/exec.c > @@ -39,22 +39,51 @@ const char *exec_get_cmd_path(void) > } > #endif > > -void exec_start_outgoing_migration(MigrationState *s, const char *command, > +/* provides the length of strList */ > +static int > +str_list_length(strList *list) > +{ > + int len = 0; > + strList *elem; > + > + for (elem = list; elem != NULL; elem = elem->next) { > + len++; > + } > + > + return len; > +} > + > +static void > +init_exec_array(strList *command, const char **argv, Error **errp) > +{ > + int i = 0; > + strList *lst; > + > + for (lst = command; lst; lst = lst->next) { > + argv[i++] = lst->value; > + } > + > + argv[i] = NULL; > + return; > +} > + > +void exec_start_outgoing_migration(MigrationState *s, strList *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 > + int length = str_list_length(command); > + const char **argv = g_malloc0(length * sizeof(const char *));
g_malloc0 is almost never desirable to use, instead: g_new0(const char *, length); > > - trace_migration_exec_outgoing(command); > + init_exec_array(command, argv, errp); > + char *new_command = g_strjoinv(" ", (char **)argv); Never freed - use g_autofree char *new_command... > + trace_migration_exec_outgoing(new_command); > ioc = QIO_CHANNEL(qio_channel_command_new_spawn(argv, > O_RDWR, > errp)); > if (!ioc) { > + g_free(argv); > return; > } argv needs freeing in success too. Simpler to declare it with g_auto(GStrv) argv = ..... > > @@ -72,21 +101,22 @@ static gboolean > exec_accept_incoming_migration(QIOChannel *ioc, > return G_SOURCE_REMOVE; > } > > -void exec_start_incoming_migration(const char *command, Error **errp) > +void exec_start_incoming_migration(strList *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 > + int length = str_list_length(command); > + const char **argv = g_malloc0(length * sizeof(const char *)); > + > + init_exec_array(command, argv, errp); > + char *new_command = g_strjoinv(" ", (char **)argv); > > - trace_migration_exec_incoming(command); > + trace_migration_exec_incoming(new_command); > ioc = QIO_CHANNEL(qio_channel_command_new_spawn(argv, > O_RDWR, > errp)); > if (!ioc) { > + g_free(argv); > return; > } All the same comments as the outgoing case. 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 :|