migration: Revert mapped-ram multifd support to fd: URI

This reverts commit decdc76772c453ff1444612e910caa0d45cd8eac in full
and also the relevant migration-tests from
7a09f092834641b7a793d50a3a261073bbb404a6.

After the addition of the new QAPI-based migration address API in 8.2
we've been converting an "fd:" URI into a SocketAddress, missing the
fact that the "fd:" syntax could also be used for a plain file instead
of a socket. This is a problem because the SocketAddress is part of
the API, so we're effectively asking users to create a "socket"
channel to pass in a plain file.

The easiest way to fix this situation is to deprecate the usage of
both SocketAddress and "fd:" when used with a plain file for
migration. Since this has been possible since 8.2, we can wait until
9.1 to deprecate it.

For 9.0, however, we should avoid adding further support to migration
to a plain file using the old "fd:" syntax or the new SocketAddress
API, and instead require the usage of either the old-style "file:" URI
or the FileMigrationArgs::filename field of the new API with the
"/dev/fdset/NN" syntax, both of which are already supported.

Signed-off-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20240319210941.1907-1-farosas@suse.de
Signed-off-by: Peter Xu <peterx@redhat.com>

diff --git a/migration/fd.c b/migration/fd.c
index fe0d096abd462353e6f46614033c29736443ce15..449adaa2deeb6698c83e762cf6c74b83876b858c 100644 (file)
--- a/migration/fd.c
+++ b/migration/fd.c
@@ -15,42 +15,19 @@
  */
 
 #include "qemu/osdep.h"
-#include "qapi/error.h"
 #include "channel.h"
 #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"
 
 
-static struct FdOutgoingArgs {
-    int fd;
-} outgoing_args;
-
-int fd_args_get_fd(void)
-{
-    return outgoing_args.fd;
-}
-
-void fd_cleanup_outgoing_migration(void)
-{
-    if (outgoing_args.fd > 0) {
-        close(outgoing_args.fd);
-        outgoing_args.fd = -1;
-    }
-}
-
 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;
-
     if (fd == -1) {
         return;
     }
@@ -62,18 +39,6 @@ void fd_start_outgoing_migration(MigrationState *s, const char *fdname, Error **
         return;
     }
 
