[PATCH] vhost: fix misleading log when setting max queue num

[PATCH] vhost: fix misleading log when setting max queue num

[Maxime Coquelin]

rte_vhost_driver_set_max_queue_num API returns early when
called for a Vhost-user device, as this API is intended to
limit the maximum number of queue pairs supported by VDUSE
devices. However, a log mentioning the maximim number of
queue pairs is being set is emitted unconditionally, which
may confuse the end user.

This patch moves this log after the backend type is
checked, so that it is only called with VDUSE backends.
The check on the requested value is also moved at the same
place.

Fixes: e1808999d36b ("vhost: restrict set max queue pair API to VDUSE")
Cc: stable@dpdk.org

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/vhost/socket.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/lib/vhost/socket.c b/lib/vhost/socket.c
index d29d15494c..07247907b0 100644
--- a/lib/vhost/socket.c
+++ b/lib/vhost/socket.c
@@ -844,13 +844,6 @@ rte_vhost_driver_set_max_queue_num(const char *path, uint32_t max_queue_pairs)
 	struct vhost_user_socket *vsocket;
 	int ret = 0;
 
-	VHOST_CONFIG_LOG(path, INFO, "Setting max queue pairs to %u", max_queue_pairs);
-
-	if (max_queue_pairs > VHOST_MAX_QUEUE_PAIRS) {
-		VHOST_CONFIG_LOG(path, ERR, "Library only supports up to %u queue pairs",
-				VHOST_MAX_QUEUE_PAIRS);
-		return -1;
-	}
 
 	pthread_mutex_lock(&vhost_user.mutex);
 	vsocket = find_vhost_user_socket(path);
@@ -872,6 +865,15 @@ rte_vhost_driver_set_max_queue_num(const char *path, uint32_t max_queue_pairs)
 		goto unlock_exit;
 	}
 
+	VHOST_CONFIG_LOG(path, INFO, "Setting max queue pairs to %u", max_queue_pairs);
+
+	if (max_queue_pairs > VHOST_MAX_QUEUE_PAIRS) {
+		VHOST_CONFIG_LOG(path, ERR, "Library only supports up to %u queue pairs",
+				VHOST_MAX_QUEUE_PAIRS);
+		ret = -1;
+		goto unlock_exit;
+	}
+
 	vsocket->max_queue_pairs = max_queue_pairs;
 
 unlock_exit:
-- 
2.47.1

Re: [PATCH] vhost: fix misleading log when setting max queue num

[Ilya Maximets]

On 1/9/25 15:31, Maxime Coquelin wrote:
> rte_vhost_driver_set_max_queue_num API returns early when
> called for a Vhost-user device, as this API is intended to
> limit the maximum number of queue pairs supported by VDUSE
> devices. However, a log mentioning the maximim number of
> queue pairs is being set is emitted unconditionally, which
> may confuse the end user.
> 
> This patch moves this log after the backend type is
> checked, so that it is only called with VDUSE backends.
> The check on the requested value is also moved at the same
> place.
> 
> Fixes: e1808999d36b ("vhost: restrict set max queue pair API to VDUSE")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---

Thanks, Maxime!  This makes the logs way less confusing.

Reviewed-by: Ilya Maximets <i.maximets@ovn.org>

>  lib/vhost/socket.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/lib/vhost/socket.c b/lib/vhost/socket.c
> index d29d15494c..07247907b0 100644
> --- a/lib/vhost/socket.c
> +++ b/lib/vhost/socket.c
> @@ -844,13 +844,6 @@ rte_vhost_driver_set_max_queue_num(const char *path, uint32_t max_queue_pairs)
>  	struct vhost_user_socket *vsocket;
>  	int ret = 0;
>  
> -	VHOST_CONFIG_LOG(path, INFO, "Setting max queue pairs to %u", max_queue_pairs);
> -
> -	if (max_queue_pairs > VHOST_MAX_QUEUE_PAIRS) {
> -		VHOST_CONFIG_LOG(path, ERR, "Library only supports up to %u queue pairs",
> -				VHOST_MAX_QUEUE_PAIRS);
> -		return -1;
> -	}
>  
>  	pthread_mutex_lock(&vhost_user.mutex);
>  	vsocket = find_vhost_user_socket(path);
> @@ -872,6 +865,15 @@ rte_vhost_driver_set_max_queue_num(const char *path, uint32_t max_queue_pairs)
>  		goto unlock_exit;
>  	}
>  
> +	VHOST_CONFIG_LOG(path, INFO, "Setting max queue pairs to %u", max_queue_pairs);
> +
> +	if (max_queue_pairs > VHOST_MAX_QUEUE_PAIRS) {
> +		VHOST_CONFIG_LOG(path, ERR, "Library only supports up to %u queue pairs",
> +				VHOST_MAX_QUEUE_PAIRS);
> +		ret = -1;
> +		goto unlock_exit;
> +	}
> +
>  	vsocket->max_queue_pairs = max_queue_pairs;
>  
>  unlock_exit:

Re: [PATCH] vhost: fix misleading log when setting max queue num

[Chenbo Xia]

Hi Maxime,

