[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Cluster-devel] [PATCH] libgfs2: Return more context from is_pathname_mounted and rename it



Hi,

On Tue, 2013-11-05 at 14:04 +0000, Andrew Price wrote:
> On 05/11/13 11:46, Steven Whitehouse wrote:
> > Hi,
> >
> > Since the path has already been transformed into an fd
> 
> This is a false assumption. For example, in fsck.gfs2's initialize() it 
> is called in order to find out whether the mount point (if any) is 
> read-only after open(path, O_RDWR | O_EXCL) fails so we don't have the 
> fd beforehand in that case. In the cases where we do have an fd it's not 
> guaranteed to be the correct one (e.g. user gives us the mount point but 
> we need to operate on the device).
> 
Well fsck has slightly different requirements - it wants to ensure that
the fs is not mounted, whereas grow wants to know that it is mounted. It
may be worth splitting the two cases if they have other differences as
well.

> >, we should be
> > using the fd rather than sending the path to lgfs2_find_mnt in order to
> > avoid any possible races and also issues wrt device names being
> > different,
> 
> There's a dependency loop here:
> 
> 1. To get the correct fd to work with we need to get the right mntent
> 2. To get the right mntent we need to know the stat info
> 3. To get the stat info without races we need to fstat the correct fd 
> (goto 1)
> 
> So I'm really not convinced that we can improve this code much more. 
> Even if there's a way to solve the loop without possible races, we'd 
> still need to send the path to lgfs2_find_mnt because we need to compare 
> it to the mount points and device names of each entry in /proc/mounts.
> 
> Btw, if we can solve it we should send a patch for e2fsck because it's 
> where the original is_pathname_mounted code came from and it's been 
> using the same algorithm for over ten years:
> 
> http://git.kernel.org/cgit/fs/ext2/e2fsprogs.git/tree/lib/ext2fs/ismounted.c#n39 
> :)
> 
> Andy
> 
Thats ok I think. What we should be doing is to open an fd for whatever
string the user has given us - in the fsck case, if O_RDWR fails, we can
try O_RDONLY. Then we can fstat to find out what it is - device, file or
directory and then look to see if we can find the mntent based on that.
Open each dev in turn and compare dev numbers if we are looking for a
dev, otherwise we know the original fd refers to a mount point and we
can compare the strings, or something along those lines.

So yes, we may need to send both the path and the fd to lfs2_find_mnt()
or maybe we should just write a/some function(s) which do the whole task
of taking the string and returning a device fd, and feeding it some
flags which tells the function whether (or not) we want to have a device
that is mounted or not, if that makes sense in the cases that we have,

Steve.




[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]