2012-02-16 21:11:32

by Andy Adamson

[permalink] [raw]
Subject: [PATCH Version 2 1/1] NFSv4.1 set the cl_hostname for data servers

From: Andy Adamson <[email protected]>

Used by /proc/fs/nfsfs/servers

Signed-off-by: Andy Adamson <[email protected]>
---
fs/nfs/client.c | 3 ++-
fs/nfs/internal.h | 3 ++-
fs/nfs/nfs4filelayoutdev.c | 11 ++++++++++-
3 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index d0f850f..c625284 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -1443,7 +1443,7 @@ error:
*/
struct nfs_client *nfs4_set_ds_client(struct nfs_client* mds_clp,
const struct sockaddr *ds_addr,
- int ds_addrlen, int ds_proto)
+ int ds_addrlen, int ds_proto, char *ds_hostname)
{
struct nfs_client_initdata cl_init = {
.addr = ds_addr,
@@ -1452,6 +1452,7 @@ struct nfs_client *nfs4_set_ds_client(struct nfs_client* mds_clp,
.proto = ds_proto,
.minorversion = mds_clp->cl_minorversion,
.net = mds_clp->net,
+ .hostname = ds_hostname,
};
struct rpc_timeout ds_timeout = {
.to_initval = 15 * HZ,
diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index 0c3648a..b870695 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -170,7 +170,8 @@ extern void nfs_mark_client_ready(struct nfs_client *clp, int state);
extern int nfs4_check_client_ready(struct nfs_client *clp);
extern struct nfs_client *nfs4_set_ds_client(struct nfs_client* mds_clp,
const struct sockaddr *ds_addr,
- int ds_addrlen, int ds_proto);
+ int ds_addrlen, int ds_proto,
+ char *ds_hostname);
#ifdef CONFIG_PROC_FS
extern int __init nfs_fs_proc_init(void);
extern void nfs_fs_proc_exit(void);
diff --git a/fs/nfs/nfs4filelayoutdev.c b/fs/nfs/nfs4filelayoutdev.c
index 41677f0..a0b1fc5 100644
--- a/fs/nfs/nfs4filelayoutdev.c
+++ b/fs/nfs/nfs4filelayoutdev.c
@@ -153,6 +153,7 @@ nfs4_ds_connect(struct nfs_server *mds_srv, struct nfs4_pnfs_ds *ds)
{
struct nfs_client *clp = ERR_PTR(-EIO);
struct nfs4_pnfs_ds_addr *da;
+ char *ds_hostname = NULL;
int status = 0;

dprintk("--> %s DS %s au_flavor %d\n", __func__, ds->ds_remotestr,
@@ -164,9 +165,17 @@ nfs4_ds_connect(struct nfs_server *mds_srv, struct nfs4_pnfs_ds *ds)
dprintk("%s: DS %s: trying address %s\n",
__func__, ds->ds_remotestr, da->da_remotestr);

+ /* For debug: OK if ds_remotestr or ds_hostname is NULL */
+ if (da->da_remotestr) {
+ char *end = strchr(da->da_remotestr, ':');
+ ds_hostname = kstrndup(da->da_remotestr,
+ end - da->da_remotestr,
+ GFP_KERNEL);
+ }
clp = nfs4_set_ds_client(mds_srv->nfs_client,
(struct sockaddr *)&da->da_addr,
- da->da_addrlen, IPPROTO_TCP);
+ da->da_addrlen, IPPROTO_TCP, ds_hostname);
+ kfree(ds_hostname);
if (!IS_ERR(clp))
break;
}
--
1.7.6.4



2012-02-17 18:53:37

by Andy Adamson

[permalink] [raw]
Subject: Re: [PATCH Version 2 1/1] NFSv4.1 set the cl_hostname for data servers

On Fri, Feb 17, 2012 at 1:46 PM, Chuck Lever <[email protected]> wrote:
> I've got a patch that already does this, which Trond wrote a year ago. ?I can submit this for 3.4.

OK. That solves the /proc/fs/nfsfs/servers HOSTNAME is null issue for
filelayout data servers.

-->Andy
>
> On Feb 17, 2012, at 1:40 PM, Andy Adamson wrote:
>
>> Isn't the clp->cl_rpcclient->cl_server always the same as
>> clp->cl_hostname? Can we just get rid of the cl_hostname and use the
>> cl_server?
>>
>> -->Andy
>>
>> On Thu, Feb 16, 2012 at 5:08 PM, Adamson, Dros
>> <[email protected]> wrote:
>>>
>>> On Feb 16, 2012, at 5:00 PM, Adamson, Dros wrote:
>>>
>>>>
>>>> On Feb 16, 2012, at 4:29 PM, Myklebust, Trond wrote:
>>>>
>>>>> On Thu, 2012-02-16 at 16:10 -0500, [email protected] wrote:
>>>>>> From: Andy Adamson <[email protected]>
>>>>>>
>>>>>> Used by /proc/fs/nfsfs/servers
>>>>>>
>>>>>> Signed-off-by: Andy Adamson <[email protected]>
>>>>>> ---
>>>>>> fs/nfs/client.c ? ? ? ? ? ?| ? ?3 ++-
>>>>>> fs/nfs/internal.h ? ? ? ? ?| ? ?3 ++-
>>>>>> fs/nfs/nfs4filelayoutdev.c | ? 11 ++++++++++-
>>>>>> 3 files changed, 14 insertions(+), 3 deletions(-)
>>>>>>
>>>>>> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
>>>>>> index d0f850f..c625284 100644
>>>>>> --- a/fs/nfs/client.c
>>>>>> +++ b/fs/nfs/client.c
>>>>>> @@ -1443,7 +1443,7 @@ error:
>>>>>> */
>>>>>> struct nfs_client *nfs4_set_ds_client(struct nfs_client* mds_clp,
>>>>>> ? ? ? ? ? ? const struct sockaddr *ds_addr,
>>>>>> - ? ? ? ? ? int ds_addrlen, int ds_proto)
>>>>>> + ? ? ? ? ? int ds_addrlen, int ds_proto, char *ds_hostname)
>>>>>
>>>>> Can you make this (and the other instances below) 'const char *', so
>>>>> that we avoid any confusion? Otherwise it looks more or less OK...
>>>>>
>>>>> Note that we should consider splitting da->da_remotestr into a
>>>>> hostname/port part at some point so that we avoid the copy below.
>>>>
>>>> da_remotestr (and ds_remotestr) were meant to be for display only (and iirc they can actually be NULL).
>>>
>>> ^^ dprintk() display, that is.
>>>
>>>>
>>>> The right way to do this is from the actual sin_addr (da_addr).
>>>>
>>>> -dros
>>>
>> --
>> 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
>
> --
> Chuck Lever
> chuck[dot]lever[at]oracle[dot]com
>
>
>
>

2012-02-17 18:46:57

by Chuck Lever III

[permalink] [raw]
Subject: Re: [PATCH Version 2 1/1] NFSv4.1 set the cl_hostname for data servers

I've got a patch that already does this, which Trond wrote a year ago. I can submit this for 3.4.

On Feb 17, 2012, at 1:40 PM, Andy Adamson wrote:

> Isn't the clp->cl_rpcclient->cl_server always the same as
> clp->cl_hostname? Can we just get rid of the cl_hostname and use the
> cl_server?
>
> -->Andy
>
> On Thu, Feb 16, 2012 at 5:08 PM, Adamson, Dros
> <[email protected]> wrote:
>>
>> On Feb 16, 2012, at 5:00 PM, Adamson, Dros wrote:
>>
>>>
>>> On Feb 16, 2012, at 4:29 PM, Myklebust, Trond wrote:
>>>
>>>> On Thu, 2012-02-16 at 16:10 -0500, [email protected] wrote:
>>>>> From: Andy Adamson <[email protected]>
>>>>>
>>>>> Used by /proc/fs/nfsfs/servers
>>>>>
>>>>> Signed-off-by: Andy Adamson <[email protected]>
>>>>> ---
>>>>> fs/nfs/client.c | 3 ++-
>>>>> fs/nfs/internal.h | 3 ++-
>>>>> fs/nfs/nfs4filelayoutdev.c | 11 ++++++++++-
>>>>> 3 files changed, 14 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
>>>>> index d0f850f..c625284 100644
>>>>> --- a/fs/nfs/client.c
>>>>> +++ b/fs/nfs/client.c
>>>>> @@ -1443,7 +1443,7 @@ error:
>>>>> */
>>>>> struct nfs_client *nfs4_set_ds_client(struct nfs_client* mds_clp,
>>>>> const struct sockaddr *ds_addr,
>>>>> - int ds_addrlen, int ds_proto)
>>>>> + int ds_addrlen, int ds_proto, char *ds_hostname)
>>>>
>>>> Can you make this (and the other instances below) 'const char *', so
>>>> that we avoid any confusion? Otherwise it looks more or less OK...
>>>>
>>>> Note that we should consider splitting da->da_remotestr into a
>>>> hostname/port part at some point so that we avoid the copy below.
>>>
>>> da_remotestr (and ds_remotestr) were meant to be for display only (and iirc they can actually be NULL).
>>
>> ^^ dprintk() display, that is.
>>
>>>
>>> The right way to do this is from the actual sin_addr (da_addr).
>>>
>>> -dros
>>
> --
> 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

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com





