From owner-freebsd-current@FreeBSD.ORG Tue Nov 18 07:43:04 2003 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 7D34116A4CE; Tue, 18 Nov 2003 07:43:04 -0800 (PST) Received: from kazi.fit.vutbr.cz (kazi.fit.vutbr.cz [147.229.8.12]) by mx1.FreeBSD.org (Postfix) with ESMTP id 4AA1C43FA3; Tue, 18 Nov 2003 07:43:02 -0800 (PST) (envelope-from cejkar@fit.vutbr.cz) Received: from kazi.fit.vutbr.cz (localhost [127.0.0.1]) by kazi.fit.vutbr.cz (8.12.10/8.12.9) with ESMTP id hAIFh0UC084395 (version=TLSv1/SSLv3 cipher=EDH-RSA-DES-CBC3-SHA bits=168 verify=NO); Tue, 18 Nov 2003 16:43:00 +0100 (CET) Received: (from cejkar@localhost) by kazi.fit.vutbr.cz (8.12.10/8.12.5/Submit) id hAIFh00O084392; Tue, 18 Nov 2003 16:43:00 +0100 (CET) X-Authentication-Warning: kazi.fit.vutbr.cz: cejkar set sender to cejkar@fit.vutbr.cz using -f Date: Tue, 18 Nov 2003 16:43:00 +0100 From: Rudolf Cejka To: Ian Dowse Message-ID: <20031118154259.GA70896@fit.vutbr.cz> References: <200311161648.hAGGmIZD097598@repoman.freebsd.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <200311161648.hAGGmIZD097598@repoman.freebsd.org> User-Agent: Mutt/1.4.1i X-Scanned-By: MIMEDefang 2.16 (www . roaringpenguin . com / mimedefang) cc: freebsd-current@FreeBSD.org cc: "Tim J. Robbins" Subject: Re: cvs commit: src/sbin/umount umount.c X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 18 Nov 2003 15:43:04 -0000 Ian Dowse wrote (2003/11/16): > Modified files: > sbin/umount umount.c > Log: > If the unmount by file system ID fails, don't warn before retrying > a non-fsid unmount if the file system ID is all zeros. This is a > temporary workaround for warnings that occur in the vfs.usermount=1 > case because non-root users get a zeroed filesystem ID. I have a > more complete fix in the works, but I won't get it done for 5.2. > Revision Changes Path > 1.41 +4 -1 src/sbin/umount/umount.c Hello and thanks for fixing this! I had a plan to report this, but you were faster :o) I'm interested in this area - please, can you tell, what do you plan to do in your more complete fix? When I looked at this issue, I thought about some things: * Why is f_fsid zeroed for non-root users at all? Is there any reason? Maybe it would be good idea to document it in /usr/src/lib/libc/sys/getfsstat.2 and /usr/src/lib/libc/sys/statfs.2 - so one small point for Tim: Tim J. Robbins wrote (2003/11/15): > Modified files: > lib/libc/sys statfs.2 > Log: > Resync. struct statfs and flag definitions with sys/mount.h. > Revision Changes Path > 1.22 +57 -22 src/lib/libc/sys/statfs.2 * Tim, can you please update /usr/src/lib/libc/sys/getfsstat.2 to reflect current sys/mount.h too, as you did for /usr/src/lib/libc/sys/statfs.2? Manual page getfsstat.2 lists struct statfs too and there are still old fields and values like MNAMELEN is 90 and so on. And if there are no objections, is it possible to add sentence "Non-root users get always f_fsid zeroed." (with better english...) to both manual pages? * There are small typos in umount.c: --- umount.c.orig Tue Nov 18 16:24:43 2003 +++ umount.c Tue Nov 18 16:24:54 2003 @@ -365,7 +365,7 @@ warn("unmount of %s failed", sfs->f_mntonname); if (errno != ENOENT) return (1); - /* Compatability for old kernels. */ + /* Compatibility for old kernels. */ warnx("retrying using path instead of file system ID"); if (unmount(sfs->f_mntonname, fflag) != 0) { warn("unmount of %s failed", sfs->f_mntonname); @@ -557,7 +557,7 @@ } /* - * Convert a hexidecimal filesystem ID to an fsid_t. + * Convert a hexadecimal filesystem ID to an fsid_t. * Returns 0 on success. */ int --- * Do you understand, why there is line in umount.c:376 getmntentry(NULL, NULL, &sfs->f_fsid, REMOVE) ? I'm not sure, but if it is needed for some reason, I think that there should be used different getmntentry() according to the used unmount() method, like in this patch: --- umount.c.orig Tue Nov 18 14:59:00 2003 +++ umount.c Tue Nov 18 15:26:59 2003 @@ -370,10 +370,14 @@ if (unmount(sfs->f_mntonname, fflag) != 0) { warn("unmount of %s failed", sfs->f_mntonname); return (1); + } else { + /* Mark this this file system as unmounted. */ + getmntentry(NULL, sfs->f_mntonname, NULL, REMOVE); } + } else { + /* Mark this this file system as unmounted. */ + getmntentry(NULL, NULL, &sfs->f_fsid, REMOVE); } - /* Mark this this file system as unmounted. */ - getmntentry(NULL, NULL, &sfs->f_fsid, REMOVE); if (vflag) (void)printf("%s: unmount from %s\n", sfs->f_mntfromname, sfs->f_mntonname); --- * /usr/src/sbin/mount/mount.c: If user uses mount -v, it prints false zeroed fsids - isn't it better to print just non-zero fsids, so that nobody is "mystified"? I have created two patches - I do not know which do you consider as a better: --- mount.c.orig Tue Nov 18 14:46:24 2003 +++ mount.c Tue Nov 18 14:50:46 2003 @@ -535,9 +535,11 @@ if (sfp->f_syncreads != 0 || sfp->f_asyncreads != 0) (void)printf(", reads: sync %ld async %ld", sfp->f_syncreads, sfp->f_asyncreads); - printf(", fsid "); - for (i = 0; i < sizeof(sfp->f_fsid); i++) - printf("%02x", ((u_char *)&sfp->f_fsid)[i]); + if (sfp->f_fsid.val[0] != 0 || sfp->f_fsid.val[1] != 0) { + printf(", fsid "); + for (i = 0; i < sizeof(sfp->f_fsid); i++) + printf("%02x", ((u_char *)&sfp->f_fsid)[i]); + } } (void)printf(")\n"); } --- or --- mount.c.orig Tue Nov 18 14:46:24 2003 +++ mount.c Tue Nov 18 14:57:30 2003 @@ -508,7 +508,7 @@ prmount(sfp) struct statfs *sfp; { - int flags, i; + int flags, i, fsid; struct opt *o; struct passwd *pw; @@ -535,9 +535,18 @@ if (sfp->f_syncreads != 0 || sfp->f_asyncreads != 0) (void)printf(", reads: sync %ld async %ld", sfp->f_syncreads, sfp->f_asyncreads); - printf(", fsid "); - for (i = 0; i < sizeof(sfp->f_fsid); i++) - printf("%02x", ((u_char *)&sfp->f_fsid)[i]); + fsid = 0; + for (i = 0; i < sizeof(sfp->f_fsid); i++) { + if (((u_char *)&sfp->f_fsid)[i] != 0) { + fsid = 1; + break; + } + } + if (fsid) { + printf(", fsid "); + for (i = 0; i < sizeof(sfp->f_fsid); i++) + printf("%02x", ((u_char *)&sfp->f_fsid)[i]); + } } (void)printf(")\n"); } --- Thanks. -- Rudolf Cejka http://www.fit.vutbr.cz/~cejkar Brno University of Technology, Faculty of Information Technology Bozetechova 2, 612 66 Brno, Czech Republic