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

[Peter Xu <peterx@redhat.com>]

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@suse.de>
> ---
>  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.

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?

-- 
Peter Xu