Re: [PATCH v2 6/6] migration: Deprecate fd: for file migration

Peter Xu <peterx(a)redhat.com&gt; writes: > On Fri, Apr 26, 2024 at 10:14:08AM -0300, Fabiano Rosas wrote: >> The fd: URI can currently trigger two different types of migration, a >> TCP migration using sockets and a file migration using a plain >> file. This is in conflict with the recently introduced (8.2) QMP >> migrate API that takes structured data as JSON-like format. We cannot >> keep the same backend for both types of migration because with the new >> API the code is more tightly coupled to the type of transport. This >> means a TCP migration must use the 'socket' transport and a file >> migration must use the 'file' transport. >> >> If we keep allowing fd: when using a file, this creates an issue when >> the user converts the old-style (fd:) to the new style ("transport": >> "socket") invocation because the file descriptor in question has >> previously been allowed to be either a plain file or a socket. >> >> To avoid creating too much confusion, we can simply deprecate the fd: >> + file usage, which is thought to be rarely used currently and instead >> establish a 1:1 correspondence between fd: URI and socket transport, >> and file: URI and file transport. >> >> Signed-off-by: Fabiano Rosas <farosas(a)suse.de&gt; >> --- >> docs/about/deprecated.rst | 14 ++++++++++++++ >> 1 file changed, 14 insertions(+) >> >> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst >> index 0fb5c82640..813f7996fe 100644 >> --- a/docs/about/deprecated.rst >> +++ b/docs/about/deprecated.rst >> @@ -464,3 +464,17 @@ both, older and future versions of QEMU. >> The ``blacklist`` config file option has been renamed to ``block-rpcs`` >> (to be in sync with the renaming of the corresponding command line >> option). >> + >> +Migration >> +--------- >> + >> +``fd:`` URI when used for file migration (since 9.1) >> +'''''''''''''''''''''''''''''''''''''''''''''''''''' >> + >> +The ``fd:`` URI can currently provide a file descriptor that >> +references either a socket or a plain file. These are two different >> +types of migration. In order to reduce ambiguity, the ``fd:`` URI >> +usage of providing a file descriptor to a plain file has been >> +deprecated in favor of explicitly using the ``file:`` URI with the >> +file descriptor being passed as an ``fdset``. Refer to the ``add-fd`` >> +command documentation for details on the ``fdset`` usage. > > Wanna do some warn_report() when detected non-socket fds alongside? Looks > like we previously do this for all deprecations. Yes, good point. > > What's the plan when it's support removed? I'm imaginging that we sanity > check fstat() + S_ISSOCK on the fd and fail otherwise? In that case we can > have the code there, dump warn_report(), then switch to failing qmp migrate > (and incoming side) later on? Something along those lines. We currently use fd_is_socket(): bool fd_is_socket(int fd) { int optval; socklen_t optlen = sizeof(optval); return !getsockopt(fd, SOL_SOCKET, SO_TYPE, &optval, &optlen); } I'm thinking of this in fd_start_outgoing_migation(): if (!fd_is_socket(fd)) { warn_report("fd: migration to a file is deprecated." " Use file: instead."); }