> On Jan 9, 2025, at 22:31, Maxime Coquelin <maxime.coquelin@redhat.com> wrote:
> 
> External email: Use caution opening links or attachments
> 
> 
> rte_vhost_driver_set_max_queue_num API returns early when
> called for a Vhost-user device, as this API is intended to
> limit the maximum number of queue pairs supported by VDUSE
> devices. However, a log mentioning the maximim number of

maximum

With above fixed:

Reviewed-by: Chenbo Xia <chenbox@nvidia.com>

> queue pairs is being set is emitted unconditionally, which
> may confuse the end user.
> 
> This patch moves this log after the backend type is
> checked, so that it is only called with VDUSE backends.
> The check on the requested value is also moved at the same
> place.
> 
> Fixes: e1808999d36b ("vhost: restrict set max queue pair API to VDUSE")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
> lib/vhost/socket.c | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/lib/vhost/socket.c b/lib/vhost/socket.c
> index d29d15494c..07247907b0 100644
> --- a/lib/vhost/socket.c
> +++ b/lib/vhost/socket.c
> @@ -844,13 +844,6 @@ rte_vhost_driver_set_max_queue_num(const char *path, uint32_t max_queue_pairs)
>        struct vhost_user_socket *vsocket;
>        int ret = 0;
> 
> -       VHOST_CONFIG_LOG(path, INFO, "Setting max queue pairs to %u", max_queue_pairs);
> -
> -       if (max_queue_pairs > VHOST_MAX_QUEUE_PAIRS) {
> -               VHOST_CONFIG_LOG(path, ERR, "Library only supports up to %u queue pairs",
> -                               VHOST_MAX_QUEUE_PAIRS);
> -               return -1;
> -       }
> 
>        pthread_mutex_lock(&vhost_user.mutex);
>        vsocket = find_vhost_user_socket(path);
> @@ -872,6 +865,15 @@ rte_vhost_driver_set_max_queue_num(const char *path, uint32_t max_queue_pairs)
>                goto unlock_exit;
>        }
> 
> +       VHOST_CONFIG_LOG(path, INFO, "Setting max queue pairs to %u", max_queue_pairs);
> +
> +       if (max_queue_pairs > VHOST_MAX_QUEUE_PAIRS) {
> +               VHOST_CONFIG_LOG(path, ERR, "Library only supports up to %u queue pairs",
> +                               VHOST_MAX_QUEUE_PAIRS);
> +               ret = -1;
> +               goto unlock_exit;
> +       }
> +
>        vsocket->max_queue_pairs = max_queue_pairs;
> 
> unlock_exit:
> --
> 2.47.1
> 

Re: [PATCH] vhost: fix misleading log when setting max queue num

[Kevin Traynor]

On 09/01/2025 14:31, Maxime Coquelin wrote:
> rte_vhost_driver_set_max_queue_num API returns early when
> called for a Vhost-user device, as this API is intended to
> limit the maximum number of queue pairs supported by VDUSE
> devices. However, a log mentioning the maximim number of
> queue pairs is being set is emitted unconditionally, which
> may confuse the end user.
> 
> This patch moves this log after the backend type is
> checked, so that it is only called with VDUSE backends.
> The check on the requested value is also moved at the same
> place.
> 
> Fixes: e1808999d36b ("vhost: restrict set max queue pair API to VDUSE")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  lib/vhost/socket.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 

Thanks, this will make it easier for vhost-user users. One nit below,

Acked-by: Kevin Traynor <ktraynor@redhat.com>

> diff --git a/lib/vhost/socket.c b/lib/vhost/socket.c
> index d29d15494c..07247907b0 100644
> --- a/lib/vhost/socket.c
> +++ b/lib/vhost/socket.c
> @@ -844,13 +844,6 @@ rte_vhost_driver_set_max_queue_num(const char *path, uint32_t max_queue_pairs)
>  	struct vhost_user_socket *vsocket;
>  	int ret = 0;
>  
> -	VHOST_CONFIG_LOG(path, INFO, "Setting max queue pairs to %u", max_queue_pairs);
> -
> -	if (max_queue_pairs > VHOST_MAX_QUEUE_PAIRS) {
> -		VHOST_CONFIG_LOG(path, ERR, "Library only supports up to %u queue pairs",
> -				VHOST_MAX_QUEUE_PAIRS);
> -		return -1;
> -	}
>  

you can remove the extra blank line here

>  	pthread_mutex_lock(&vhost_user.mutex);
>  	vsocket = find_vhost_user_socket(path);
> @@ -872,6 +865,15 @@ rte_vhost_driver_set_max_queue_num(const char *path, uint32_t max_queue_pairs)
>  		goto unlock_exit;
>  	}
>  
> +	VHOST_CONFIG_LOG(path, INFO, "Setting max queue pairs to %u", max_queue_pairs);
> +
> +	if (max_queue_pairs > VHOST_MAX_QUEUE_PAIRS) {
> +		VHOST_CONFIG_LOG(path, ERR, "Library only supports up to %u queue pairs",
> +				VHOST_MAX_QUEUE_PAIRS);
> +		ret = -1;
> +		goto unlock_exit;
> +	}
> +
>  	vsocket->max_queue_pairs = max_queue_pairs;
>  
>  unlock_exit: