Re: [RFC PATCH v3 3/3] migration: Add fd to FileMigrationArgs

[Peter Xu <peterx@redhat.com>]

On Tue, Mar 19, 2024 at 04:25:32PM +0000, Daniel P. Berrangé wrote:
> On Fri, Mar 15, 2024 at 04:54:27PM -0400, Peter Xu wrote:
> > On Fri, Mar 15, 2024 at 03:01:09PM -0300, Fabiano Rosas wrote:
> > > Peter Xu <peterx@redhat.com> writes:
> > > 
> > > > [I queued patch 1-2 into -stable, leaving this patch for further
> > > >  discussions]
> > > >
> > > > On Fri, Mar 15, 2024 at 08:55:42AM +0000, Daniel P. Berrangé wrote:
> > > >> The 'file:' protocol eventually calls into qemu_open, and this
> > > >> transparently allows for FD passing using /dev/fdset/NNN syntax
> > > >> to pass in FDs. 
> > > >
> > > > If it always use /dev/fdsets for files, does it mean that the newly added
> > > > SOCKET_ADDRESS_TYPE_FD support on mapped-ram will never be used (so we can
> > > > drop them)?
> > > 
> > > We already have SOCKET_ADDRESS_TYPE_FD + file since 8.2 when the
> > > MigrationAddress was added. So this:
> > > 
> > > 'channels': [ { 'channel-type': 'main',
> > >                 'addr': { 'transport': 'socket',
> > >                 'type': 'fd',
> > >                 'str': 'fdname' } } ]
> > > 
> > > works without multifd and without mapped-ram if the fd is a file or
> > > socket.
> > > 
> > > So yes, you're correct, but given we already have this^ it would be
> > > perhaps more confusing for users to allow it, but not allow the very
> > > same JSON when multifd=true, mapped-ram=true.
> > 
> > I don't think the fd: protocol (no matter the old "fd:", or the new JSON
> > format) is trivial to use. If libvirt didn't use it I won't be surprised to
> > see nobody using it.  I want us to avoid working on things that nobody is
> > using, or has a better replacement.
> > 
> > So even if Libvirt supports both, I'm wondering whether /dev/fdset/ works
> > for all the cases that libvirt needs.  I am aware that the old getfd has
> > the monitor limitation so that if the QMP disconnected and reconnect, the
> > fd can be gone.  However I'm not sure whether that's the only reason to
> > have add-fd, and also not sure whether it means add-fd is always preferred,
> > so that maybe we can consider obsolete getfd?
> 
> Historically libvirt primariily uses the 'fd:' protocol, with a
> socket FD. It never gives QEMU a plain file FD, since it has
> always added its "iohelper" as a MITM, in order to add O_DIRECT
> on top.
> 
> The 'getfd' command is something that is needed when talking to
> QEMU for any API that involves a "SocketAddress" QAPI type,
> which is applicable for migration.
> 
> With the introduction of 'MigrationAddress', the 'socket' protocol
> is backed by 'SocketAddress' and thus supports FD passing for
> sockets (or potentally pipes too), in combination with 'getfd'.
> 
> With the 'file' protocol in 'MigrationAddress', since it gets
> backed by qemu_open(), then /dev/fdset/NN and 'add-fd' provide
> passing for plain files.

I see.  I assume it means we still have multiple users of getfd so it's
still in use where add-fd is not yet avaiable.

But then, SOCKET_ADDRESS_TYPE_FD is then not used for libvirt in the whole
mapped-ram effort, neither do we need any support on file migrations over
"fd", e.g. fd_start_incoming_migration() for files. So we can drop these
parts, am I right?

Thanks,

-- 
Peter Xu