Patchwork [01/11] cifs: track local_nls in volume info

login
register
mail settings
Submitter Jeff Layton
Date April 24, 2010, 11:57 a.m.
Message ID <1272110272-20686-2-git-send-email-jlayton@redhat.com>
Download mbox | patch
Permalink /patch/50888/
State New
Headers show

Comments

Jeff Layton - April 24, 2010, 11:57 a.m.
Add a local_nls field to the smb_vol struct and keep a pointer to the
local_nls in it.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/cifs/connect.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)
Steve French - April 25, 2010, 7:52 p.m.
On Sat, Apr 24, 2010 at 6:57 AM, Jeff Layton <jlayton@redhat.com> wrote:
> Add a local_nls field to the smb_vol struct and keep a pointer to the
> local_nls in it.
>
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
>  fs/cifs/connect.c |   11 ++++++-----
>  1 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 58a2109..eb85dd8 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -102,6 +102,7 @@ struct smb_vol {
>        bool sockopt_tcp_nodelay:1;
>        unsigned short int port;
>        char *prepath;
> +       struct nls_table *local_nls;
>  };

If we do this here, should we take it out of the cifs_sb?
If two volumes mounted to the same cifs_sb - last nls wins?
Jeff Layton - April 25, 2010, 11:14 p.m.
On Sun, 25 Apr 2010 14:52:46 -0500
Steve French <smfrench@gmail.com> wrote:

> On Sat, Apr 24, 2010 at 6:57 AM, Jeff Layton <jlayton@redhat.com> wrote:
> > Add a local_nls field to the smb_vol struct and keep a pointer to the
> > local_nls in it.
> >
> > Signed-off-by: Jeff Layton <jlayton@redhat.com>
> > ---
> >  fs/cifs/connect.c |   11 ++++++-----
> >  1 files changed, 6 insertions(+), 5 deletions(-)
> >
> > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> > index 58a2109..eb85dd8 100644
> > --- a/fs/cifs/connect.c
> > +++ b/fs/cifs/connect.c
> > @@ -102,6 +102,7 @@ struct smb_vol {
> >        bool sockopt_tcp_nodelay:1;
> >        unsigned short int port;
> >        char *prepath;
> > +       struct nls_table *local_nls;
> >  };
> 
> If we do this here, should we take it out of the cifs_sb?

No, the smb_vol doesn't live past the session/tcon establishment.

> If two volumes mounted to the same cifs_sb - last nls wins?
>

Not sure I understand this question. The smb_vol is unfortunately
somewhat poorly named. It's mostly a struct for holding the results of
parsing mount options.
Steve French - April 25, 2010, 11:19 p.m.
On Sun, Apr 25, 2010 at 6:14 PM, Jeff Layton <jlayton@redhat.com> wrote:
> On Sun, 25 Apr 2010 14:52:46 -0500
> Steve French <smfrench@gmail.com> wrote:
>
>> On Sat, Apr 24, 2010 at 6:57 AM, Jeff Layton <jlayton@redhat.com> wrote:
>> > Add a local_nls field to the smb_vol struct and keep a pointer to the
>> > local_nls in it.
>> >
>> > Signed-off-by: Jeff Layton <jlayton@redhat.com>
>> > ---
>> >  fs/cifs/connect.c |   11 ++++++-----
>> >  1 files changed, 6 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
>> > index 58a2109..eb85dd8 100644
>> > --- a/fs/cifs/connect.c
>> > +++ b/fs/cifs/connect.c
>> > @@ -102,6 +102,7 @@ struct smb_vol {
>> >        bool sockopt_tcp_nodelay:1;
>> >        unsigned short int port;
>> >        char *prepath;
>> > +       struct nls_table *local_nls;
>> >  };
>>
>> If we do this here, should we take it out of the cifs_sb?
>
> No, the smb_vol doesn't live past the session/tcon establishment.
>
>> If two volumes mounted to the same cifs_sb - last nls wins?
>>
>
> Not sure I understand this question. The smb_vol is unfortunately
> somewhat poorly named. It's mostly a struct for holding the results of
> parsing mount options.

Sorry about that - I was confusing it with the vfsmnt structure (vs. the
subperblock)

Patch

diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 58a2109..eb85dd8 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -102,6 +102,7 @@  struct smb_vol {
 	bool sockopt_tcp_nodelay:1;
 	unsigned short int port;
 	char *prepath;
+	struct nls_table *local_nls;
 };
 
 static int ipv4_connect(struct TCP_Server_Info *server);
@@ -2353,20 +2354,20 @@  try_mount_again:
 		goto out;
 	}
 
-
 	/* this is needed for ASCII cp to Unicode converts */
 	if (volume_info->iocharset == NULL) {
-		cifs_sb->local_nls = load_nls_default();
-	/* load_nls_default can not return null */
+		/* load_nls_default cannot return null */
+		volume_info->local_nls = load_nls_default();
 	} else {
-		cifs_sb->local_nls = load_nls(volume_info->iocharset);
-		if (cifs_sb->local_nls == NULL) {
+		volume_info->local_nls = load_nls(volume_info->iocharset);
+		if (volume_info->local_nls == NULL) {
 			cERROR(1, "CIFS mount error: iocharset %s not found",
 				 volume_info->iocharset);
 			rc = -ELIBACC;
 			goto out;
 		}
 	}
+	cifs_sb->local_nls = volume_info->local_nls;
 
 	/* get a reference to a tcp session */
 	srvTcp = cifs_get_tcp_session(volume_info);