[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PULL V2 19/25] vdpa: Extract get features part from vhost_vdpa_get_
From: |
Peter Maydell |
Subject: |
Re: [PULL V2 19/25] vdpa: Extract get features part from vhost_vdpa_get_max_queue_pairs |
Date: |
Fri, 29 Jul 2022 15:08:33 +0100 |
On Wed, 20 Jul 2022 at 10:04, Jason Wang <jasowang@redhat.com> wrote:
>
> From: Eugenio Pérez <eperezma@redhat.com>
>
> To know the device features is needed for CVQ SVQ, so SVQ knows if it
> can handle all commands or not. Extract from
> vhost_vdpa_get_max_queue_pairs so we can reuse it.
>
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> Acked-by: Jason Wang <jasowang@redhat.com>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
Hi; this change introduces a resource leak in the new
error-exit return path in net_init_vhost_vdpa(). Spotted
by Coverity, CID 1490785.
> @@ -517,10 +521,11 @@ int net_init_vhost_vdpa(const Netdev *netdev, const
> char *name,
> NetClientState *peer, Error **errp)
> {
> const NetdevVhostVDPAOptions *opts;
> + uint64_t features;
> int vdpa_device_fd;
> g_autofree NetClientState **ncs = NULL;
> NetClientState *nc;
> - int queue_pairs, i, has_cvq = 0;
> + int queue_pairs, r, i, has_cvq = 0;
>
> assert(netdev->type == NET_CLIENT_DRIVER_VHOST_VDPA);
> opts = &netdev->u.vhost_vdpa;
> @@ -534,7 +539,12 @@ int net_init_vhost_vdpa(const Netdev *netdev, const char
> *name,
> return -errno;
> }
>
> - queue_pairs = vhost_vdpa_get_max_queue_pairs(vdpa_device_fd,
> + r = vhost_vdpa_get_features(vdpa_device_fd, &features, errp);
> + if (unlikely(r < 0)) {
> + return r;
At this point in the code we have allocated the file descriptor
vdpa_device_fd, but this return path fails to close it.
> + }
> +
> + queue_pairs = vhost_vdpa_get_max_queue_pairs(vdpa_device_fd, features,
> &has_cvq, errp);
> if (queue_pairs < 0) {
> qemu_close(vdpa_device_fd);
Compare this pre-existing error-exit path, which correctly
calls qemu_close() on the fd.
Related question: is this function supposed to return -1 on
failure, or negative-errno ? At the moment it has a mix
of both. I think that the sole caller only really wants "<0 on
error", in which case the error-exit code paths could probably
be tidied up so that instead of explicitly calling
qemu_close() and returning r, queue_pairs, or whatever
they got back from the function they just called, they
could just 'goto err_svq' which will do the "close the fd
and return -1" work. Better still, by initializing 'i'
to 0 at the top of the function (and naming it something
clearer, ideally), all the code paths after the initial
qemu_open() succeeds could be made to use 'goto err'
for the error-exit case.
thanks
-- PMM
- [PULL V2 06/25] vhost: Move vhost_svq_kick call to vhost_svq_add, (continued)
- [PULL V2 06/25] vhost: Move vhost_svq_kick call to vhost_svq_add, Jason Wang, 2022/07/20
- [PULL V2 07/25] vhost: Check for queue full at vhost_svq_add, Jason Wang, 2022/07/20
- [PULL V2 08/25] vhost: Decouple vhost_svq_add from VirtQueueElement, Jason Wang, 2022/07/20
- [PULL V2 09/25] vhost: Add SVQDescState, Jason Wang, 2022/07/20
- [PULL V2 10/25] vhost: Track number of descs in SVQDescState, Jason Wang, 2022/07/20
- [PULL V2 11/25] vhost: add vhost_svq_push_elem, Jason Wang, 2022/07/20
- [PULL V2 05/25] vhost: Reorder vhost_svq_kick, Jason Wang, 2022/07/20
- [PULL V2 17/25] vdpa: manual forward CVQ buffers, Jason Wang, 2022/07/20
- [PULL V2 15/25] vdpa: Export vhost_vdpa_dma_map and unmap calls, Jason Wang, 2022/07/20
- [PULL V2 19/25] vdpa: Extract get features part from vhost_vdpa_get_max_queue_pairs, Jason Wang, 2022/07/20
- Re: [PULL V2 19/25] vdpa: Extract get features part from vhost_vdpa_get_max_queue_pairs,
Peter Maydell <=
- [PULL V2 22/25] softmmu/runstate.c: add RunStateTransition support form COLO to PRELAUNCH, Jason Wang, 2022/07/20
- [PULL V2 12/25] vhost: Expose vhost_svq_add, Jason Wang, 2022/07/20
- [PULL V2 14/25] vhost: Add svq avail_handler callback, Jason Wang, 2022/07/20
- [PULL V2 13/25] vhost: add vhost_svq_poll, Jason Wang, 2022/07/20
- [PULL V2 16/25] vhost-net-vdpa: add stubs for when no virtio-net device is present, Jason Wang, 2022/07/20
- [PULL V2 18/25] vdpa: Buffer CVQ support on shadow virtqueue, Jason Wang, 2022/07/20
- [PULL V2 20/25] vdpa: Add device migration blocker, Jason Wang, 2022/07/20
- [PULL V2 21/25] vdpa: Add x-svq to NetdevVhostVDPAOptions, Jason Wang, 2022/07/20
- [PULL V2 23/25] net/colo: Fix a "double free" crash to clear the conn_list, Jason Wang, 2022/07/20