Dexuan Cui <de...@microsoft.com> writes: >> -----Original Message----- >> From: Vitaly Kuznetsov [mailto:vkuzn...@redhat.com] >> Sent: Tuesday, April 21, 2015 16:18 >> To: KY Srinivasan >> Cc: Haiyang Zhang; de...@linuxdriverproject.org; linux- >> ker...@vger.kernel.org; Dexuan Cui >> Subject: [PATCH 1/3] Drivers: hv: vmbus: do cleanup on all vmbus_open() >> failure paths >> >> In case there was an error reported in the response to the >> CHANNELMSG_OPENCHANNEL >> call we need to do the cleanup as a vmbus_open() user won't be doing it >> after >> receiving an error. The cleanup should be done on all failure paths. >> >> Signed-off-by: Vitaly Kuznetsov <vkuzn...@redhat.com> >> --- >> drivers/hv/channel.c | 7 ++++--- >> 1 file changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c >> index 54da66d..836386f 100644 >> --- a/drivers/hv/channel.c >> +++ b/drivers/hv/channel.c >> @@ -186,11 +186,12 @@ int vmbus_open(struct vmbus_channel >> *newchannel, u32 send_ringbuffer_size, >> list_del(&open_info->msglistentry); >> spin_unlock_irqrestore(&vmbus_connection.channelmsg_lock, >> flags); >> >> - if (err == 0) >> - newchannel->state = CHANNEL_OPENED_STATE; >> + if (err != 0) >> + goto error_gpadl; >> >> + newchannel->state = CHANNEL_OPENED_STATE; >> kfree(open_info); >> - return err; >> + return 0; >> >> error1: >> spin_lock_irqsave(&vmbus_connection.channelmsg_lock, flags); >> -- > > Thanks for the patch, Vitaly! > > The patch looks good to me except for 1 small issue: > I suppose open_info->response.open_result.status is different from > Linux-style -EXXX error codes. > > Maybe here we can return -EAGAIN if err != 0 ?
I think I inspected all vmbus_open() callers and they all just check ret != 0 but I agree we'd better return some -EXXXX code here. I'm not exactly sure if open_result.status != 0 usually means a permanent or a temporary failure but if it's temporary -EAGAIN here seems reasonable. I'll send v2, thanks for the review! > > -- Dexuan -- Vitaly _______________________________________________ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
