[PATCH] xfsprogs: skip over "rootfs" entry if mtab links to /proc/mounts

[PATCH] xfsprogs: skip over "rootfs" entry if mtab links to /proc/mounts

[Eric Sandeen]

This is for RH bug 727938, xfs_fsr regression for root file system

Fedora has made /etc/mtab a symlink to /proc/mounts, but when
we issue "xfs_fsr /" and fsr's getmntany() goes looking for
the "/" entry, the first one it finds is

rootfs / rootfs rw 0 0

it says no way, that's a rootfs filesystem type, not xfs!
And it never finds this later:

/dev/sda2 / xfs rw,relatime,attr2,noquota 0 0

This patch to skip over the rootfs entry seems to fix it.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

diff --git a/fsr/xfs_fsr.c b/fsr/xfs_fsr.c
index 40c2e6f..eaf8ac7 100644
--- a/fsr/xfs_fsr.c
+++ b/fsr/xfs_fsr.c
@@ -1648,6 +1648,9 @@ getmntany(FILE *fp, struct mntent *mp, struct mntent *mpref, struct stat64 *s)
 	struct stat64 ms;
 
 	while ((t = getmntent(fp))) {
+		/* skip over "rootfs / rootfs" if mtab is really /proc/mounts */
+		if (strcmp(t->mnt_type, "rootfs") == 0)
+			continue;
 		if (mpref->mnt_fsname) {	/* device */
 			if (stat64(t->mnt_fsname, &ms) < 0)
 				continue;

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfsprogs: skip over "rootfs" entry if mtab links to /proc/mounts

[Christoph Hellwig]

On Wed, Aug 03, 2011 at 01:41:59PM -0500, Eric Sandeen wrote:
> This is for RH bug 727938, xfs_fsr regression for root file system
> 
> Fedora has made /etc/mtab a symlink to /proc/mounts, but when
> we issue "xfs_fsr /" and fsr's getmntany() goes looking for
> the "/" entry, the first one it finds is
> 
> rootfs / rootfs rw 0 0
> 
> it says no way, that's a rootfs filesystem type, not xfs!
> And it never finds this later:
> 
> /dev/sda2 / xfs rw,relatime,attr2,noquota 0 0
> 
> This patch to skip over the rootfs entry seems to fix it.

I don't like this.  rootfs is the symptom, but the underlying problem
is that in Linux we're perfectly fine to have multiple filesystems
mounted on a single mountpoint, and the getmntany can't deal with it.

I think the right fix is to simply remove the break from the loop,
and thus let a second match for our fs override the first.  This relies
on getmntent returning entries in the order they were mounted, but
without that I can't think of a reliable way for getmntent to work
with the multiple mounts in Linux.

It would also be nice to have a testcase for this behaviour in xfstests,
creating two loop devices with different backing files and mounting them
at the same mount point.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfsprogs: skip over "rootfs" entry if mtab links to /proc/mounts

[Eric Sandeen]

On 8/3/11 2:02 PM, Christoph Hellwig wrote:
> On Wed, Aug 03, 2011 at 01:41:59PM -0500, Eric Sandeen wrote:
>> This is for RH bug 727938, xfs_fsr regression for root file system
>>
>> Fedora has made /etc/mtab a symlink to /proc/mounts, but when
>> we issue "xfs_fsr /" and fsr's getmntany() goes looking for
>> the "/" entry, the first one it finds is
>>
>> rootfs / rootfs rw 0 0
>>
>> it says no way, that's a rootfs filesystem type, not xfs!
>> And it never finds this later:
>>
>> /dev/sda2 / xfs rw,relatime,attr2,noquota 0 0
>>
>> This patch to skip over the rootfs entry seems to fix it.
> 
> I don't like this.  rootfs is the symptom, but the underlying problem
> is that in Linux we're perfectly fine to have multiple filesystems
> mounted on a single mountpoint, and the getmntany can't deal with it.

But in this case, xfs_fsr has been told to "defrag mountpoint /bar"
and if /dev/X is mounted on /foo, /bar, and /baz, it'll do the
right thing, won't it? 

oh wait I'm thinking about multiple mountpoints for the same device,
not multiple devices on the same mountpoint.

Ok, duh, you're right of course.

Well, this patch certainly makes the current situation no worse,
and fixes the immediate problem with /proc/mounts... and we really
never do want to try to do something with "rootfs" do we?

Could the other behavioral change be done as a separate patch?

Well, maybe there's no point, what do you think?

> I think the right fix is to simply remove the break from the loop,
> and thus let a second match for our fs override the first.  This relies
> on getmntent returning entries in the order they were mounted, but
> without that I can't think of a reliable way for getmntent to work
> with the multiple mounts in Linux.

fair enough.

> It would also be nice to have a testcase for this behaviour in xfstests,
> creating two loop devices with different backing files and mounting them
> at the same mount point.

easy enough :)

-Eric


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfsprogs: skip over "rootfs" entry if mtab links to /proc/mounts

[Eric Sandeen]

On 8/3/11 2:02 PM, Christoph Hellwig wrote:
> On Wed, Aug 03, 2011 at 01:41:59PM -0500, Eric Sandeen wrote:
>> This is for RH bug 727938, xfs_fsr regression for root file system
>>
>> Fedora has made /etc/mtab a symlink to /proc/mounts, but when
>> we issue "xfs_fsr /" and fsr's getmntany() goes looking for
>> the "/" entry, the first one it finds is
>>
>> rootfs / rootfs rw 0 0
>>
>> it says no way, that's a rootfs filesystem type, not xfs!
>> And it never finds this later:
>>
>> /dev/sda2 / xfs rw,relatime,attr2,noquota 0 0
>>
>> This patch to skip over the rootfs entry seems to fix it.
> 
> I don't like this.  rootfs is the symptom, but the underlying problem
> is that in Linux we're perfectly fine to have multiple filesystems
> mounted on a single mountpoint, and the getmntany can't deal with it.
> 
> I think the right fix is to simply remove the break from the loop,

It's not quite that simple.

when we do

	*mp = *t;
	/* break; */

we are pointing the mp string pointers at the strings in the getmntent entry,
which will keep moving as we iterate.  We'll need a strdup or something
to copy it out on each good find, with appropriate frees etc, I guess.

Is there a better way to do it?  This seems pretty ugly.

        memset(mp, 0, sizeof(struct mntent));
        while ((t = getmntent(fp))) {
                if (mpref->mnt_fsname) {        /* device */
                        if (stat64(t->mnt_fsname, &ms) < 0)
                                continue;
                        if (s->st_rdev != ms.st_rdev)
                                continue;
                }
                if (mpref->mnt_dir) {           /* mount point */
                        if (stat64(t->mnt_dir, &ms) < 0)
                                continue;
                        if (s->st_ino != ms.st_ino || s->st_dev != ms.st_dev)
                                continue;
                }
                /* This one matches */
                found = 1;
                free(mp->mnt_fsname);
                free(mp->mnt_dir);
                free(mp->mnt_type);
                mp->mnt_fsname = strdup(t->mnt_fsname);
                mp->mnt_dir = strdup(t->mnt_dir);
                mp->mnt_type = strdup(t->mnt_type);
        }
        return (found);


(and then free the strings in the caller when we're done ...)

-Eric

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs