Hi Maxime,
> -----Original Message-----
> From: Maxime Coquelin <maxime.coque...@redhat.com>
> Sent: Wednesday, January 20, 2021 5:25 AM
> To: dev@dpdk.org; Xia, Chenbo <chenbo....@intel.com>; olivier.m...@6wind.com;
> amore...@redhat.com; david.march...@redhat.com
> Cc: Maxime Coquelin <maxime.coque...@redhat.com>
> Subject: [PATCH v2 44/44] net/virtio: handle Virtio-user setup failure
> properly
>
> This patch fixes virtio_user_dev_setup() error path,
> by cleaning all resources it allocates.
>
> Suggested-by: Adrian Moreno <amore...@redhat.com>
> Signed-off-by: Maxime Coquelin <maxime.coque...@redhat.com>
> ---
> drivers/net/virtio/virtio_user/virtio_user_dev.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> index a1e32158bb..2f7dc574b6 100644
> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> @@ -427,15 +427,22 @@ virtio_user_dev_setup(struct virtio_user_dev *dev)
>
> if (virtio_user_dev_init_notify(dev) < 0) {
> PMD_INIT_LOG(ERR, "(%s) Failed to init notifiers\n", dev->path);
> - return -1;
> + goto destroy;
> }
>
> if (virtio_user_fill_intr_handle(dev) < 0) {
> PMD_INIT_LOG(ERR, "(%s) Failed to init interrupt handler\n",
> dev-
> >path);
> - return -1;
> + goto uninit;
> }
>
> return 0;
> +
> +uninit:
> + virtio_user_dev_uninit(dev);
IMHO, it may not be the best choice to call virtio_user_dev_uninit here..
Logically when virtio_user_fill_intr_handle fails, we want to release the
resources
which is allocated in virtio_user_dev_init_notify (i.e., fds), but
virtio_user_dev_uninit
does more. For example, unregister the event callback that have not been
registered yet and
it also leads to destroy callback called twice. Although things done but not
needed will
not lead to error, but it looks not in the best way. What do you think?
Thanks,
Chenbo
> +destroy:
> + dev->ops->destroy(dev);
> +
> + return -1;
> }
>
> /* Use below macro to filter features from vhost backend */
> --
> 2.29.2