-    /*
-     * 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);
     object_unref(OBJECT(ioc));
@@ -104,20 +69,9 @@ void fd_start_incoming_migration(const char *fdname, Error **errp)
         return;
     }
 
-    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");
-        qio_channel_add_watch_full(ioc, G_IO_IN,
-                                   fd_accept_incoming_migration,
-                                   NULL, NULL,
-                                   g_main_context_get_thread_default());
-    }
+    qio_channel_set_name(ioc, "migration-fd-incoming");
+    qio_channel_add_watch_full(ioc, G_IO_IN,
+                               fd_accept_incoming_migration,
+                               NULL, NULL,
+                               g_main_context_get_thread_default());
 }

diff --git a/migration/fd.h b/migration/fd.h
index 0c0a18d9e76eaea815d4303c47a183e67444fe2f..b901bc014eb4f519e5cb404fa3944e5ab3399d18 100644 (file)
--- a/migration/fd.h
+++ b/migration/fd.h
@@ -20,6 +20,4 @@ void fd_start_incoming_migration(const char *fdname, Error **errp);
 
 void fd_start_outgoing_migration(MigrationState *s, const char *fdname,
                                  Error **errp);
-void fd_cleanup_outgoing_migration(void);
-int fd_args_get_fd(void);
 #endif

diff --git a/migration/file.c b/migration/file.c
index b6e8ba13f2d9c9e57cdc52456750a0ffa145bd57..ab18ba505a1d2d860898279a7230b137f0011e07 100644 (file)
--- a/migration/file.c
+++ b/migration/file.c
@@ -11,7 +11,6 @@
 #include "qemu/error-report.h"
 #include "qapi/error.h"
 #include "channel.h"
-#include "fd.h"
 #include "file.h"
 #include "migration.h"
 #include "io/channel-file.h"
@@ -55,27 +54,15 @@ bool file_send_channel_create(gpointer opaque, Error **errp)
 {
     QIOChannelFile *ioc;
     int flags = O_WRONLY;
-    bool ret = false;
-    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);
-    }
+    bool ret = true;
 
+    ioc = qio_channel_file_new_path(outgoing_args.fname, flags, 0, errp);
     if (!ioc) {
+        ret = false;
         goto out;
     }
 
     multifd_channel_connect(opaque, QIO_CHANNEL(ioc));
-    ret = true;
 
 out:
     /*

diff --git a/migration/migration.c b/migration/migration.c
index f60bd371e3f896a74df8be4282a15b4280eba732..047b6b49cf81fb98e7895250501ea4d56dea3664 100644 (file)
--- 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;
 }
 

diff --git a/migration/multifd.c b/migration/multifd.c
index 0179422f6dc191f65453321c42b0eaf263a1da7f..d2f0238f70f841349a99d44e2a54c5d139b188bd 100644 (file)
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -18,7 +18,6 @@
 #include "exec/ramblock.h"
 #include "qemu/error-report.h"
 #include "qapi/error.h"
-#include "fd.h"
 #include "file.h"
 #include "migration.h"
 #include "migration-stats.h"
@@ -794,7 +793,6 @@ static bool multifd_send_cleanup_channel(MultiFDSendParams *p, Error **errp)
 static void multifd_send_cleanup_state(void)
 {
     file_cleanup_outgoing_migration();
-    fd_cleanup_outgoing_migration();
     socket_cleanup_outgoing_migration();
     qemu_sem_destroy(&multifd_send_state->channels_created);
     qemu_sem_destroy(&multifd_send_state->channels_ready);

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 71895abb7fb10d391b41dae132ab97cf9f04efea..1d2cee87ea310e751551055f6e4422aeb9b8c425 100644 (file)
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -2536,13 +2536,6 @@ static void *migrate_precopy_fd_file_start(QTestState *from, QTestState *to)
     return NULL;
 }
 
-static void *migrate_fd_file_mapped_ram_start(QTestState *from, QTestState *to)
-{
-    migrate_mapped_ram_start(from, to);
-
-    return migrate_precopy_fd_file_start(from, to);
-}
-
 static void test_migrate_precopy_fd_file(void)
 {
     MigrateCommon args = {
@@ -2553,36 +2546,6 @@ static void test_migrate_precopy_fd_file(void)
     };
     test_file_common(&args, true);
 }
-
-static void test_migrate_precopy_fd_file_mapped_ram(void)
-{
-    MigrateCommon args = {
-        .listen_uri = "defer",
-        .connect_uri = "fd:fd-mig",
-        .start_hook = migrate_fd_file_mapped_ram_start,
-        .finish_hook = test_migrate_fd_finish_hook
-    };
-    test_file_common(&args, true);
-}
-
-static void *migrate_multifd_fd_mapped_ram_start(QTestState *from,
-                                                QTestState *to)
-{
-    migrate_multifd_mapped_ram_start(from, to);
-    return migrate_precopy_fd_file_start(from, to);
-}
-
-static void test_multifd_fd_mapped_ram(void)
-{
-    MigrateCommon args = {
-        .connect_uri = "fd:fd-mig",
-        .listen_uri = "defer",
-        .start_hook = migrate_multifd_fd_mapped_ram_start,
-        .finish_hook = test_migrate_fd_finish_hook
-    };
-
-    test_file_common(&args, true);
-}
 #endif /* _WIN32 */
 
 static void do_test_validate_uuid(MigrateStart *args, bool should_fail)
@@ -3687,10 +3650,6 @@ int main(int argc, char **argv)
                        test_multifd_file_mapped_ram);
     migration_test_add("/migration/multifd/file/mapped-ram/live",
                        test_multifd_file_mapped_ram_live);
-#ifndef _WIN32
-    migration_test_add("/migration/multifd/fd/mapped-ram",
-                       test_multifd_fd_mapped_ram);
-#endif
 
 #ifdef CONFIG_GNUTLS
     migration_test_add("/migration/precopy/unix/tls/psk",
@@ -3753,8 +3712,6 @@ int main(int argc, char **argv)
                        test_migrate_precopy_fd_socket);
     migration_test_add("/migration/precopy/fd/file",
                        test_migrate_precopy_fd_file);
-    migration_test_add("/migration/precopy/fd/file/mapped-ram",
-                       test_migrate_precopy_fd_file_mapped_ram);
 #endif
     migration_test_add("/migration/validate_uuid", test_validate_uuid);
     migration_test_add("/migration/validate_uuid_error",