* [Qemu-devel] [PATCH] vhost: Fix vhostfd leak in error branch
@ 2014-11-28 9:26 arei.gonglei
2014-12-01 5:03 ` Jason Wang
2014-12-01 9:37 ` [Qemu-devel] [for-2.2] " Michael S. Tsirkin
0 siblings, 2 replies; 6+ messages in thread
From: arei.gonglei @ 2014-11-28 9:26 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, Gonglei, peter.huangpeng, mst
From: Gonglei <arei.gonglei@huawei.com>
Signed-off-by: Gonglei <arei.gonglei@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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] vhost: Fix vhostfd leak in error branch
2014-11-28 9:26 [Qemu-devel] [PATCH] vhost: Fix vhostfd leak in error branch arei.gonglei
@ 2014-12-01 5:03 ` Jason Wang
2014-12-01 6:27 ` Gonglei
2014-12-01 9:37 ` [Qemu-devel] [for-2.2] " Michael S. Tsirkin
1 sibling, 1 reply; 6+ messages in thread
From: Jason Wang @ 2014-12-01 5:03 UTC (permalink / raw)
To: arei.gonglei; +Cc: pbonzini, mst, qemu-devel, peter.huangpeng
On Fri, Nov 28, 2014 at 5:26 PM, arei.gonglei@huawei.com wrote:
> From: Gonglei <arei.gonglei@huawei.com>
>
> Signed-off-by: Gonglei <arei.gonglei@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).
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] vhost: Fix vhostfd leak in error branch
2014-12-01 5:03 ` Jason Wang
@ 2014-12-01 6:27 ` Gonglei
2014-12-01 7:52 ` Jason Wang
0 siblings, 1 reply; 6+ messages in thread
From: Gonglei @ 2014-12-01 6:27 UTC (permalink / raw)
To: Jason Wang; +Cc: pbonzini, mst, qemu-devel, Huangpeng (Peter)
On 2014/12/1 13:03, Jason Wang wrote:
>
>
> On Fri, Nov 28, 2014 at 5:26 PM, arei.gonglei@huawei.com wrote:
>> From: Gonglei <arei.gonglei@huawei.com>
>>
>> Signed-off-by: Gonglei <arei.gonglei@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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] vhost: Fix vhostfd leak in error branch
2014-12-01 6:27 ` Gonglei
@ 2014-12-01 7:52 ` Jason Wang
0 siblings, 0 replies; 6+ messages in thread
From: Jason Wang @ 2014-12-01 7:52 UTC (permalink / raw)
To: Gonglei; +Cc: pbonzini, mst, qemu-devel, Huangpeng (Peter)
On Mon, Dec 1, 2014 at 2:27 PM, Gonglei <arei.gonglei@huawei.com> wrote:
> On 2014/12/1 13:03, Jason Wang wrote:
>
>>
>>
>> On Fri, Nov 28, 2014 at 5:26 PM, arei.gonglei@huawei.com wrote:
>>> From: Gonglei <arei.gonglei@huawei.com>
>>>
>>> Signed-off-by: Gonglei <arei.gonglei@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 <jasowang@redhat.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [for-2.2] Re: [PATCH] vhost: Fix vhostfd leak in error branch
2014-11-28 9:26 [Qemu-devel] [PATCH] vhost: Fix vhostfd leak in error branch arei.gonglei
2014-12-01 5:03 ` Jason Wang
@ 2014-12-01 9:37 ` Michael S. Tsirkin
2014-12-01 13:23 ` Peter Maydell
1 sibling, 1 reply; 6+ messages in thread
From: Michael S. Tsirkin @ 2014-12-01 9:37 UTC (permalink / raw)
To: arei.gonglei; +Cc: peter.maydell, pbonzini, qemu-devel, peter.huangpeng
On Fri, Nov 28, 2014 at 05:26:29PM +0800, arei.gonglei@huawei.com wrote:
> From: Gonglei <arei.gonglei@huawei.com>
>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Peter, could you pick this up for 2.2 please?
Reviewed-by: Michael S. Tsirkin <mst@redhat.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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [for-2.2] Re: [PATCH] vhost: Fix vhostfd leak in error branch
2014-12-01 9:37 ` [Qemu-devel] [for-2.2] " Michael S. Tsirkin
@ 2014-12-01 13:23 ` Peter Maydell
0 siblings, 0 replies; 6+ messages in thread
From: Peter Maydell @ 2014-12-01 13:23 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Paolo Bonzini, Gonglei (Arei), QEMU Developers, Huangpeng (Peter)
On 1 December 2014 at 09:37, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Fri, Nov 28, 2014 at 05:26:29PM +0800, arei.gonglei@huawei.com wrote:
>> From: Gonglei <arei.gonglei@huawei.com>
>>
>> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
>
> Peter, could you pick this up for 2.2 please?
>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Applied, thanks.
-- PMM
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-12-01 13:23 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-11-28 9:26 [Qemu-devel] [PATCH] vhost: Fix vhostfd leak in error branch arei.gonglei
2014-12-01 5:03 ` Jason Wang
2014-12-01 6:27 ` Gonglei
2014-12-01 7:52 ` Jason Wang
2014-12-01 9:37 ` [Qemu-devel] [for-2.2] " Michael S. Tsirkin
2014-12-01 13:23 ` Peter Maydell
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.