qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 2/2] migration/multifd: Ensure we're not given a socket fo


From: Peter Xu
Subject: Re: [PATCH v2 2/2] migration/multifd: Ensure we're not given a socket for file migration
Date: Thu, 14 Mar 2024 13:53:34 -0400

On Thu, Mar 14, 2024 at 01:44:13PM -0300, Fabiano Rosas wrote:
> Peter Xu <peterx@redhat.com> writes:
> 
> > 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?
> 
> It's been a while since I looked at this. Looking into it once more
> today, I think the issue is actually that we only fetch the fds from the
> monitor at fd_start_outgoing|incoming_migration().

Yes that looks more reasonable.

It means we may want to touch up transport_supports_seeking()'s comment on
this if needed.

> 
> >
> > IOW, why can't we rely on a single fd_is_socket() check for
> > SOCKET_ADDRESS_TYPE_FD in transport_supports_multi_channels()?
> >
> 
> There's no fd at that point. Just a string.
> 
> I think the right fix here would be to move the
> monitor_fd_get/monitor_fd_param (why two different functions?)

Yes this is all confusing.

I think it makes sense to use the same guideline for both sides on the fd:
protocol, perhaps it means we should always use monitor_fd_param() to be
consistent and compatible.

> earlier into migrate_uri_parse. And possibly also extend
> FileMigrationArgs to contain an fd. Not sure how easy would that be.

But if so IIUC the 'filename' parameter will need to be optional if "fd"
exists.  While that will break QAPI for 8.2.

I think it's fine we keep what we do right now (with the above comment
fixed on why that check needs to be delayed, though), or if you find it
easy to unify the check in some undestructive way.

-- 
Peter Xu




reply via email to

[Prev in Thread] Current Thread [Next in Thread]