2012-02-16 22:08:44

by Adamson, Dros

[permalink] [raw]
Subject: Re: [PATCH Version 2 1/1] NFSv4.1 set the cl_hostname for data servers


On Feb 16, 2012, at 5:00 PM, Adamson, Dros wrote:

>
> On Feb 16, 2012, at 4:29 PM, Myklebust, Trond wrote:
>
>> On Thu, 2012-02-16 at 16:10 -0500, [email protected] wrote:
>>> From: Andy Adamson <[email protected]>
>>>
>>> Used by /proc/fs/nfsfs/servers
>>>
>>> Signed-off-by: Andy Adamson <[email protected]>
>>> ---
>>> fs/nfs/client.c | 3 ++-
>>> fs/nfs/internal.h | 3 ++-
>>> fs/nfs/nfs4filelayoutdev.c | 11 ++++++++++-
>>> 3 files changed, 14 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
>>> index d0f850f..c625284 100644
>>> --- a/fs/nfs/client.c
>>> +++ b/fs/nfs/client.c
>>> @@ -1443,7 +1443,7 @@ error:
>>> */
>>> struct nfs_client *nfs4_set_ds_client(struct nfs_client* mds_clp,
>>> const struct sockaddr *ds_addr,
>>> - int ds_addrlen, int ds_proto)
>>> + int ds_addrlen, int ds_proto, char *ds_hostname)
>>
>> Can you make this (and the other instances below) 'const char *', so
>> that we avoid any confusion? Otherwise it looks more or less OK...
>>
>> Note that we should consider splitting da->da_remotestr into a
>> hostname/port part at some point so that we avoid the copy below.
>
> da_remotestr (and ds_remotestr) were meant to be for display only (and iirc they can actually be NULL).

