On 15/05/23 3:59 pm, Daniel P. Berrangé wrote:
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;
+}
+
Juan for your comment : "I can't believe tat we have a list type and we
don't have a length() function for that type."
I had seen come patches regarding adding utility for finding length of a
strList in util.h file as a MACRO, but I think the patches have still
not gone in.
+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);
Ack. Will change that.
- 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...
Ack.
+ 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 = .....
Yes, Ack.
@@ -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.
Ack. Thanks Daniel for the inputs.
With regards,
Daniel
Regards,
Het Gala