* [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@redhat.com, mst@redhat.com, qemu-devel@nongnu.org, 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@redhat.com, mst@redhat.com, qemu-devel@nongnu.org, 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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).