Re: [PATCH 04/18] vhost-vdpa: remove the unnecessary check in vhost_vdpa_add()

[Jason Wang]

在 2021/6/23 下午10:53, Stefano Garzarella 写道:

On Mon, Jun 21, 2021 at 12:16:36PM +0800, Jason Wang wrote:

The VhostVDPAState is just allocated by qemu_new_net_client() via
g_malloc0() in net_vhost_vdpa_init(). So s->vhost_net is NULL for
sure, let's remove this unnecessary check in vhost_vdpa_add().

Signed-off-by: Jason Wang 
---
net/vhost-vdpa.c | 4 
1 file changed, 4 deletions(-)

diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 18b45ad777..728e63ff54 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -112,10 +112,6 @@ static int vhost_vdpa_add(NetClientState *ncs, 
void *be)

    error_report("failed to init vhost_net for queue");
    goto err;
    }
-    if (s->vhost_net) {
-    vhost_net_cleanup(s->vhost_net);
-    g_free(s->vhost_net);
-    }


Maybe we can add an assert() to discover future issues, but I don't 
have a strong opinion.



I think the assumption of qemu_new_net_client() is that it will always 
succeed (see other caller).


So I tend not to bother.

Thanks




It is fine:

Reviewed-by: Stefano Garzarella 


    s->vhost_net = net;
    ret = vhost_vdpa_net_check_device_id(net);
    if (ret) {
-- 2.25.1
2.25.1

Re: [PATCH 04/18] vhost-vdpa: remove the unnecessary check in vhost_vdpa_add()

[Eli Cohen]

On Mon, Jun 21, 2021 at 12:16:36PM +0800, Jason Wang wrote:
> The VhostVDPAState is just allocated by qemu_new_net_client() via
> g_malloc0() in net_vhost_vdpa_init(). So s->vhost_net is NULL for
> sure, let's remove this unnecessary check in vhost_vdpa_add().
> 
> Signed-off-by: Jason Wang 

Reviewed-by: Eli Cohen 

> ---
> net/vhost-vdpa.c | 4 
> 1 file changed, 4 deletions(-)
> 
> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> index 18b45ad777..728e63ff54 100644
> --- a/net/vhost-vdpa.c
> +++ b/net/vhost-vdpa.c
> @@ -112,10 +112,6 @@ static int vhost_vdpa_add(NetClientState *ncs, void *be)
> error_report("failed to init vhost_net for queue");
> goto err;
> }
> -if (s->vhost_net) {
> -vhost_net_cleanup(s->vhost_net);
> -g_free(s->vhost_net);
> -}
> s->vhost_net = net;
> ret = vhost_vdpa_net_check_device_id(net);
> if (ret) {
> -- 2.25.1
> 2.25.1
> 
> 

Re: [PATCH 04/18] vhost-vdpa: remove the unnecessary check in vhost_vdpa_add()

[Stefano Garzarella]

On Mon, Jun 21, 2021 at 12:16:36PM +0800, Jason Wang wrote:

The VhostVDPAState is just allocated by qemu_new_net_client() via
g_malloc0() in net_vhost_vdpa_init(). So s->vhost_net is NULL for
sure, let's remove this unnecessary check in vhost_vdpa_add().

Signed-off-by: Jason Wang 
---
net/vhost-vdpa.c | 4 
1 file changed, 4 deletions(-)

diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 18b45ad777..728e63ff54 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -112,10 +112,6 @@ static int vhost_vdpa_add(NetClientState *ncs, void *be)
error_report("failed to init vhost_net for queue");
goto err;
}
-if (s->vhost_net) {
-vhost_net_cleanup(s->vhost_net);
-g_free(s->vhost_net);
-}


Maybe we can add an assert() to discover future issues, but I don't have 
a strong opinion.


It is fine:

Reviewed-by: Stefano Garzarella 


s->vhost_net = net;
ret = vhost_vdpa_net_check_device_id(net);
if (ret) {
-- 2.25.1
2.25.1

[PATCH 04/18] vhost-vdpa: remove the unnecessary check in vhost_vdpa_add()

[Jason Wang]

The VhostVDPAState is just allocated by qemu_new_net_client() via
g_malloc0() in net_vhost_vdpa_init(). So s->vhost_net is NULL for
sure, let's remove this unnecessary check in vhost_vdpa_add().

Signed-off-by: Jason Wang 
---
 net/vhost-vdpa.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 18b45ad777..728e63ff54 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -112,10 +112,6 @@ static int vhost_vdpa_add(NetClientState *ncs, void *be)
 error_report("failed to init vhost_net for queue");
 goto err;
 }
-if (s->vhost_net) {
-vhost_net_cleanup(s->vhost_net);
-g_free(s->vhost_net);
-}
 s->vhost_net = net;
 ret = vhost_vdpa_net_check_device_id(net);
 if (ret) {
-- 
2.25.1