diff mbox

nfs: flexfilelayout: remove v3-only data server limitation

Message ID 20170403121026.14142-1-tigran.mkrtchyan@desy.de (mailing list archive)
State New, archived
Headers show

Commit Message

Mkrtchyan, Tigran April 3, 2017, 12:10 p.m. UTC 
Flexfilelayout supports data servers which talk NFS v3 and v4.{0,1}.
However, this code path is disabled and v3 only servers are accepted.
This change removes this limitation.
Signed-off-by: Tigran Mkrtchyan <tigran.mkrtchyan@desy.de>
---
 fs/nfs/flexfilelayout/flexfilelayoutdev.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

Comments

Weston Andros Adamson April 3, 2017, 1:54 p.m. UTC | #1 
Hey Tigran!

> On Apr 3, 2017, at 8:10 AM, Tigran Mkrtchyan <tigran.mkrtchyan@desy.de> wrote:
> 
> Flexfilelayout supports data servers which talk NFS v3 and v4.{0,1}.
> However, this code path is disabled and v3 only servers are accepted.
> This change removes this limitation.
> Signed-off-by: Tigran Mkrtchyan <tigran.mkrtchyan@desy.de>
> ---
> fs/nfs/flexfilelayout/flexfilelayoutdev.c | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/nfs/flexfilelayout/flexfilelayoutdev.c b/fs/nfs/flexfilelayout/flexfilelayoutdev.c
> index 457cfeb..fac0ef2 100644
> --- a/fs/nfs/flexfilelayout/flexfilelayoutdev.c
> +++ b/fs/nfs/flexfilelayout/flexfilelayoutdev.c
> @@ -119,12 +119,18 @@ nfs4_ff_alloc_deviceid_node(struct nfs_server *server, struct pnfs_device *pdev,
> 		if (ds_versions[i].wsize > NFS_MAX_FILE_IO_SIZE)
> 			ds_versions[i].wsize = NFS_MAX_FILE_IO_SIZE;
> 
> -		if (ds_versions[i].version != 3 || ds_versions[i].minor_version != 0) {
> -			dprintk("%s: [%d] unsupported ds version %d-%d\n", __func__,
> -				i, ds_versions[i].version,
> -				ds_versions[i].minor_version);
> -			ret = -EPROTONOSUPPORT;
> -			goto out_err_drain_dsaddrs;
> +		/* check for valid major minor combination */
> +		switch (ds_versions[i].version * 100 + ds_versions[i].minor_version) {

I'm not sure I love this method of matching

> +			case 300: /* v3   */
> +			case 400: /* v4.0 */
> +			case 401: /* v4.1 */
> +				break;

v4.2?

-dros

> +			default:
> +				dprintk("%s: [%d] unsupported ds version %d-%d\n", __func__,
> +					i, ds_versions[i].version,
> +					ds_versions[i].minor_version);
> +				ret = -EPROTONOSUPPORT;
> +				goto out_err_drain_dsaddrs;
> 		}
> 
> 		dprintk("%s: [%d] vers %u minor_ver %u rsize %u wsize %u coupled %d\n",
> -- 
> 2.9.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mkrtchyan, Tigran April 3, 2017, 2:10 p.m. UTC | #2 
Hi Dros,

----- Original Message -----
> From: "Weston Andros Adamson" <dros@monkey.org>
> To: "Tigran Mkrtchyan" <tigran.mkrtchyan@desy.de>
> Cc: "linux-nfs list" <linux-nfs@vger.kernel.org>, "Trond Myklebust" <trond.myklebust@primarydata.com>
> Sent: Monday, April 3, 2017 3:54:59 PM
> Subject: Re: [PATCH] nfs: flexfilelayout: remove v3-only data server limitation

> Hey Tigran!
> 
>> On Apr 3, 2017, at 8:10 AM, Tigran Mkrtchyan <tigran.mkrtchyan@desy.de> wrote:
>> 
>> Flexfilelayout supports data servers which talk NFS v3 and v4.{0,1}.
>> However, this code path is disabled and v3 only servers are accepted.
>> This change removes this limitation.
>> Signed-off-by: Tigran Mkrtchyan <tigran.mkrtchyan@desy.de>
>> ---
>> fs/nfs/flexfilelayout/flexfilelayoutdev.c | 18 ++++++++++++------
>> 1 file changed, 12 insertions(+), 6 deletions(-)
>> 
>> diff --git a/fs/nfs/flexfilelayout/flexfilelayoutdev.c
>> b/fs/nfs/flexfilelayout/flexfilelayoutdev.c
>> index 457cfeb..fac0ef2 100644
>> --- a/fs/nfs/flexfilelayout/flexfilelayoutdev.c
>> +++ b/fs/nfs/flexfilelayout/flexfilelayoutdev.c
>> @@ -119,12 +119,18 @@ nfs4_ff_alloc_deviceid_node(struct nfs_server *server,
>> struct pnfs_device *pdev,
>> 		if (ds_versions[i].wsize > NFS_MAX_FILE_IO_SIZE)
>> 			ds_versions[i].wsize = NFS_MAX_FILE_IO_SIZE;
>> 
>> -		if (ds_versions[i].version != 3 || ds_versions[i].minor_version != 0) {
>> -			dprintk("%s: [%d] unsupported ds version %d-%d\n", __func__,
>> -				i, ds_versions[i].version,
>> -				ds_versions[i].minor_version);
>> -			ret = -EPROTONOSUPPORT;
>> -			goto out_err_drain_dsaddrs;
>> +		/* check for valid major minor combination */
>> +		switch (ds_versions[i].version * 100 + ds_versions[i].minor_version) {
> 
> I'm not sure I love this method of matching

I didn't find a better way without flood of if-statements. Any suggestions?

> 
>> +			case 300: /* v3   */
>> +			case 400: /* v4.0 */
>> +			case 401: /* v4.1 */
>> +				break;
> 
> v4.2?

Right!

Tigran.
> 
> -dros
> 
>> +			default:
>> +				dprintk("%s: [%d] unsupported ds version %d-%d\n", __func__,
>> +					i, ds_versions[i].version,
>> +					ds_versions[i].minor_version);
>> +				ret = -EPROTONOSUPPORT;
>> +				goto out_err_drain_dsaddrs;
>> 		}
>> 
>> 		dprintk("%s: [%d] vers %u minor_ver %u rsize %u wsize %u coupled %d\n",
>> --
>> 2.9.3
>> 
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Weston Andros Adamson April 4, 2017, 2:17 a.m. UTC | #3 
> On Apr 3, 2017, at 10:10 AM, Mkrtchyan, Tigran <tigran.mkrtchyan@desy.de> wrote:
> 
> Hi Dros,
> 
> ----- Original Message -----
>> From: "Weston Andros Adamson" <dros@monkey.org>
>> To: "Tigran Mkrtchyan" <tigran.mkrtchyan@desy.de>
>> Cc: "linux-nfs list" <linux-nfs@vger.kernel.org>, "Trond Myklebust" <trond.myklebust@primarydata.com>
>> Sent: Monday, April 3, 2017 3:54:59 PM
>> Subject: Re: [PATCH] nfs: flexfilelayout: remove v3-only data server limitation
> 
>> Hey Tigran!
>> 
>>> On Apr 3, 2017, at 8:10 AM, Tigran Mkrtchyan <tigran.mkrtchyan@desy.de> wrote:
>>> 
>>> Flexfilelayout supports data servers which talk NFS v3 and v4.{0,1}.
>>> However, this code path is disabled and v3 only servers are accepted.
>>> This change removes this limitation.
>>> Signed-off-by: Tigran Mkrtchyan <tigran.mkrtchyan@desy.de>
>>> ---
>>> fs/nfs/flexfilelayout/flexfilelayoutdev.c | 18 ++++++++++++------
>>> 1 file changed, 12 insertions(+), 6 deletions(-)
>>> 
>>> diff --git a/fs/nfs/flexfilelayout/flexfilelayoutdev.c
>>> b/fs/nfs/flexfilelayout/flexfilelayoutdev.c
>>> index 457cfeb..fac0ef2 100644
>>> --- a/fs/nfs/flexfilelayout/flexfilelayoutdev.c
>>> +++ b/fs/nfs/flexfilelayout/flexfilelayoutdev.c
>>> @@ -119,12 +119,18 @@ nfs4_ff_alloc_deviceid_node(struct nfs_server *server,
>>> struct pnfs_device *pdev,
>>> 		if (ds_versions[i].wsize > NFS_MAX_FILE_IO_SIZE)
>>> 			ds_versions[i].wsize = NFS_MAX_FILE_IO_SIZE;
>>> 
>>> -		if (ds_versions[i].version != 3 || ds_versions[i].minor_version != 0) {
>>> -			dprintk("%s: [%d] unsupported ds version %d-%d\n", __func__,
>>> -				i, ds_versions[i].version,
>>> -				ds_versions[i].minor_version);
>>> -			ret = -EPROTONOSUPPORT;
>>> -			goto out_err_drain_dsaddrs;
>>> +		/* check for valid major minor combination */
>>> +		switch (ds_versions[i].version * 100 + ds_versions[i].minor_version) {
>> 
>> I'm not sure I love this method of matching
> 
> I didn't find a better way without flood of if-statements. Any suggestions?

Not really. That is clever, just not immediately obvious (to me). Plus what happens when we have NFSv4.100? ;)

I was thinking you could simply check for > nfsv3, but that's not very specific.

Lets see what Trond and Anna say.

-dros

> 
>> 
>>> +			case 300: /* v3   */
>>> +			case 400: /* v4.0 */
>>> +			case 401: /* v4.1 */
>>> +				break;
>> 
>> v4.2?
> 
> Right!
> 
> Tigran.
>> 
>> -dros
>> 
>>> +			default:
>>> +				dprintk("%s: [%d] unsupported ds version %d-%d\n", __func__,
>>> +					i, ds_versions[i].version,
>>> +					ds_versions[i].minor_version);
>>> +				ret = -EPROTONOSUPPORT;
>>> +				goto out_err_drain_dsaddrs;
>>> 		}
>>> 
>>> 		dprintk("%s: [%d] vers %u minor_ver %u rsize %u wsize %u coupled %d\n",
>>> --
>>> 2.9.3
>>> 
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> 
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/nfs/flexfilelayout/flexfilelayoutdev.c b/fs/nfs/flexfilelayout/flexfilelayoutdev.c
index 457cfeb..fac0ef2 100644
--- a/fs/nfs/flexfilelayout/flexfilelayoutdev.c
+++ b/fs/nfs/flexfilelayout/flexfilelayoutdev.c
@@ -119,12 +119,18 @@  nfs4_ff_alloc_deviceid_node(struct nfs_server *server, struct pnfs_device *pdev,
 		if (ds_versions[i].wsize > NFS_MAX_FILE_IO_SIZE)
 			ds_versions[i].wsize = NFS_MAX_FILE_IO_SIZE;
 
-		if (ds_versions[i].version != 3 || ds_versions[i].minor_version != 0) {
-			dprintk("%s: [%d] unsupported ds version %d-%d\n", __func__,
-				i, ds_versions[i].version,
-				ds_versions[i].minor_version);
-			ret = -EPROTONOSUPPORT;
-			goto out_err_drain_dsaddrs;
+		/* check for valid major minor combination */
+		switch (ds_versions[i].version * 100 + ds_versions[i].minor_version) {
+			case 300: /* v3   */
+			case 400: /* v4.0 */
+			case 401: /* v4.1 */
+				break;
+			default:
+				dprintk("%s: [%d] unsupported ds version %d-%d\n", __func__,
+					i, ds_versions[i].version,
+					ds_versions[i].minor_version);
+				ret = -EPROTONOSUPPORT;
+				goto out_err_drain_dsaddrs;
 		}
 
 		dprintk("%s: [%d] vers %u minor_ver %u rsize %u wsize %u coupled %d\n",