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

[Peter Xu <peterx@redhat.com>]

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?

> 
> That's the only reason I didn't propose reverting commit decdc76772
> ("migration/multifd: Add mapped-ram support to fd: URI").
> 
> For mapped-ram in libvirt, we'll definitely not use
> SOCKET_ADDRESS_TYPE_FD (as in the JSON), because I don't think libvirt
> supports the new API.
> 
> As for SOCKET_ADDRESS_TYPE_FD as in "fd:", we could use it when
> direct-io is disabled. With direct-io, the fdset will be required.
> 
> >
> > What about the old getfd?  Is it obsolete because it relies on monitor
> > object?  Or maybe it's still in use?
> >
> > It would be greatly helpful if there can be a summary of how libvirt uses
> > fd for migration purpose.
> >
> > Thanks,
> 

-- 
Peter Xu