[v2] TAP initialization refactoring

[PATCH v2 05/20] net/tap: net_init_tap_one(): move parameter checking earlier

Posted by Vladimir Sementsov-Ogievskiy 2 months, 3 weeks ago

Let's keep all similar argument checking in net_init_tap() function.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 net/tap.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/net/tap.c b/net/tap.c
index 58c3318b1c..3fe99ef63f 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -765,9 +765,6 @@ static int net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
                        "vhost-net requested but could not be initialized");
             goto failed;
         }
-    } else if (vhostfdname) {
-        error_setg(errp, "vhostfd(s)= is not valid without vhost");
-        goto failed;
     }
 
     return 0;
@@ -829,6 +826,11 @@ int net_init_tap(const Netdev *netdev, const char *name,
         return -1;
     }
 
+    if (tap->has_vhost && !tap->vhost && (tap->vhostfds || tap->vhostfd)) {
+        error_setg(errp, "vhostfd(s)= is not valid without vhost");
+        return -1;
+    }
+
     if (tap->fd) {
         if (tap->ifname || tap->script || tap->downscript ||
             tap->has_vnet_hdr || tap->helper || tap->has_queues ||
-- 
2.48.1

Re: [PATCH v2 05/20] net/tap: net_init_tap_one(): move parameter checking earlier

Posted by Jason Wang 2 months, 1 week ago

On Sun, Aug 24, 2025 at 12:03 AM Vladimir Sementsov-Ogievskiy
<vsementsov@yandex-team.ru> wrote:
>
> Let's keep all similar argument checking in net_init_tap() function.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
>  net/tap.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/net/tap.c b/net/tap.c
> index 58c3318b1c..3fe99ef63f 100644
> --- a/net/tap.c
> +++ b/net/tap.c
> @@ -765,9 +765,6 @@ static int net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
>                         "vhost-net requested but could not be initialized");
>              goto failed;
>          }
> -    } else if (vhostfdname) {
> -        error_setg(errp, "vhostfd(s)= is not valid without vhost");
> -        goto failed;
>      }
>
>      return 0;
> @@ -829,6 +826,11 @@ int net_init_tap(const Netdev *netdev, const char *name,
>          return -1;
>      }
>
> +    if (tap->has_vhost && !tap->vhost && (tap->vhostfds || tap->vhostfd)) {
> +        error_setg(errp, "vhostfd(s)= is not valid without vhost");
> +        return -1;
> +    }

So this doesn't deal with vhost force or is this expected to be
applied on top of the deprecation of vhostforce?

Thanks

> +
>      if (tap->fd) {
>          if (tap->ifname || tap->script || tap->downscript ||
>              tap->has_vnet_hdr || tap->helper || tap->has_queues ||
> --
> 2.48.1
>

Re: [PATCH v2 05/20] net/tap: net_init_tap_one(): move parameter checking earlier

Posted by Vladimir Sementsov-Ogievskiy 2 months, 1 week ago

On 03.09.25 07:18, Jason Wang wrote:
> On Sun, Aug 24, 2025 at 12:03 AM Vladimir Sementsov-Ogievskiy
> <vsementsov@yandex-team.ru> wrote:
>>
>> Let's keep all similar argument checking in net_init_tap() function.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>> ---
>>   net/tap.c | 8 +++++---
>>   1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/net/tap.c b/net/tap.c
>> index 58c3318b1c..3fe99ef63f 100644
>> --- a/net/tap.c
>> +++ b/net/tap.c
>> @@ -765,9 +765,6 @@ static int net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
>>                          "vhost-net requested but could not be initialized");
>>               goto failed;
>>           }
>> -    } else if (vhostfdname) {
>> -        error_setg(errp, "vhostfd(s)= is not valid without vhost");
>> -        goto failed;
>>       }
>>
>>       return 0;
>> @@ -829,6 +826,11 @@ int net_init_tap(const Netdev *netdev, const char *name,
>>           return -1;
>>       }
>>
>> +    if (tap->has_vhost && !tap->vhost && (tap->vhostfds || tap->vhostfd)) {
>> +        error_setg(errp, "vhostfd(s)= is not valid without vhost");
>> +        return -1;
>> +    }
> 
> So this doesn't deal with vhost force or is this expected to be
> applied on top of the deprecation of vhostforce?
> 

This patch doesn't modify existing logic. And deprecation doesn't modify logic in tap.c as well.
So, this series and deprecation may be applied in any order.

> 
>> +
>>       if (tap->fd) {
>>           if (tap->ifname || tap->script || tap->downscript ||
>>               tap->has_vnet_hdr || tap->helper || tap->has_queues ||
>> --
>> 2.48.1
>>
> 


-- 
Best regards,
Vladimir