Date: Wed, 23 Jun 2010 21:58:24 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Bruce Cran <bruce@cran.org.uk> Cc: freebsd-fs@freebsd.org Subject: Re: Patch to fix reported mountpoint when mounting dirty filesystem Message-ID: <20100623213423.Y45477@delplex.bde.org> In-Reply-To: <201006211847.27788.bruce@cran.org.uk> References: <201006211847.27788.bruce@cran.org.uk>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 21 Jun 2010, Bruce Cran wrote: > I've been investigating PR bin/19683 which is about the fact that when a R/W > mount is denied due to the filesystem being dirty, the reported mountpoint is > that which has been recorded in the superblock, not the directory that it's > currently being mounted on. I've attached a potential patch but I'm not sure > if f_mntonname is valid at the first print statement. I think it is, but this is unclear. Even if it is always initialized, it might be the wrong name to use. (ISTR cases where MNT_RELOAD and/or MNT_UPDATE changes names from one alias to another. Usually the new name is better but this is not clear. Anyway, all uses of the name except the final recording of it in the superblock involve an error, and it is hard to think of a case where the new name is not relevant (the old name might be relevant too).) The old name is used in several other contexts that probably need the same change. E.g., in the error message just after the ones that you changed. If none are left that actually need to know the old name, then it would be clearer to change the old name in the superblock at the beginning of ffs_mount() instead of at the end of ffs_mountfs(). I think the change is discarded unless the mount succeeds. > Also, I tried to > replicate the problem with ext2fs but the despite code being present which > prints the mountpoint, that code was never hit so I'm not sure if it needs > updated too? ext2fs has the same problem. Other clones of ffs that keep the old name in the superblock probably have the same problem. Several read-only clones of ffs don't have the problem since they don't write so the can't have a writable name in a superblock. BTW, ffs_mountfs() is now bogus, and its comment "/* common code for mount and mountroot */ is more bogus. `mountroot' no longer exists so ffs_mountfs() cannot be common for it. ffs_mountfs() is now only called once near the end of ffs_mount() where it could easily be inlined. Its splitting apparently dates from when it was common code for ffs and mfs. Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20100623213423.Y45477>