Re: [PATCH v4 25/34] migration/multifd: Add fixed-ram support to fd: URI

[Peter Xu]

On Tue, Feb 20, 2024 at 07:41:29PM -0300, Fabiano Rosas wrote:
> If we receive a file descriptor that points to a regular file, there's
> nothing stopping us from doing multifd migration with fixed-ram to
> that file.
> 
> Enable the fd: URI to work with multifd + fixed-ram.
> 
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
>  migration/fd.c        | 30 ++++++++++++++++++++++++++++++
>  migration/fd.h        |  1 +
>  migration/file.c      | 12 +++++++++---
>  migration/migration.c |  4 ++++
>  4 files changed, 44 insertions(+), 3 deletions(-)
> 
> diff --git a/migration/fd.c b/migration/fd.c
> index 0eb677dcae..b7e4d071a4 100644
> --- a/migration/fd.c
> +++ b/migration/fd.c
> @@ -19,14 +19,28 @@
>  #include "fd.h"
>  #include "migration.h"
>  #include "monitor/monitor.h"
> +#include "io/channel-file.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_start_outgoing_migration(MigrationState *s, const char *fdname, 
> Error **errp)
>  {
>      QIOChannel *ioc;
>      int fd = monitor_get_fd(monitor_cur(), fdname, errp);
> +
> +    outgoing_args.fd = -1;

I suggest we either drop this, or close() it before releasing; basically
each fd reference holds a real fd index would be easier, IMHO.

Also, we'd want to free the fd (by closing it) just like we free
outgoing_args.fname?  Otherwise if a fd is passed in, who is responsible to
close it and release the fd resource?

> +
>      if (fd == -1) {
>          return;
>      }
> @@ -38,6 +52,8 @@ void fd_start_outgoing_migration(MigrationState *s, const 
> char *fdname, Error **
>          return;
>      }
>  
> +    outgoing_args.fd = fd;

If agree with above, then dup(fd) here.

> +
>      qio_channel_set_name(ioc, "migration-fd-outgoing");
>      migration_channel_connect(s, ioc, NULL, NULL);
>      object_unref(OBJECT(ioc));
> @@ -73,4 +89,18 @@ void fd_start_incoming_migration(const char *fdname, Error 
> **errp)
>                                 fd_accept_incoming_migration,
>                                 NULL, NULL,
>                                 g_main_context_get_thread_default());
> +
> +    if (migrate_multifd()) {
> +        int channels = migrate_multifd_channels();
> +
> +        while (channels--) {
> +            ioc = QIO_CHANNEL(qio_channel_file_new_fd(fd));

dup(fd)?

> +
> +            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 b901bc014e..1be980c130 100644
> --- a/migration/fd.h
> +++ b/migration/fd.h
> @@ -20,4 +20,5 @@ void fd_start_incoming_migration(const char *fdname, Error 
> **errp);
>  
>  void fd_start_outgoing_migration(MigrationState *s, const char *fdname,
>                                   Error **errp);
> +int fd_args_get_fd(void);
>  #endif
> diff --git a/migration/file.c b/migration/file.c
> index 1a18e608fc..27ccfc6a1d 100644
> --- a/migration/file.c
> +++ b/migration/file.c
> @@ -11,6 +11,7 @@
>  #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"
> @@ -58,10 +59,15 @@ bool file_send_channel_create(gpointer opaque, Error 
> **errp)
>  {
>      QIOChannelFile *ioc;
>      int flags = O_WRONLY;
> +    int fd = fd_args_get_fd();
>  
> -    ioc = qio_channel_file_new_path(outgoing_args.fname, flags, 0, errp);
> -    if (!ioc) {
> -        return false;
> +    if (fd && fd != -1) {
> +        ioc = qio_channel_file_new_fd(fd);
> +    } else {
> +        ioc = qio_channel_file_new_path(outgoing_args.fname, flags, 0, errp);
> +        if (!ioc) {
> +            return false;
> +        }
>      }
>  
>      if (!multifd_channel_connect(opaque, QIO_CHANNEL(ioc), errp)) {
> diff --git a/migration/migration.c b/migration/migration.c
> index 32b291a282..ce7e6f5065 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -134,6 +134,10 @@ 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_fixed_ram();
> +        }
> +
>          return (saddr->type == SOCKET_ADDRESS_TYPE_INET ||
>                  saddr->type == SOCKET_ADDRESS_TYPE_UNIX ||
>                  saddr->type == SOCKET_ADDRESS_TYPE_VSOCK);
> -- 
> 2.35.3
> 

-- 
Peter Xu