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

Vladimir Sementsov-Ogievskiy posted 1 patch 1 week, 1 day ago
Failed in applying to current master (apply log)
There is a newer version of this series
docs/about/deprecated.rst | 7 +++++++
qapi/net.json             | 6 +++++-
2 files changed, 12 insertions(+), 1 deletion(-)
[PATCH] qapi: net/tap: deprecate vhostforce option
Posted by Vladimir Sementsov-Ogievskiy 1 week, 1 day ago
This option simply duplicates the @vhost option since long ago
(10 years!)
commit 1e7398a140f7a6 ("vhost: enable vhost without without MSI-X").
Let's finally deprecate it.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 docs/about/deprecated.rst | 7 +++++++
 qapi/net.json             | 6 +++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index d50645a071..d14cb37480 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 just duplicates the main ``vhost`` option.
+Use ``vhost`` alone.
+
+
 VFIO device options
 '''''''''''''''''''
 
diff --git a/qapi/net.json b/qapi/net.json
index 78bcc9871e..d1216bb60a 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -353,6 +353,10 @@
 # @poll-us: maximum number of microseconds that could be spent on busy
 #     polling for tap (since 2.7)
 #
+# Features:
+#
+# @deprecated: Member @vhostforce is deprecated.  Simply use @vhost.
+#
 # Since: 1.2
 ##
 { 'struct': 'NetdevTapOptions',
@@ -369,7 +373,7 @@
     '*vhost':      'bool',
     '*vhostfd':    'str',
     '*vhostfds':   'str',
-    '*vhostforce': 'bool',
+    '*vhostforce': { 'type': 'bool', 'features': [ 'deprecated' ] },
     '*queues':     'uint32',
     '*poll-us':    'uint32'} }
 
-- 
2.48.1
Re: [PATCH] qapi: net/tap: deprecate vhostforce option
Posted by Markus Armbruster via Devel 1 week ago
Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:

> This option simply duplicates the @vhost option since long ago
> (10 years!)
> commit 1e7398a140f7a6 ("vhost: enable vhost without without MSI-X").

This isn't obvious to me.

As far as I can see, their only use is in net_init_tap_one():

    if (tap->has_vhost ? tap->vhost :
        vhostfdname || (tap->has_vhostforce && tap->vhostforce)) {

Can you take this apart for me?

> Let's finally deprecate it.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
>  docs/about/deprecated.rst | 7 +++++++
>  qapi/net.json             | 6 +++++-
>  2 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> index d50645a071..d14cb37480 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 just duplicates the main ``vhost`` option.
> +Use ``vhost`` alone.

Would "Use instead ``vhost`` instead" be clearer?

> +
> +
>  VFIO device options
>  '''''''''''''''''''
>  
> diff --git a/qapi/net.json b/qapi/net.json
> index 78bcc9871e..d1216bb60a 100644
> --- a/qapi/net.json
> +++ b/qapi/net.json
> @@ -353,6 +353,10 @@
>  # @poll-us: maximum number of microseconds that could be spent on busy
>  #     polling for tap (since 2.7)
>  #
> +# Features:
> +#
> +# @deprecated: Member @vhostforce is deprecated.  Simply use @vhost.

@deprecated text is commonly of the form "FOO is deprecated.  Use BAR
instead."

Recommend "Use @vhost instead."

> +#
>  # Since: 1.2
>  ##
>  { 'struct': 'NetdevTapOptions',
> @@ -369,7 +373,7 @@
>      '*vhost':      'bool',
>      '*vhostfd':    'str',
>      '*vhostfds':   'str',
> -    '*vhostforce': 'bool',
> +    '*vhostforce': { 'type': 'bool', 'features': [ 'deprecated' ] },
>      '*queues':     'uint32',
>      '*poll-us':    'uint32'} }
Re: [PATCH] qapi: net/tap: deprecate vhostforce option
Posted by Vladimir Sementsov-Ogievskiy 5 days, 17 hours ago
On 30.08.25 11:17, Markus Armbruster wrote:
> Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
> 
>> This option simply duplicates the @vhost option since long ago
>> (10 years!)
>> commit 1e7398a140f7a6 ("vhost: enable vhost without without MSI-X").
> 
> This isn't obvious to me.
> 
> As far as I can see, their only use is in net_init_tap_one():
> 
>      if (tap->has_vhost ? tap->vhost :
>          vhostfdname || (tap->has_vhostforce && tap->vhostforce)) {
> 
> Can you take this apart for me?

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

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

So we simply have redundant options:

vhost=on / vhost=off : vhostforce ignored, doesn't make sense

vhost unset : vhostforce counts, enabling vhost

So 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 it.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>> ---
>>   docs/about/deprecated.rst | 7 +++++++
>>   qapi/net.json             | 6 +++++-
>>   2 files changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
>> index d50645a071..d14cb37480 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 just duplicates the main ``vhost`` option.
>> +Use ``vhost`` alone.
> 
> Would "Use instead ``vhost`` instead" be clearer?

I meant, that user should not use vhost=on + vhostforce=on anymore.

My be just "Use ``vhost``", without "alone"/"instead"?

> 
>> +
>> +
>>   VFIO device options
>>   '''''''''''''''''''
>>   
>> diff --git a/qapi/net.json b/qapi/net.json
>> index 78bcc9871e..d1216bb60a 100644
>> --- a/qapi/net.json
>> +++ b/qapi/net.json
>> @@ -353,6 +353,10 @@
>>   # @poll-us: maximum number of microseconds that could be spent on busy
>>   #     polling for tap (since 2.7)
>>   #
>> +# Features:
>> +#
>> +# @deprecated: Member @vhostforce is deprecated.  Simply use @vhost.
> 
> @deprecated text is commonly of the form "FOO is deprecated.  Use BAR
> instead."
> 
> Recommend "Use @vhost instead."
> 
>> +#
>>   # Since: 1.2
>>   ##
>>   { 'struct': 'NetdevTapOptions',
>> @@ -369,7 +373,7 @@
>>       '*vhost':      'bool',
>>       '*vhostfd':    'str',
>>       '*vhostfds':   'str',
>> -    '*vhostforce': 'bool',
>> +    '*vhostforce': { 'type': 'bool', 'features': [ 'deprecated' ] },
>>       '*queues':     'uint32',
>>       '*poll-us':    'uint32'} }
> 


