[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [v5, 09/31] vhost: fix calling vhost_dev_cleanup() afte
From: |
Ilya Maximets |
Subject: |
Re: [Qemu-devel] [v5, 09/31] vhost: fix calling vhost_dev_cleanup() after vhost_dev_init() |
Date: |
Mon, 25 Jul 2016 15:33:46 +0300 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.8.0 |
On 21.07.2016 11:57, Marc-André Lureau wrote:
> From: Marc-André Lureau <address@hidden>
>
> vhost_net_init() calls vhost_dev_init() and in case of failure, calls
> vhost_dev_cleanup() directly. However, the structure is already
> partially cleaned on error. Calling vhost_dev_cleanup() again will call
> vhost_virtqueue_cleanup() on already clean queues, and causing potential
> double-close. Instead, adjust dev->nvqs and simplify vhost_dev_init()
> code to not call vhost_virtqueue_cleanup() but vhost_dev_cleanup()
> instead.
>
> Signed-off-by: Marc-André Lureau <address@hidden>
> ---
> hw/virtio/vhost.c | 13 ++++---------
> 1 file changed, 4 insertions(+), 9 deletions(-)
>
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 9400b47..c61302a 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -1047,7 +1047,8 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
> for (i = 0; i < hdev->nvqs; ++i) {
> r = vhost_virtqueue_init(hdev, hdev->vqs + i, hdev->vq_index + i);
> if (r < 0) {
> - goto fail_vq;
> + hdev->nvqs = i;
> + goto fail;
> }
> }
>
> @@ -1104,19 +1105,13 @@ int vhost_dev_init(struct vhost_dev *hdev, void
> *opaque,
> memory_listener_register(&hdev->memory_listener, &address_space_memory);
> QLIST_INSERT_HEAD(&vhost_devices, hdev, entry);
> return 0;
> +
> fail_busyloop:
> while (--i >= 0) {
> vhost_virtqueue_set_busyloop_timeout(hdev, hdev->vq_index + i, 0);
> }
> - i = hdev->nvqs;
> -fail_vq:
> - while (--i >= 0) {
> - vhost_virtqueue_cleanup(hdev->vqs + i);
> - }
> fail:
> - r = -errno;
> - hdev->vhost_ops->vhost_backend_cleanup(hdev);
> - QLIST_REMOVE(hdev, entry);
> + vhost_dev_cleanup(hdev);
> return r;
> }
>
>
This patch introduces closing of zero fd on backend init failure or any
other error before virtqueue_init loop because of calling
'vhost_virtqueue_cleanup()' on not initialized virtqueues.
I'm suggesting following fixup:
----------------------------------------------------------------------
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 6175d8b..d7428c5 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1038,8 +1038,9 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
VhostBackendType backend_type, uint32_t busyloop_timeout)
{
uint64_t features;
- int i, r;
+ int i, r, n_initialized_vqs;
+ n_initialized_vqs = 0;
hdev->migration_blocker = NULL;
r = vhost_set_backend_type(hdev, backend_type);
@@ -1069,10 +1071,9 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
goto fail;
}
- for (i = 0; i < hdev->nvqs; ++i) {
+ for (i = 0; i < hdev->nvqs; ++i, ++n_initialized_vqs) {
r = vhost_virtqueue_init(hdev, hdev->vqs + i, hdev->vq_index + i);
if (r < 0) {
- hdev->nvqs = i;
goto fail;
}
}
@@ -1136,6 +1137,7 @@ fail_busyloop:
vhost_virtqueue_set_busyloop_timeout(hdev, hdev->vq_index + i, 0);
}
fail:
+ hdev->nvqs = n_initialized_vqs;
vhost_dev_cleanup(hdev);
return r;
}
----------------------------------------------------------------------
Best regards, Ilya Maximets.
- [Qemu-devel] [PATCH v5 00/31] vhost-user reconnect fixes, marcandre . lureau, 2016/07/21
- [Qemu-devel] [PATCH v5 01/31] misc: indentation, marcandre . lureau, 2016/07/21
- [Qemu-devel] [PATCH v5 02/31] vhost-user: minor simplification, marcandre . lureau, 2016/07/21
- [Qemu-devel] [PATCH v5 03/31] vhost: don't assume opaque is a fd, use backend cleanup, marcandre . lureau, 2016/07/21
- [Qemu-devel] [PATCH v5 04/31] vhost: make vhost_log_put() idempotent, marcandre . lureau, 2016/07/21
- [Qemu-devel] [PATCH v5 05/31] vhost: assert the log was cleaned up, marcandre . lureau, 2016/07/21
- [Qemu-devel] [PATCH v5 06/31] vhost: fix cleanup on not fully initialized device, marcandre . lureau, 2016/07/21
- [Qemu-devel] [PATCH v5 07/31] vhost: make vhost_dev_cleanup() idempotent, marcandre . lureau, 2016/07/21
- [Qemu-devel] [PATCH v5 08/31] vhost-net: always call vhost_dev_cleanup() on failure, marcandre . lureau, 2016/07/21
- [Qemu-devel] [PATCH v5 09/31] vhost: fix calling vhost_dev_cleanup() after vhost_dev_init(), marcandre . lureau, 2016/07/21
- Re: [Qemu-devel] [v5, 09/31] vhost: fix calling vhost_dev_cleanup() after vhost_dev_init(),
Ilya Maximets <=
- Re: [Qemu-devel] [v5, 09/31] vhost: fix calling vhost_dev_cleanup() after vhost_dev_init(), Marc-André Lureau, 2016/07/25
- Re: [Qemu-devel] [v5, 09/31] vhost: fix calling vhost_dev_cleanup() after vhost_dev_init(), Ilya Maximets, 2016/07/25
- Re: [Qemu-devel] [v5, 09/31] vhost: fix calling vhost_dev_cleanup() after vhost_dev_init(), Marc-André Lureau, 2016/07/25
- Re: [Qemu-devel] [v5, 09/31] vhost: fix calling vhost_dev_cleanup() after vhost_dev_init(), Ilya Maximets, 2016/07/25
[Qemu-devel] [PATCH v5 10/31] vhost: do not assert() on vhost_ops failure, marcandre . lureau, 2016/07/21
[Qemu-devel] [PATCH v5 11/31] vhost: add missing VHOST_OPS_DEBUG, marcandre . lureau, 2016/07/21
[Qemu-devel] [PATCH v5 12/31] vhost: use error_report() instead of fprintf(stderr, ...), marcandre . lureau, 2016/07/21
[Qemu-devel] [PATCH v5 13/31] qemu-char: fix qemu_chr_fe_set_msgfds() crash when disconnected, marcandre . lureau, 2016/07/21
[Qemu-devel] [PATCH v5 15/31] vhost-user: check qemu_chr_fe_set_msgfds() return value, marcandre . lureau, 2016/07/21
[Qemu-devel] [PATCH v5 14/31] vhost-user: call set_msgfds unconditionally, marcandre . lureau, 2016/07/21