[{"id":3629533,"web_url":"http://patchwork.ozlabs.org/comment/3629533/","msgid":"","list_archive_url":null,"date":"2025-12-29T21:08:26","subject":"Re: [RFC PATCH 24/25] migration: Move URI parsing to channel.c","submitter":{"id":67717,"url":"http://patchwork.ozlabs.org/api/people/67717/","name":"Peter Xu","email":"peterx@redhat.com"},"content":"On Fri, Dec 26, 2025 at 06:19:26PM -0300, Fabiano Rosas wrote:\n> The migrate_uri_parse function is responsible for converting the URI\n> string into a MigrationChannel for consumption by the rest of the\n> code. Move it to channel.c and add a wrapper that calls both URI and\n> channels parsing.\n> \n> While here, add some words about the memory management of the\n> MigrationChannel object, which is slightly different from\n> migrate_uri_parse() and migrate_channels_parse(). The former takes a\n> string and has to allocate a MigrationChannel, the latter takes a QAPI\n> object that is managed by the QAPI code.\n> \n> Signed-off-by: Fabiano Rosas \n\nThere're some comments added into migration_connect(), that I still don't\nthink I fully agree upon.. but the rest looks all good to me.\n\nThat's probably to be discussed separately anyway, I think you can take\nmine here:\n\nReviewed-by: Peter Xu \n\n> ---\n> migration/channel.c | 97 +++++++++++++++++++++++++++++++++++++++++--\n> migration/channel.h | 9 ++--\n> migration/migration.c | 95 ++----------------------------------------\n> 3 files changed, 101 insertions(+), 100 deletions(-)\n> \n> diff --git a/migration/channel.c b/migration/channel.c\n> index 8b43c3d983..d30e29c9b3 100644\n> --- a/migration/channel.c\n> +++ b/migration/channel.c\n> @@ -40,6 +40,12 @@ bool migration_connect(MigrationAddress *addr, bool out, Error **errp)\n> SocketAddress *saddr;\n> ERRP_GUARD();\n> \n> + /*\n> + * This is reached from a QMP command, the transport code below\n> + * must copy the relevant parts of 'addr' before this function\n> + * returns because the QAPI code will free it.\n> + */\n> +\n> switch (addr->transport) {\n> case MIGRATION_ADDRESS_TYPE_SOCKET:\n> saddr = &addr->u.socket;\n> @@ -318,10 +324,10 @@ int migration_channel_read_peek(QIOChannel *ioc,\n> return 0;\n> }\n> \n> -bool migrate_channels_parse(MigrationChannelList *channels,\n> - MigrationChannel **main_channelp,\n> - MigrationChannel **cpr_channelp,\n> - Error **errp)\n> +static bool migrate_channels_parse(MigrationChannelList *channels,\n> + MigrationChannel **main_channelp,\n> + MigrationChannel **cpr_channelp,\n> + Error **errp)\n> {\n> MigrationChannel *channelv[MIGRATION_CHANNEL_TYPE__MAX] = { NULL };\n> bool single_channel;\n> @@ -353,6 +359,15 @@ bool migrate_channels_parse(MigrationChannelList *channels,\n> }\n> }\n> \n> + /*\n> + * These don't technically need to be cloned because they come\n> + * from a QAPI object ('channels'). The top-level caller\n> + * (qmp_migrate) needs to copy the necessary information before\n> + * returning from the QMP command. Cloning here is just to keep\n> + * the interface consistent with migrate_uri_parse() that _does\n> + * not_ take a QAPI object and instead allocates and transfers\n> + * ownership of a MigrationChannel to qmp_migrate.\n> + */\n> if (cpr_channelp) {\n> *cpr_channelp = QAPI_CLONE(MigrationChannel,\n> channelv[MIGRATION_CHANNEL_TYPE_CPR]);\n> @@ -368,3 +383,77 @@ bool migrate_channels_parse(MigrationChannelList *channels,\n> \n> return true;\n> }\n> +\n> +bool migrate_uri_parse(const char *uri, MigrationChannel **channel,\n> + Error **errp)\n> +{\n> + g_autoptr(MigrationChannel) val = g_new0(MigrationChannel, 1);\n> + g_autoptr(MigrationAddress) addr = g_new0(MigrationAddress, 1);\n> + InetSocketAddress *isock = &addr->u.rdma;\n> + strList **tail = &addr->u.exec.args;\n> +\n> + if (strstart(uri, \"exec:\", NULL)) {\n> + addr->transport = MIGRATION_ADDRESS_TYPE_EXEC;\n> +#ifdef WIN32\n> + QAPI_LIST_APPEND(tail, g_strdup(exec_get_cmd_path()));\n> + QAPI_LIST_APPEND(tail, g_strdup(\"/c\"));\n> +#else\n> + QAPI_LIST_APPEND(tail, g_strdup(\"/bin/sh\"));\n> + QAPI_LIST_APPEND(tail, g_strdup(\"-c\"));\n> +#endif\n> + QAPI_LIST_APPEND(tail, g_strdup(uri + strlen(\"exec:\")));\n> + } else if (strstart(uri, \"rdma:\", NULL)) {\n> + if (inet_parse(isock, uri + strlen(\"rdma:\"), errp)) {\n> + qapi_free_InetSocketAddress(isock);\n> + return false;\n> + }\n> + addr->transport = MIGRATION_ADDRESS_TYPE_RDMA;\n> + } else if (strstart(uri, \"tcp:\", NULL) ||\n> + strstart(uri, \"unix:\", NULL) ||\n> + strstart(uri, \"vsock:\", NULL) ||\n> + strstart(uri, \"fd:\", NULL)) {\n> + addr->transport = MIGRATION_ADDRESS_TYPE_SOCKET;\n> + SocketAddress *saddr = socket_parse(uri, errp);\n> + if (!saddr) {\n> + return false;\n> + }\n> + addr->u.socket.type = saddr->type;\n> + addr->u.socket.u = saddr->u;\n> + /* Don't free the objects inside; their ownership moved to \"addr\" */\n> + g_free(saddr);\n> + } else if (strstart(uri, \"file:\", NULL)) {\n> + addr->transport = MIGRATION_ADDRESS_TYPE_FILE;\n> + addr->u.file.filename = g_strdup(uri + strlen(\"file:\"));\n> + if (file_parse_offset(addr->u.file.filename, &addr->u.file.offset,\n> + errp)) {\n> + return false;\n> + }\n> + } else {\n> + error_setg(errp, \"unknown migration protocol: %s\", uri);\n> + return false;\n> + }\n> +\n> + val->channel_type = MIGRATION_CHANNEL_TYPE_MAIN;\n> + val->addr = g_steal_pointer(&addr);\n> + *channel = g_steal_pointer(&val);\n> + return true;\n> +}\n> +\n> +bool migration_channel_parse_input(const char *uri,\n> + MigrationChannelList *channels,\n> + MigrationChannel **main_channelp,\n> + MigrationChannel **cpr_channelp,\n> + Error **errp)\n> +{\n> + if (!uri == !channels) {\n> + error_setg(errp, \"need either 'uri' or 'channels' argument\");\n> + return false;\n> + }\n> +\n> + if (channels) {\n> + return migrate_channels_parse(channels, main_channelp, cpr_channelp,\n> + errp);\n> + } else {\n> + return migrate_uri_parse(uri, main_channelp, errp);\n> + }\n> +}\n> diff --git a/migration/channel.h b/migration/channel.h\n> index b3276550b7..3724b0493a 100644\n> --- a/migration/channel.h\n> +++ b/migration/channel.h\n> @@ -52,8 +52,9 @@ static inline bool migration_connect_incoming(MigrationAddress *addr,\n> return migration_connect(addr, false, errp);\n> }\n> \n> -bool migrate_channels_parse(MigrationChannelList *channels,\n> - MigrationChannel **main_channelp,\n> - MigrationChannel **cpr_channelp,\n> - Error **errp);\n> +bool migration_channel_parse_input(const char *uri,\n> + MigrationChannelList *channels,\n> + MigrationChannel **main_channelp,\n> + MigrationChannel **cpr_channelp,\n> + Error **errp);\n> #endif\n> diff --git a/migration/migration.c b/migration/migration.c\n> index 6064f1e5ea..15d8459a81 100644\n> --- a/migration/migration.c\n> +++ b/migration/migration.c\n> @@ -15,7 +15,6 @@\n> \n> #include \"qemu/osdep.h\"\n> #include \"qemu/ctype.h\"\n> -#include \"qemu/cutils.h\"\n> #include \"qemu/error-report.h\"\n> #include \"qemu/main-loop.h\"\n> #include \"migration/blocker.h\"\n> @@ -659,61 +658,6 @@ bool migrate_is_uri(const char *uri)\n> return *uri == ':';\n> }\n> \n> -bool migrate_uri_parse(const char *uri, MigrationChannel **channel,\n> - Error **errp)\n> -{\n> - g_autoptr(MigrationChannel) val = g_new0(MigrationChannel, 1);\n> - g_autoptr(MigrationAddress) addr = g_new0(MigrationAddress, 1);\n> - InetSocketAddress *isock = &addr->u.rdma;\n> - strList **tail = &addr->u.exec.args;\n> -\n> - if (strstart(uri, \"exec:\", NULL)) {\n> - addr->transport = MIGRATION_ADDRESS_TYPE_EXEC;\n> -#ifdef WIN32\n> - QAPI_LIST_APPEND(tail, g_strdup(exec_get_cmd_path()));\n> - QAPI_LIST_APPEND(tail, g_strdup(\"/c\"));\n> -#else\n> - QAPI_LIST_APPEND(tail, g_strdup(\"/bin/sh\"));\n> - QAPI_LIST_APPEND(tail, g_strdup(\"-c\"));\n> -#endif\n> - QAPI_LIST_APPEND(tail, g_strdup(uri + strlen(\"exec:\")));\n> - } else if (strstart(uri, \"rdma:\", NULL)) {\n> - if (inet_parse(isock, uri + strlen(\"rdma:\"), errp)) {\n> - qapi_free_InetSocketAddress(isock);\n> - return false;\n> - }\n> - addr->transport = MIGRATION_ADDRESS_TYPE_RDMA;\n> - } else if (strstart(uri, \"tcp:\", NULL) ||\n> - strstart(uri, \"unix:\", NULL) ||\n> - strstart(uri, \"vsock:\", NULL) ||\n> - strstart(uri, \"fd:\", NULL)) {\n> - addr->transport = MIGRATION_ADDRESS_TYPE_SOCKET;\n> - SocketAddress *saddr = socket_parse(uri, errp);\n> - if (!saddr) {\n> - return false;\n> - }\n> - addr->u.socket.type = saddr->type;\n> - addr->u.socket.u = saddr->u;\n> - /* Don't free the objects inside; their ownership moved to \"addr\" */\n> - g_free(saddr);\n> - } else if (strstart(uri, \"file:\", NULL)) {\n> - addr->transport = MIGRATION_ADDRESS_TYPE_FILE;\n> - addr->u.file.filename = g_strdup(uri + strlen(\"file:\"));\n> - if (file_parse_offset(addr->u.file.filename, &addr->u.file.offset,\n> - errp)) {\n> - return false;\n> - }\n> - } else {\n> - error_setg(errp, \"unknown migration protocol: %s\", uri);\n> - return false;\n> - }\n> -\n> - val->channel_type = MIGRATION_CHANNEL_TYPE_MAIN;\n> - val->addr = g_steal_pointer(&addr);\n> - *channel = g_steal_pointer(&val);\n> - return true;\n> -}\n> -\n> static bool\n> migration_incoming_state_setup(MigrationIncomingState *mis, Error **errp)\n> {\n> @@ -744,27 +688,10 @@ static void qemu_setup_incoming_migration(const char *uri, bool has_channels,\n> g_autoptr(MigrationChannel) main_ch = NULL;\n> MigrationIncomingState *mis = migration_incoming_get_current();\n> \n> - /*\n> - * Having preliminary checks for uri and channel\n> - */\n> - if (!uri == !channels) {\n> - error_setg(errp, \"need either 'uri' or 'channels' argument\");\n> + if (!migration_channel_parse_input(uri, channels, &main_ch, NULL, errp)) {\n> return;\n> }\n> \n> - if (channels) {\n> - if (!migrate_channels_parse(channels, &main_ch, NULL, errp)) {\n> - return;\n> - }\n> - }\n> -\n> - if (uri) {\n> - /* caller uses the old URI syntax */\n> - if (!migrate_uri_parse(uri, &main_ch, errp)) {\n> - return;\n> - }\n> - }\n> -\n> /* transport mechanism not suitable for migration? */\n> if (!migration_transport_compatible(main_ch->addr, errp)) {\n> return;\n> @@ -2124,27 +2051,11 @@ void qmp_migrate(const char *uri, bool has_channels,\n> g_autoptr(MigrationChannel) main_ch = NULL;\n> g_autoptr(MigrationChannel) cpr_ch = NULL;\n> \n> - /*\n> - * Having preliminary checks for uri and channel\n> - */\n> - if (!uri == !channels) {\n> - error_setg(errp, \"need either 'uri' or 'channels' argument\");\n> + if (!migration_channel_parse_input(uri, channels, &main_ch, &cpr_ch,\n> + errp)) {\n> return;\n> }\n> \n> - if (channels) {\n> - if (!migrate_channels_parse(channels, &main_ch, &cpr_ch, errp)) {\n> - return;\n> - }\n> - }\n> -\n> - if (uri) {\n> - /* caller uses the old URI syntax */\n> - if (!migrate_uri_parse(uri, &main_ch, errp)) {\n> - return;\n> - }\n> - }\n> -\n> /* transport mechanism not suitable for migration? */\n> if (!migration_transport_compatible(main_ch->addr, errp)) {\n> return;\n> -- \n> 2.51.0\n>","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@legolas.ozlabs.org","Authentication-Results":["legolas.ozlabs.org;\n\tdkim=pass (1024-bit key;\n unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256\n header.s=mimecast20190719 header.b=RDR5UlbJ;\n\tdkim=pass (2048-bit key;\n unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256\n header.s=google header.b=mXER4qEP;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org\n (client-ip=209.51.188.17; helo=lists.gnu.org;\n envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n receiver=patchwork.ozlabs.org)"],"Received":["from lists.gnu.org (lists.gnu.org [209.51.188.17])\n\t(using TLSv1.2 with cipher ECDHE-ECDSA-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4dg83l1xn4z1xqH\n\tfor ; Tue, 30 Dec 2025 08:09:23 +1100 (AEDT)","from localhost ([::1] helo=lists1p.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.90_1)\n\t(envelope-from )\n\tid 1vaKU3-0007sU-13; Mon, 29 Dec 2025 16:08:55 -0500","from eggs.gnu.org ([2001:470:142:3::10])\n by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)\n (Exim 4.90_1) (envelope-from ) id 1vaKTm-0007rR-GL\n for qemu-devel@nongnu.org; Mon, 29 Dec 2025 16:08:42 -0500","from us-smtp-delivery-124.mimecast.com ([170.10.133.124])\n by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)\n (Exim 4.90_1) (envelope-from ) id 1vaKTi-0007Jn-Bw\n for qemu-devel@nongnu.org; Mon, 29 Dec 2025 16:08:36 -0500","from mail-qk1-f197.google.com (mail-qk1-f197.google.com\n [209.85.222.197]) by relay.mimecast.com with ESMTP with STARTTLS\n (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n us-mta-638-6FaZLs3YOoqGDzImyGlMGg-1; Mon, 29 Dec 2025 16:08:29 -0500","by mail-qk1-f197.google.com with SMTP id\n af79cd13be357-8b245c49d0cso2273252185a.3\n for ; Mon, 29 Dec 2025 13:08:29 -0800 (PST)","from x1.local ([142.188.210.156]) by smtp.gmail.com with ESMTPSA id\n af79cd13be357-8c0968935b5sm2457058785a.19.2025.12.29.13.08.26\n (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n Mon, 29 Dec 2025 13:08:27 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n s=mimecast20190719; t=1767042510;\n h=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n to:to:cc:cc:mime-version:mime-version:content-type:content-type:\n in-reply-to:in-reply-to:references:references;\n bh=hjxSvEhUEgwirgYPIkzu3ItMTRzCZGDgbDzqeZBBdqk=;\n b=RDR5UlbJZsI6XypHZyvGi+mRC8+8SMNdJVhd74hVajO8JrIbxYfKPROy1ZwRQ8vq1eTKbz\n VpKR8/Jarpd/Z30qUjo6NXZizhc8APDvdsXmhxCuTFjmCh7oqLNCRAek7+qBxzcBYcrIeQ\n YiPI9FiGfRE9XYL8lRkmbz2Udhh9Zjk=","v=1; a=rsa-sha256; c=relaxed/relaxed;\n d=redhat.com; s=google; t=1767042508; x=1767647308; darn=nongnu.org;\n h=in-reply-to:content-disposition:mime-version:references:message-id\n :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to;\n bh=hjxSvEhUEgwirgYPIkzu3ItMTRzCZGDgbDzqeZBBdqk=;\n b=mXER4qEPl7zttwz1CMvxg6Dwepq7PTG3tH3eNDhmR2cijf8LBnZlAFUhyiYt8dLKzM\n 1QcD27+0GMLKAEMQ6xH5E90NscNrKLtFGzYOmM4ojpi9XS1aw4z4ei/0aIoC+OWnoVU2\n 5UzeasnXhYhUzg4wnMqbm3lI8pBnHEJUq0HSjb20lJ6uN75LORE9JrauogTlIMBAlQez\n PZW4DBAsBcDuPqmFBY6/HLthduxLqs4MwYEIlGX1CL/bfgZQfJaLBm1YIoX47LbHaLPA\n pfac7aI9w8Khk7fkweVMVofeUyCXAe2MNYW6TIxjbNG+u5kJYsQFt24BAPyVPDamCpF4\n Aj2Q=="],"X-MC-Unique":"6FaZLs3YOoqGDzImyGlMGg-1","X-Mimecast-MFC-AGG-ID":"6FaZLs3YOoqGDzImyGlMGg_1767042508","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n d=1e100.net; s=20230601; t=1767042508; x=1767647308;\n h=in-reply-to:content-disposition:mime-version:references:message-id\n :subject:cc:to:from:date:x-gm-gg:x-gm-message-state:from:to:cc\n :subject:date:message-id:reply-to;\n bh=hjxSvEhUEgwirgYPIkzu3ItMTRzCZGDgbDzqeZBBdqk=;\n b=ubDoPSM5oDNshRpwVjowhUE4VVrMjG1cfe6qLgzIHl7iahUoIT/QvCAfeoiDaOwHAj\n NaJrGWVjhfnROO/paAMoNbElM2RK+JkF+/eO/sVWMHip75z0x2/hakqlkX84W+ohPcmU\n j/ZbfWchFwE3ulF0h2dZMvojX9ALEB7JjL/oXrsqSWi0hmgSAPH3EzQpiwwQA5Ap20xJ\n CdJUBXi6TZWcll19u/gNCCGg5ZPWZw2WbcLox2u1nYyZTlGFZxP9owvrMLKVXZwrFY7t\n LjEJXe/yeT4vrtIF7pSlIcmCOPgyXShe0lRCJLxGB9UNpAK/lpFQY1s17mSUCoSDqL+a\n g6Ag==","X-Gm-Message-State":"AOJu0YwjJV3whVgW1fYNxYyagGbkUbY3fIMHQUF8UfJD5i7kHfl2lx9V\n +rpURClUJSkBFvMeieZvfoPUGSAtKgOWLJnE4HiTOXoNagEKGDFcQRJakRZIHTnVAniAmATkDaF\n LleJn8ZiMweumUfiIdQ33g3HSNxey5GykgjOZvawpJIO8AmFSzDuiTnCGl86cpkCI","X-Gm-Gg":"AY/fxX4IkIXbKHyFhUTAvcaioOqfNgCzNHUAPtzyzWcsZYvseg7EKpHUx/BHOVNbyQg\n AktchAdU5cRT+KbfPslbayYXhUkgjmkRhjQVd5Y0OSATPLiX9fczFi1PBB+qpUkr2sIb7CbajEz\n MTtYzGtEV7ihz28PYIMy3zbX8kIkZhbGDf1kK9PnN9edQfeujbCLXxLpTxLgN9T+TRkC3qGLxq+\n 1TPy/k+1wimihm9Tdt7VWY3wNBsTOE/DaJULt7PIpf15YrM1WtVJP5Ly0F+HKRPhhNpv+0RHjvb\n TnYnbubvBsSt4ABwVcLZA3FGZskecFR0SqnODmoTyau+k1q80iuJdLJEWzUJ5hQGOET/Yp30Mwo\n KSQQ=","X-Received":["by 2002:a05:620a:4495:b0:8b2:e4f7:bfc9 with SMTP id\n af79cd13be357-8c08f68d65cmr4804802785a.39.1767042508062;\n Mon, 29 Dec 2025 13:08:28 -0800 (PST)","by 2002:a05:620a:4495:b0:8b2:e4f7:bfc9 with SMTP id\n af79cd13be357-8c08f68d65cmr4804799785a.39.1767042507606;\n Mon, 29 Dec 2025 13:08:27 -0800 (PST)"],"X-Google-Smtp-Source":"\n AGHT+IFFwedXBagVwcXhRGmbk2u8V99G0kjcdgs9q5DQ31/HhsR8Pj3OYrS5xudTqfxy5oa5k1dvnw==","Date":"Mon, 29 Dec 2025 16:08:26 -0500","From":"Peter Xu ","To":"Fabiano Rosas ","Cc":"qemu-devel@nongnu.org","Subject":"Re: [RFC PATCH 24/25] migration: Move URI parsing to channel.c","Message-ID":"","References":"<20251226211930.27565-1-farosas@suse.de>\n <20251226211930.27565-25-farosas@suse.de>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20251226211930.27565-25-farosas@suse.de>","Received-SPF":"pass client-ip=170.10.133.124; envelope-from=peterx@redhat.com;\n helo=us-smtp-delivery-124.mimecast.com","X-Spam_score_int":"-20","X-Spam_score":"-2.1","X-Spam_bar":"--","X-Spam_report":"(-2.1 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001,\n DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1,\n RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001,\n RCVD_IN_VALIDITY_RPBL_BLOCKED=0.001, RCVD_IN_VALIDITY_SAFE_BLOCKED=0.001,\n SPF_HELO_PASS=-0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no","X-Spam_action":"no action","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n ","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n ","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org"}},{"id":3629539,"web_url":"http://patchwork.ozlabs.org/comment/3629539/","msgid":"<87344t7zlv.fsf@suse.de>","list_archive_url":null,"date":"2025-12-29T21:22:04","subject":"Re: [RFC PATCH 24/25] migration: Move URI parsing to channel.c","submitter":{"id":85343,"url":"http://patchwork.ozlabs.org/api/people/85343/","name":"Fabiano Rosas","email":"farosas@suse.de"},"content":"Peter Xu writes:\n\n> On Fri, Dec 26, 2025 at 06:19:26PM -0300, Fabiano Rosas wrote:\n>> The migrate_uri_parse function is responsible for converting the URI\n>> string into a MigrationChannel for consumption by the rest of the\n>> code. Move it to channel.c and add a wrapper that calls both URI and\n>> channels parsing.\n>> \n>> While here, add some words about the memory management of the\n>> MigrationChannel object, which is slightly different from\n>> migrate_uri_parse() and migrate_channels_parse(). The former takes a\n>> string and has to allocate a MigrationChannel, the latter takes a QAPI\n>> object that is managed by the QAPI code.\n>> \n>> Signed-off-by: Fabiano Rosas \n>\n> There're some comments added into migration_connect(), that I still don't\n> think I fully agree upon.. but the rest looks all good to me.\n>\n\nYou're talking about the comments not being at the right place? I can\nduplicate them in migration_connect_outgoing|incoming.\n\n> That's probably to be discussed separately anyway, I think you can take\n> mine here:\n>\n> Reviewed-by: Peter Xu \n>\n>> ---\n>> migration/channel.c | 97 +++++++++++++++++++++++++++++++++++++++++--\n>> migration/channel.h | 9 ++--\n>> migration/migration.c | 95 ++----------------------------------------\n>> 3 files changed, 101 insertions(+), 100 deletions(-)\n>> \n>> diff --git a/migration/channel.c b/migration/channel.c\n>> index 8b43c3d983..d30e29c9b3 100644\n>> --- a/migration/channel.c\n>> +++ b/migration/channel.c\n>> @@ -40,6 +40,12 @@ bool migration_connect(MigrationAddress *addr, bool out, Error **errp)\n>> SocketAddress *saddr;\n>> ERRP_GUARD();\n>> \n>> + /*\n>> + * This is reached from a QMP command, the transport code below\n>> + * must copy the relevant parts of 'addr' before this function\n>> + * returns because the QAPI code will free it.\n>> + */\n>> +\n>> switch (addr->transport) {\n>> case MIGRATION_ADDRESS_TYPE_SOCKET:\n>> saddr = &addr->u.socket;\n>> @@ -318,10 +324,10 @@ int migration_channel_read_peek(QIOChannel *ioc,\n>> return 0;\n>> }\n>> \n>> -bool migrate_channels_parse(MigrationChannelList *channels,\n>> - MigrationChannel **main_channelp,\n>> - MigrationChannel **cpr_channelp,\n>> - Error **errp)\n>> +static bool migrate_channels_parse(MigrationChannelList *channels,\n>> + MigrationChannel **main_channelp,\n>> + MigrationChannel **cpr_channelp,\n>> + Error **errp)\n>> {\n>> MigrationChannel *channelv[MIGRATION_CHANNEL_TYPE__MAX] = { NULL };\n>> bool single_channel;\n>> @@ -353,6 +359,15 @@ bool migrate_channels_parse(MigrationChannelList *channels,\n>> }\n>> }\n>> \n>> + /*\n>> + * These don't technically need to be cloned because they come\n>> + * from a QAPI object ('channels'). The top-level caller\n>> + * (qmp_migrate) needs to copy the necessary information before\n>> + * returning from the QMP command. Cloning here is just to keep\n>> + * the interface consistent with migrate_uri_parse() that _does\n>> + * not_ take a QAPI object and instead allocates and transfers\n>> + * ownership of a MigrationChannel to qmp_migrate.\n>> + */\n>> if (cpr_channelp) {\n>> *cpr_channelp = QAPI_CLONE(MigrationChannel,\n>> channelv[MIGRATION_CHANNEL_TYPE_CPR]);\n>> @@ -368,3 +383,77 @@ bool migrate_channels_parse(MigrationChannelList *channels,\n>> \n>> return true;\n>> }\n>> +\n>> +bool migrate_uri_parse(const char *uri, MigrationChannel **channel,\n>> + Error **errp)\n>> +{\n>> + g_autoptr(MigrationChannel) val = g_new0(MigrationChannel, 1);\n>> + g_autoptr(MigrationAddress) addr = g_new0(MigrationAddress, 1);\n>> + InetSocketAddress *isock = &addr->u.rdma;\n>> + strList **tail = &addr->u.exec.args;\n>> +\n>> + if (strstart(uri, \"exec:\", NULL)) {\n>> + addr->transport = MIGRATION_ADDRESS_TYPE_EXEC;\n>> +#ifdef WIN32\n>> + QAPI_LIST_APPEND(tail, g_strdup(exec_get_cmd_path()));\n>> + QAPI_LIST_APPEND(tail, g_strdup(\"/c\"));\n>> +#else\n>> + QAPI_LIST_APPEND(tail, g_strdup(\"/bin/sh\"));\n>> + QAPI_LIST_APPEND(tail, g_strdup(\"-c\"));\n>> +#endif\n>> + QAPI_LIST_APPEND(tail, g_strdup(uri + strlen(\"exec:\")));\n>> + } else if (strstart(uri, \"rdma:\", NULL)) {\n>> + if (inet_parse(isock, uri + strlen(\"rdma:\"), errp)) {\n>> + qapi_free_InetSocketAddress(isock);\n>> + return false;\n>> + }\n>> + addr->transport = MIGRATION_ADDRESS_TYPE_RDMA;\n>> + } else if (strstart(uri, \"tcp:\", NULL) ||\n>> + strstart(uri, \"unix:\", NULL) ||\n>> + strstart(uri, \"vsock:\", NULL) ||\n>> + strstart(uri, \"fd:\", NULL)) {\n>> + addr->transport = MIGRATION_ADDRESS_TYPE_SOCKET;\n>> + SocketAddress *saddr = socket_parse(uri, errp);\n>> + if (!saddr) {\n>> + return false;\n>> + }\n>> + addr->u.socket.type = saddr->type;\n>> + addr->u.socket.u = saddr->u;\n>> + /* Don't free the objects inside; their ownership moved to \"addr\" */\n>> + g_free(saddr);\n>> + } else if (strstart(uri, \"file:\", NULL)) {\n>> + addr->transport = MIGRATION_ADDRESS_TYPE_FILE;\n>> + addr->u.file.filename = g_strdup(uri + strlen(\"file:\"));\n>> + if (file_parse_offset(addr->u.file.filename, &addr->u.file.offset,\n>> + errp)) {\n>> + return false;\n>> + }\n>> + } else {\n>> + error_setg(errp, \"unknown migration protocol: %s\", uri);\n>> + return false;\n>> + }\n>> +\n>> + val->channel_type = MIGRATION_CHANNEL_TYPE_MAIN;\n>> + val->addr = g_steal_pointer(&addr);\n>> + *channel = g_steal_pointer(&val);\n>> + return true;\n>> +}\n>> +\n>> +bool migration_channel_parse_input(const char *uri,\n>> + MigrationChannelList *channels,\n>> + MigrationChannel **main_channelp,\n>> + MigrationChannel **cpr_channelp,\n>> + Error **errp)\n>> +{\n>> + if (!uri == !channels) {\n>> + error_setg(errp, \"need either 'uri' or 'channels' argument\");\n>> + return false;\n>> + }\n>> +\n>> + if (channels) {\n>> + return migrate_channels_parse(channels, main_channelp, cpr_channelp,\n>> + errp);\n>> + } else {\n>> + return migrate_uri_parse(uri, main_channelp, errp);\n>> + }\n>> +}\n>> diff --git a/migration/channel.h b/migration/channel.h\n>> index b3276550b7..3724b0493a 100644\n>> --- a/migration/channel.h\n>> +++ b/migration/channel.h\n>> @@ -52,8 +52,9 @@ static inline bool migration_connect_incoming(MigrationAddress *addr,\n>> return migration_connect(addr, false, errp);\n>> }\n>> \n>> -bool migrate_channels_parse(MigrationChannelList *channels,\n>> - MigrationChannel **main_channelp,\n>> - MigrationChannel **cpr_channelp,\n>> - Error **errp);\n>> +bool migration_channel_parse_input(const char *uri,\n>> + MigrationChannelList *channels,\n>> + MigrationChannel **main_channelp,\n>> + MigrationChannel **cpr_channelp,\n>> + Error **errp);\n>> #endif\n>> diff --git a/migration/migration.c b/migration/migration.c\n>> index 6064f1e5ea..15d8459a81 100644\n>> --- a/migration/migration.c\n>> +++ b/migration/migration.c\n>> @@ -15,7 +15,6 @@\n>> \n>> #include \"qemu/osdep.h\"\n>> #include \"qemu/ctype.h\"\n>> -#include \"qemu/cutils.h\"\n>> #include \"qemu/error-report.h\"\n>> #include \"qemu/main-loop.h\"\n>> #include \"migration/blocker.h\"\n>> @@ -659,61 +658,6 @@ bool migrate_is_uri(const char *uri)\n>> return *uri == ':';\n>> }\n>> \n>> -bool migrate_uri_parse(const char *uri, MigrationChannel **channel,\n>> - Error **errp)\n>> -{\n>> - g_autoptr(MigrationChannel) val = g_new0(MigrationChannel, 1);\n>> - g_autoptr(MigrationAddress) addr = g_new0(MigrationAddress, 1);\n>> - InetSocketAddress *isock = &addr->u.rdma;\n>> - strList **tail = &addr->u.exec.args;\n>> -\n>> - if (strstart(uri, \"exec:\", NULL)) {\n>> - addr->transport = MIGRATION_ADDRESS_TYPE_EXEC;\n>> -#ifdef WIN32\n>> - QAPI_LIST_APPEND(tail, g_strdup(exec_get_cmd_path()));\n>> - QAPI_LIST_APPEND(tail, g_strdup(\"/c\"));\n>> -#else\n>> - QAPI_LIST_APPEND(tail, g_strdup(\"/bin/sh\"));\n>> - QAPI_LIST_APPEND(tail, g_strdup(\"-c\"));\n>> -#endif\n>> - QAPI_LIST_APPEND(tail, g_strdup(uri + strlen(\"exec:\")));\n>> - } else if (strstart(uri, \"rdma:\", NULL)) {\n>> - if (inet_parse(isock, uri + strlen(\"rdma:\"), errp)) {\n>> - qapi_free_InetSocketAddress(isock);\n>> - return false;\n>> - }\n>> - addr->transport = MIGRATION_ADDRESS_TYPE_RDMA;\n>> - } else if (strstart(uri, \"tcp:\", NULL) ||\n>> - strstart(uri, \"unix:\", NULL) ||\n>> - strstart(uri, \"vsock:\", NULL) ||\n>> - strstart(uri, \"fd:\", NULL)) {\n>> - addr->transport = MIGRATION_ADDRESS_TYPE_SOCKET;\n>> - SocketAddress *saddr = socket_parse(uri, errp);\n>> - if (!saddr) {\n>> - return false;\n>> - }\n>> - addr->u.socket.type = saddr->type;\n>> - addr->u.socket.u = saddr->u;\n>> - /* Don't free the objects inside; their ownership moved to \"addr\" */\n>> - g_free(saddr);\n>> - } else if (strstart(uri, \"file:\", NULL)) {\n>> - addr->transport = MIGRATION_ADDRESS_TYPE_FILE;\n>> - addr->u.file.filename = g_strdup(uri + strlen(\"file:\"));\n>> - if (file_parse_offset(addr->u.file.filename, &addr->u.file.offset,\n>> - errp)) {\n>> - return false;\n>> - }\n>> - } else {\n>> - error_setg(errp, \"unknown migration protocol: %s\", uri);\n>> - return false;\n>> - }\n>> -\n>> - val->channel_type = MIGRATION_CHANNEL_TYPE_MAIN;\n>> - val->addr = g_steal_pointer(&addr);\n>> - *channel = g_steal_pointer(&val);\n>> - return true;\n>> -}\n>> -\n>> static bool\n>> migration_incoming_state_setup(MigrationIncomingState *mis, Error **errp)\n>> {\n>> @@ -744,27 +688,10 @@ static void qemu_setup_incoming_migration(const char *uri, bool has_channels,\n>> g_autoptr(MigrationChannel) main_ch = NULL;\n>> MigrationIncomingState *mis = migration_incoming_get_current();\n>> \n>> - /*\n>> - * Having preliminary checks for uri and channel\n>> - */\n>> - if (!uri == !channels) {\n>> - error_setg(errp, \"need either 'uri' or 'channels' argument\");\n>> + if (!migration_channel_parse_input(uri, channels, &main_ch, NULL, errp)) {\n>> return;\n>> }\n>> \n>> - if (channels) {\n>> - if (!migrate_channels_parse(channels, &main_ch, NULL, errp)) {\n>> - return;\n>> - }\n>> - }\n>> -\n>> - if (uri) {\n>> - /* caller uses the old URI syntax */\n>> - if (!migrate_uri_parse(uri, &main_ch, errp)) {\n>> - return;\n>> - }\n>> - }\n>> -\n>> /* transport mechanism not suitable for migration? */\n>> if (!migration_transport_compatible(main_ch->addr, errp)) {\n>> return;\n>> @@ -2124,27 +2051,11 @@ void qmp_migrate(const char *uri, bool has_channels,\n>> g_autoptr(MigrationChannel) main_ch = NULL;\n>> g_autoptr(MigrationChannel) cpr_ch = NULL;\n>> \n>> - /*\n>> - * Having preliminary checks for uri and channel\n>> - */\n>> - if (!uri == !channels) {\n>> - error_setg(errp, \"need either 'uri' or 'channels' argument\");\n>> + if (!migration_channel_parse_input(uri, channels, &main_ch, &cpr_ch,\n>> + errp)) {\n>> return;\n>> }\n>> \n>> - if (channels) {\n>> - if (!migrate_channels_parse(channels, &main_ch, &cpr_ch, errp)) {\n>> - return;\n>> - }\n>> - }\n>> -\n>> - if (uri) {\n>> - /* caller uses the old URI syntax */\n>> - if (!migrate_uri_parse(uri, &main_ch, errp)) {\n>> - return;\n>> - }\n>> - }\n>> -\n>> /* transport mechanism not suitable for migration? */\n>> if (!migration_transport_compatible(main_ch->addr, errp)) {\n>> return;\n>> -- \n>> 2.51.0\n>>","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@legolas.ozlabs.org","Authentication-Results":["legolas.ozlabs.org;\n\tdkim=pass (1024-bit key;\n unprotected) header.d=suse.de header.i=@suse.de header.a=rsa-sha256\n header.s=susede2_rsa header.b=wuhQBwYF;\n\tdkim=pass header.d=suse.de header.i=@suse.de header.a=ed25519-sha256\n header.s=susede2_ed25519 header.b=ZYGVQq0c;\n\tdkim=pass (1024-bit key) header.d=suse.de header.i=@suse.de\n header.a=rsa-sha256 header.s=susede2_rsa header.b=wuhQBwYF;\n\tdkim=neutral header.d=suse.de header.i=@suse.de header.a=ed25519-sha256\n header.s=susede2_ed25519 header.b=ZYGVQq0c;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org\n (client-ip=209.51.188.17; helo=lists.gnu.org;\n envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n receiver=patchwork.ozlabs.org)","smtp-out2.suse.de;\n\tnone"],"Received":["from lists.gnu.org (lists.gnu.org [209.51.188.17])\n\t(using TLSv1.2 with cipher ECDHE-ECDSA-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4dg8Lq2ql2z1xqH\n\tfor ; Tue, 30 Dec 2025 08:22:27 +1100 (AEDT)","from localhost ([::1] helo=lists1p.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.90_1)\n\t(envelope-from )\n\tid 1vaKgv-00047A-BD; Mon, 29 Dec 2025 16:22:13 -0500","from eggs.gnu.org ([2001:470:142:3::10])\n by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)\n (Exim 4.90_1) (envelope-from ) id 1vaKgu-000461-1g\n for qemu-devel@nongnu.org; Mon, 29 Dec 2025 16:22:12 -0500","from smtp-out2.suse.de ([195.135.223.131])\n by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128)\n (Exim 4.90_1) (envelope-from ) id 1vaKgr-0000fP-Pd\n for qemu-devel@nongnu.org; Mon, 29 Dec 2025 16:22:11 -0500","from imap1.dmz-prg2.suse.org (unknown [10.150.64.97])\n (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest\n SHA256)\n (No client certificate requested)\n by smtp-out2.suse.de (Postfix) with ESMTPS id 5CFE95BCD3;\n Mon, 29 Dec 2025 21:22:08 +0000 (UTC)","from imap1.dmz-prg2.suse.org (localhost [127.0.0.1])\n (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest\n SHA256)\n (No client certificate requested)\n by imap1.dmz-prg2.suse.org (Postfix) with ESMTPS id BF719137C3;\n Mon, 29 Dec 2025 21:22:07 +0000 (UTC)","from dovecot-director2.suse.de ([2a07:de40:b281:106:10:150:64:167])\n by imap1.dmz-prg2.suse.org with ESMTPSA id IUzZH//wUmnCZQAAD6G6ig\n (envelope-from ); Mon, 29 Dec 2025 21:22:07 +0000"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de;\n s=susede2_rsa;\n t=1767043328;\n h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc:\n mime-version:mime-version:content-type:content-type:\n in-reply-to:in-reply-to:references:references;\n bh=iQFBJXUUhTlANDTeSnhA2qJdD/ZEjGxEpPv48HVj3MU=;\n b=wuhQBwYFVPUIxIQxr0TP5mhQAd8KP+qx4Q/TJHbrtbe4WMbjQl2hzwRZN6PE2sPnbAgl+f\n ZTFal9aBG84fpoGURidqyVbAjFmWbZTC9IsCHO4P+F0pZW0UU+sZ8o6/ljvPe6Ath+p+tc\n A6laJHCNsqLfgP7bWq9RWeXDmetgStg=","v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de;\n s=susede2_ed25519; t=1767043328;\n h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc:\n mime-version:mime-version:content-type:content-type:\n in-reply-to:in-reply-to:references:references;\n bh=iQFBJXUUhTlANDTeSnhA2qJdD/ZEjGxEpPv48HVj3MU=;\n b=ZYGVQq0cXu9S1kWnP8JZM59ZW6pUSKXNucu4JIoG3RMVtX2sv+WnS5s1zDqo9Zvae1qcdb\n CUtoiyETHCSjS5Dg==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de;\n s=susede2_rsa;\n t=1767043328;\n h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc:\n mime-version:mime-version:content-type:content-type:\n in-reply-to:in-reply-to:references:references;\n bh=iQFBJXUUhTlANDTeSnhA2qJdD/ZEjGxEpPv48HVj3MU=;\n b=wuhQBwYFVPUIxIQxr0TP5mhQAd8KP+qx4Q/TJHbrtbe4WMbjQl2hzwRZN6PE2sPnbAgl+f\n ZTFal9aBG84fpoGURidqyVbAjFmWbZTC9IsCHO4P+F0pZW0UU+sZ8o6/ljvPe6Ath+p+tc\n A6laJHCNsqLfgP7bWq9RWeXDmetgStg=","v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de;\n s=susede2_ed25519; t=1767043328;\n h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc:\n mime-version:mime-version:content-type:content-type:\n in-reply-to:in-reply-to:references:references;\n bh=iQFBJXUUhTlANDTeSnhA2qJdD/ZEjGxEpPv48HVj3MU=;\n b=ZYGVQq0cXu9S1kWnP8JZM59ZW6pUSKXNucu4JIoG3RMVtX2sv+WnS5s1zDqo9Zvae1qcdb\n CUtoiyETHCSjS5Dg=="],"From":"Fabiano Rosas ","To":"Peter Xu ","Cc":"qemu-devel@nongnu.org","Subject":"Re: [RFC PATCH 24/25] migration: Move URI parsing to channel.c","In-Reply-To":"","References":"<20251226211930.27565-1-farosas@suse.de>\n <20251226211930.27565-25-farosas@suse.de> ","Date":"Mon, 29 Dec 2025 18:22:04 -0300","Message-ID":"<87344t7zlv.fsf@suse.de>","MIME-Version":"1.0","Content-Type":"text/plain","X-Spam-Score":"-4.30","X-Spamd-Result":"default: False [-4.30 / 50.00]; BAYES_HAM(-3.00)[100.00%];\n NEURAL_HAM_LONG(-1.00)[-1.000];\n NEURAL_HAM_SHORT(-0.20)[-0.992]; MIME_GOOD(-0.10)[text/plain];\n FROM_HAS_DN(0.00)[];\n URIBL_BLOCKED(0.00)[suse.de:email,suse.de:mid,imap1.dmz-prg2.suse.org:helo];\n DKIM_SIGNED(0.00)[suse.de:s=susede2_rsa,suse.de:s=susede2_ed25519];\n FUZZY_RATELIMITED(0.00)[rspamd.com]; ARC_NA(0.00)[];\n RCPT_COUNT_TWO(0.00)[2]; MIME_TRACE(0.00)[0:+];\n TO_MATCH_ENVRCPT_ALL(0.00)[]; RCVD_TLS_ALL(0.00)[];\n MISSING_XM_UA(0.00)[]; FROM_EQ_ENVFROM(0.00)[];\n TO_DN_SOME(0.00)[]; RCVD_COUNT_TWO(0.00)[2];\n RCVD_VIA_SMTP_AUTH(0.00)[]; MID_RHS_MATCH_FROM(0.00)[];\n DBL_BLOCKED_OPENRESOLVER(0.00)[suse.de:email, suse.de:mid,\n imap1.dmz-prg2.suse.org:helo]","Received-SPF":"pass client-ip=195.135.223.131; envelope-from=farosas@suse.de;\n helo=smtp-out2.suse.de","X-Spam_score_int":"-43","X-Spam_score":"-4.4","X-Spam_bar":"----","X-Spam_report":"(-4.4 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1,\n DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1,\n RCVD_IN_DNSWL_MED=-2.3, RCVD_IN_VALIDITY_RPBL_BLOCKED=0.001,\n RCVD_IN_VALIDITY_SAFE_BLOCKED=0.001, SPF_HELO_NONE=0.001,\n SPF_PASS=-0.001 autolearn=ham autolearn_force=no","X-Spam_action":"no action","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n ","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n ","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org"}},{"id":3629544,"web_url":"http://patchwork.ozlabs.org/comment/3629544/","msgid":"","list_archive_url":null,"date":"2025-12-29T22:11:18","subject":"Re: [RFC PATCH 24/25] migration: Move URI parsing to channel.c","submitter":{"id":67717,"url":"http://patchwork.ozlabs.org/api/people/67717/","name":"Peter Xu","email":"peterx@redhat.com"},"content":"On Mon, Dec 29, 2025 at 06:22:04PM -0300, Fabiano Rosas wrote:\n> You're talking about the comments not being at the right place? I can\n> duplicate them in migration_connect_outgoing|incoming.\n\nYep, dup it is fine, or just drop them?\n\nNormally we should assume all parameters to be freed anytime by default for\na function. AFAIU that's the common case.\n\nOTOH, IMHO we may better need comments where the function can transit\nownership of the memory of the parameters... while this is not the case.","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@legolas.ozlabs.org","Authentication-Results":["legolas.ozlabs.org;\n\tdkim=pass (1024-bit key;\n unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256\n header.s=mimecast20190719 header.b=in8p8+bX;\n\tdkim=pass (2048-bit key;\n unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256\n header.s=google header.b=MiYaEGYi;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org\n (client-ip=209.51.188.17; helo=lists.gnu.org;\n envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n receiver=patchwork.ozlabs.org)"],"Received":["from lists.gnu.org (lists.gnu.org [209.51.188.17])\n\t(using TLSv1.2 with cipher ECDHE-ECDSA-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4dg9Rl3l30z1xpV\n\tfor ; Tue, 30 Dec 2025 09:11:47 +1100 (AEDT)","from localhost ([::1] helo=lists1p.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.90_1)\n\t(envelope-from )\n\tid 1vaLSa-0008Kg-7I; Mon, 29 Dec 2025 17:11:31 -0500","from eggs.gnu.org ([2001:470:142:3::10])\n by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)\n (Exim 4.90_1) (envelope-from ) id 1vaLSY-0008KD-0v\n for qemu-devel@nongnu.org; Mon, 29 Dec 2025 17:11:26 -0500","from us-smtp-delivery-124.mimecast.com ([170.10.129.124])\n by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)\n (Exim 4.90_1) (envelope-from ) id 1vaLSW-0001uq-Gn\n for qemu-devel@nongnu.org; Mon, 29 Dec 2025 17:11:25 -0500","from mail-qk1-f200.google.com (mail-qk1-f200.google.com\n [209.85.222.200]) by relay.mimecast.com with ESMTP with STARTTLS\n (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n us-mta-571-yozoMwf6Oa2Bgd0FSJVMDA-1; Mon, 29 Dec 2025 17:11:21 -0500","by mail-qk1-f200.google.com with SMTP id\n af79cd13be357-8b2f0be2cf0so3521812685a.0\n for ; Mon, 29 Dec 2025 14:11:21 -0800 (PST)","from x1.local ([142.188.210.156]) by smtp.gmail.com with ESMTPSA id\n af79cd13be357-8c0968913ccsm2525860885a.16.2025.12.29.14.11.19\n (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n Mon, 29 Dec 2025 14:11:19 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n s=mimecast20190719; t=1767046283;\n h=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n to:to:cc:cc:mime-version:mime-version:content-type:content-type:\n in-reply-to:in-reply-to:references:references;\n bh=abQIuGVnkPQl727MqNYmLNB8lMqv7+ASsf/FWIKMjmE=;\n b=in8p8+bXhRRcPSQnWFevPKSuigfKeJ+jtw1cUgBmSfZp/lR1mupFamqOBaBLOT6g6Tc8vu\n 0ueqL2KCGuYSTN0duWlpC0eUiQVTuofPzs101mWO2jewQGqMSJhv44kH8q27BGAS5q2/RL\n 9NvY6e4w7jzjRdqiXgc473TiKq8xapY=","v=1; a=rsa-sha256; c=relaxed/relaxed;\n d=redhat.com; s=google; t=1767046280; x=1767651080; darn=nongnu.org;\n h=in-reply-to:content-disposition:mime-version:references:message-id\n :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to;\n bh=abQIuGVnkPQl727MqNYmLNB8lMqv7+ASsf/FWIKMjmE=;\n b=MiYaEGYiUxj7/cm4h46u6/K0r4nXDhLUWDpPyyd+sskBYfSEXKPaEidMYKQDYnoslx\n DOlzrIwTfVrWx5ZgGQE4K6Zgc2kwfz1UZ3JGo0AVruQia4e8W3qM0+3i+CWgMvf+cmRC\n JYavXC9PScCF6TMvEnklvEYVZCoYJ87VK9x/ZTOxpVCeC9MXFGjRkPazEhiO4F7nXqSc\n S6gK8DTHlpe91hBoyjICPRTwDQztPLS4jsWHOLrG4Q8SU244O2pFD8xUjeJw5/naKgMc\n HN17vSm9ry8b3zUYI8MewmDyVRKh2Kx1gttJdaVGCTKMF18GvNX9pioUjUIOcXAaAJZ3\n pyUQ=="],"X-MC-Unique":"yozoMwf6Oa2Bgd0FSJVMDA-1","X-Mimecast-MFC-AGG-ID":"yozoMwf6Oa2Bgd0FSJVMDA_1767046281","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n d=1e100.net; s=20230601; t=1767046280; x=1767651080;\n h=in-reply-to:content-disposition:mime-version:references:message-id\n :subject:cc:to:from:date:x-gm-gg:x-gm-message-state:from:to:cc\n :subject:date:message-id:reply-to;\n bh=abQIuGVnkPQl727MqNYmLNB8lMqv7+ASsf/FWIKMjmE=;\n b=mcQLZtJfE+6XpBS+Ec2GGazp+mT6O7CGYwi2bbmmHzVwwmJ+VYx2bWUe0uIOYXf3g0\n z7XyqEOtV3TPUL3f6afosHHcsxE8RfnfRgAP9IMw0Cx1H4ZgOEnZM24sylJmygNCT3r/\n kq6tTZKefOk3lvkNz2p3EzOH6nnRuvgC72UnPRtiXqOsmhSBl7U2mHVY/d+if1gb46o9\n PKcnUlf9AwebqVfZwOJlqX4wQ+wzn636BGOnhzgoN0K+nM8aOONHJwgcCnCsUVkgh9Xy\n GVpJXfqQsYbfGVtkQ74fvpnxGpwwjY7J8g0FcHW7EkYOKtFabNlgQzLmhJ9iV382S1SI\n wMmQ==","X-Gm-Message-State":"AOJu0Yxq1VhuyRrUFl+8RyiiFiRxMNS6qMRlQEYnrFhzx03g1v71Ye1V\n hwNBq6937cT1Kba1I6oGS7hADAZuuGkrtFktc7KEB3I+QzqwvDXiEa8Ll262D+28DE68bIBbwVk\n P6ZWn6I2zxL++n6wA6mh27MgHGITIW96jZRFyxvMRuJWobvOBOxl7hmf4Jx6lwszX","X-Gm-Gg":"AY/fxX4DWDffDRWPxGX3t94rLSBpvN6sYsYW2OQGLlPJc2yNRtJb1lRCJ9vVeut0KTM\n iP5WaKKVczk4+8cL69I6+BVfIVi4RoSdOVPVYW1f/a9dkbxUviFhTprjil45VnPsWKqGY7BYLCy\n 1lDe2iQ87esT0lik5M/r4VIEe/oF8mojOa021yuG3oxrE8YPppnjK6k4dKxScF5fW5Wdnjeo35S\n d/dRvpBcuxqpZn4e8yo+5tBNBTamzbNFfwqh5QmeYT9hYTk946KmczbcXqKWf4nEXsk/kmDl+2D\n IMUH0i9znKQzhp5HfXMb/XH9w/wtkh5+VkgMs3GXMSG84Jp/2NdoPxfcb11N8Gve6iJ4vVfMy2P\n 2QRc=","X-Received":["by 2002:a05:620a:319f:b0:8b2:3a3c:f261 with SMTP id\n af79cd13be357-8c08fbba6f5mr4523612685a.32.1767046280580;\n Mon, 29 Dec 2025 14:11:20 -0800 (PST)","by 2002:a05:620a:319f:b0:8b2:3a3c:f261 with SMTP id\n af79cd13be357-8c08fbba6f5mr4523610285a.32.1767046280147;\n Mon, 29 Dec 2025 14:11:20 -0800 (PST)"],"X-Google-Smtp-Source":"\n AGHT+IG8mPDmm0WIrllWfrDzKF2mWmrvwCsmWGGIMhn7Fsh/cJJ5s/o6rMt8K3SXJhW7LZUmH4JNsw==","Date":"Mon, 29 Dec 2025 17:11:18 -0500","From":"Peter Xu ","To":"Fabiano Rosas ","Cc":"qemu-devel@nongnu.org","Subject":"Re: [RFC PATCH 24/25] migration: Move URI parsing to channel.c","Message-ID":"","References":"<20251226211930.27565-1-farosas@suse.de>\n <20251226211930.27565-25-farosas@suse.de>\n <87344t7zlv.fsf@suse.de>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<87344t7zlv.fsf@suse.de>","Received-SPF":"pass client-ip=170.10.129.124; envelope-from=peterx@redhat.com;\n helo=us-smtp-delivery-124.mimecast.com","X-Spam_score_int":"-20","X-Spam_score":"-2.1","X-Spam_bar":"--","X-Spam_report":"(-2.1 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001,\n DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1,\n RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H2=0.001,\n RCVD_IN_VALIDITY_RPBL_BLOCKED=0.001, RCVD_IN_VALIDITY_SAFE_BLOCKED=0.001,\n SPF_HELO_PASS=-0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no","X-Spam_action":"no action","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n ","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n ","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org"}}]