Skip to content

Commit

Permalink
migration: Revert mapped-ram multifd support to fd: URI
Browse files Browse the repository at this point in the history 
This reverts commit decdc76 in full
and also the relevant migration-tests from
7a09f09.

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>
  • Loading branch information
@xzpeter
Fabiano Rosas authored and xzpeter committed Mar 22, 2024
1 parent 853546f commit bd4480b
Show file tree
Hide file tree
Showing 6 changed files with 8 additions and 127 deletions.
56 changes: 5 additions & 51 deletions migration/fd.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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));
Expand Down Expand Up @@ -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());
}
2 changes: 0 additions & 2 deletions migration/fd.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
19 changes: 3 additions & 16 deletions migration/file.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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:
/*
Expand Down
13 changes: 0 additions & 13 deletions migration/migration.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
}

Expand Down
2 changes: 0 additions & 2 deletions migration/multifd.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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);
Expand Down
43 changes: 0 additions & 43 deletions tests/qtest/migration-test.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -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)
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down

0 comments on commit bd4480b

Please sign in to comment.