From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55955) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XvLmh-0006oV-U1 for qemu-devel@nongnu.org; Mon, 01 Dec 2014 02:52:44 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XvLmd-0002GP-3r for qemu-devel@nongnu.org; Mon, 01 Dec 2014 02:52:39 -0500 Received: from mx1.redhat.com ([209.132.183.28]:41221) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XvLmc-0002GG-QT for qemu-devel@nongnu.org; Mon, 01 Dec 2014 02:52:35 -0500 Date: Mon, 01 Dec 2014 08:00:23 +0008 From: Jason Wang Message-Id: <1417420343.877.0@smtp.corp.redhat.com> In-Reply-To: <547C0A3E.9070106@huawei.com> References: <1417166789-1960-1-git-send-email-arei.gonglei@huawei.com> <1417410209.4969.0@smtp.corp.redhat.com> <547C0A3E.9070106@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Subject: Re: [Qemu-devel] [PATCH] vhost: Fix vhostfd leak in error branch List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , 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 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 >>> >>> Signed-off-by: Gonglei >>> --- >>> 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