2017-04-03 12:10:32

by Mkrtchyan, Tigran

[permalink] [raw]
Subject: [PATCH] nfs: flexfilelayout: remove v3-only data server limitation

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 <[email protected]>
---
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) {
+ 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",
--
2.9.3



2017-04-03 13:55:01

by Weston Andros Adamson

[permalink] [raw]
Subject: Re: [PATCH] nfs: flexfilelayout: remove v3-only data server limitation

Hey Tigran!

> On Apr 3, 2017, at 8:10 AM, Tigran Mkrtchyan =
<[email protected]> wrote:
>=20
> 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 <[email protected]>
> ---
> fs/nfs/flexfilelayout/flexfilelayoutdev.c | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)
>=20
> 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 =3D NFS_MAX_FILE_IO_SIZE;
>=20
> - if (ds_versions[i].version !=3D 3 || =
ds_versions[i].minor_version !=3D 0) {
> - dprintk("%s: [%d] unsupported ds version =
%d-%d\n", __func__,
> - i, ds_versions[i].version,
> - ds_versions[i].minor_version);
> - ret =3D -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 =3D -EPROTONOSUPPORT;
> + goto out_err_drain_dsaddrs;
> }
>=20
> dprintk("%s: [%d] vers %u minor_ver %u rsize %u wsize %u =
coupled %d\n",
> --=20
> 2.9.3
>=20
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" =
in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html


2017-04-03 14:10:15

by Mkrtchyan, Tigran

[permalink] [raw]
Subject: Re: [PATCH] nfs: flexfilelayout: remove v3-only data server limitation

Hi Dros,

----- Original Message -----
> From: "Weston Andros Adamson" <[email protected]>
> To: "Tigran Mkrtchyan" <[email protected]>
> Cc: "linux-nfs list" <[email protected]>, "Trond Myklebust" <[email protected]>
> 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 <[email protected]> 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 <[email protected]>
>> ---
>> 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 [email protected]
>> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2017-04-04 02:17:16

by Weston Andros Adamson

[permalink] [raw]
Subject: Re: [PATCH] nfs: flexfilelayout: remove v3-only data server limitation


> On Apr 3, 2017, at 10:10 AM, Mkrtchyan, Tigran =
<[email protected]> wrote:
>=20
> Hi Dros,
>=20
> ----- Original Message -----
>> From: "Weston Andros Adamson" <[email protected]>
>> To: "Tigran Mkrtchyan" <[email protected]>
>> Cc: "linux-nfs list" <[email protected]>, "Trond Myklebust" =
<[email protected]>
>> Sent: Monday, April 3, 2017 3:54:59 PM
>> Subject: Re: [PATCH] nfs: flexfilelayout: remove v3-only data server =
limitation
>=20
>> Hey Tigran!
>>=20
>>> On Apr 3, 2017, at 8:10 AM, Tigran Mkrtchyan =
<[email protected]> wrote:
>>>=20
>>> 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 <[email protected]>
>>> ---
>>> fs/nfs/flexfilelayout/flexfilelayoutdev.c | 18 ++++++++++++------
>>> 1 file changed, 12 insertions(+), 6 deletions(-)
>>>=20
>>> 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 =3D NFS_MAX_FILE_IO_SIZE;
>>>=20
>>> - if (ds_versions[i].version !=3D 3 || =
ds_versions[i].minor_version !=3D 0) {
>>> - dprintk("%s: [%d] unsupported ds version =
%d-%d\n", __func__,
>>> - i, ds_versions[i].version,
>>> - ds_versions[i].minor_version);
>>> - ret =3D -EPROTONOSUPPORT;
>>> - goto out_err_drain_dsaddrs;
>>> + /* check for valid major minor combination */
>>> + switch (ds_versions[i].version * 100 + =
ds_versions[i].minor_version) {
>>=20
>> I'm not sure I love this method of matching
>=20
> 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

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