-- 
Best regards,
Vladimir
Re: [PATCH] qapi: net/tap: deprecate vhostforce option
Posted by Markus Armbruster via Devel 5 days, 15 hours ago
Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:

> On 30.08.25 11:17, Markus Armbruster wrote:
>> Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
>> 
>>> This option simply duplicates the @vhost option since long ago
>>> (10 years!)
>>> commit 1e7398a140f7a6 ("vhost: enable vhost without without MSI-X").
>>
>> This isn't obvious to me.
>>
>> As far as I can see, their only use is in net_init_tap_one():
>>
>>      if (tap->has_vhost ? tap->vhost :
>>          vhostfdname || (tap->has_vhostforce && tap->vhostforce)) {
>>
>> Can you take this apart for me?
>
> Prior 1e7398a140f7a6, to enable vhost for some specific kind of guests
> (that don't have MSI-X support), you should hav set vhostforce=on
> (with vhost=on or unset).
>
> Since 1e7398a140f7a6, guest type doesn't matter, all guests are equal
> for vhost-enabling options logic.
>
> So we simply have redundant options:
>
> vhost=on / vhost=off : vhostforce ignored, doesn't make sense
>
> vhost unset : vhostforce counts, enabling vhost
>
> So you may enable vhost several ways:
> - vhost=on
> - vhostforce=on
> - vhost=on + vhostforce=on
> - and even vhost=on + vhostforce=off
>
> - they are all equal.

So @vhostforce doesn't quite duplicate @vhost: if they conflict, @vhost
silently takes precedence.

>>> Let's finally deprecate it.
>>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>>> ---
>>>   docs/about/deprecated.rst | 7 +++++++
>>>   qapi/net.json             | 6 +++++-
>>>   2 files changed, 12 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
>>> index d50645a071..d14cb37480 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 just duplicates the main ``vhost`` option.
>>> +Use ``vhost`` alone.
>>
>> Would "Use instead ``vhost`` instead" be clearer?
>
> I meant, that user should not use vhost=on + vhostforce=on anymore.
>
> My be just "Use ``vhost``", without "alone"/"instead"?

Suggest

     The ``vhostforce`` option is redundant with the ``vhost`` option.
     If they conflict, ``vhost`` takes precedence.  Just use ``vhost``.

Thanks!

[...]
Re: [PATCH] qapi: net/tap: deprecate vhostforce option
Posted by Vladimir Sementsov-Ogievskiy 5 days, 13 hours ago
On 01.09.25 14:50, Markus Armbruster wrote:
> Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
> 
>> On 30.08.25 11:17, Markus Armbruster wrote:
>>> Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:
>>>
>>>> This option simply duplicates the @vhost option since long ago
>>>> (10 years!)
>>>> commit 1e7398a140f7a6 ("vhost: enable vhost without without MSI-X").
>>>
>>> This isn't obvious to me.
>>>
>>> As far as I can see, their only use is in net_init_tap_one():
>>>
>>>       if (tap->has_vhost ? tap->vhost :
>>>           vhostfdname || (tap->has_vhostforce && tap->vhostforce)) {
>>>
>>> Can you take this apart for me?
>>
>> Prior 1e7398a140f7a6, to enable vhost for some specific kind of guests
>> (that don't have MSI-X support), you should hav set vhostforce=on
>> (with vhost=on or unset).
>>
>> Since 1e7398a140f7a6, guest type doesn't matter, all guests are equal
>> for vhost-enabling options logic.
>>
>> So we simply have redundant options:
>>
>> vhost=on / vhost=off : vhostforce ignored, doesn't make sense
>>
>> vhost unset : vhostforce counts, enabling vhost
>>
>> So you may enable vhost several ways:
>> - vhost=on
>> - vhostforce=on
>> - vhost=on + vhostforce=on
>> - and even vhost=on + vhostforce=off
>>
>> - they are all equal.
> 
> So @vhostforce doesn't quite duplicate @vhost: if they conflict, @vhost
> silently takes precedence.

Right. My description was too simplified, I'll update and resend, thanks!

> 
>>>> Let's finally deprecate it.
>>>>
>>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>>>> ---
>>>>    docs/about/deprecated.rst | 7 +++++++
>>>>    qapi/net.json             | 6 +++++-
>>>>    2 files changed, 12 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
>>>> index d50645a071..d14cb37480 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 just duplicates the main ``vhost`` option.
>>>> +Use ``vhost`` alone.
>>>
>>> Would "Use instead ``vhost`` instead" be clearer?
>>
>> I meant, that user should not use vhost=on + vhostforce=on anymore.
>>
>> My be just "Use ``vhost``", without "alone"/"instead"?
> 
> Suggest
> 
>       The ``vhostforce`` option is redundant with the ``vhost`` option.
>       If they conflict, ``vhost`` takes precedence.  Just use ``vhost``.
> 
> Thanks!
> 
> [...]
> 


-- 
Best regards,
Vladimir