[PATCH 0/2] libxfs: fix up mount check handling

[PATCH 0/2] libxfs: fix up mount check handling

[Eric Sandeen]

A 2 patch series to more cleanly handle the disappearance
of ustat() on some platforms, used in platform_check_ismounted()
on Linux.

-Eric

[PATCH 1/2] libxfs: move iswritable "fatal" decision to caller

[Eric Sandeen]

Simplify platform_check_iswritable by moving the
"fatal" decision up to the (one) caller.  In other words,
simply return whether mounted+writable is true, and
return 1 if so.  Caller decides what to do with that info
based on /its/ "fatal" argument.

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

diff --git a/libxfs/darwin.c b/libxfs/darwin.c
index 017e190..19d2ab6 100644
--- a/libxfs/darwin.c
+++ b/libxfs/darwin.c
@@ -33,7 +33,7 @@ platform_check_ismounted(char *name, char *block, struct stat64 *s, int verbose)
 }
 
 int
-platform_check_iswritable(char *name, char *block, struct stat64 *s, int fatal)
+platform_check_iswritable(char *name, char *block, struct stat64 *s)
 {
 	int	fd, writable;
 
diff --git a/libxfs/freebsd.c b/libxfs/freebsd.c
index 6c9f089..9e22183 100644
--- a/libxfs/freebsd.c
+++ b/libxfs/freebsd.c
@@ -66,7 +66,7 @@ platform_check_ismounted(char *name, char *block, struct stat64 *s, int verbose)
 }
 
 int
