On 21.07.2016 11:57, Marc-André Lureau wrote: > From: Marc-André Lureau <marcandre.lur...@redhat.com> > > 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 <marcandre.lur...@redhat.com> > --- > 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.