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

[Markus Armbruster <armbru@redhat.com>]

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!

[...]