* Re: [PATCH 2/2] vhost-user-base: clean up vhost_dev on realize failure
2026-06-29 9:26 ` [PATCH 2/2] vhost-user-base: clean up vhost_dev on realize failure zhaoguohan
@ 2026-06-29 9:57 ` Michael S. Tsirkin
2026-06-30 1:20 ` [PATCH v2 " zhaoguohan
1 sibling, 0 replies; 4+ messages in thread
From: Michael S. Tsirkin @ 2026-06-29 9:57 UTC (permalink / raw)
To: zhaoguohan; +Cc: qemu-devel, Alex Bennée, Stefano Garzarella
On Mon, Jun 29, 2026 at 05:26:19PM +0800, zhaoguohan@kylinos.cn wrote:
> From: GuoHan Zhao <zhaoguohan@kylinos.cn>
>
> vhost-user-base queries the backend shared memory configuration after
> vhost_dev_init() succeeds. If that query fails, or if the shared memory
> configuration fails validation, realization jumps to the error path
> without cleaning up the initialized vhost_dev.
>
> Track whether vhost_dev_init() succeeded and call vhost_dev_cleanup()
> from the error path in that case. Keep a copy of vhost_dev.vqs so the
> allocation can still be freed after vhost_dev_cleanup() clears struct
> vhost_dev.
>
> Fixes: 6608dca74ecf (vhost-user-device: Add shared memory BAR)
> Signed-off-by: GuoHan Zhao <zhaoguohan@kylinos.cn>
> ---
> hw/virtio/vhost-user-base.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/hw/virtio/vhost-user-base.c b/hw/virtio/vhost-user-base.c
> index 6ac523f9ebef..000e91d50331 100644
> --- a/hw/virtio/vhost-user-base.c
> +++ b/hw/virtio/vhost-user-base.c
> @@ -287,6 +287,8 @@ static void vub_device_realize(DeviceState *dev, Error **errp)
> VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> VHostUserBase *vub = VHOST_USER_BASE(dev);
> uint64_t memory_sizes[VIRTIO_MAX_SHMEM_REGIONS];
> + struct vhost_virtqueue *vhost_vqs = NULL;
> + bool vhost_dev_initialized = false;
> int i, ret, nregions, regions_processed = 0;
>
> if (!vub->chardev.chr) {
> @@ -338,6 +340,7 @@ static void vub_device_realize(DeviceState *dev, Error **errp)
>
> vub->vhost_dev.nvqs = vub->num_vqs;
> vub->vhost_dev.vqs = g_new0(struct vhost_virtqueue, vub->vhost_dev.nvqs);
> + vhost_vqs = vub->vhost_dev.vqs;
>
> /* connect to backend */
> ret = vhost_dev_init(&vub->vhost_dev, &vub->vhost_user,
> @@ -346,6 +349,7 @@ static void vub_device_realize(DeviceState *dev, Error **errp)
> if (ret < 0) {
> goto err;
> }
> + vhost_dev_initialized = true;
>
> ret = vub->vhost_dev.vhost_ops->vhost_get_shmem_config(&vub->vhost_dev,
> &nregions,
> @@ -386,6 +390,10 @@ static void vub_device_realize(DeviceState *dev, Error **errp)
> dev, NULL, true);
> return;
> err:
> + if (vhost_dev_initialized) {
> + vhost_dev_cleanup(&vub->vhost_dev);
> + }
> + g_free(vhost_vqs);
> do_vhost_user_cleanup(vdev, vub);
> }
Pls just add another label.
> --
> 2.43.0
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH v2 2/2] vhost-user-base: clean up vhost_dev on realize failure
2026-06-29 9:26 ` [PATCH 2/2] vhost-user-base: clean up vhost_dev on realize failure zhaoguohan
2026-06-29 9:57 ` Michael S. Tsirkin
@ 2026-06-30 1:20 ` zhaoguohan
1 sibling, 0 replies; 4+ messages in thread
From: zhaoguohan @ 2026-06-30 1:20 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael S. Tsirkin, Stefano Garzarella, Alex Bennée
From: GuoHan Zhao <zhaoguohan@kylinos.cn>
Failures after vhost_dev_init() currently skip vhost_dev_cleanup(),
leaking initialized vhost_dev state.
Add a separate unwind label for those paths. Keep a copy of
vhost_dev.vqs so the array can still be freed after vhost_dev_cleanup()
clears struct vhost_dev.
Fixes: 6608dca74ecf (vhost-user-device: Add shared memory BAR)
Signed-off-by: GuoHan Zhao <zhaoguohan@kylinos.cn>
---
v2:
- Use a separate error label instead of a bool.
hw/virtio/vhost-user-base.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/hw/virtio/vhost-user-base.c b/hw/virtio/vhost-user-base.c
index 39b5e637fc37..62a139ebbdfe 100644
--- a/hw/virtio/vhost-user-base.c
+++ b/hw/virtio/vhost-user-base.c
@@ -283,6 +283,7 @@ static void vub_device_realize(DeviceState *dev, Error **errp)
VirtIODevice *vdev = VIRTIO_DEVICE(dev);
VHostUserBase *vub = VHOST_USER_BASE(dev);
uint64_t memory_sizes[VIRTIO_MAX_SHMEM_REGIONS];
+ struct vhost_virtqueue *vhost_vqs = NULL;
int i, ret, nregions, regions_processed = 0;
if (!vub->chardev.chr) {
@@ -334,6 +335,7 @@ static void vub_device_realize(DeviceState *dev, Error **errp)
vub->vhost_dev.nvqs = vub->num_vqs;
vub->vhost_dev.vqs = g_new0(struct vhost_virtqueue, vub->vhost_dev.nvqs);
+ vhost_vqs = vub->vhost_dev.vqs;
/* connect to backend */
ret = vhost_dev_init(&vub->vhost_dev, &vub->vhost_user,
@@ -349,7 +351,7 @@ static void vub_device_realize(DeviceState *dev, Error **errp)
errp);
if (ret < 0) {
- goto err;
+ goto err_vhost_dev;
}
for (i = 0; i < VIRTIO_MAX_SHMEM_REGIONS && regions_processed < nregions; i++) {
@@ -364,14 +366,14 @@ static void vub_device_realize(DeviceState *dev, Error **errp)
errp);
if (ret < 0) {
- goto err;
+ goto err_vhost_dev;
}
}
if (memory_sizes[i] % qemu_real_host_page_size() != 0) {
error_setg(errp, "Shared memory %d size must be a multiple of "
"the host page size", i);
- goto err;
+ goto err_vhost_dev;
}
virtio_new_shmem_region(vdev, i, memory_sizes[i]);
@@ -381,7 +383,10 @@ static void vub_device_realize(DeviceState *dev, Error **errp)
qemu_chr_fe_set_handlers(&vub->chardev, NULL, NULL, vub_event, NULL,
dev, NULL, true);
return;
+err_vhost_dev:
+ vhost_dev_cleanup(&vub->vhost_dev);
err:
+ g_free(vhost_vqs);
do_vhost_user_cleanup(vdev, vub);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 4+ messages in thread