Another patch that can be pulled from
git://neil.brown.name/nfs-utils for-steved
This and the next deal with problems with the new 'string based
option' code when portmap can only be contacted via TCP.
Some people seem to turn of string based options to avoid this
problem:
https://bugs.launchpad.net/ubuntu/+bug/213444/comments/23
This can partly be avoided using mountproto=tcp, but that
a/ shouldn't be needed (isn't with the old code)
b/ doesn't help the unmount case.
This first patch make umount work better if mountproto=tcp was given
to mount.
The next patch gives mountproto=tcp to mount if it is needed.
NeilBrown
>From 863af3c54b574a588ac6d7bd05a1f250784159bd Mon Sep 17 00:00:00 2001
From: Neil Brown <[email protected]>
Date: Tue, 15 Jul 2008 17:36:57 +1000
If portmap is not listening on UDP (as apparently happens with
MS-Windows-Server2003R2SP2), then nfs mounts have to be mounted
with -o mountproto=tcp to succeed.
In this case a umount will still try UDP and will fail to contact the
server. It will still succeed with the local unmount (after a
timeout) but exits with a non-zero exit status. This causes
/bin/mount to retry so we get a strange error about the filesystem
not being mounted.
So:
get umount to use tcp if "mountproto=tcp" appears in mtab
ignore any failure message from the server that would overwrite
a success message from the local umount syscall.
Signed-off-by: NeilBrown <[email protected]>
---
utils/mount/nfsumount.c | 19 +++++++++++++++----
1 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/utils/mount/nfsumount.c b/utils/mount/nfsumount.c
index 285273b..9e1855d 100644
--- a/utils/mount/nfsumount.c
+++ b/utils/mount/nfsumount.c
@@ -203,9 +203,15 @@ static int do_nfs_umount23(const char *spec, char *opts)
pmap->pm_vers = nfsvers_to_mnt(atoi(p+5));
if (opts && (p = strstr(opts, "mountvers=")) && isdigit(*(p+10)))
pmap->pm_vers = atoi(p+10);
- if (opts && (hasmntopt(&mnt, "udp") || hasmntopt(&mnt, "proto=udp")))
+ if (opts && (hasmntopt(&mnt, "udp")
+ || hasmntopt(&mnt, "proto=udp")
+ || hasmntopt(&mnt, "mountproto=udp")
+ ))
pmap->pm_prot = IPPROTO_UDP;
- if (opts && (hasmntopt(&mnt, "tcp") || hasmntopt(&mnt, "proto=tcp")))
+ if (opts && (hasmntopt(&mnt, "tcp")
+ || hasmntopt(&mnt, "proto=tcp")
+ || hasmntopt(&mnt, "mountproto=tcp")
+ ))
pmap->pm_prot = IPPROTO_TCP;
if (!nfs_gethostbyname(hostname, &mnt_server.saddr)) {
@@ -347,8 +353,13 @@ int nfsumount(int argc, char *argv[])
ret = 0;
if (mc) {
if (!lazy && strcmp(mc->m.mnt_type, "nfs4") != 0)
- ret = do_nfs_umount23(mc->m.mnt_fsname, mc->m.mnt_opts);
- ret = del_mtab(mc->m.mnt_fsname, mc->m.mnt_dir) ?: ret;
+ /* We ignore the error from do_nfs_umount23.
+ * If the actual umount succeeds (in del_mtab),
+ * we don't want to signal an error, as that
+ * could cause /sbin/mount to retry!
+ */
+ do_nfs_umount23(mc->m.mnt_fsname, mc->m.mnt_opts);
+ ret = del_mtab(mc->m.mnt_fsname, mc->m.mnt_dir);
} else if (*spec != '/') {
if (!lazy)
ret = do_nfs_umount23(spec, "tcp,v3");
--
1.5.6.2
On Tue, Jul 15, 2008 at 8:52 AM, Neil Brown <[email protected]> wrote:
>
> Another patch that can be pulled from
> git://neil.brown.name/nfs-utils for-steved
>
> This and the next deal with problems with the new 'string based
> option' code when portmap can only be contacted via TCP.
>
> Some people seem to turn of string based options to avoid this
> problem:
> https://bugs.launchpad.net/ubuntu/+bug/213444/comments/23
>
> This can partly be avoided using mountproto=tcp, but that
> a/ shouldn't be needed (isn't with the old code)
> b/ doesn't help the unmount case.
>
> This first patch make umount work better if mountproto=tcp was given
> to mount.
> The next patch gives mountproto=tcp to mount if it is needed.
>
> NeilBrown
>
>
>
> From 863af3c54b574a588ac6d7bd05a1f250784159bd Mon Sep 17 00:00:00 2001
> From: Neil Brown <[email protected]>
> Date: Tue, 15 Jul 2008 17:36:57 +1000
>
> If portmap is not listening on UDP (as apparently happens with
> MS-Windows-Server2003R2SP2), then nfs mounts have to be mounted
> with -o mountproto=tcp to succeed.
>
> In this case a umount will still try UDP and will fail to contact the
> server. It will still succeed with the local unmount (after a
> timeout) but exits with a non-zero exit status. This causes
> /bin/mount to retry so we get a strange error about the filesystem
> not being mounted.
>
> So:
> get umount to use tcp if "mountproto=tcp" appears in mtab
> ignore any failure message from the server that would overwrite
> a success message from the local umount syscall.
>
> Signed-off-by: NeilBrown <[email protected]>
> ---
> utils/mount/nfsumount.c | 19 +++++++++++++++----
> 1 files changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/utils/mount/nfsumount.c b/utils/mount/nfsumount.c
> index 285273b..9e1855d 100644
> --- a/utils/mount/nfsumount.c
> +++ b/utils/mount/nfsumount.c
> @@ -203,9 +203,15 @@ static int do_nfs_umount23(const char *spec, char *opts)
> pmap->pm_vers = nfsvers_to_mnt(atoi(p+5));
> if (opts && (p = strstr(opts, "mountvers=")) && isdigit(*(p+10)))
> pmap->pm_vers = atoi(p+10);
> - if (opts && (hasmntopt(&mnt, "udp") || hasmntopt(&mnt, "proto=udp")))
> + if (opts && (hasmntopt(&mnt, "udp")
> + || hasmntopt(&mnt, "proto=udp")
> + || hasmntopt(&mnt, "mountproto=udp")
> + ))
> pmap->pm_prot = IPPROTO_UDP;
> - if (opts && (hasmntopt(&mnt, "tcp") || hasmntopt(&mnt, "proto=tcp")))
> + if (opts && (hasmntopt(&mnt, "tcp")
> + || hasmntopt(&mnt, "proto=tcp")
> + || hasmntopt(&mnt, "mountproto=tcp")
> + ))
> pmap->pm_prot = IPPROTO_TCP;
That's a good fix. I have a patch later in my IPv6 series that does
this along with many other things. Again, it's reasonable to address
this now.
>
> if (!nfs_gethostbyname(hostname, &mnt_server.saddr)) {
> @@ -347,8 +353,13 @@ int nfsumount(int argc, char *argv[])
> ret = 0;
> if (mc) {
> if (!lazy && strcmp(mc->m.mnt_type, "nfs4") != 0)
> - ret = do_nfs_umount23(mc->m.mnt_fsname, mc->m.mnt_opts);
> - ret = del_mtab(mc->m.mnt_fsname, mc->m.mnt_dir) ?: ret;
> + /* We ignore the error from do_nfs_umount23.
> + * If the actual umount succeeds (in del_mtab),
> + * we don't want to signal an error, as that
> + * could cause /sbin/mount to retry!
> + */
> + do_nfs_umount23(mc->m.mnt_fsname, mc->m.mnt_opts);
> + ret = del_mtab(mc->m.mnt_fsname, mc->m.mnt_dir);
Well, NFSv2/v3 UMNT is advisory only. I think this should have been
ignoring the result of the UMNT RPC already anyway. In fact, Solaris
doesn't even wait for the RPC reply... (and we shouldn't either --
that will just block the umount if the server isn't available).
But I've never seen /sbin/mount retry a umount request... so I don't
quite understand the new comment here. I could be off base...
> } else if (*spec != '/') {
> if (!lazy)
> ret = do_nfs_umount23(spec, "tcp,v3");
If you're going to ignore the return code above, why not here as well?
And you might as well make do_nfs_umount23() return void while you're
at it.
If you do that, you can move your new comment to a block comment in
front of do_nfs_umount23() -- that would be easier to read, I think.
> --
> 1.5.6.2
--
Chuck Lever
Neil Brown wrote:
> Another patch that can be pulled from
> git://neil.brown.name/nfs-utils for-steved
>
> This and the next deal with problems with the new 'string based
> option' code when portmap can only be contacted via TCP.
>
> Some people seem to turn of string based options to avoid this
> problem:
> https://bugs.launchpad.net/ubuntu/+bug/213444/comments/23
>
> This can partly be avoided using mountproto=tcp, but that
> a/ shouldn't be needed (isn't with the old code)
> b/ doesn't help the unmount case.
>
> This first patch make umount work better if mountproto=tcp was given
> to mount.
> The next patch gives mountproto=tcp to mount if it is needed.
Committed
steved.
On Wednesday July 16, [email protected] wrote:
>
>
> Neil Brown wrote:
> > Another patch that can be pulled from
> > git://neil.brown.name/nfs-utils for-steved
> >
> > This and the next deal with problems with the new 'string based
> > option' code when portmap can only be contacted via TCP.
> >
> > Some people seem to turn of string based options to avoid this
> > problem:
> > https://bugs.launchpad.net/ubuntu/+bug/213444/comments/23
> >
> > This can partly be avoided using mountproto=tcp, but that
> > a/ shouldn't be needed (isn't with the old code)
> > b/ doesn't help the unmount case.
> >
> > This first patch make umount work better if mountproto=tcp was given
> > to mount.
> > The next patch gives mountproto=tcp to mount if it is needed.
> Committed
>
> steved.
Thanks!
Do you have plans for a 1.1.3?
15th of September would be 6 months since 1.1.2 :-)
NeilBrown
Neil Brown wrote:
>
> Thanks!
> Do you have plans for a 1.1.3?
Sure do... I'm taking some PTO over the next few days
but I come back I plan to cut the 1.1.3 release...
steved.