2013-09-11 11:20:54

by Wei Yongjun

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

From: Wei Yongjun <[email protected]>

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 <[email protected]>
---
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",


2013-09-11 20:03:12

by KY Srinivasan

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



> -----Original Message-----
> From: Wei Yongjun [mailto:[email protected]]
> Sent: Wednesday, September 11, 2013 4:20 AM
> To: KY Srinivasan; Haiyang Zhang
> Cc: [email protected]; [email protected]; linux-
> [email protected]
> Subject: [PATCH] Drivers: hv: vmbus: fix error return code in vmbus_connect()
>
> From: Wei Yongjun <[email protected]>
>
> 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

2013-09-12 00:25:49

by Wei Yongjun

[permalink] [raw]
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:[email protected]]
>> Sent: Wednesday, September 11, 2013 4:20 AM
>> To: KY Srinivasan; Haiyang Zhang
>> Cc: [email protected]; [email protected]; linux-
>> [email protected]
>> Subject: [PATCH] Drivers: hv: vmbus: fix error return code in vmbus_connect()
>>
>> From: Wei Yongjun <[email protected]>
>>
>> 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.




2013-09-12 00:36:51

by KY Srinivasan

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



> -----Original Message-----
> From: Wei Yongjun [mailto:[email protected]]
> Sent: Wednesday, September 11, 2013 5:26 PM
> To: KY Srinivasan
> Cc: Haiyang Zhang; [email protected];
> [email protected]; [email protected]
> 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:[email protected]]
> >> Sent: Wednesday, September 11, 2013 4:20 AM
> >> To: KY Srinivasan; Haiyang Zhang
> >> Cc: [email protected]; [email protected]; linux-
> >> [email protected]
> >> Subject: [PATCH] Drivers: hv: vmbus: fix error return code in vmbus_connect()
> >>
> >> From: Wei Yongjun <[email protected]>
> >>
> >> 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
>
>
>
>

2013-09-12 11:11:34

by Dan Carpenter

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

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