From patchwork Fri Mar 15 03:20:38 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Fabiano Rosas X-Patchwork-Id: 13593024 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 7F640C54E60 for ; Fri, 15 Mar 2024 03:22:06 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1rky80-0003zF-IQ; Thu, 14 Mar 2024 23:21:04 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1rky7u-0003yO-G1 for qemu-devel@nongnu.org; Thu, 14 Mar 2024 23:20:58 -0400 Received: from smtp-out1.suse.de ([195.135.223.130]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1rky7m-0004IF-9y for qemu-devel@nongnu.org; Thu, 14 Mar 2024 23:20:58 -0400 Received: from imap1.dmz-prg2.suse.org (imap1.dmz-prg2.suse.org [10.150.64.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 25ED021D44; Fri, 15 Mar 2024 03:20:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1710472846; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=niLMWk+PhaKVhiINpnrFIJE7tK7ZrL4Zx8VBbFS5GxA=; b=edrMNs18Xa6uB38d9MqhEnj7SaNgTqq7kjHyA0vBP9HnAUv3Qg1L4ZLMw05iNSBwVhXuYw jHIBm02E/mYezNYf2TmyhLCxbSFPCd7wvbciZWaqaRl4Q1ErIlJUl393ogEAcxtAYGWzC2 /65u2C4zjwuW3YOlNGEWJyyIwWk6sak= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1710472846; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=niLMWk+PhaKVhiINpnrFIJE7tK7ZrL4Zx8VBbFS5GxA=; b=aAxkkDEXxZ67eSDUTnDQmDfRsY6ycn2TtkE/RFETtLSgSN4y03t8AaKbh9RL0EGvoDGWGO T3cJ1YhXnTNXqkDQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1710472846; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=niLMWk+PhaKVhiINpnrFIJE7tK7ZrL4Zx8VBbFS5GxA=; b=edrMNs18Xa6uB38d9MqhEnj7SaNgTqq7kjHyA0vBP9HnAUv3Qg1L4ZLMw05iNSBwVhXuYw jHIBm02E/mYezNYf2TmyhLCxbSFPCd7wvbciZWaqaRl4Q1ErIlJUl393ogEAcxtAYGWzC2 /65u2C4zjwuW3YOlNGEWJyyIwWk6sak= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1710472846; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=niLMWk+PhaKVhiINpnrFIJE7tK7ZrL4Zx8VBbFS5GxA=; b=aAxkkDEXxZ67eSDUTnDQmDfRsY6ycn2TtkE/RFETtLSgSN4y03t8AaKbh9RL0EGvoDGWGO T3cJ1YhXnTNXqkDQ== Received: from imap1.dmz-prg2.suse.org (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by imap1.dmz-prg2.suse.org (Postfix) with ESMTPS id F271A1368C; Fri, 15 Mar 2024 03:20:44 +0000 (UTC) Received: from dovecot-director2.suse.de ([10.150.64.162]) by imap1.dmz-prg2.suse.org with ESMTPSA id eMaYLYy+82XZcQAAD6G6ig (envelope-from ); Fri, 15 Mar 2024 03:20:44 +0000 From: Fabiano Rosas To: qemu-devel@nongnu.org Cc: =?utf-8?q?Daniel_P_=2E_Berrang=C3=A9?= , Peter Xu Subject: [PATCH v3 1/3] migration/multifd: Ensure we're not given a socket for file migration Date: Fri, 15 Mar 2024 00:20:38 -0300 Message-Id: <20240315032040.7974-2-farosas@suse.de> X-Mailer: git-send-email 2.35.3 In-Reply-To: <20240315032040.7974-1-farosas@suse.de> References: <20240315032040.7974-1-farosas@suse.de> MIME-Version: 1.0 Authentication-Results: smtp-out1.suse.de; none X-Spamd-Result: default: False [0.70 / 50.00]; ARC_NA(0.00)[]; RCVD_VIA_SMTP_AUTH(0.00)[]; FROM_HAS_DN(0.00)[]; RCPT_COUNT_THREE(0.00)[3]; R_MISSING_CHARSET(2.50)[]; TO_MATCH_ENVRCPT_ALL(0.00)[]; MIME_GOOD(-0.10)[text/plain]; BROKEN_CONTENT_TYPE(1.50)[]; TO_DN_SOME(0.00)[]; NEURAL_HAM_LONG(-1.00)[-1.000]; RCVD_COUNT_THREE(0.00)[3]; DKIM_SIGNED(0.00)[suse.de:s=susede2_rsa,suse.de:s=susede2_ed25519]; NEURAL_HAM_SHORT(-0.20)[-0.987]; MID_CONTAINS_FROM(1.00)[]; DBL_BLOCKED_OPENRESOLVER(0.00)[suse.de:email]; FUZZY_BLOCKED(0.00)[rspamd.com]; FROM_EQ_ENVFROM(0.00)[]; MIME_TRACE(0.00)[0:+]; RCVD_TLS_ALL(0.00)[]; BAYES_HAM(-3.00)[100.00%] Received-SPF: pass client-ip=195.135.223.130; envelope-from=farosas@suse.de; helo=smtp-out1.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, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_MED=-2.3, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 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: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org When doing migration using the fd: URI, QEMU will fetch the file descriptor passed in via the monitor at fd_start_outgoing|incoming_migration(), which means the checks at migration_channels_and_transport_compatible() happen too soon and we don't know at that point whether the FD refers to a plain file or a socket. For this reason, we've been allowing a migration channel of type SOCKET_ADDRESS_TYPE_FD to pass the initial verifications in scenarios where the socket migration is not supported, such as with fd + multifd. The commit decdc76772 ("migration/multifd: Add mapped-ram support to fd: URI") was supposed to add a second check prior to starting migration to make sure a socket fd is not passed instead of a file fd, but failed to do so. Add the missing verification and update the comment explaining this situation which is currently incorrect. Fixes: decdc76772 ("migration/multifd: Add mapped-ram support to fd: URI") Signed-off-by: Fabiano Rosas Reviewed-by: Peter Xu --- migration/fd.c | 8 ++++++++ migration/file.c | 7 +++++++ migration/migration.c | 6 +++--- 3 files changed, 18 insertions(+), 3 deletions(-) diff --git a/migration/fd.c b/migration/fd.c index 39a52e5c90..c07030f715 100644 --- a/migration/fd.c +++ b/migration/fd.c @@ -22,6 +22,7 @@ #include "migration.h" #include "monitor/monitor.h" #include "io/channel-file.h" +#include "io/channel-socket.h" #include "io/channel-util.h" #include "options.h" #include "trace.h" @@ -95,6 +96,13 @@ void fd_start_incoming_migration(const char *fdname, Error **errp) } if (migrate_multifd()) { + if (fd_is_socket(fd)) { + error_setg(errp, + "Multifd migration to a socket FD is not supported"); + object_unref(ioc); + return; + } + file_create_incoming_channels(ioc, errp); } else { qio_channel_set_name(ioc, "migration-fd-incoming"); diff --git a/migration/file.c b/migration/file.c index ddde0ca818..b6e8ba13f2 100644 --- a/migration/file.c +++ b/migration/file.c @@ -15,6 +15,7 @@ #include "file.h" #include "migration.h" #include "io/channel-file.h" +#include "io/channel-socket.h" #include "io/channel-util.h" #include "options.h" #include "trace.h" @@ -58,6 +59,12 @@ bool file_send_channel_create(gpointer opaque, Error **errp) int fd = fd_args_get_fd(); if (fd && fd != -1) { + if (fd_is_socket(fd)) { + error_setg(errp, + "Multifd migration to a socket FD is not supported"); + goto out; + } + ioc = qio_channel_file_new_dupfd(fd, errp); } else { ioc = qio_channel_file_new_path(outgoing_args.fname, flags, 0, errp); diff --git a/migration/migration.c b/migration/migration.c index 644e073b7d..f60bd371e3 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -166,9 +166,9 @@ static bool transport_supports_seeking(MigrationAddress *addr) } /* - * At this point, the user might not yet have passed the file - * descriptor to QEMU, so we cannot know for sure whether it - * refers to a plain file or a socket. Let it through anyway. + * At this point QEMU has not yet fetched the fd passed in by the + * user, so we cannot know for sure whether it refers to a plain + * file or a socket. Let it through anyway and check at fd.c. */ if (addr->transport == MIGRATION_ADDRESS_TYPE_SOCKET) { return addr->u.socket.type == SOCKET_ADDRESS_TYPE_FD; From patchwork Fri Mar 15 03:20:39 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Fabiano Rosas X-Patchwork-Id: 13593022 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 225A7C54E67 for ; Fri, 15 Mar 2024 03:21:44 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1rky8C-00040E-Au; Thu, 14 Mar 2024 23:21:16 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1rky7z-0003zG-Ru for qemu-devel@nongnu.org; Thu, 14 Mar 2024 23:21:04 -0400 Received: from smtp-out1.suse.de ([195.135.223.130]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1rky7m-0004IN-5K for qemu-devel@nongnu.org; Thu, 14 Mar 2024 23:21:03 -0400 Received: from imap1.dmz-prg2.suse.org (imap1.dmz-prg2.suse.org [IPv6:2a07:de40:b281:104:10:150:64:97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id BBF1F21D93; Fri, 15 Mar 2024 03:20:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1710472847; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=rmlLzyqrfZPWaaTdawgpYY7pzIQgR8FqjE/xWFU8Qe4=; b=Ai/PQL6A0pAi7IIH3Sc59ZdcwbRjYj6I5RCHeIbvg0zFeoSlqqPrm8RuTQxB1xvLDMqKyz Q9c0SkxdRPeLtA17SXZuAG41JahnZD2kpfrY+vrEmR54mjTJqbO6X7uSarM7e5ZOYOYuu9 0PXCPuOGuUU68xoIPvmmfJVKyOWoxPw= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1710472847; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=rmlLzyqrfZPWaaTdawgpYY7pzIQgR8FqjE/xWFU8Qe4=; b=T3cMslVqCvI/jlPvJTigwKJxuWDultsQWdUXdmRn+i5ASknwMkZICF1n+jFSrKtbWq6vyY yn4i718xAksTKTAw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1710472847; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=rmlLzyqrfZPWaaTdawgpYY7pzIQgR8FqjE/xWFU8Qe4=; b=Ai/PQL6A0pAi7IIH3Sc59ZdcwbRjYj6I5RCHeIbvg0zFeoSlqqPrm8RuTQxB1xvLDMqKyz Q9c0SkxdRPeLtA17SXZuAG41JahnZD2kpfrY+vrEmR54mjTJqbO6X7uSarM7e5ZOYOYuu9 0PXCPuOGuUU68xoIPvmmfJVKyOWoxPw= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1710472847; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=rmlLzyqrfZPWaaTdawgpYY7pzIQgR8FqjE/xWFU8Qe4=; b=T3cMslVqCvI/jlPvJTigwKJxuWDultsQWdUXdmRn+i5ASknwMkZICF1n+jFSrKtbWq6vyY yn4i718xAksTKTAw== Received: from imap1.dmz-prg2.suse.org (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by imap1.dmz-prg2.suse.org (Postfix) with ESMTPS id 9515D1368C; Fri, 15 Mar 2024 03:20:46 +0000 (UTC) Received: from dovecot-director2.suse.de ([10.150.64.162]) by imap1.dmz-prg2.suse.org with ESMTPSA id oKMEF46+82XZcQAAD6G6ig (envelope-from ); Fri, 15 Mar 2024 03:20:46 +0000 From: Fabiano Rosas To: qemu-devel@nongnu.org Cc: =?utf-8?q?Daniel_P_=2E_Berrang=C3=A9?= , Peter Xu Subject: [PATCH v3 2/3] migration/multifd: Duplicate the fd for the outgoing_args Date: Fri, 15 Mar 2024 00:20:39 -0300 Message-Id: <20240315032040.7974-3-farosas@suse.de> X-Mailer: git-send-email 2.35.3 In-Reply-To: <20240315032040.7974-1-farosas@suse.de> References: <20240315032040.7974-1-farosas@suse.de> MIME-Version: 1.0 X-Spamd-Bar: +++ Authentication-Results: smtp-out1.suse.de; dkim=pass header.d=suse.de header.s=susede2_rsa header.b="Ai/PQL6A"; dkim=pass header.d=suse.de header.s=susede2_ed25519 header.b=T3cMslVq X-Spamd-Result: default: False [3.03 / 50.00]; ARC_NA(0.00)[]; RCVD_VIA_SMTP_AUTH(0.00)[]; R_DKIM_ALLOW(-0.20)[suse.de:s=susede2_rsa,suse.de:s=susede2_ed25519]; FROM_HAS_DN(0.00)[]; RCPT_COUNT_THREE(0.00)[3]; R_MISSING_CHARSET(2.50)[]; TO_MATCH_ENVRCPT_ALL(0.00)[]; MIME_GOOD(-0.10)[text/plain]; BROKEN_CONTENT_TYPE(1.50)[]; TO_DN_SOME(0.00)[]; NEURAL_HAM_LONG(-1.00)[-1.000]; RCVD_COUNT_THREE(0.00)[3]; DKIM_SIGNED(0.00)[suse.de:s=susede2_rsa,suse.de:s=susede2_ed25519]; DKIM_TRACE(0.00)[suse.de:+]; MX_GOOD(-0.01)[]; MID_CONTAINS_FROM(1.00)[]; DBL_BLOCKED_OPENRESOLVER(0.00)[suse.de:dkim,suse.de:email]; FUZZY_BLOCKED(0.00)[rspamd.com]; FROM_EQ_ENVFROM(0.00)[]; MIME_TRACE(0.00)[0:+]; NEURAL_HAM_SHORT(-0.20)[-1.000]; RCVD_TLS_ALL(0.00)[]; BAYES_HAM(-0.46)[79.08%]; RBL_SPAMHAUS_BLOCKED_OPENRESOLVER(0.00)[2a07:de40:b281:104:10:150:64:97:from] X-Rspamd-Server: rspamd1.dmz-prg2.suse.org X-Rspamd-Queue-Id: BBF1F21D93 Received-SPF: pass client-ip=195.135.223.130; envelope-from=farosas@suse.de; helo=smtp-out1.suse.de 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, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 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: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org We currently store the file descriptor used during the main outgoing channel creation to use it again when creating the multifd channels. Since this fd is used for the first iochannel, there's risk that the QIOChannel gets freed and the fd closed while outgoing_args.fd still has it available. This could lead to an fd-reuse bug. Duplicate the outgoing_args fd to avoid this issue. Suggested-by: Peter Xu Signed-off-by: Fabiano Rosas Reviewed-by: Peter Xu --- migration/fd.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/migration/fd.c b/migration/fd.c index c07030f715..fe0d096abd 100644 --- a/migration/fd.c +++ b/migration/fd.c @@ -49,8 +49,7 @@ void fd_start_outgoing_migration(MigrationState *s, const char *fdname, Error ** { QIOChannel *ioc; int fd = monitor_get_fd(monitor_cur(), fdname, errp); - - outgoing_args.fd = -1; + int newfd; if (fd == -1) { return; @@ -63,7 +62,17 @@ void fd_start_outgoing_migration(MigrationState *s, const char *fdname, Error ** return; } - outgoing_args.fd = fd; + /* + * This is dup()ed just to avoid referencing an fd that might + * be already closed by the iochannel. + */ + newfd = dup(fd); + if (newfd == -1) { + error_setg_errno(errp, errno, "Could not dup FD %d", fd); + object_unref(ioc); + return; + } + outgoing_args.fd = newfd; qio_channel_set_name(ioc, "migration-fd-outgoing"); migration_channel_connect(s, ioc, NULL, NULL); From patchwork Fri Mar 15 03:20:40 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Fabiano Rosas X-Patchwork-Id: 13593023 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 208FAC54E5D for ; Fri, 15 Mar 2024 03:21:44 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1rky7w-0003ya-SS; Thu, 14 Mar 2024 23:21:02 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1rky7t-0003xx-Ax for qemu-devel@nongnu.org; Thu, 14 Mar 2024 23:20:57 -0400 Received: from smtp-out1.suse.de ([2a07:de40:b251:101:10:150:64:1]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1rky7n-0004IW-0s for qemu-devel@nongnu.org; Thu, 14 Mar 2024 23:20:56 -0400 Received: from imap1.dmz-prg2.suse.org (imap1.dmz-prg2.suse.org [IPv6:2a07:de40:b281:104:10:150:64:97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id D62C721D94; Fri, 15 Mar 2024 03:20:49 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1710472849; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=74RrPFY9zjw0wZcmCjd84GqQxrgaar52OmG+YS4MfCg=; b=T3AmefCsFJZn6A7+Ium+jlClDi9Mi8Q+y8m9HE+XYfxRuVKNVeY2bgjqGescdz59S4vAUR xa/h3jHmXWzn9eH9cSULjNsJO4Ucbca7Fl39QQ8lautB7oHrBJ8Qj+GtN+bzQ0dhfmTtbZ fSX3dMJ9hE7jDgUePe7lqUAwEt3TTZs= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1710472849; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=74RrPFY9zjw0wZcmCjd84GqQxrgaar52OmG+YS4MfCg=; b=sWyHrU94irLQV6iv56gz4vBTWMRXd0a1t4ETR/qHhZawXZnNBu65odEJ5kbOZ6wv2q4nVw PdD8d39xyBeaZUDw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1710472849; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=74RrPFY9zjw0wZcmCjd84GqQxrgaar52OmG+YS4MfCg=; b=T3AmefCsFJZn6A7+Ium+jlClDi9Mi8Q+y8m9HE+XYfxRuVKNVeY2bgjqGescdz59S4vAUR xa/h3jHmXWzn9eH9cSULjNsJO4Ucbca7Fl39QQ8lautB7oHrBJ8Qj+GtN+bzQ0dhfmTtbZ fSX3dMJ9hE7jDgUePe7lqUAwEt3TTZs= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1710472849; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=74RrPFY9zjw0wZcmCjd84GqQxrgaar52OmG+YS4MfCg=; b=sWyHrU94irLQV6iv56gz4vBTWMRXd0a1t4ETR/qHhZawXZnNBu65odEJ5kbOZ6wv2q4nVw PdD8d39xyBeaZUDw== Received: from imap1.dmz-prg2.suse.org (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by imap1.dmz-prg2.suse.org (Postfix) with ESMTPS id 3EECA1368C; Fri, 15 Mar 2024 03:20:48 +0000 (UTC) Received: from dovecot-director2.suse.de ([10.150.64.162]) by imap1.dmz-prg2.suse.org with ESMTPSA id wMj3AZC+82XZcQAAD6G6ig (envelope-from ); Fri, 15 Mar 2024 03:20:48 +0000 From: Fabiano Rosas To: qemu-devel@nongnu.org Cc: =?utf-8?q?Daniel_P_=2E_Berrang=C3=A9?= , Peter Xu , Eric Blake , Markus Armbruster Subject: [RFC PATCH v3 3/3] migration: Add fd to FileMigrationArgs Date: Fri, 15 Mar 2024 00:20:40 -0300 Message-Id: <20240315032040.7974-4-farosas@suse.de> X-Mailer: git-send-email 2.35.3 In-Reply-To: <20240315032040.7974-1-farosas@suse.de> References: <20240315032040.7974-1-farosas@suse.de> MIME-Version: 1.0 X-Spamd-Bar: / Authentication-Results: smtp-out1.suse.de; dkim=pass header.d=suse.de header.s=susede2_rsa header.b=T3AmefCs; dkim=pass header.d=suse.de header.s=susede2_ed25519 header.b=sWyHrU94 X-Rspamd-Server: rspamd2.dmz-prg2.suse.org X-Spamd-Result: default: False [0.49 / 50.00]; ARC_NA(0.00)[]; RCVD_VIA_SMTP_AUTH(0.00)[]; R_DKIM_ALLOW(-0.20)[suse.de:s=susede2_rsa,suse.de:s=susede2_ed25519]; SPAMHAUS_XBL(0.00)[2a07:de40:b281:104:10:150:64:97:from]; FROM_HAS_DN(0.00)[]; TO_DN_SOME(0.00)[]; R_MISSING_CHARSET(2.50)[]; TO_MATCH_ENVRCPT_ALL(0.00)[]; MIME_GOOD(-0.10)[text/plain]; BROKEN_CONTENT_TYPE(1.50)[]; RCPT_COUNT_FIVE(0.00)[5]; NEURAL_HAM_LONG(-1.00)[-1.000]; RCVD_COUNT_THREE(0.00)[3]; DKIM_SIGNED(0.00)[suse.de:s=susede2_rsa,suse.de:s=susede2_ed25519]; DKIM_TRACE(0.00)[suse.de:+]; MX_GOOD(-0.01)[]; MID_CONTAINS_FROM(1.00)[]; DBL_BLOCKED_OPENRESOLVER(0.00)[suse.de:dkim,suse.de:email]; FUZZY_BLOCKED(0.00)[rspamd.com]; FROM_EQ_ENVFROM(0.00)[]; MIME_TRACE(0.00)[0:+]; NEURAL_HAM_SHORT(-0.20)[-0.999]; RCVD_TLS_ALL(0.00)[]; BAYES_HAM(-3.00)[100.00%] X-Rspamd-Queue-Id: D62C721D94 Received-SPF: pass client-ip=2a07:de40:b251:101:10:150:64:1; envelope-from=farosas@suse.de; helo=smtp-out1.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, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_MED=-2.3, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 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: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org The fd: URI has supported migration to a file or socket since before QEMU 8.2. In 8.2 we added the file: URI that supported migration to a file. So now we have two ways (three if you count exec:>cat) to migrate to a file. Fine. However, In 8.2 we also added the new qmp_migrate API that uses a JSON channel list instead of the URI. It added two migration transports SOCKET and FILE. It was decided that the new API would classify the fd migration as a type of socket migration, neglecting the fact that the fd.c code also supported file migrations. In 9.0 we're adding support for fd + multifd + mapped-ram, which is tied to the file migration. This was implemented in fd.c, which is only reachable when the SOCKET address type is used. The result of this is that we're asking users of the new API to create (1) something called a "socket" to perform migration to a plain file. And creating something called a "file" provides no way of passing in a file descriptor. This is confusing. The new API also parses the old-style URI into the new-style data structures, so the old and correct fd: now also considers fd: to be socket-related only. Unlike the other migration addresses, the fd comes already setup from the user, it's not just a string that QEMU will use to start a connection. We need to actually fetch the fd from the monitor before even being able to poke at it to know if it is a socket. Aside from the issue (1) above, the current approach of parsing the fd into a SOCKET and only later deciding if it is file-backed doesn't work well now that we're adding multifd support for fd: and file: migration via the mapped-ram feature. With a larger number of combinations, we need to be able to know upfront when an fd is backed by a plain file vs. a socket. We're currently using a trick of allowing socket channels to pass some (2) validation that they shouldn't, to only later verify if the fd was indeed a socket. To clean this up I'm proposing we start requiring users of the new API to use the "file" structure when the migration stream will end up in a file and use the "socket" structure when the fd points to an actual socket. This improves (1). To keep backward compatibility, I'm still accepting everything that was accepted before and only changing the structures internally to allow the rest of the code to rely on the MIGRATION_ADDRESS_TYPE. This addresses (2). We can then slowly deprecate the wrong way, i.e. using type socket for fd + file migration. In this patch you'll find: - a new function migrate_resolve_fd() that fetches the fd from the monitor and converts the MigrationAddress into the correct type; - the removal of the hacks for (2); - a helper function to convert the resolved fd that's represented as a string for storage in the MigrationAddress back into an integer for consumption. Signed-off-by: Fabiano Rosas --- migration/fd.c | 15 +------ migration/file.c | 14 +++--- migration/migration.c | 100 ++++++++++++++++++++++++++++++++++++------ migration/migration.h | 1 + qapi/migration.json | 11 ++++- 5 files changed, 107 insertions(+), 34 deletions(-) diff --git a/migration/fd.c b/migration/fd.c index fe0d096abd..2d4414c7ea 100644 --- a/migration/fd.c +++ b/migration/fd.c @@ -20,9 +20,6 @@ #include "fd.h" #include "file.h" #include "migration.h" -#include "monitor/monitor.h" -#include "io/channel-file.h" -#include "io/channel-socket.h" #include "io/channel-util.h" #include "options.h" #include "trace.h" @@ -48,8 +45,7 @@ void fd_cleanup_outgoing_migration(void) void fd_start_outgoing_migration(MigrationState *s, const char *fdname, Error **errp) { QIOChannel *ioc; - int fd = monitor_get_fd(monitor_cur(), fdname, errp); - int newfd; + int newfd, fd = migrate_fd_str_to_int(fdname, errp); if (fd == -1) { return; @@ -91,7 +87,7 @@ static gboolean fd_accept_incoming_migration(QIOChannel *ioc, void fd_start_incoming_migration(const char *fdname, Error **errp) { QIOChannel *ioc; - int fd = monitor_fd_param(monitor_cur(), fdname, errp); + int fd = migrate_fd_str_to_int(fdname, errp); if (fd == -1) { return; } @@ -105,13 +101,6 @@ void fd_start_incoming_migration(const char *fdname, Error **errp) } if (migrate_multifd()) { - if (fd_is_socket(fd)) { - error_setg(errp, - "Multifd migration to a socket FD is not supported"); - object_unref(ioc); - return; - } - file_create_incoming_channels(ioc, errp); } else { qio_channel_set_name(ioc, "migration-fd-incoming"); diff --git a/migration/file.c b/migration/file.c index b6e8ba13f2..b29521ffa7 100644 --- a/migration/file.c +++ b/migration/file.c @@ -59,12 +59,6 @@ bool file_send_channel_create(gpointer opaque, Error **errp) int fd = fd_args_get_fd(); if (fd && fd != -1) { - if (fd_is_socket(fd)) { - error_setg(errp, - "Multifd migration to a socket FD is not supported"); - goto out; - } - ioc = qio_channel_file_new_dupfd(fd, errp); } else { ioc = qio_channel_file_new_path(outgoing_args.fname, flags, 0, errp); @@ -95,6 +89,10 @@ void file_start_outgoing_migration(MigrationState *s, uint64_t offset = file_args->offset; QIOChannel *ioc; + if (file_args->fd) { + return fd_start_outgoing_migration(s, file_args->fd, errp); + } + trace_migration_file_outgoing(filename); fioc = qio_channel_file_new_path(filename, O_CREAT | O_WRONLY | O_TRUNC, @@ -163,6 +161,10 @@ void file_start_incoming_migration(FileMigrationArgs *file_args, Error **errp) QIOChannelFile *fioc = NULL; uint64_t offset = file_args->offset; + if (file_args->fd) { + return fd_start_incoming_migration(file_args->fd, errp); + } + trace_migration_file_incoming(filename); fioc = qio_channel_file_new_path(filename, O_RDONLY, 0, errp); diff --git a/migration/migration.c b/migration/migration.c index f60bd371e3..88162205a3 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -140,10 +140,6 @@ static bool transport_supports_multi_channels(MigrationAddress *addr) if (addr->transport == MIGRATION_ADDRESS_TYPE_SOCKET) { SocketAddress *saddr = &addr->u.socket; - if (saddr->type == SOCKET_ADDRESS_TYPE_FD) { - return migrate_mapped_ram(); - } - return (saddr->type == SOCKET_ADDRESS_TYPE_INET || saddr->type == SOCKET_ADDRESS_TYPE_UNIX || saddr->type == SOCKET_ADDRESS_TYPE_VSOCK); @@ -165,15 +161,6 @@ static bool transport_supports_seeking(MigrationAddress *addr) return true; } - /* - * At this point QEMU has not yet fetched the fd passed in by the - * user, so we cannot know for sure whether it refers to a plain - * file or a socket. Let it through anyway and check at fd.c. - */ - if (addr->transport == MIGRATION_ADDRESS_TYPE_SOCKET) { - return addr->u.socket.type == SOCKET_ADDRESS_TYPE_FD; - } - return false; } @@ -608,6 +595,85 @@ bool migrate_uri_parse(const char *uri, MigrationChannel **channel, return true; } +/* + * Unlike other types of migration addresses, a file descriptor is not + * just a string that QEMU uses to initiate a connection. An fd has to + * be prepared in advance by the user or management app. We only know + * whether a file descriptor refers to a socket or a file after + * retrieving it from the monitor. + * + * If the method being used to reference the fds for migration is the + * old "fd:" URI, migrate_uri_parse() always parses it into a + * MIGRATION_ADDRESS_TYPE_SOCKET. + * + * If the method is the new channels API, a file descriptor might come + * via either the MIGRATION_ADDRESS_TYPE_SOCKET or the + * MIGRATION_ADDRESS_TYPE_FILE. For backward compatibility with 8.2 + * which allowed a file migration with MIGRATION_ADDRESS_TYPE_SOCKET, + * this is still allowed. + * + * This function converts all of the above into + * MIGRATION_ADDRESS_TYPE_FILE so the rest of code doesn't need to + * care about none of this. + */ +static bool migrate_resolve_fd(MigrationAddress *addr, Error **errp) +{ + int fd; + + if (addr->transport == MIGRATION_ADDRESS_TYPE_FILE) { + FileMigrationArgs *fargs = &addr->u.file; + + if (fargs->fd) { + fd = monitor_fd_param(monitor_cur(), fargs->fd, errp); + if (fd == -1) { + return false; + } + + if (!fd_is_socket(fd)) { + addr->u.file.fd = g_strdup_printf("%d", fd); + } else { + error_setg(errp, + "Migration transport 'file' does not support socket file descriptors"); + return false; + } + } + } else if (addr->transport == MIGRATION_ADDRESS_TYPE_SOCKET) { + SocketAddress *saddr = &addr->u.socket; + + if (saddr->type == SOCKET_ADDRESS_TYPE_FD) { + fd = monitor_fd_param(monitor_cur(), saddr->u.fd.str, errp); + if (fd == -1) { + return false; + } + + g_free(saddr->u.fd.str); + + if (!fd_is_socket(fd)) { + addr->transport = MIGRATION_ADDRESS_TYPE_FILE; + addr->u.file.fd = g_strdup_printf("%d", fd); + addr->u.file.filename = NULL; + addr->u.file.offset = 0; + } else { + saddr->u.fd.str = g_strdup_printf("%d", fd); + } + } + } + + return true; +} + +int migrate_fd_str_to_int(const char *fdname, Error **errp) +{ + int err, fd; + + err = qemu_strtoi(fdname, NULL, 0, &fd); + if (err) { + error_setg_errno(errp, err, "failed to convert fd"); + return -1; + } + return fd; +} + static void qemu_start_incoming_migration(const char *uri, bool has_channels, MigrationChannelList *channels, Error **errp) @@ -641,6 +707,10 @@ static void qemu_start_incoming_migration(const char *uri, bool has_channels, addr = channel->addr; } + if (!migrate_resolve_fd(addr, errp)) { + return; + } + /* transport mechanism not suitable for migration? */ if (!migration_channels_and_transport_compatible(addr, errp)) { return; @@ -2086,6 +2156,10 @@ void qmp_migrate(const char *uri, bool has_channels, addr = channel->addr; } + if (!migrate_resolve_fd(addr, errp)) { + return; + } + /* transport mechanism not suitable for migration? */ if (!migration_channels_and_transport_compatible(addr, errp)) { return; diff --git a/migration/migration.h b/migration/migration.h index 8045e39c26..e265d1732c 100644 --- a/migration/migration.h +++ b/migration/migration.h @@ -543,5 +543,6 @@ int migration_rp_wait(MigrationState *s); * to remember the target is always the migration thread. */ void migration_rp_kick(MigrationState *s); +int migrate_fd_str_to_int(const char *fdname, Error **errp); #endif diff --git a/qapi/migration.json b/qapi/migration.json index aa1b39bce1..37f4b9c6fb 100644 --- a/qapi/migration.json +++ b/qapi/migration.json @@ -1656,13 +1656,20 @@ # # @filename: The file to receive the migration stream # +# @fd: A file descriptor name or number. File descriptors must be +# first added with the 'getfd' command. (since 9.0). +# # @offset: The file offset where the migration stream will start # +# Since 9.0, all members are optional, but at least one of @filename +# or @fd are required. +# # Since: 8.2 ## { 'struct': 'FileMigrationArgs', - 'data': { 'filename': 'str', - 'offset': 'uint64' } } + 'data': { '*filename': 'str', + '*fd': 'str', + '*offset': 'uint64' } } ## # @MigrationExecCommand: