[PATCH v2] qapi: net/tap: deprecate vhostforce option

[PATCH v2] qapi: net/tap: deprecate vhostforce option

[Vladimir Sementsov-Ogievskiy]

This option doesn't make sense since long ago (10 years!)
commit 1e7398a140f7a6 ("vhost: enable vhost without without MSI-X").

Prior 1e7398a140f7a6, to enable vhost for some specific kind of guests
(that don't have MSI-X support), you should have set vhostforce=on
(with vhost=on or unset).

Since 1e7398a140f7a6, guest type doesn't matter, all guests are equal
for vhost-enabling options logic.

The current logic is:
  vhost=on / vhost=off : vhostforce ignored, doesn't make sense
  vhost unset : vhostforce counts, enabling vhost

Currently you may enable vhost several ways:
- vhost=on
- vhostforce=on
- vhost=on + vhostforce=on
- and even vhost=on + vhostforce=off

- they are all equal.

Let's finally deprecate the extra option.

Also, fix @vhostforce documentation.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---

v2:
- improve wording
- add documentation fix

 docs/about/deprecated.rst |  7 +++++++
 qapi/net.json             | 11 +++++++++--
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index d50645a071..b17a5a41aa 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -516,6 +516,13 @@ Stream ``reconnect`` (since 9.2)
 The ``reconnect`` option only allows specifying second granularity timeouts,
 which is not enough for all types of use cases, use ``reconnect-ms`` instead.
 
+TAP ``vhostforce`` (since 10.2)
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+The ``vhostforce`` option is redundant with the ``vhost`` option.
+If they conflict, ``vhost`` takes precedence.  Just use ``vhost``.
+
+
 VFIO device options
 '''''''''''''''''''
 
diff --git a/qapi/net.json b/qapi/net.json
index 78bcc9871e..e89c7aff5f 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -346,13 +346,20 @@
 # @vhostfds: file descriptors of multiple already opened vhost net
 #     devices
 #
-# @vhostforce: vhost on for non-MSIX virtio guests
+# @vhostforce: enable vhost-net network accelerator. Ignored when
+#    @vhost is set.
 #
 # @queues: number of queues to be created for multiqueue capable tap
 #
 # @poll-us: maximum number of microseconds that could be spent on busy
 #     polling for tap (since 2.7)
 #
+# Features:
+#
+# @deprecated: Member @vhostforce is deprecated.  The @vhostforce
+#    option is redundant with the @vhost option. If they conflict,
+#    @vhost takes precedence.  Just use @vhost.
+#
 # Since: 1.2
 ##
 { 'struct': 'NetdevTapOptions',
@@ -369,7 +376,7 @@
     '*vhost':      'bool',
     '*vhostfd':    'str',
     '*vhostfds':   'str',
-    '*vhostforce': 'bool',
+    '*vhostforce': { 'type': 'bool', 'features': [ 'deprecated' ] },
     '*queues':     'uint32',
     '*poll-us':    'uint32'} }
 
-- 
2.48.1

Re: [PATCH v2] qapi: net/tap: deprecate vhostforce option

[Markus Armbruster]

Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:

> This option doesn't make sense since long ago (10 years!)
> commit 1e7398a140f7a6 ("vhost: enable vhost without without MSI-X").
>
> Prior 1e7398a140f7a6, to enable vhost for some specific kind of guests
> (that don't have MSI-X support), you should have set vhostforce=on
> (with vhost=on or unset).
>
> Since 1e7398a140f7a6, guest type doesn't matter, all guests are equal
> for vhost-enabling options logic.
>
> The current logic is:
>   vhost=on / vhost=off : vhostforce ignored, doesn't make sense
>   vhost unset : vhostforce counts, enabling vhost
>
> Currently you may enable vhost several ways:
> - vhost=on
> - vhostforce=on
> - vhost=on + vhostforce=on
> - and even vhost=on + vhostforce=off
>
> - they are all equal.
>
> Let's finally deprecate the extra option.
>
> Also, fix @vhostforce documentation.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
>
> v2:
> - improve wording
> - add documentation fix
>
>  docs/about/deprecated.rst |  7 +++++++
>  qapi/net.json             | 11 +++++++++--
>  2 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> index d50645a071..b17a5a41aa 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -516,6 +516,13 @@ Stream ``reconnect`` (since 9.2)
>  The ``reconnect`` option only allows specifying second granularity timeouts,
>  which is not enough for all types of use cases, use ``reconnect-ms`` instead.
>  
> +TAP ``vhostforce`` (since 10.2)
> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +The ``vhostforce`` option is redundant with the ``vhost`` option.
> +If they conflict, ``vhost`` takes precedence.  Just use ``vhost``.
> +
> +
>  VFIO device options
>  '''''''''''''''''''
>  
> diff --git a/qapi/net.json b/qapi/net.json
> index 78bcc9871e..e89c7aff5f 100644
> --- a/qapi/net.json
> +++ b/qapi/net.json
> @@ -346,13 +346,20 @@
>  # @vhostfds: file descriptors of multiple already opened vhost net
>  #     devices
>  #
> -# @vhostforce: vhost on for non-MSIX virtio guests
> +# @vhostforce: enable vhost-net network accelerator. Ignored when
> +#    @vhost is set.

Two spaces between sentences for consistency, please.

>  #
>  # @queues: number of queues to be created for multiqueue capable tap
>  #
>  # @poll-us: maximum number of microseconds that could be spent on busy
>  #     polling for tap (since 2.7)
>  #
> +# Features:
> +#
> +# @deprecated: Member @vhostforce is deprecated.  The @vhostforce
> +#    option is redundant with the @vhost option. If they conflict,
> +#    @vhost takes precedence.  Just use @vhost.
> +#
>  # Since: 1.2
>  ##
>  { 'struct': 'NetdevTapOptions',
> @@ -369,7 +376,7 @@
>      '*vhost':      'bool',
>      '*vhostfd':    'str',
>      '*vhostfds':   'str',
> -    '*vhostforce': 'bool',
> +    '*vhostforce': { 'type': 'bool', 'features': [ 'deprecated' ] },
>      '*queues':     'uint32',
>      '*poll-us':    'uint32'} }

Reviewed-by: Markus Armbruster <armbru@redhat.com>