| Message ID | 20240426131408.25410-7-farosas@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | migration removals & deprecations | expand |
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 <peterx@redhat.com> 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@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. 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."); }
On Mon, Apr 29, 2024 at 03:47:39PM -0300, Fabiano Rosas wrote: > Peter Xu <peterx@redhat.com> 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@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. > > 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."); > } Sounds good, perhaps also in fd_start_incoming_migration().
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.
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(+)