[v1] vhost-vDPA multiqueue

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

Posted by Jason Wang 4 years, 7 months ago

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 <jasowang@redhat.com>
---
 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

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

Posted by Stefano Garzarella 4 years, 7 months ago

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 <jasowang@redhat.com>
>---
> 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 <sgarzare@redhat.com>

>     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()

Posted by Eli Cohen 4 years, 7 months ago

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 <jasowang@redhat.com>

Reviewed-by: Eli Cohen <elic@nvidia.com>

> ---
> 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()

Posted by Jason Wang 4 years, 7 months ago

在 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 <jasowang@redhat.com>
>> ---
>> 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 <sgarzare@redhat.com>
>
>>     s->vhost_net = net;
>>     ret = vhost_vdpa_net_check_device_id(net);
>>     if (ret) {
>> -- 2.25.1
>> 2.25.1
>>
>>
>