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.retnakaran@nutanix.com>
Signed-off-by: Het Gala <het.gala@nutanix.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
migration/exec.c | 71 +++++++++++++++++++++++++++++++------------
migration/exec.h | 4 +--
migration/migration.c | 10 +++---
3 files changed, 57 insertions(+), 28 deletions(-)
diff --git a/migration/exec.c b/migration/exec.c
index 32f5143dfd..8bc321c66b 100644
--- a/migration/exec.c
+++ b/migration/exec.c
@@ -39,20 +39,50 @@ const char *exec_get_cmd_path(void)
}
#endif
-void exec_start_outgoing_migration(MigrationState *s, const char *command, Error **errp)
+/* 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, 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);
+ g_auto(GStrv) argv = (char **) g_new0(const char *, length);
- trace_migration_exec_outgoing(command);
- ioc = QIO_CHANNEL(qio_channel_command_new_spawn(argv,
- O_RDWR,
- errp));
+ init_exec_array(command, argv, errp);
+ g_autofree char *new_command = g_strjoinv(" ", (char **)argv);
+
+ trace_migration_exec_outgoing(new_command);
+ ioc = QIO_CHANNEL(
+ qio_channel_command_new_spawn((const char * const *) argv,
+ O_RDWR,
+ errp));
if (!ioc) {
return;
}
@@ -71,20 +101,21 @@ 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);
+ g_auto(GStrv) argv = (char **) g_new0(const char *, length);
+
+ init_exec_array(command, argv, errp);
+ g_autofree char *new_command = g_strjoinv(" ", (char **)argv);
- trace_migration_exec_incoming(command);
- ioc = QIO_CHANNEL(qio_channel_command_new_spawn(argv,
- O_RDWR,
- errp));
+ trace_migration_exec_incoming(new_command);
+ ioc = QIO_CHANNEL(
+ qio_channel_command_new_spawn((const char * const *) argv,
+ O_RDWR,
+ errp));
if (!ioc) {
return;
}
diff --git a/migration/exec.h b/migration/exec.h
index 736cd71028..3107f205e3 100644
--- a/migration/exec.h
+++ b/migration/exec.h
@@ -23,8 +23,8 @@
#ifdef WIN32
const char *exec_get_cmd_path(void);
#endif
-void exec_start_incoming_migration(const char *host_port, Error **errp);
+void exec_start_incoming_migration(strList *host_port, Error **errp);
-void exec_start_outgoing_migration(MigrationState *s, const char *host_port,
+void exec_start_outgoing_migration(MigrationState *s, strList *host_port,
Error **errp);
#endif
diff --git a/migration/migration.c b/migration/migration.c
index b41fda6f80..ebe14b9c38 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -474,7 +474,6 @@ static bool migrate_uri_parse(const char *uri,
static void qemu_start_incoming_migration(const char *uri, Error **errp)
{
- const char *p = NULL;
g_autoptr(MigrationAddress) channel = g_new0(MigrationAddress, 1);
/* URI is not suitable for migration? */
@@ -500,8 +499,8 @@ static void qemu_start_incoming_migration(const char *uri, Error **errp)
} else if (channel->transport == MIGRATION_ADDRESS_TYPE_RDMA) {
rdma_start_incoming_migration(&channel->u.rdma, errp);
#endif
- } else if (strstart(uri, "exec:", &p)) {
- exec_start_incoming_migration(p, errp);
+ } else if (channel->transport == MIGRATION_ADDRESS_TYPE_EXEC) {
+ exec_start_incoming_migration(channel->u.exec.args, errp);
} else {
error_setg(errp, "unknown migration protocol: %s", uri);
}
@@ -1723,7 +1722,6 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
bool resume_requested;
Error *local_err = NULL;
MigrationState *s = migrate_get_current();
- const char *p = NULL;
g_autoptr(MigrationAddress) channel = g_new0(MigrationAddress, 1);
/* URI is not suitable for migration? */
@@ -1761,8 +1759,8 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
} else if (channel->transport == MIGRATION_ADDRESS_TYPE_RDMA) {
rdma_start_outgoing_migration(s, &channel->u.rdma, &local_err);
#endif
- } else if (strstart(uri, "exec:", &p)) {
- exec_start_outgoing_migration(s, p, &local_err);
+ } else if (channel->transport == MIGRATION_ADDRESS_TYPE_EXEC) {
+ exec_start_outgoing_migration(s, channel->u.exec.args, &local_err);
} else {
error_setg(&local_err, QERR_INVALID_PARAMETER_VALUE, "uri",
"a valid migration protocol");
--
2.22.3
Het Gala <het.gala@nutanix.com> writes:
> 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.retnakaran@nutanix.com>
> Signed-off-by: Het Gala <het.gala@nutanix.com>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> migration/exec.c | 71 +++++++++++++++++++++++++++++++------------
> migration/exec.h | 4 +--
> migration/migration.c | 10 +++---
> 3 files changed, 57 insertions(+), 28 deletions(-)
>
> diff --git a/migration/exec.c b/migration/exec.c
> index 32f5143dfd..8bc321c66b 100644
> --- a/migration/exec.c
> +++ b/migration/exec.c
> @@ -39,20 +39,50 @@ const char *exec_get_cmd_path(void)
> }
> #endif
>
> -void exec_start_outgoing_migration(MigrationState *s, const char *command, Error **errp)
> +/* 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, char **argv, Error **errp)
> +{
> + int i = 0;
> + strList *lst;
> +
> + for (lst = command; lst; lst = lst->next) {
> + argv[i++] = lst->value;
> + }
> +
> + argv[i] = NULL;
This will write out of bounds.
> + 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);
> + g_auto(GStrv) argv = (char **) g_new0(const char *, length);
This allocation does not leave space for the NULL byte.
>
> - trace_migration_exec_outgoing(command);
> - ioc = QIO_CHANNEL(qio_channel_command_new_spawn(argv,
> - O_RDWR,
> - errp));
> + init_exec_array(command, argv, errp);
> + g_autofree char *new_command = g_strjoinv(" ", (char **)argv);
> +
> + trace_migration_exec_outgoing(new_command);
> + ioc = QIO_CHANNEL(
> + qio_channel_command_new_spawn((const char * const *) argv,
> + O_RDWR,
> + errp));
> if (!ioc) {
> return;
> }
> @@ -71,20 +101,21 @@ 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);
> + g_auto(GStrv) argv = (char **) g_new0(const char *, length);
Here as well.
> +
> + init_exec_array(command, argv, errp);
> + g_autofree char *new_command = g_strjoinv(" ", (char **)argv);
>
> - trace_migration_exec_incoming(command);
> - ioc = QIO_CHANNEL(qio_channel_command_new_spawn(argv,
> - O_RDWR,
> - errp));
> + trace_migration_exec_incoming(new_command);
> + ioc = QIO_CHANNEL(
> + qio_channel_command_new_spawn((const char * const *) argv,
> + O_RDWR,
> + errp));
> if (!ioc) {
> return;
> }
> diff --git a/migration/exec.h b/migration/exec.h
> index 736cd71028..3107f205e3 100644
> --- a/migration/exec.h
> +++ b/migration/exec.h
> @@ -23,8 +23,8 @@
> #ifdef WIN32
> const char *exec_get_cmd_path(void);
> #endif
> -void exec_start_incoming_migration(const char *host_port, Error **errp);
> +void exec_start_incoming_migration(strList *host_port, Error **errp);
>
> -void exec_start_outgoing_migration(MigrationState *s, const char *host_port,
> +void exec_start_outgoing_migration(MigrationState *s, strList *host_port,
> Error **errp);
> #endif
> diff --git a/migration/migration.c b/migration/migration.c
> index b41fda6f80..ebe14b9c38 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -474,7 +474,6 @@ static bool migrate_uri_parse(const char *uri,
>
> static void qemu_start_incoming_migration(const char *uri, Error **errp)
> {
> - const char *p = NULL;
> g_autoptr(MigrationAddress) channel = g_new0(MigrationAddress, 1);
>
> /* URI is not suitable for migration? */
> @@ -500,8 +499,8 @@ static void qemu_start_incoming_migration(const char *uri, Error **errp)
> } else if (channel->transport == MIGRATION_ADDRESS_TYPE_RDMA) {
> rdma_start_incoming_migration(&channel->u.rdma, errp);
> #endif
> - } else if (strstart(uri, "exec:", &p)) {
> - exec_start_incoming_migration(p, errp);
> + } else if (channel->transport == MIGRATION_ADDRESS_TYPE_EXEC) {
> + exec_start_incoming_migration(channel->u.exec.args, errp);
> } else {
> error_setg(errp, "unknown migration protocol: %s", uri);
> }
> @@ -1723,7 +1722,6 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
> bool resume_requested;
> Error *local_err = NULL;
> MigrationState *s = migrate_get_current();
> - const char *p = NULL;
> g_autoptr(MigrationAddress) channel = g_new0(MigrationAddress, 1);
>
> /* URI is not suitable for migration? */
> @@ -1761,8 +1759,8 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
> } else if (channel->transport == MIGRATION_ADDRESS_TYPE_RDMA) {
> rdma_start_outgoing_migration(s, &channel->u.rdma, &local_err);
> #endif
> - } else if (strstart(uri, "exec:", &p)) {
> - exec_start_outgoing_migration(s, p, &local_err);
> + } else if (channel->transport == MIGRATION_ADDRESS_TYPE_EXEC) {
> + exec_start_outgoing_migration(s, channel->u.exec.args, &local_err);
> } else {
> error_setg(&local_err, QERR_INVALID_PARAMETER_VALUE, "uri",
> "a valid migration protocol");
On 10/4/2023 8:25 PM, Fabiano Rosas wrote:
> Het Gala<het.gala@nutanix.com> writes:
>
>> 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.retnakaran@nutanix.com>
>> Signed-off-by: Het Gala<het.gala@nutanix.com>
>> Reviewed-by: Daniel P. Berrangé<berrange@redhat.com>
>> ---
>> migration/exec.c | 71 +++++++++++++++++++++++++++++++------------
>> migration/exec.h | 4 +--
>> migration/migration.c | 10 +++---
>> 3 files changed, 57 insertions(+), 28 deletions(-)
>>
>> -void exec_start_outgoing_migration(MigrationState *s, const char *command, Error **errp)
>> +/* 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, char **argv, Error **errp)
>> +{
>> + int i = 0;
>> + strList *lst;
>> +
>> + for (lst = command; lst; lst = lst->next) {
>> + argv[i++] = lst->value;
>> + }
>> +
>> + argv[i] = NULL;
> This will write out of bounds.
Yes, will increase the length of argv in exec_start_outgoing_migration()
by one, that would solve the issue right.
>> + 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);
>> + g_auto(GStrv) argv = (char **) g_new0(const char *, length);
> This allocation does not leave space for the NULL byte.
Yes you are rigght. I assumed the user will itself have to end the list
of argument with a 'NULL', but that's not correct. Thanks for pointing
it out. Will make the length of argv from length -> length+1.
>>
>> - trace_migration_exec_outgoing(command);
>> - ioc = QIO_CHANNEL(qio_channel_command_new_spawn(argv,
>> - O_RDWR,
>> - errp));
>> + init_exec_array(command, argv, errp);
>> + g_autofree char *new_command = g_strjoinv(" ", (char **)argv);
>> +
>> + trace_migration_exec_outgoing(new_command);
>> + ioc = QIO_CHANNEL(
>> + qio_channel_command_new_spawn((const char * const *) argv,
>> + O_RDWR,
>> + errp));
>> if (!ioc) {
>> return;
>> }
>> @@ -71,20 +101,21 @@ 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);
>> + g_auto(GStrv) argv = (char **) g_new0(const char *, length);
> Here as well.
Ack, will increase length of argv by one while initalization.
Regards,
Het Gala
© 2016 - 2026 Red Hat, Inc.