[PATCH] Drivers: hv: vmbus: fix error return code in vmbus_connect()

[PATCH] Drivers: hv: vmbus: fix error return code in vmbus_connect()

[Wei Yongjun]

From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>

Fix to return -EINVAL in the version check error handling
case instead of 0, as done elsewhere in this function.

Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
 drivers/hv/connection.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index 8f4743a..b3eb50f 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -204,8 +204,10 @@ int vmbus_connect(void)
 		version = vmbus_get_next_version(version);
 	} while (version != VERSION_INVAL);
 
-	if (version == VERSION_INVAL)
+	if (version == VERSION_INVAL) {
+		ret = -EINVAL;
 		goto cleanup;
+	}
 
 	vmbus_proto_version = version;
 	pr_info("Hyper-V Host Build:%d-%d.%d-%d-%d.%d; Vmbus version:%d.%d\n",

RE: [PATCH] Drivers: hv: vmbus: fix error return code in vmbus_connect()

[KY Srinivasan]

> -----Original Message-----
> From: Wei Yongjun [mailto:weiyj.lk@gmail.com]
> Sent: Wednesday, September 11, 2013 4:20 AM
> To: KY Srinivasan; Haiyang Zhang
> Cc: yongjun_wei@trendmicro.com.cn; devel@linuxdriverproject.org; linux-
> kernel@vger.kernel.org
> Subject: [PATCH] Drivers: hv: vmbus: fix error return code in vmbus_connect()
> 
> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> 
> Fix to return -EINVAL in the version check error handling
> case instead of 0, as done elsewhere in this function.

The return will not be zero in this case. If you look at the function 
vmbus_negotiate_version(), in case the host refuses the version, the
return value will be set to -ECONNREFUSED

Regards,

K. Y

Re: [PATCH] Drivers: hv: vmbus: fix error return code in vmbus_connect()

[Wei Yongjun]

On 09/12/2013 04:03 AM, KY Srinivasan wrote:
>
>> -----Original Message-----
>> From: Wei Yongjun [mailto:weiyj.lk@gmail.com]
>> Sent: Wednesday, September 11, 2013 4:20 AM
>> To: KY Srinivasan; Haiyang Zhang
>> Cc: yongjun_wei@trendmicro.com.cn; devel@linuxdriverproject.org; linux-
>> kernel@vger.kernel.org
>> Subject: [PATCH] Drivers: hv: vmbus: fix error return code in vmbus_connect()
>>
>> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
>>
>> Fix to return -EINVAL in the version check error handling
>> case instead of 0, as done elsewhere in this function.
> The return will not be zero in this case. If you look at the function 
> vmbus_negotiate_version(), in case the host refuses the version, the
> return value will be set to -ECONNREFUSED

look at the code:

 196         do {
 197                 ret = vmbus_negotiate_version(msginfo, version);
 198                 if (ret)
 199                         goto cleanup;
 200 
 201                 if (vmbus_connection.conn_state == CONNECTED)
 202                         break;
 203 
 204                 version = vmbus_get_next_version(version);
 205         } while (version != VERSION_INVAL);
 206 
 207         if (version == VERSION_INVAL)
 208                 goto cleanup;

if function vmbus_negotiate_version() return error, the code
will goto cleanup. 

If 'version == VERSION_INVAL' is true, I think we tried all
of the VERSION_WS2008/VERSION_WIN7/VERSION_WIN8, but still
can not get the connection.

 

RE: [PATCH] Drivers: hv: vmbus: fix error return code in vmbus_connect()

[KY Srinivasan]

> -----Original Message-----
> From: Wei Yongjun [mailto:weiyj.lk@gmail.com]
> Sent: Wednesday, September 11, 2013 5:26 PM
> To: KY Srinivasan
> Cc: Haiyang Zhang; yongjun_wei@trendmicro.com.cn;
> devel@linuxdriverproject.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] Drivers: hv: vmbus: fix error return code in
> vmbus_connect()
> 
> On 09/12/2013 04:03 AM, KY Srinivasan wrote:
> >
> >> -----Original Message-----
> >> From: Wei Yongjun [mailto:weiyj.lk@gmail.com]
> >> Sent: Wednesday, September 11, 2013 4:20 AM
> >> To: KY Srinivasan; Haiyang Zhang
> >> Cc: yongjun_wei@trendmicro.com.cn; devel@linuxdriverproject.org; linux-
> >> kernel@vger.kernel.org
> >> Subject: [PATCH] Drivers: hv: vmbus: fix error return code in vmbus_connect()
> >>
> >> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> >>
> >> Fix to return -EINVAL in the version check error handling
> >> case instead of 0, as done elsewhere in this function.
> > The return will not be zero in this case. If you look at the function
> > vmbus_negotiate_version(), in case the host refuses the version, the
> > return value will be set to -ECONNREFUSED
> 
> look at the code:
> 
>  196         do {
>  197                 ret = vmbus_negotiate_version(msginfo, version);
>  198                 if (ret)
>  199                         goto cleanup;
>  200
>  201                 if (vmbus_connection.conn_state == CONNECTED)
>  202                         break;
>  203
>  204                 version = vmbus_get_next_version(version);
>  205         } while (version != VERSION_INVAL);
>  206
>  207         if (version == VERSION_INVAL)
>  208                 goto cleanup;
> 
> if function vmbus_negotiate_version() return error, the code
> will goto cleanup.
> 
> If 'version == VERSION_INVAL' is true, I think we tried all
> of the VERSION_WS2008/VERSION_WIN7/VERSION_WIN8, but still
> can not get the connection.

When you try a given version, vmbus_negotiate_version() returns an error code
if the host does not support that version. Hence, in this code you will not be able to
run on anything prior to ws2012. I have already sent in a patch to fix that - if we timeout
only then we jump to cleanup. In any event, if we tried all versions and none was acceptable
to the host, we would return the value from vmbus_negotiate_version().

K. Y
> 
> 
> 
> 

Re: [PATCH] Drivers: hv: vmbus: fix error return code in vmbus_connect()

[Dan Carpenter]

On Thu, Sep 12, 2013 at 12:36:46AM +0000, KY Srinivasan wrote:
> When you try a given version, vmbus_negotiate_version() returns an error code
> if the host does not support that version. Hence, in this code you will not be able to
> run on anything prior to ws2012. I have already sent in a patch to fix that - if we timeout
> only then we jump to cleanup. In any event, if we tried all versions and none was acceptable
> to the host, we would return the value from vmbus_negotiate_version().
> 

First of all Wei's patch fixes an obvious bug.  Read the code.

If you already fixed it then what's the URL or the subject of your
email so we can look for it in our inbox.  Give us a clue here.

regards,
dan carpenter