Het Gala <het.g...@nutanix.com> wrote:
> This patch introduces code that can parse 'uri' string parameter and
> spit out 'MigrateAddress' struct. All the required migration parameters
> are stored in the struct.
>
> Suggested-by: Aravind Retnakaran <aravind.retnaka...@nutanix.com>
> Signed-off-by: Het Gala <het.g...@nutanix.com>
Markus, could you comment?
> ---
> migration/migration.c | 63 +++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 61 insertions(+), 2 deletions(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index 0ee07802a5..a7e4e286aa 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -64,6 +64,7 @@
> #include "yank_functions.h"
> #include "sysemu/qtest.h"
> #include "options.h"
> +#include "qemu/sockets.h"
>
> static NotifierList migration_state_notifiers =
> NOTIFIER_LIST_INITIALIZER(migration_state_notifiers);
> @@ -408,13 +409,58 @@ void migrate_add_address(SocketAddress *address)
> QAPI_CLONE(SocketAddress, address));
> }
>
> +static bool migrate_uri_parse(const char *uri,
> + MigrateAddress **channel,
> + Error **errp)
> +{
> + Error *local_err = NULL;
Remove this.
> + MigrateAddress *addrs = g_new0(MigrateAddress, 1);
> + SocketAddress *saddr;
> + InetSocketAddress *isock = &addrs->u.rdma;
> + strList **tail = &addrs->u.exec.args;
> +
> + if (strstart(uri, "exec:", NULL)) {
> + addrs->transport = MIGRATE_TRANSPORT_EXEC;
> + QAPI_LIST_APPEND(tail, g_strdup("/bin/sh"));
> + QAPI_LIST_APPEND(tail, g_strdup("-c"));
> + QAPI_LIST_APPEND(tail, g_strdup(uri + strlen("exec:")));
> + } else if (strstart(uri, "rdma:", NULL) &&
> + !inet_parse(isock, uri + strlen("rdma:"), errp)) {
> + addrs->transport = MIGRATE_TRANSPORT_RDMA;
> + } else if (strstart(uri, "tcp:", NULL) ||
> + strstart(uri, "unix:", NULL) ||
> + strstart(uri, "vsock:", NULL) ||
> + strstart(uri, "fd:", NULL)) {
> + addrs->transport = MIGRATE_TRANSPORT_SOCKET;
> + saddr = socket_parse(uri, &local_err);
saddr = socket_parse(uri, &local_err);
if (saddr == NULL) {
qapi_free_MigrateAddress(addrs);
return false;
}
> + addrs->u.socket = *saddr;
> + }
> +
> + if (local_err) {
> + qapi_free_SocketAddress(saddr);
> + qapi_free_InetSocketAddress(isock);
> + error_propagate(errp, local_err);
> + return false;
> + }
And remove this last chunk?
Or I am missing something?
> +
> + *channel = addrs;
> + return true;
> +}
> +
> static void qemu_start_incoming_migration(const char *uri, Error **errp)
> {
> const char *p = NULL;
> + MigrateAddress *channel = g_new0(MigrateAddress, 1);
>
> /* URI is not suitable for migration? */
> if (!migration_channels_and_uri_compatible(uri, errp)) {
> - return;
> + goto out;
> + }
> +
> + if (uri && !migrate_uri_parse(uri, &channel, errp)) {
> + error_setg(errp, "Error parsing uri");
Why are we doing this error_setg? We are supposed to have a better
error assigned to errp already, no?
> + goto out;
> }
>
> qapi_event_send_migration(MIGRATION_STATUS_SETUP);
> @@ -433,6 +479,9 @@ static void qemu_start_incoming_migration(const char
> *uri, Error **errp)
> } else {
> error_setg(errp, "unknown migration protocol: %s", uri);
> }
> +
> +out:
> + qapi_free_MigrateAddress(channel);
I wish, I really wish, that there was a way to free things on error.
> }
>
> static void process_incoming_migration_bh(void *opaque)
> @@ -1638,10 +1687,16 @@ void qmp_migrate(const char *uri, bool has_blk, bool
> blk,
> Error *local_err = NULL;
> MigrationState *s = migrate_get_current();
> const char *p = NULL;
> + MigrateAddress *channel = g_new0(MigrateAddress, 1);
>
> /* URI is not suitable for migration? */
> if (!migration_channels_and_uri_compatible(uri, errp)) {
> - return;
> + goto out;
> + }
> +
> + if (!migrate_uri_parse(uri, &channel, &local_err)) {
> + error_setg(errp, "Error parsing uri");
Same here about setting errp.
> + goto out;
> }
>
> if (!migrate_prepare(s, has_blk && blk, has_inc && inc,
> @@ -1688,6 +1743,10 @@ void qmp_migrate(const char *uri, bool has_blk, bool
> blk,
> error_propagate(errp, local_err);
> return;
> }
> +
> +out:
> + qapi_free_MigrateAddress(channel);
> + return;
> }
>
> void qmp_migrate_cancel(Error **errp)
