Re: [Qemu-devel] [PATCH] vhost: Fix vhostfd leak in error branch

[Gonglei]

On 2014/12/1 13:03, Jason Wang wrote:

> 
> 
> On Fri, Nov 28, 2014 at 5:26 PM, address@hidden wrote:
>> From: Gonglei <address@hidden>
>>
>> Signed-off-by: Gonglei <address@hidden>
>> ---
>>  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