* [PATCH] cifs: handle cifs_get_tcon() errors properly
@ 2010-11-15 12:45 Suresh Jayaraman
[not found] ` <1289825123-13716-1-git-send-email-sjayaraman-l3A5Bk7waGM@public.gmane.org>
0 siblings, 1 reply; 5+ messages in thread
From: Suresh Jayaraman @ 2010-11-15 12:45 UTC (permalink / raw)
To: Steve French; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA
cifs_get_tcon() could return any of the following errors:
-ENOMEM/-ENODEV/EREMOTEIO. We should follow the DFS referral code path only
when we get -EREMOTEIO (STATUS_PATH_NOT_COVERED) from the server and not
otherwise.
Compile-tested only.
Signed-off-by: Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>
---
fs/cifs/connect.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 251a17c..c3a2323 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -2780,7 +2780,10 @@ try_mount_again:
if (IS_ERR(tcon)) {
rc = PTR_ERR(tcon);
tcon = NULL;
- goto remote_path_check;
+ if (rc == -EREMOTEIO)
+ goto remote_path_check;
+ else
+ goto mount_fail_check;
}
/* do not care if following two calls succeed - informational */
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] cifs: handle cifs_get_tcon() errors properly
[not found] ` <1289825123-13716-1-git-send-email-sjayaraman-l3A5Bk7waGM@public.gmane.org>
@ 2010-11-15 13:25 ` Jeff Layton
[not found] ` <20101115082535.0d7200a5-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org>
0 siblings, 1 reply; 5+ messages in thread
From: Jeff Layton @ 2010-11-15 13:25 UTC (permalink / raw)
To: Suresh Jayaraman; +Cc: Steve French, linux-cifs-u79uwXL29TY76Z2rM5mHXA
On Mon, 15 Nov 2010 18:15:23 +0530
Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org> wrote:
> cifs_get_tcon() could return any of the following errors:
> -ENOMEM/-ENODEV/EREMOTEIO. We should follow the DFS referral code path only
> when we get -EREMOTEIO (STATUS_PATH_NOT_COVERED) from the server and not
> otherwise.
>
> Compile-tested only.
>
> Signed-off-by: Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>
> ---
> fs/cifs/connect.c | 5 ++++-
> 1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 251a17c..c3a2323 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -2780,7 +2780,10 @@ try_mount_again:
> if (IS_ERR(tcon)) {
> rc = PTR_ERR(tcon);
> tcon = NULL;
> - goto remote_path_check;
> + if (rc == -EREMOTEIO)
> + goto remote_path_check;
> + else
> + goto mount_fail_check;
> }
>
> /* do not care if following two calls succeed - informational */
Don't you mean "EREMOTE" here? I've always interpreted "EREMOTEIO" to
mean that the server had the equivalent of an I/O error, whereas
"EREMOTE" means "Object is remote".
I don't see how this patch materially changes anything. Pseudocode,
assuming that rc == -EREMOTE:
remote_path_check:
/* check if a whole path (including prepath) is not remote */
if (!rc && cifs_sb->prepathlen && tcon) {
/* stuff we won't hit because rc is non-zero */
}
if (rc == -EREMOTE) {
/* handle a DFS referral */
}
if (rc)
goto mount_fail_check;
I'll grant that cifs_mount is still a mess, but I'm not sure this
really improves anything.
--
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] cifs: handle cifs_get_tcon() errors properly
[not found] ` <20101115082535.0d7200a5-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org>
@ 2010-11-15 14:54 ` Suresh Jayaraman
[not found] ` <4CE149B1.5090901-l3A5Bk7waGM@public.gmane.org>
0 siblings, 1 reply; 5+ messages in thread
From: Suresh Jayaraman @ 2010-11-15 14:54 UTC (permalink / raw)
To: Jeff Layton; +Cc: Steve French, linux-cifs-u79uwXL29TY76Z2rM5mHXA
On 11/15/2010 06:55 PM, Jeff Layton wrote:
> On Mon, 15 Nov 2010 18:15:23 +0530
> Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org> wrote:
>
>> cifs_get_tcon() could return any of the following errors:
>> -ENOMEM/-ENODEV/EREMOTEIO. We should follow the DFS referral code path only
>> when we get -EREMOTEIO (STATUS_PATH_NOT_COVERED) from the server and not
>> otherwise.
>>
>> Compile-tested only.
>>
>> Signed-off-by: Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>
>> ---
>> fs/cifs/connect.c | 5 ++++-
>> 1 files changed, 4 insertions(+), 1 deletions(-)
>>
>> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
>> index 251a17c..c3a2323 100644
>> --- a/fs/cifs/connect.c
>> +++ b/fs/cifs/connect.c
>> @@ -2780,7 +2780,10 @@ try_mount_again:
>> if (IS_ERR(tcon)) {
>> rc = PTR_ERR(tcon);
>> tcon = NULL;
>> - goto remote_path_check;
>> + if (rc == -EREMOTEIO)
>> + goto remote_path_check;
>> + else
>> + goto mount_fail_check;
>> }
>>
>> /* do not care if following two calls succeed - informational */
>
> Don't you mean "EREMOTE" here? I've always interpreted "EREMOTEIO" to
Doh, yes.
> mean that the server had the equivalent of an I/O error, whereas
> "EREMOTE" means "Object is remote".
>
> I don't see how this patch materially changes anything. Pseudocode,
> assuming that rc == -EREMOTE:
I agree that this is not an improvement (more of a cleanup), but I think
it improves readability (given that the single function runs for more than
hundred lines), and skips a couple of unneeded checks.
Here's a fixed version (but this patch can be ignored if we feel so):
cifs_get_tcon() could return any of the following errors:
-ENOMEM/-ENODEV/EREMOTEIO. We should follow the DFS referral code path only
when we get -EREMOTEIO (STATUS_PATH_NOT_COVERED) from the server and not
otherwise.
Compile-tested only.
Signed-off-by: Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>
---
fs/cifs/connect.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 251a17c..6dbc145 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -2780,7 +2780,10 @@ try_mount_again:
if (IS_ERR(tcon)) {
rc = PTR_ERR(tcon);
tcon = NULL;
- goto remote_path_check;
+ if (rc == -EREMOTE)
+ goto remote_path_check;
+ else
+ goto mount_fail_check;
}
/* do not care if following two calls succeed - informational */
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] cifs: handle cifs_get_tcon() errors properly
[not found] ` <4CE149B1.5090901-l3A5Bk7waGM@public.gmane.org>
@ 2010-11-15 15:28 ` Jeff Layton
[not found] ` <20101115102843.6f425b27-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org>
0 siblings, 1 reply; 5+ messages in thread
From: Jeff Layton @ 2010-11-15 15:28 UTC (permalink / raw)
To: Suresh Jayaraman; +Cc: Steve French, linux-cifs-u79uwXL29TY76Z2rM5mHXA
On Mon, 15 Nov 2010 20:24:41 +0530
Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org> wrote:
> On 11/15/2010 06:55 PM, Jeff Layton wrote:
> > On Mon, 15 Nov 2010 18:15:23 +0530
> > Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org> wrote:
> >
> >> cifs_get_tcon() could return any of the following errors:
> >> -ENOMEM/-ENODEV/EREMOTEIO. We should follow the DFS referral code path only
> >> when we get -EREMOTEIO (STATUS_PATH_NOT_COVERED) from the server and not
> >> otherwise.
> >>
> >> Compile-tested only.
> >>
> >> Signed-off-by: Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>
> >> ---
> >> fs/cifs/connect.c | 5 ++++-
> >> 1 files changed, 4 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> >> index 251a17c..c3a2323 100644
> >> --- a/fs/cifs/connect.c
> >> +++ b/fs/cifs/connect.c
> >> @@ -2780,7 +2780,10 @@ try_mount_again:
> >> if (IS_ERR(tcon)) {
> >> rc = PTR_ERR(tcon);
> >> tcon = NULL;
> >> - goto remote_path_check;
> >> + if (rc == -EREMOTEIO)
> >> + goto remote_path_check;
> >> + else
> >> + goto mount_fail_check;
> >> }
> >>
> >> /* do not care if following two calls succeed - informational */
> >
> > Don't you mean "EREMOTE" here? I've always interpreted "EREMOTEIO" to
>
> Doh, yes.
>
> > mean that the server had the equivalent of an I/O error, whereas
> > "EREMOTE" means "Object is remote".
> >
> > I don't see how this patch materially changes anything. Pseudocode,
> > assuming that rc == -EREMOTE:
>
> I agree that this is not an improvement (more of a cleanup), but I think
> it improves readability (given that the single function runs for more than
> hundred lines), and skips a couple of unneeded checks.
>
> Here's a fixed version (but this patch can be ignored if we feel so):
>
>
> cifs_get_tcon() could return any of the following errors:
> -ENOMEM/-ENODEV/EREMOTEIO. We should follow the DFS referral code path only
> when we get -EREMOTEIO (STATUS_PATH_NOT_COVERED) from the server and not
> otherwise.
>
> Compile-tested only.
>
> Signed-off-by: Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>
> ---
> fs/cifs/connect.c | 5 ++++-
> 1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 251a17c..6dbc145 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -2780,7 +2780,10 @@ try_mount_again:
> if (IS_ERR(tcon)) {
> rc = PTR_ERR(tcon);
> tcon = NULL;
> - goto remote_path_check;
> + if (rc == -EREMOTE)
> + goto remote_path_check;
> + else
> + goto mount_fail_check;
> }
>
> /* do not care if following two calls succeed - informational */
I just don't the point here. cifs_mount is quite messy, but I don't
think this really does much to improve its readability.
--
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] cifs: handle cifs_get_tcon() errors properly
[not found] ` <20101115102843.6f425b27-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org>
@ 2010-11-15 16:51 ` Suresh Jayaraman
0 siblings, 0 replies; 5+ messages in thread
From: Suresh Jayaraman @ 2010-11-15 16:51 UTC (permalink / raw)
To: Jeff Layton; +Cc: Steve French, linux-cifs-u79uwXL29TY76Z2rM5mHXA
On 11/15/2010 08:58 PM, Jeff Layton wrote:
> On Mon, 15 Nov 2010 20:24:41 +0530
> Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org> wrote:
>
>> I agree that this is not an improvement (more of a cleanup), but I think
>> it improves readability (given that the single function runs for more than
>> hundred lines), and skips a couple of unneeded checks.
>>
>> Here's a fixed version (but this patch can be ignored if we feel so):
>>
>>
>> cifs_get_tcon() could return any of the following errors:
>> -ENOMEM/-ENODEV/EREMOTEIO. We should follow the DFS referral code path only
>> when we get -EREMOTEIO (STATUS_PATH_NOT_COVERED) from the server and not
>> otherwise.
>>
>> Compile-tested only.
>>
>> Signed-off-by: Suresh Jayaraman <sjayaraman-l3A5Bk7waGM@public.gmane.org>
>> ---
>> fs/cifs/connect.c | 5 ++++-
>> 1 files changed, 4 insertions(+), 1 deletions(-)
>>
>> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
>> index 251a17c..6dbc145 100644
>> --- a/fs/cifs/connect.c
>> +++ b/fs/cifs/connect.c
>> @@ -2780,7 +2780,10 @@ try_mount_again:
>> if (IS_ERR(tcon)) {
>> rc = PTR_ERR(tcon);
>> tcon = NULL;
>> - goto remote_path_check;
>> + if (rc == -EREMOTE)
>> + goto remote_path_check;
>> + else
>> + goto mount_fail_check;
>> }
>>
>> /* do not care if following two calls succeed - informational */
>
> I just don't the point here. cifs_mount is quite messy, but I don't
> think this really does much to improve its readability.
>
Ok, let's drop this patch..
--
Suresh Jayaraman
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-11-15 16:51 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-11-15 12:45 [PATCH] cifs: handle cifs_get_tcon() errors properly Suresh Jayaraman
[not found] ` <1289825123-13716-1-git-send-email-sjayaraman-l3A5Bk7waGM@public.gmane.org>
2010-11-15 13:25 ` Jeff Layton
[not found] ` <20101115082535.0d7200a5-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org>
2010-11-15 14:54 ` Suresh Jayaraman
[not found] ` <4CE149B1.5090901-l3A5Bk7waGM@public.gmane.org>
2010-11-15 15:28 ` Jeff Layton
[not found] ` <20101115102843.6f425b27-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org>
2010-11-15 16:51 ` Suresh Jayaraman
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.