^^ dprintk() display, that is.

>
> The right way to do this is from the actual sin_addr (da_addr).
>
> -dros


Attachments:
smime.p7s (1.34 kB)

2012-02-16 22:01:07

by Adamson, Dros

[permalink] [raw]
Subject: Re: [PATCH Version 2 1/1] NFSv4.1 set the cl_hostname for data servers


On Feb 16, 2012, at 4:29 PM, Myklebust, Trond wrote:

> On Thu, 2012-02-16 at 16:10 -0500, [email protected] wrote:
>> From: Andy Adamson <[email protected]>
>>
>> Used by /proc/fs/nfsfs/servers
>>
>> Signed-off-by: Andy Adamson <[email protected]>
>> ---
>> fs/nfs/client.c | 3 ++-
>> fs/nfs/internal.h | 3 ++-
>> fs/nfs/nfs4filelayoutdev.c | 11 ++++++++++-
>> 3 files changed, 14 insertions(+), 3 deletions(-)
>>
>> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
>> index d0f850f..c625284 100644
>> --- a/fs/nfs/client.c
>> +++ b/fs/nfs/client.c
>> @@ -1443,7 +1443,7 @@ error:
>> */
>> struct nfs_client *nfs4_set_ds_client(struct nfs_client* mds_clp,
>> const struct sockaddr *ds_addr,
>> - int ds_addrlen, int ds_proto)
>> + int ds_addrlen, int ds_proto, char *ds_hostname)
>
> Can you make this (and the other instances below) 'const char *', so
> that we avoid any confusion? Otherwise it looks more or less OK...
>
> Note that we should consider splitting da->da_remotestr into a
> hostname/port part at some point so that we avoid the copy below.

