Re: [Qemu-devel] [PATCH] vhost: Fix vhostfd leak in error branch
On Fri, Nov 28, 2014 at 5:26 PM, arei.gong...@huawei.com wrote: From: Gonglei arei.gong...@huawei.com Signed-off-by: Gonglei arei.gong...@huawei.com --- hw/scsi/vhost-scsi.c | 1 + hw/virtio/vhost.c| 2 ++ 2 files changed, 3 insertions(+) diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c index 308b393..dcb2bc5 100644 --- a/hw/scsi/vhost-scsi.c +++ b/hw/scsi/vhost-scsi.c @@ -233,6 +233,7 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp) vhost_dummy_handle_output); if (err != NULL) { error_propagate(errp, err); +close(vhostfd); return; } diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index 5d7c40a..5a12861 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -817,10 +817,12 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque, int i, r; if (vhost_set_backend_type(hdev, backend_type) 0) { +close((uintptr_t)opaque); return -1; } if (hdev-vhost_ops-vhost_backend_init(hdev, opaque) 0) { +close((uintptr_t)opaque); return -errno; } Patch looks fine. I wonder whether setting errno and goto fail would be better here? This will let vhost_backend_cleanup() to do the cleanup, e.g closeing fd or purging queue (for vhost uesr).
Re: [Qemu-devel] [PATCH] vhost: Fix vhostfd leak in error branch
On 2014/12/1 13:03, Jason Wang wrote: On Fri, Nov 28, 2014 at 5:26 PM, arei.gong...@huawei.com wrote: From: Gonglei arei.gong...@huawei.com Signed-off-by: Gonglei arei.gong...@huawei.com --- hw/scsi/vhost-scsi.c | 1 + hw/virtio/vhost.c| 2 ++ 2 files changed, 3 insertions(+) diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c index 308b393..dcb2bc5 100644 --- a/hw/scsi/vhost-scsi.c +++ b/hw/scsi/vhost-scsi.c @@ -233,6 +233,7 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp) vhost_dummy_handle_output); if (err != NULL) { error_propagate(errp, err); +close(vhostfd); return; } diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index 5d7c40a..5a12861 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -817,10 +817,12 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque, int i, r; if (vhost_set_backend_type(hdev, backend_type) 0) { +close((uintptr_t)opaque); return -1; } if (hdev-vhost_ops-vhost_backend_init(hdev, opaque) 0) { +close((uintptr_t)opaque); return -errno; } Patch looks fine. I wonder whether setting errno and goto fail would be better here? This will let vhost_backend_cleanup() to do the cleanup, e.g closeing fd or purging queue (for vhost uesr). Hi, Jason Actually, vhost_backend_init() can not fail for both vhost-usr and vhost-backend-type-kernel at present. Besides, vhost-usr' s vhost_backend_cleanup() just set dev-opaque to 0, don't purge queues. Regards, -Gonglei
Re: [Qemu-devel] [PATCH] vhost: Fix vhostfd leak in error branch
On Mon, Dec 1, 2014 at 2:27 PM, Gonglei arei.gong...@huawei.com wrote: On 2014/12/1 13:03, Jason Wang wrote: On Fri, Nov 28, 2014 at 5:26 PM, arei.gong...@huawei.com wrote: From: Gonglei arei.gong...@huawei.com Signed-off-by: Gonglei arei.gong...@huawei.com --- hw/scsi/vhost-scsi.c | 1 + hw/virtio/vhost.c| 2 ++ 2 files changed, 3 insertions(+) diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c index 308b393..dcb2bc5 100644 --- a/hw/scsi/vhost-scsi.c +++ b/hw/scsi/vhost-scsi.c @@ -233,6 +233,7 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp) vhost_dummy_handle_output); if (err != NULL) { error_propagate(errp, err); +close(vhostfd); return; } diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index 5d7c40a..5a12861 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -817,10 +817,12 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque, int i, r; if (vhost_set_backend_type(hdev, backend_type) 0) { +close((uintptr_t)opaque); return -1; } if (hdev-vhost_ops-vhost_backend_init(hdev, opaque) 0) { +close((uintptr_t)opaque); return -errno; } Patch looks fine. I wonder whether setting errno and goto fail would be better here? This will let vhost_backend_cleanup() to do the cleanup, e.g closeing fd or purging queue (for vhost uesr). Hi, Jason Actually, vhost_backend_init() can not fail for both vhost-usr and vhost-backend-type-kernel at present. Besides, vhost-usr' s vhost_backend_cleanup() just set dev-opaque to 0, don't purge queues. I see, thanks for explaining. Reviewed-by: Jason Wang jasow...@redhat.com
[Qemu-devel] [PATCH] vhost: Fix vhostfd leak in error branch
From: Gonglei arei.gong...@huawei.com Signed-off-by: Gonglei arei.gong...@huawei.com --- hw/scsi/vhost-scsi.c | 1 + hw/virtio/vhost.c| 2 ++ 2 files changed, 3 insertions(+) diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c index 308b393..dcb2bc5 100644 --- a/hw/scsi/vhost-scsi.c +++ b/hw/scsi/vhost-scsi.c @@ -233,6 +233,7 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp) vhost_dummy_handle_output); if (err != NULL) { error_propagate(errp, err); +close(vhostfd); return; } diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index 5d7c40a..5a12861 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -817,10 +817,12 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque, int i, r; if (vhost_set_backend_type(hdev, backend_type) 0) { +close((uintptr_t)opaque); return -1; } if (hdev-vhost_ops-vhost_backend_init(hdev, opaque) 0) { +close((uintptr_t)opaque); return -errno; } -- 1.7.12.4