On Thu, Mar 14, 2024 at 01:50:07PM -0300, Fabiano Rosas wrote: > Peter Xu <pet...@redhat.com> writes: > > > On Thu, Mar 14, 2024 at 11:10:12AM -0400, Peter Xu wrote: > >> On Wed, Mar 13, 2024 at 06:28:24PM -0300, Fabiano Rosas wrote: > >> > When doing migration using the fd: URI, the incoming migration starts > >> > before the user has passed the file descriptor to QEMU. This means > >> > that the checks at migration_channels_and_transport_compatible() > >> > happen too soon and we need to allow a migration channel of type > >> > SOCKET_ADDRESS_TYPE_FD even though socket migration is not supported > >> > with multifd. > >> > >> Hmm, bare with me if this is a stupid one.. why the incoming migration can > >> start _before_ the user passed in the fd? > >> > >> IOW, why can't we rely on a single fd_is_socket() check for > >> SOCKET_ADDRESS_TYPE_FD in transport_supports_multi_channels()? > >> > >> > > >> > 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. > >> > > >> > Fixes: decdc76772 ("migration/multifd: Add mapped-ram support to fd: > >> > URI") > >> > Signed-off-by: Fabiano Rosas <faro...@suse.de> > >> > --- > >> > migration/fd.c | 8 ++++++++ > >> > migration/file.c | 7 +++++++ > >> > 2 files changed, 15 insertions(+) > >> > > >> > 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; > >> > + } > > > > And... I just noticed this is forbiding multifd+socket+fd in general? But > > isn't that the majority of multifd usage when with libvirt over sockets? > > I didn't think multifd supported socket fds, does it? I don't see code > to create the multiple channels anywhere. How would that work? Multiple > threads writing to a single socket fd? I'm a bit confused.
You're probably right. I somehow had the assumption that Libvirt always used fds to passover to QEMU for migration, but indeed multifd at least shouldn't support it as I read the code again.. It'll be good if Dan would help to clarify when fd will be used in migrations. -- Peter Xu