On Mon, Dec 26, 2022 at 05:33:27AM +0000, Het Gala wrote:
> From: Author Het Gala <het.g...@nutanix.com>
>
> Existing uri is encoded at multiple levels to extract the relevant
> migration information.
>
> The modified QAPI design maps migration parameters into MigrateChannel
> struct before, thus avoiding double-level uri encoding.
>
> socket_outgoing_migration() has been depricated as It's only purpose was
> uri parsing.
> Renamed socket_outgoing_migration_internal() as socket_outgoing_migration().
> qemu_uri_parsing() has been introduced to parse uri string (backward
> compatibility) and populate the MigrateChannel struct parameters. Note that
> the function will no longer be needed once the 'uri' parameter is depricated.
>
> Suggested-by: Daniel P. Berrange <berra...@redhat.com>
> Suggested-by: Manish Mishra <manish.mis...@nutanix.com>
> Suggested-by: Aravind Retnakaran <aravind.retnaka...@nutanix.com>
> Signed-off-by: Het Gala <het.g...@nutanix.com>
> ---
> migration/migration.c | 78 +++++++++++++++++++++++++++++++++++--------
> migration/socket.c | 15 +--------
> migration/socket.h | 3 +-
> 3 files changed, 67 insertions(+), 29 deletions(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index 1b6e62612a..36de9f6a6b 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -61,6 +61,7 @@
> #include "sysemu/cpus.h"
> #include "yank_functions.h"
> #include "sysemu/qtest.h"
> +#include "qemu/sockets.h"
>
> #define MAX_THROTTLE (128 << 20) /* Migration transfer speed
> throttling */
>
> @@ -486,6 +487,39 @@ void migrate_add_address(SocketAddress *address)
> QAPI_CLONE(SocketAddress, address));
> }
>
> +static void qemu_uri_parsing(const char *uri,
> + MigrateChannel **channel,
> + Error **errp)
Coding style would prefer 'bool' instad of 'void'...
Also lets call this 'migrate_uri_parse'
> +{
> + Error *local_err = NULL;
> + const char *p = NULL;
> + MigrateChannel *val = g_new0(MigrateChannel, 1);
> + MigrateAddress *addrs = g_new0(MigrateAddress, 1);
> + SocketAddress *saddr = g_new0(SocketAddress, 1);
> +
> + if (strstart(uri, "exec:", &p)) {
> + addrs->transport = MIGRATE_TRANSPORT_EXEC;
> + addrs->u.exec.exec_str = g_strdup(p + strlen("exec:"));
> + } else if (strstart(uri, "rdma:", NULL)) {
> + addrs->transport = MIGRATE_TRANSPORT_RDMA;
> + addrs->u.rdma.rdma_str = g_strdup(p + strlen("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);
> + addrs->u.socket.socket_type = saddr;
> + }
> + val->channeltype = MIGRATE_CHANNEL_TYPE_MAIN;
> + val->addr = addrs;
> + *channel = val;
> +
> + if (local_err) {
> + error_propagate(errp, local_err);
... 'return false';
> + }
... 'return true;'
> +}
> +
> static void qemu_start_incoming_migration(const char *uri, Error **errp)
> {
> const char *p = NULL;
> @@ -2397,7 +2431,8 @@ void qmp_migrate(const char *uri, MigrateChannel
> *channel, bool has_blk,
> {
> Error *local_err = NULL;
> MigrationState *s = migrate_get_current();
> - const char *p = NULL;
> + MigrateAddress *addrs = g_new0(MigrateAddress, 1);
> + SocketAddress *saddr = g_new0(SocketAddress, 1);
>
> if (!migrate_prepare(s, has_blk && blk, has_inc && inc,
> has_resume && resume, errp)) {
> @@ -2411,20 +2446,35 @@ void qmp_migrate(const char *uri, MigrateChannel
> *channel, bool has_blk,
> }
> }
>
> + /*
> + * motive here is just to have checks and convert uri into
> + * socketaddress struct
> + */
> + if (uri && channel) {
> + error_setg(errp, "uri and channels options should be"
> + "mutually exclusive");
Needs a 'return' statement after reporting the error. ALso, this
check should be moved to the earlier patch that introduced the
'channel' field.
> + } else if (uri) {
> + qemu_uri_parsing(uri, &channel, &local_err);
Needs to 'return' on error, eg
} else if (uri && !qemu_uri_parsing(...))
return;
> + }
> +
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 :|
