On Tue, Jul 15, 2008 at 8:52 AM, Neil Brown <neilb@xxxxxxx> 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 <neilb@xxxxxxx> > 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 <neilb@xxxxxxx> > --- > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
