From mboxrd@z Thu Jan  1 00:00:00 1970
From: Yuanhan Liu <yliu@fridaylinux.org>
Subject: Re: [PATCH] vhost: fix vhost-user init failed
Date: Fri, 14 Jul 2017 10:43:52 +0800
Message-ID: <20170714024351.GG11626@yliu-home>
References: <20170710080648.63443-1-zhiyong.yang@intel.com>
 <20170710094827.zdykowebtm3m3w2k@dhcp-192-218.str.redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: Zhiyong Yang <zhiyong.yang@intel.com>, dev@dpdk.org,
 maxime.coquelin@redhat.com, lei.a.yao@intel.com
To: Jens Freimann <jfreimann@redhat.com>
Return-path: <dev-bounces@dpdk.org>
Received: from mail-pf0-f194.google.com (mail-pf0-f194.google.com
 [209.85.192.194]) by dpdk.org (Postfix) with ESMTP id 3A21829D9
 for <dev@dpdk.org>; Fri, 14 Jul 2017 04:44:01 +0200 (CEST)
Received: by mail-pf0-f194.google.com with SMTP id z6so9240239pfk.3
 for <dev@dpdk.org>; Thu, 13 Jul 2017 19:44:01 -0700 (PDT)
Content-Disposition: inline
In-Reply-To: <20170710094827.zdykowebtm3m3w2k@dhcp-192-218.str.redhat.com>
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

On Mon, Jul 10, 2017 at 11:48:27AM +0200, Jens Freimann wrote:
> >@@ -668,7 +668,7 @@ rte_vhost_driver_register(const char *path, uint64_t flags)
> >	}
> >
> >	vhost_user.vsockets[vhost_user.vsocket_cnt++] = vsocket;
> >-
> >+	goto out;
> >out_mutex:
> >	if (pthread_mutex_destroy(&vsocket->conn_mutex)) {
> >		RTE_LOG(ERR, VHOST_CONFIG,
> 
> Thanks for fixing this!
> 
> Sorry for introducing this bug, I was about to send this before I saw
> your fix:
> 
> diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c
> index 57b86c0..b2158a7 100644
> --- a/lib/librte_vhost/socket.c
> +++ b/lib/librte_vhost/socket.c
> @@ -668,6 +668,9 @@ rte_vhost_driver_register(const char *path, uint64_t flags)
>        }
> 
>        vhost_user.vsockets[vhost_user.vsocket_cnt++] = vsocket;
> +out:
> +       pthread_mutex_unlock(&vhost_user.mutex);
> +       return ret;
> 
> out_mutex:
>        if (pthread_mutex_destroy(&vsocket->conn_mutex)) {
> @@ -677,9 +680,7 @@ rte_vhost_driver_register(const char *path, uint64_t flags)
> out_free:
>        free(vsocket->path);
>        free(vsocket);
> -out:
>        pthread_mutex_unlock(&vhost_user.mutex);
> -
>        return ret;
> }
> 
> Both works fine, so I leave it up to the maintainers how to fix.
> 
> Reviewed-by: Jens Freimann <jfreimann@redhat.com>

I actually prefers below: I don't want a simple "out" on top of "out_mutex"
and "out_free", and an unconditional "goto" looks weird to me.

---
@@ -669,6 +669,9 @@ rte_vhost_driver_register(const char *path, uint64_t flags)

        vhost_user.vsockets[vhost_user.vsocket_cnt++] = vsocket;

+       pthread_mutex_unlock(&vhost_user.mutex);
+       return ret;
+
 out_mutex:
        if (pthread_mutex_destroy(&vsocket->conn_mutex)) {
                RTE_LOG(ERR, VHOST_CONFIG,


Applied to dpdk-next-virtio, with above changes.

Thanks!

	--yliu