-platform_check_iswritable(char *name, char *block, struct stat64 *s, int fatal)
+platform_check_iswritable(char *name, char *block, struct stat64 *s)
 {
         int cnt, i;
         struct statfs *fsinfo;
@@ -74,7 +74,7 @@ platform_check_iswritable(char *name, char *block, struct stat64 *s, int fatal)
         if ((cnt = getmntinfo(&fsinfo, MNT_NOWAIT)) == 0) {
 		fprintf(stderr, _("%s: %s contains a possibly writable, "
 				"mounted filesystem\n"), progname, name);
-			return fatal;
+			return 1;
 	}
 
         for (i = 0; i < cnt; i++) {
@@ -88,7 +88,7 @@ platform_check_iswritable(char *name, char *block, struct stat64 *s, int fatal)
         if (i == cnt) {
 		fprintf(stderr, _("%s: %s contains a mounted and writable "
 				"filesystem\n"), progname, name);
-		return fatal;
+		return 1;
 	}
 	return 0;
 }
diff --git a/libxfs/init.c b/libxfs/init.c
index c13b123..828ae3e 100644
--- a/libxfs/init.c
+++ b/libxfs/init.c
@@ -75,7 +75,9 @@ check_isactive(char *name, char *block, int fatal)
 		return 0;
 	if (platform_check_ismounted(name, block, &st, 0) == 0)
 		return 0;
-	return platform_check_iswritable(name, block, &st, fatal);
+	if (platform_check_iswritable(name, block, &st))
+		return fatal ? 1 : 0;
+	return 0;
 }
 
 /* libxfs_device_to_fd:
diff --git a/libxfs/init.h b/libxfs/init.h
index 112febb..4dda3ee 100644
--- a/libxfs/init.h
+++ b/libxfs/init.h
@@ -22,8 +22,7 @@ struct stat64;
 
 extern int platform_check_ismounted (char *path, char *block,
 					struct stat64 *sptr, int verbose);
-extern int platform_check_iswritable (char *path, char *block,
-					struct stat64 *sptr, int fatal);
+extern int platform_check_iswritable (char *path, char *block, struct stat64 *sptr);
 extern int platform_set_blocksize (int fd, char *path, dev_t device, int bsz, int fatal);
 extern void platform_flush_device (int fd, dev_t device);
 extern char *platform_findrawpath(char *path);
diff --git a/libxfs/irix.c b/libxfs/irix.c
index 65aaa7e..c23ebe0 100644
--- a/libxfs/irix.c
+++ b/libxfs/irix.c
@@ -31,7 +31,7 @@ platform_check_ismounted(char *name, char *block, struct stat64 *s, int verbose)
 }
 
 int
-platform_check_iswritable(char *name, char *block, struct stat64 *s, int fatal)
+platform_check_iswritable(char *name, char *block, struct stat64 *s)
 {
 	return 1;
 }
diff --git a/libxfs/linux.c b/libxfs/linux.c
index c9f2baf..67b615b 100644
--- a/libxfs/linux.c
+++ b/libxfs/linux.c
@@ -74,9 +74,8 @@ platform_check_ismounted(char *name, char *block, struct stat64 *s, int verbose)
 }
 
 int
-platform_check_iswritable(char *name, char *block, struct stat64 *s, int fatal)
+platform_check_iswritable(char *name, char *block, struct stat64 *s)
 {
-	int		sts = 0;
 	FILE		*f;
 	struct stat64	mst;
 	struct mntent	*mnt;
@@ -86,7 +85,7 @@ platform_check_iswritable(char *name, char *block, struct stat64 *s, int fatal)
 	if ((f = setmntent(mounts, "r")) == NULL) {
 		fprintf(stderr, _("%s: %s contains a possibly writable, "
 				"mounted filesystem\n"), progname, name);
-			return fatal;
+			return 1;
 	}
 	while ((mnt = getmntent(f)) != NULL) {
 		if (stat64(mnt->mnt_fsname, &mst) < 0)
@@ -97,13 +96,14 @@ platform_check_iswritable(char *name, char *block, struct stat64 *s, int fatal)
 		    && hasmntopt(mnt, MNTOPT_RO) != NULL)
 			break;
 	}
+	endmntent(f);
+
 	if (mnt == NULL) {
 		fprintf(stderr, _("%s: %s contains a mounted and writable "
 				"filesystem\n"), progname, name);
-		sts = fatal;
+		return 1;
 	}
-	endmntent(f);
-	return sts;
+	return 0;
 }
 
 int

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

[PATCH 2/2] libxfs: remove ustat() use, factor mount checks into helper function

[Eric Sandeen]

ustat() has been deprecated for a very long time, and newer
architectures (aarch64, for one) have not implemented it.
If called, it always returns false, so we can no longer use
it to determine a device's mounted status.

We already have another mechanism for determining the mounted
status of a device in platform_check_iswritable; it iterates
over getmntent looking for the device, and checks its mount
options.  We can do the same thing to check for a simple mount,
and not caring about the "ro" mount option.

Because the loop is essentially the same, factor it into a
helper which accepts a VERBOSE flag to print info if the device
is found in the checked-for state, and a WRITABLE flag which
only checks specifically for a mounted and /writable/ device.

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

Dave, up to you if you want to take all this in lieu of the
fixed-up patch from Felix, or if you'd like me to rebase this
once you have merged that patch.  Happy to do that if you'd like -
now that I think about it, that may be the cleaner progression
of changes.

Thanks,
-Eric

diff --git a/libxfs/linux.c b/libxfs/linux.c
index 67b615b..fad9a4a 100644
--- a/libxfs/linux.c
+++ b/libxfs/linux.c
@@ -48,14 +48,24 @@ static int max_block_alignment;
 
 #define PROC_MOUNTED	"/proc/mounts"
 
-int
-platform_check_ismounted(char *name, char *block, struct stat64 *s, int verbose)
+/*
+ * Check if the filesystem is mounted.  Be verbose if asked, and
+ * optionally restrict check to writable mounts.
+ */
+#define	CHECK_MOUNT_VERBOSE	0x1
+#define	CHECK_MOUNT_WRITABLE	0x2
+
+static int
+platform_check_mount(char *name, char *block, struct stat64 *s, int flags)
 {
-	/* Pad ust; pre-2.6.28 linux copies out too much in 32bit compat mode */
-	struct ustat	ust[2];
 	struct stat64	st;
+	struct stat64	mst;
+	FILE		*f;
+	struct mntent	*mnt;
+	char		mounts[MAXPATHLEN];
 
 	if (!s) {
+		/* If either fails we are neither mounted nor writable */
 		if (stat64(block, &st) < 0)
 			return 0;
 		if ((st.st_mode & S_IFMT) != S_IFBLK)
@@ -63,47 +73,64 @@ platform_check_ismounted(char *name, char *block, struct stat64 *s, int verbose)
 		s = &st;
 	}
 
-	if (ustat(s->st_rdev, ust) >= 0) {
-		if (verbose)
-			fprintf(stderr,
-				_("%s: %s contains a mounted filesystem\n"),
-				progname, name);
-		return 1;
-	}
-	return 0;
-}
-
-int
-platform_check_iswritable(char *name, char *block, struct stat64 *s)
-{
-	FILE		*f;
-	struct stat64	mst;
-	struct mntent	*mnt;
-	char		mounts[MAXPATHLEN];
-
 	strcpy(mounts, (!access(PROC_MOUNTED, R_OK)) ? PROC_MOUNTED : MOUNTED);
 	if ((f = setmntent(mounts, "r")) == NULL) {
+		/* Unexpected failure, warn unconditionally */
 		fprintf(stderr, _("%s: %s contains a possibly writable, "
 				"mounted filesystem\n"), progname, name);
-			return 1;
+		return 1;
 	}
+
 	while ((mnt = getmntent(f)) != NULL) {
 		if (stat64(mnt->mnt_fsname, &mst) < 0)
 			continue;
 		if ((mst.st_mode & S_IFMT) != S_IFBLK)
 			continue;
-		if (mst.st_rdev == s->st_rdev
-		    && hasmntopt(mnt, MNTOPT_RO) != NULL)
-			break;
+		if (mst.st_rdev == s->st_rdev) {
+			/* Found our device */
+			if (!(flags & CHECK_MOUNT_WRITABLE) ||
+			    !hasmntopt(mnt, MNTOPT_RO))
+				break;
+		}
 	}
 	endmntent(f);
 
-	if (mnt == NULL) {
-		fprintf(stderr, _("%s: %s contains a mounted and writable "
-				"filesystem\n"), progname, name);
-		return 1;
+	/* No mounts contained the condition we were looking for */
+	if (mnt == NULL)
+		return 0;
+
+	if (flags & CHECK_MOUNT_VERBOSE) {
+		if (flags & CHECK_MOUNT_WRITABLE) {
+			fprintf(stderr,
+_("%s: %s contains a mounted and writable filesystem\n"),
+				progname, name);
+		} else {
+			fprintf(stderr,
+_("%s: %s contains a mounted filesystem\n"),
+				progname, name);
+		}
 	}
-	return 0;
+
+	return 1;
+}
+
+int
+platform_check_ismounted(char *name, char *block, struct stat64 *s, int verbose)
+{
+	int flags;
+
+	flags = verbose ? CHECK_MOUNT_VERBOSE : 0;
+	return platform_check_mount(name, block, s, flags);
+}
+
+int
+platform_check_iswritable(char *name, char *block, struct stat64 *s)
+{
+	int flags;
+
+	/* Writable checks are always verbose */
+	flags = CHECK_MOUNT_WRITABLE | CHECK_MOUNT_VERBOSE;
+	return platform_check_mount(name, block, s, flags);
 }
 
 int

Re: [PATCH 2/2] libxfs: remove ustat() use, factor mount checks into helper function

[Dave Chinner]

On Thu, Sep 08, 2016 at 09:16:11AM -0500, Eric Sandeen wrote:
> ustat() has been deprecated for a very long time, and newer
> architectures (aarch64, for one) have not implemented it.
> If called, it always returns false, so we can no longer use
> it to determine a device's mounted status.
> 
> We already have another mechanism for determining the mounted
> status of a device in platform_check_iswritable; it iterates
> over getmntent looking for the device, and checks its mount
> options.  We can do the same thing to check for a simple mount,
> and not caring about the "ro" mount option.
> 
> Because the loop is essentially the same, factor it into a
> helper which accepts a VERBOSE flag to print info if the device
> is found in the checked-for state, and a WRITABLE flag which
> only checks specifically for a mounted and /writable/ device.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> Dave, up to you if you want to take all this in lieu of the
> fixed-up patch from Felix, or if you'd like me to rebase this
> once you have merged that patch.  Happy to do that if you'd like -
> now that I think about it, that may be the cleaner progression
> of changes.

Rebase, please, as it does not apply to the current tree.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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