Thanks for taking care of this, but this just seems to make the already
horribly ugly code even worse.
What do you think about the version below?
---
From: Christoph Hellwig <hch@xxxxxx>
Subject: fsr: fix /proc/mounts parsing
Make sure we do not reject an XFS root mount just because /dev/root is also
listed in /proc/mounts. The root cause for this was the awkward getmntany
function, which is replaced with a broader reach find_mountpoint function
which replace getmntany and the surrounding code from the main routine in
a structured way. This changes the flow from finding a mounted filesystem
matching the argument and checking that it's XFS to find a mounted XFS
filesystem and thus fixes the bug.
Based on analysis and an earlier patch from
Carlos Maiolino <cmaiolino@xxxxxxxxxx>.
Signed-off-by: Christoph Hellwig <hch@xxxxxx>
---
fsr/xfs_fsr.c | 142 +++++++++++++++++++++++++++++-----------------------------
1 file changed, 72 insertions(+), 70 deletions(-)
Index: xfsprogs-dev/fsr/xfs_fsr.c
===================================================================
--- xfsprogs-dev.orig/fsr/xfs_fsr.c 2012-02-12 16:30:07.286766766 -0800
+++ xfsprogs-dev/fsr/xfs_fsr.c 2012-02-12 16:42:39.293447376 -0800
@@ -109,7 +109,6 @@ static void tmp_init(char *mnt);
static char * tmp_next(char *mnt);
static void tmp_close(char *mnt);
int xfs_getgeom(int , xfs_fsop_geom_v1_t * );
-static int getmntany(FILE *, struct mntent *, struct mntent *, struct stat64 *);
xfs_fsop_geom_v1_t fsgeom; /* geometry of active mounted system */
@@ -178,18 +177,73 @@ aborter(int unused)
exit(1);
}
+/*
+ * Check if the argument is either the device name or mountpoint of an XFS
+ * filesystem. Note that we do not care about bind mounted regular files
+ * here - the code that handles defragmentation of invidual files takes care
+ * of that.
+ */
+static char *
+find_mountpoint(char *mtab, char *argname, struct stat64 *sb)
+{
+ struct mntent *t;
+ struct stat64 ms;
+ FILE *mtabp;
+ char *mntp = NULL;
+
+ mtabp = setmntent(mtab, "r");
+ if (!mtabp) {
+ fprintf(stderr, _("%s: cannot read %s\n"),
+ progname, mtab);
+ exit(1);
+ }
+
+ while ((t = getmntent(mtabp))) {
+ if (S_ISDIR(sb->st_mode)) {
+ if (stat64(t->mnt_dir, &ms) < 0)
+ continue;
+ if (sb->st_ino != ms.st_ino)
+ continue;
+ if (sb->st_dev != ms.st_dev)
+ continue;
+ } else {
+ if (stat64(t->mnt_fsname, &ms) < 0)
+ continue;
+ if (sb->st_rdev != ms.st_rdev)
+ continue;
+ }
+
+ if (strcmp(t->mnt_type, MNTTYPE_XFS) != 0)
+ continue;
+
+ /*
+ * If we found an entry based on the device name make sure we
+ * stat the mountpoint that the mtab gave actually is accessible
+ * before using it.
+ */
+ if (S_ISBLK(sb->st_mode)) {
+ struct stat64 sb2;
+
+ if (stat64(t->mnt_dir, &sb2) < 0)
+ continue;
+ }
+
+ mntp = t->mnt_dir;
+ break;
+ }
+
+ endmntent(mtabp);
+ return mntp;
+}
+
int
main(int argc, char **argv)
{
- struct stat64 sb, sb2;
+ struct stat64 sb;
char *argname;
- char *cp;
int c;
- struct mntent mntpref;
- register struct mntent *mntp;
- struct mntent ment;
+ char *mntp;
char *mtab = NULL;
- register FILE *mtabp;
setlinebuf(stdout);
progname = basename(argv[0]);
@@ -281,49 +335,26 @@ main(int argc, char **argv)
if (optind < argc) {
for (; optind < argc; optind++) {
argname = argv[optind];
- mntp = NULL;
+
if (lstat64(argname, &sb) < 0) {
fprintf(stderr,
_("%s: could not stat: %s: %s\n"),
progname, argname, strerror(errno));
continue;
}
- if (S_ISLNK(sb.st_mode) && stat64(argname, &sb2) == 0 &&
- (S_ISBLK(sb2.st_mode) || S_ISCHR(sb2.st_mode)))
- sb = sb2;
- if (S_ISBLK(sb.st_mode) || (S_ISDIR(sb.st_mode))) {
- if ((mtabp = setmntent(mtab, "r")) == NULL) {
- fprintf(stderr,
- _("%s: cannot read %s\n"),
- progname, mtab);
- exit(1);
- }
- bzero(&mntpref, sizeof(mntpref));
- if (S_ISDIR(sb.st_mode))
- mntpref.mnt_dir = argname;
- else
- mntpref.mnt_fsname = argname;
- if (getmntany(mtabp, &ment, &mntpref, &sb) &&
- strcmp(ment.mnt_type, MNTTYPE_XFS) == 0) {
- mntp = &ment;
- if (S_ISBLK(sb.st_mode)) {
- cp = mntp->mnt_dir;
- if (cp == NULL ||
- stat64(cp, &sb2) < 0) {
- fprintf(stderr, _(
- "%s: could not stat: %s: %s\n"),
- progname, argname,
- strerror(errno));
- continue;
- }
- sb = sb2;
- argname = cp;
- }
- }
+ if (S_ISLNK(sb.st_mode)) {
+ struct stat64 sb2;
+
+ if (stat64(argname, &sb2) == 0 &&
+ (S_ISBLK(sb2.st_mode) ||
+ S_ISCHR(sb2.st_mode)))
+ sb = sb2;
}
+
+ mntp = find_mountpoint(mtab, argname, &sb);
if (mntp != NULL) {
- fsrfs(mntp->mnt_dir, 0, 100);
+ fsrfs(mntp, 0, 100);
} else if (S_ISCHR(sb.st_mode)) {
fprintf(stderr, _(
"%s: char special not supported: %s\n"),
@@ -1639,35 +1670,6 @@ fsrprintf(const char *fmt, ...)
}
/*
- * emulate getmntany
- */
-static int
-getmntany(FILE *fp, struct mntent *mp, struct mntent *mpref, struct stat64 *s)
-{
- struct mntent *t;
- struct stat64 ms;
-
- 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;
- }
- *mp = *t;
- break;
- }
- return (t != NULL);
-}
-
-
-/*
* Initialize a directory for tmp file use. This is used
* by the full filesystem defragmentation when we're walking
* the inodes and do not know the path for the individual
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
