[dpdk-dev,1/2] vhost: make sure vhost fdset-thread created successfully

Message ID 1521722398-93353-1-git-send-email-xiangxia.m.yue@gmail.com (mailing list archive)
State Superseded, archived
Delegated to: Maxime Coquelin
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues

Commit Message

Tonghao Zhang March 22, 2018, 12:39 p.m. UTC
  From: Tonghao Zhang <xiangxia.m.yue@gmail.com>

When first call the 'rte_vhost_driver_start', the
fdset_event_dispatch thread should be created successfully.
Because the vhost uses it to poll socket events for vhost
server or clients. Without it, for example, vhost will not
get the connection event.

This patch returns err code directly when created not successful.

Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
---
 lib/librte_vhost/socket.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
  

Comments

Jianfeng Tan March 23, 2018, 1:30 a.m. UTC | #1
Hi Xiangxia,

> -----Original Message-----
> From: xiangxia.m.yue@gmail.com [mailto:xiangxia.m.yue@gmail.com]
> Sent: Thursday, March 22, 2018 8:40 PM
> To: Tan, Jianfeng
> Cc: dev@dpdk.org; Tonghao Zhang
> Subject: [PATCH 1/2] vhost: make sure vhost fdset-thread created
> successfully
> 
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> 
> When first call the 'rte_vhost_driver_start', the
> fdset_event_dispatch thread should be created successfully.
> Because the vhost uses it to poll socket events for vhost
> server or clients. Without it, for example, vhost will not
> get the connection event.
> 
> This patch returns err code directly when created not successful.
> 
> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>

Except a nit below,

Reviewed-by: Jianfeng Tan <jianfeng.tan@intel.com>

> ---
>  lib/librte_vhost/socket.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c
> index 83befdc..8ca01df 100644
> --- a/lib/librte_vhost/socket.c
> +++ b/lib/librte_vhost/socket.c
> @@ -831,9 +831,11 @@ struct vhost_device_ops const *
>  	if (fdset_tid == 0) {
>  		int ret = pthread_create(&fdset_tid, NULL,
> fdset_event_dispatch,
>  				     &vhost_user.fdset);
> -		if (ret != 0)
> +		if (ret != 0) {
>  			RTE_LOG(ERR, VHOST_CONFIG,
>  				"failed to create fdset handling thread");
> +			return ret;

According to the declaration below, we shall return 0 or -1.

/**
 *
 * Start the vhost-user driver.
 *
 * This function triggers the vhost-user negotiation.
 *
 * @param path
 *  The vhost-user socket file path
 * @return
 *  0 on success, -1 on failure
 */
int rte_vhost_driver_start(const char *path);

> +		}
>  	}
> 
>  	if (vsocket->is_server)
> --
> 1.8.3.1
  
Tonghao Zhang March 23, 2018, 1:53 a.m. UTC | #2
On Fri, Mar 23, 2018 at 9:30 AM, Tan, Jianfeng <jianfeng.tan@intel.com> wrote:
> Hi Xiangxia,
>
>> -----Original Message-----
>> From: xiangxia.m.yue@gmail.com [mailto:xiangxia.m.yue@gmail.com]
>> Sent: Thursday, March 22, 2018 8:40 PM
>> To: Tan, Jianfeng
>> Cc: dev@dpdk.org; Tonghao Zhang
>> Subject: [PATCH 1/2] vhost: make sure vhost fdset-thread created
>> successfully
>>
>> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>>
>> When first call the 'rte_vhost_driver_start', the
>> fdset_event_dispatch thread should be created successfully.
>> Because the vhost uses it to poll socket events for vhost
>> server or clients. Without it, for example, vhost will not
>> get the connection event.
>>
>> This patch returns err code directly when created not successful.
>>
>> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>
> Except a nit below,
>
> Reviewed-by: Jianfeng Tan <jianfeng.tan@intel.com>
>
>> ---
>>  lib/librte_vhost/socket.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c
>> index 83befdc..8ca01df 100644
>> --- a/lib/librte_vhost/socket.c
>> +++ b/lib/librte_vhost/socket.c
>> @@ -831,9 +831,11 @@ struct vhost_device_ops const *
>>       if (fdset_tid == 0) {
>>               int ret = pthread_create(&fdset_tid, NULL,
>> fdset_event_dispatch,
>>                                    &vhost_user.fdset);
>> -             if (ret != 0)
>> +             if (ret != 0) {
>>                       RTE_LOG(ERR, VHOST_CONFIG,
>>                               "failed to create fdset handling thread");
>> +                     return ret;
>
> According to the declaration below, we shall return 0 or -1.
I will send v2

> /**
>  *
>  * Start the vhost-user driver.
>  *
>  * This function triggers the vhost-user negotiation.
>  *
>  * @param path
>  *  The vhost-user socket file path
>  * @return
>  *  0 on success, -1 on failure
>  */
> int rte_vhost_driver_start(const char *path);
>
>> +             }
>>       }
>>
>>       if (vsocket->is_server)
>> --
>> 1.8.3.1
>
  

Patch

diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c
index 83befdc..8ca01df 100644
--- a/lib/librte_vhost/socket.c
+++ b/lib/librte_vhost/socket.c
@@ -831,9 +831,11 @@  struct vhost_device_ops const *
 	if (fdset_tid == 0) {
 		int ret = pthread_create(&fdset_tid, NULL, fdset_event_dispatch,
 				     &vhost_user.fdset);
-		if (ret != 0)
+		if (ret != 0) {
 			RTE_LOG(ERR, VHOST_CONFIG,
 				"failed to create fdset handling thread");
+			return ret;
+		}
 	}
 
 	if (vsocket->is_server)