* Re: [PATCH] xfsprogs: skip over "rootfs" entry if mtab links to /proc/mounts
2011-08-03 19:02 ` Christoph Hellwig
@ 2011-08-03 19:12 ` Eric Sandeen
2011-08-04 22:17 ` Eric Sandeen
1 sibling, 0 replies; 4+ messages in thread
From: Eric Sandeen @ 2011-08-03 19:12 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs-oss
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] xfsprogs: skip over "rootfs" entry if mtab links to /proc/mounts
2011-08-03 19:02 ` Christoph Hellwig
2011-08-03 19:12 ` Eric Sandeen
@ 2011-08-04 22:17 ` Eric Sandeen
1 sibling, 0 replies; 4+ messages in thread
From: Eric Sandeen @ 2011-08-04 22:17 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Eric Sandeen, xfs-oss
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
^ permalink raw reply [flat|nested] 4+ messages in thread