da_remotestr (and ds_remotestr) were meant to be for display only (and iirc they can actually be NULL).

The right way to do this is from the actual sin_addr (da_addr).

-dros


Attachments:
smime.p7s (1.34 kB)

2012-02-16 21:29:24

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH Version 2 1/1] NFSv4.1 set the cl_hostname for data servers

T24gVGh1LCAyMDEyLTAyLTE2IGF0IDE2OjEwIC0wNTAwLCBhbmRyb3NAbmV0YXBwLmNvbSB3cm90
ZToNCj4gRnJvbTogQW5keSBBZGFtc29uIDxhbmRyb3NAbmV0YXBwLmNvbT4NCj4gDQo+IFVzZWQg
YnkgL3Byb2MvZnMvbmZzZnMvc2VydmVycw0KPiANCj4gU2lnbmVkLW9mZi1ieTogQW5keSBBZGFt
c29uIDxhbmRyb3NAbmV0YXBwLmNvbT4NCj4gLS0tDQo+ICBmcy9uZnMvY2xpZW50LmMgICAgICAg
ICAgICB8ICAgIDMgKystDQo+ICBmcy9uZnMvaW50ZXJuYWwuaCAgICAgICAgICB8ICAgIDMgKyst
DQo+ICBmcy9uZnMvbmZzNGZpbGVsYXlvdXRkZXYuYyB8ICAgMTEgKysrKysrKysrKy0NCj4gIDMg
ZmlsZXMgY2hhbmdlZCwgMTQgaW5zZXJ0aW9ucygrKSwgMyBkZWxldGlvbnMoLSkNCj4gDQo+IGRp
ZmYgLS1naXQgYS9mcy9uZnMvY2xpZW50LmMgYi9mcy9uZnMvY2xpZW50LmMNCj4gaW5kZXggZDBm
ODUwZi4uYzYyNTI4NCAxMDA2NDQNCj4gLS0tIGEvZnMvbmZzL2NsaWVudC5jDQo+ICsrKyBiL2Zz
L25mcy9jbGllbnQuYw0KPiBAQCAtMTQ0Myw3ICsxNDQzLDcgQEAgZXJyb3I6DQo+ICAgKi8NCj4g
IHN0cnVjdCBuZnNfY2xpZW50ICpuZnM0X3NldF9kc19jbGllbnQoc3RydWN0IG5mc19jbGllbnQq
IG1kc19jbHAsDQo+ICAJCWNvbnN0IHN0cnVjdCBzb2NrYWRkciAqZHNfYWRkciwNCj4gLQkJaW50
IGRzX2FkZHJsZW4sIGludCBkc19wcm90bykNCj4gKwkJaW50IGRzX2FkZHJsZW4sIGludCBkc19w
cm90bywgY2hhciAqZHNfaG9zdG5hbWUpDQoNCkNhbiB5b3UgbWFrZSB0aGlzIChhbmQgdGhlIG90
aGVyIGluc3RhbmNlcyBiZWxvdykgJ2NvbnN0IGNoYXIgKicsIHNvDQp0aGF0IHdlIGF2b2lkIGFu
eSBjb25mdXNpb24/IE90aGVyd2lzZSBpdCBsb29rcyBtb3JlIG9yIGxlc3MgT0suLi4NCg0KTm90
ZSB0aGF0IHdlIHNob3VsZCBjb25zaWRlciBzcGxpdHRpbmcgZGEtPmRhX3JlbW90ZXN0ciBpbnRv
IGENCmhvc3RuYW1lL3BvcnQgcGFydCBhdCBzb21lIHBvaW50IHNvIHRoYXQgd2UgYXZvaWQgdGhl
IGNvcHkgYmVsb3cuDQoNCj4gIHsNCj4gIAlzdHJ1Y3QgbmZzX2NsaWVudF9pbml0ZGF0YSBjbF9p
bml0ID0gew0KPiAgCQkuYWRkciA9IGRzX2FkZHIsDQo+IEBAIC0xNDUyLDYgKzE0NTIsNyBAQCBz
dHJ1Y3QgbmZzX2NsaWVudCAqbmZzNF9zZXRfZHNfY2xpZW50KHN0cnVjdCBuZnNfY2xpZW50KiBt
ZHNfY2xwLA0KPiAgCQkucHJvdG8gPSBkc19wcm90bywNCj4gIAkJLm1pbm9ydmVyc2lvbiA9IG1k
c19jbHAtPmNsX21pbm9ydmVyc2lvbiwNCj4gIAkJLm5ldCA9IG1kc19jbHAtPm5ldCwNCj4gKwkJ
Lmhvc3RuYW1lID0gZHNfaG9zdG5hbWUsDQo+ICAJfTsNCj4gIAlzdHJ1Y3QgcnBjX3RpbWVvdXQg
ZHNfdGltZW91dCA9IHsNCj4gIAkJLnRvX2luaXR2YWwgPSAxNSAqIEhaLA0KPiBkaWZmIC0tZ2l0
IGEvZnMvbmZzL2ludGVybmFsLmggYi9mcy9uZnMvaW50ZXJuYWwuaA0KPiBpbmRleCAwYzM2NDhh
Li5iODcwNjk1IDEwMDY0NA0KPiAtLS0gYS9mcy9uZnMvaW50ZXJuYWwuaA0KPiArKysgYi9mcy9u
ZnMvaW50ZXJuYWwuaA0KPiBAQCAtMTcwLDcgKzE3MCw4IEBAIGV4dGVybiB2b2lkIG5mc19tYXJr
X2NsaWVudF9yZWFkeShzdHJ1Y3QgbmZzX2NsaWVudCAqY2xwLCBpbnQgc3RhdGUpOw0KPiAgZXh0
ZXJuIGludCBuZnM0X2NoZWNrX2NsaWVudF9yZWFkeShzdHJ1Y3QgbmZzX2NsaWVudCAqY2xwKTsN
Cj4gIGV4dGVybiBzdHJ1Y3QgbmZzX2NsaWVudCAqbmZzNF9zZXRfZHNfY2xpZW50KHN0cnVjdCBu
ZnNfY2xpZW50KiBtZHNfY2xwLA0KPiAgCQkJCQkgICAgIGNvbnN0IHN0cnVjdCBzb2NrYWRkciAq
ZHNfYWRkciwNCj4gLQkJCQkJICAgICBpbnQgZHNfYWRkcmxlbiwgaW50IGRzX3Byb3RvKTsNCj4g
KwkJCQkJICAgICBpbnQgZHNfYWRkcmxlbiwgaW50IGRzX3Byb3RvLA0KPiArCQkJCQkgICAgIGNo
YXIgKmRzX2hvc3RuYW1lKTsNCj4gICNpZmRlZiBDT05GSUdfUFJPQ19GUw0KPiAgZXh0ZXJuIGlu
dCBfX2luaXQgbmZzX2ZzX3Byb2NfaW5pdCh2b2lkKTsNCj4gIGV4dGVybiB2b2lkIG5mc19mc19w
cm9jX2V4aXQodm9pZCk7DQo+IGRpZmYgLS1naXQgYS9mcy9uZnMvbmZzNGZpbGVsYXlvdXRkZXYu
YyBiL2ZzL25mcy9uZnM0ZmlsZWxheW91dGRldi5jDQo+IGluZGV4IDQxNjc3ZjAuLmEwYjFmYzUg
MTAwNjQ0DQo+IC0tLSBhL2ZzL25mcy9uZnM0ZmlsZWxheW91dGRldi5jDQo+ICsrKyBiL2ZzL25m
cy9uZnM0ZmlsZWxheW91dGRldi5jDQo+IEBAIC0xNTMsNiArMTUzLDcgQEAgbmZzNF9kc19jb25u
ZWN0KHN0cnVjdCBuZnNfc2VydmVyICptZHNfc3J2LCBzdHJ1Y3QgbmZzNF9wbmZzX2RzICpkcykN
Cj4gIHsNCj4gIAlzdHJ1Y3QgbmZzX2NsaWVudCAqY2xwID0gRVJSX1BUUigtRUlPKTsNCj4gIAlz
dHJ1Y3QgbmZzNF9wbmZzX2RzX2FkZHIgKmRhOw0KPiArCWNoYXIgKmRzX2hvc3RuYW1lID0gTlVM
TDsNCj4gIAlpbnQgc3RhdHVzID0gMDsNCj4gIA0KPiAgCWRwcmludGsoIi0tPiAlcyBEUyAlcyBh
dV9mbGF2b3IgJWRcbiIsIF9fZnVuY19fLCBkcy0+ZHNfcmVtb3Rlc3RyLA0KPiBAQCAtMTY0LDkg
KzE2NSwxNyBAQCBuZnM0X2RzX2Nvbm5lY3Qoc3RydWN0IG5mc19zZXJ2ZXIgKm1kc19zcnYsIHN0
cnVjdCBuZnM0X3BuZnNfZHMgKmRzKQ0KPiAgCQlkcHJpbnRrKCIlczogRFMgJXM6IHRyeWluZyBh
ZGRyZXNzICVzXG4iLA0KPiAgCQkJX19mdW5jX18sIGRzLT5kc19yZW1vdGVzdHIsIGRhLT5kYV9y
ZW1vdGVzdHIpOw0KPiAgDQo+ICsJCS8qIEZvciBkZWJ1ZzogT0sgaWYgZHNfcmVtb3Rlc3RyIG9y
IGRzX2hvc3RuYW1lIGlzIE5VTEwgKi8NCj4gKwkJaWYgKGRhLT5kYV9yZW1vdGVzdHIpIHsNCj4g
KwkJCWNoYXIgKmVuZCA9IHN0cmNocihkYS0+ZGFfcmVtb3Rlc3RyLCAnOicpOw0KPiArCQkJZHNf
aG9zdG5hbWUgPSBrc3RybmR1cChkYS0+ZGFfcmVtb3Rlc3RyLA0KPiArCQkJCQkgICAgICAgZW5k
IC0gZGEtPmRhX3JlbW90ZXN0ciwNCj4gKwkJCQkJICAgICAgIEdGUF9LRVJORUwpOw0KPiArCQl9
DQo+ICAJCWNscCA9IG5mczRfc2V0X2RzX2NsaWVudChtZHNfc3J2LT5uZnNfY2xpZW50LA0KPiAg
CQkJCSAoc3RydWN0IHNvY2thZGRyICopJmRhLT5kYV9hZGRyLA0KPiAtCQkJCSBkYS0+ZGFfYWRk
cmxlbiwgSVBQUk9UT19UQ1ApOw0KPiArCQkJCSBkYS0+ZGFfYWRkcmxlbiwgSVBQUk9UT19UQ1As
IGRzX2hvc3RuYW1lKTsNCj4gKwkJa2ZyZWUoZHNfaG9zdG5hbWUpOw0KPiAgCQlpZiAoIUlTX0VS
UihjbHApKQ0KPiAgCQkJYnJlYWs7DQo+ICAJfQ0KDQotLSANClRyb25kIE15a2xlYnVzdA0KTGlu
dXggTkZTIGNsaWVudCBtYWludGFpbmVyDQoNCk5ldEFwcA0KVHJvbmQuTXlrbGVidXN0QG5ldGFw
cC5jb20NCnd3dy5uZXRhcHAuY29tDQoNCg==

2012-02-17 18:40:54

by Andy Adamson

[permalink] [raw]
Subject: Re: [PATCH Version 2 1/1] NFSv4.1 set the cl_hostname for data servers

Isn't the clp->cl_rpcclient->cl_server always the same as
clp->cl_hostname? Can we just get rid of the cl_hostname and use the
cl_server?

-->Andy

On Thu, Feb 16, 2012 at 5:08 PM, Adamson, Dros
<[email protected]> wrote:
>
> On Feb 16, 2012, at 5:00 PM, Adamson, Dros wrote:
>
>>
>> On Feb 16, 2012, at 4:29 PM, Myklebust, Trond wrote:
>>
>>> On Thu, 2012-02-16 at 16:10 -0500, [email protected] wrote:
>>>> From: Andy Adamson <[email protected]>
>>>>
>>>> Used by /proc/fs/nfsfs/servers
>>>>
>>>> Signed-off-by: Andy Adamson <[email protected]>
>>>> ---
>>>> fs/nfs/client.c ? ? ? ? ? ?| ? ?3 ++-
>>>> fs/nfs/internal.h ? ? ? ? ?| ? ?3 ++-
>>>> fs/nfs/nfs4filelayoutdev.c | ? 11 ++++++++++-
>>>> 3 files changed, 14 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
>>>> index d0f850f..c625284 100644
>>>> --- a/fs/nfs/client.c
>>>> +++ b/fs/nfs/client.c
>>>> @@ -1443,7 +1443,7 @@ error:
>>>> */
>>>> struct nfs_client *nfs4_set_ds_client(struct nfs_client* mds_clp,
>>>> ? ? ? ? ? ? const struct sockaddr *ds_addr,
>>>> - ? ? ? ? ? int ds_addrlen, int ds_proto)
>>>> + ? ? ? ? ? int ds_addrlen, int ds_proto, char *ds_hostname)
>>>
>>> Can you make this (and the other instances below) 'const char *', so
>>> that we avoid any confusion? Otherwise it looks more or less OK...
>>>
>>> Note that we should consider splitting da->da_remotestr into a
>>> hostname/port part at some point so that we avoid the copy below.
>>
>> da_remotestr (and ds_remotestr) were meant to be for display only (and iirc they can actually be NULL).
>
> ^^ dprintk() display, that is.
>
>>
>> The right way to do this is from the actual sin_addr (da_addr).
>>
>> -dros
>

2012-02-17 18:55:41

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH Version 2 1/1] NFSv4.1 set the cl_hostname for data servers

T24gRnJpLCAyMDEyLTAyLTE3IGF0IDEzOjQwIC0wNTAwLCBBbmR5IEFkYW1zb24gd3JvdGU6DQo+
IElzbid0IHRoZSBjbHAtPmNsX3JwY2NsaWVudC0+Y2xfc2VydmVyIGFsd2F5cyB0aGUgc2FtZSBh
cw0KPiBjbHAtPmNsX2hvc3RuYW1lPyBDYW4gd2UganVzdCBnZXQgcmlkIG9mIHRoZSBjbF9ob3N0
bmFtZSBhbmQgdXNlIHRoZQ0KPiBjbF9zZXJ2ZXI/DQo+IA0KDQpZZXMuIFRoYXQgd291bGQgYWxz
byBiZSBhbiBhY2NlcHRhYmxlIGFsdGVybmF0aXZlLi4uDQoNCi0tIA0KVHJvbmQgTXlrbGVidXN0
DQpMaW51eCBORlMgY2xpZW50IG1haW50YWluZXINCg0KTmV0QXBwDQpUcm9uZC5NeWtsZWJ1c3RA
bmV0YXBwLmNvbQ0Kd3d3Lm5ldGFwcC5jb20NCg0K