[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,

Since the path has already been transformed into an fd, 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,

Steve.

On Mon, 2013-11-04 at 15:38 +0000, Andrew Price wrote:
> This one fell off my radar somehow. Reviews appreciated.
> 
> Andy
> 
> On 12/10/13 23:14, Andrew Price wrote:
> > is_pathname_mounted is renamed lgfs2_find_mnt and now returns more
> > context through the isdev and mnt parameters. It now only takes one path
> > and returns whether it is the name of the device or the mount point
> > using *isdev. It also sets *mnt to the struct mntent found by
> > getmntent() so that the caller can check its contents itself.
> >
> > Callers have been updated and a bug where gfs2_grow wasn't working when
> > the device name was provided by the user has been fixed.
> >
> > Signed-off-by: Andrew Price <anprice redhat com>
> > ---
> >   gfs2/fsck/initialize.c |  9 ++++-----
> >   gfs2/libgfs2/libgfs2.h |  3 ++-
> >   gfs2/libgfs2/misc.c    | 55 ++++++++++++++++++++++++++++++--------------------
> >   gfs2/mkfs/main_grow.c  | 24 ++++++++++++++++------
> >   gfs2/mkfs/main_jadd.c  | 23 ++++++++++++++++-----
> >   5 files changed, 75 insertions(+), 39 deletions(-)
> >
> > diff --git a/gfs2/fsck/initialize.c b/gfs2/fsck/initialize.c
> > index 6612fe7..1cc15ec 100644
> > --- a/gfs2/fsck/initialize.c
> > +++ b/gfs2/fsck/initialize.c
> > @@ -1475,7 +1475,8 @@ int initialize(struct gfs2_sbd *sdp, int force_check, int preen,
> >
> >   	sdp->device_fd = open(opts.device, open_flag);
> >   	if (sdp->device_fd < 0) {
> > -		int is_mounted, ro;
> > +		int isdev;
> > +		struct mntent *mnt;
> >
> >   		if (open_flag == O_RDONLY || errno != EBUSY) {
> >   			log_crit( _("Unable to open device: %s\n"),
> > @@ -1490,18 +1491,16 @@ int initialize(struct gfs2_sbd *sdp, int force_check, int preen,
> >   		   function checks for device as well. */
> >   		strncpy(sdp->device_name, opts.device,
> >   			sizeof(sdp->device_name));
> > -		sdp->path_name = sdp->device_name; /* This gets overwritten */
> > -		is_mounted = is_pathname_mounted(sdp->path_name, sdp->device_name, &ro);
> >   		/* If the device is busy, but not because it's mounted, fail.
> >   		   This protects against cases where the file system is LVM
> >   		   and perhaps mounted on a different node. */
> > -		if (!is_mounted)
> > +		if (lgfs2_find_mnt(opts.device, &isdev, &mnt) || mnt == NULL)
> >   			goto mount_fail;
> >   		/* If the device is mounted, but not mounted RO, fail.  This
> >   		   protects them against cases where the file system is
> >   		   mounted RW, but still allows us to check our own root
> >   		   file system. */
> > -		if (!ro)
> > +		if (!hasmntopt(mnt, MNTOPT_RO))
> >   			goto mount_fail;
> >   		/* The device is mounted RO, so it's likely our own root
> >   		   file system.  We can only do so much to protect the users
> > diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h
> > index f864a08..993f605 100644
> > --- a/gfs2/libgfs2/libgfs2.h
> > +++ b/gfs2/libgfs2/libgfs2.h
> > @@ -12,6 +12,7 @@
> >   #include <linux/limits.h>
> >   #include <endian.h>
> >   #include <byteswap.h>
> > +#include <mntent.h>
> >
> >   #include <linux/gfs2_ondisk.h>
> >   #include "osi_list.h"
> > @@ -714,7 +715,7 @@ extern int metafs_interrupted;
> >   extern int compute_heightsize(struct gfs2_sbd *sdp, uint64_t *heightsize,
> >   		uint32_t *maxheight, uint32_t bsize1, int diptrs, int inptrs);
> >   extern int compute_constants(struct gfs2_sbd *sdp);
> > -extern int is_pathname_mounted(char *path_name, char *device_name, int *ro_mount);
> > +extern int lgfs2_find_mnt(const char *path, int *isdev, struct mntent **mnt);
> >   extern int find_gfs2_meta(struct gfs2_sbd *sdp);
> >   extern int dir_exists(const char *dir);
> >   extern int mount_gfs2_meta(struct gfs2_sbd *sdp);
> > diff --git a/gfs2/libgfs2/misc.c b/gfs2/libgfs2/misc.c
> > index 7f500e6..6bc95a9 100644
> > --- a/gfs2/libgfs2/misc.c
> > +++ b/gfs2/libgfs2/misc.c
> > @@ -102,20 +102,27 @@ int compute_constants(struct gfs2_sbd *sdp)
> >   	return 0;
> >   }
> >
> > -int is_pathname_mounted(char *path_name, char *device_name, int *ro_mount)
> > +/**
> > + * lgfs2_find_mnt - Find a mounted file system relating to a path.
> > + * path:  The path
> > + * isdev: Will be non-zero if the path is the device or zero if the path is the mount directory
> > + * mnt:   When the return value is 0, this will either be NULL for "not found" or
> > + *        it will point to the found mount point.
> > + * Returns 0 to indicate a successful scan of the mount points or non-zero on error with errno set.
> > + *         On success, *mnt will be NULL or a valid pointer as noted above.
> > + */
> > +int lgfs2_find_mnt(const char *path, int *isdev, struct mntent **mnt)
> >   {
> >   	FILE *fp;
> > -	struct mntent *mnt;
> >   	dev_t file_dev=0, file_rdev=0;
> >   	ino_t file_ino=0;
> >   	struct stat st_buf;
> >
> > -	*ro_mount = 0;
> >   	if ((fp = setmntent("/proc/mounts", "r")) == NULL) {
> >   		perror("open: /proc/mounts");
> > -		return 0;
> > +		return 1;
> >   	}
> > -	if (stat(path_name, &st_buf) == 0) {
> > +	if (stat(path, &st_buf) == 0) {
> >   		if (S_ISBLK(st_buf.st_mode)) {
> >   #ifndef __GNU__ /* The GNU hurd is broken with respect to stat devices */
> >   			file_rdev = st_buf.st_rdev;
> > @@ -125,42 +132,46 @@ int is_pathname_mounted(char *path_name, char *device_name, int *ro_mount)
> >   			file_ino = st_buf.st_ino;
> >   		}
> >   	}
> > -	while ((mnt = getmntent (fp)) != NULL) {
> > +	while ((*mnt = getmntent(fp)) != NULL) {
> >   		/* Check if they specified the device instead of mnt point */
> > -		if (strcmp(device_name, mnt->mnt_fsname) == 0) {
> > -			strcpy(path_name, mnt->mnt_dir); /* fix it */
> > +		if (strcmp(path, (*mnt)->mnt_fsname) == 0) {
> > +			*isdev = 1;
> >   			break;
> >   		}
> > -		if (strcmp(path_name, mnt->mnt_dir) == 0) {
> > -			strcpy(device_name, mnt->mnt_fsname); /* fix it */
> > +		if (strcmp(path, (*mnt)->mnt_dir) == 0) {
> > +			*isdev = 0;
> >   			break;
> >   		}
> > -		if (stat(mnt->mnt_fsname, &st_buf) == 0) {
> > +		if (stat((*mnt)->mnt_fsname, &st_buf) == 0) {
> >   			if (S_ISBLK(st_buf.st_mode)) {
> >   #ifndef __GNU__
> > -				if (file_rdev && (file_rdev == st_buf.st_rdev))
> > +				if (file_rdev && (file_rdev == st_buf.st_rdev)) {
> > +					*isdev = 1;
> >   					break;
> > +				}
> >   #endif  /* __GNU__ */
> >   			} else {
> >   				if (file_dev && ((file_dev == st_buf.st_dev) &&
> > -						 (file_ino == st_buf.st_ino)))
> > +						 (file_ino == st_buf.st_ino))) {
> > +					*isdev = 0;
> >   					break;
> > +				}
> >   			}
> >   		}
> >   	}
> >   	endmntent (fp);
> > -	if (mnt == NULL)
> > -		return 0;
> > -	if (stat(mnt->mnt_dir, &st_buf) < 0) {
> > +	if (*mnt == NULL)
> > +		return 0; /* Success. Answer is no. */
> > +	if (stat((*mnt)->mnt_dir, &st_buf) < 0) {
> >   		if (errno == ENOENT)
> > -			return 0;
> > +			return 1;
> >   	}
> >   	/* Can't trust fstype because / has "rootfs". */
> > -	if (file_rdev && (st_buf.st_dev != file_rdev))
> > -		return 0;
> > -	if (hasmntopt(mnt, MNTOPT_RO))
> > -               *ro_mount = 1;
> > -	return 1; /* mounted */
> > +	if (file_rdev && (st_buf.st_dev != file_rdev)) {
> > +		errno = EINVAL;
> > +		return 1;
> > +	}
> > +	return 0; /* Success. Answer is yes. */
> >   }
> >
> >   static int lock_for_admin(struct gfs2_sbd *sdp)
> > diff --git a/gfs2/mkfs/main_grow.c b/gfs2/mkfs/main_grow.c
> > index 5db91d9..4c8807e 100644
> > --- a/gfs2/mkfs/main_grow.c
> > +++ b/gfs2/mkfs/main_grow.c
> > @@ -323,7 +323,6 @@ main_grow(int argc, char *argv[])
> >   	int rgcount, rindex_fd;
> >   	char rindex_name[PATH_MAX];
> >   	int error = EXIT_SUCCESS;
> > -	int ro_mnt = 0;
> >
> >   	memset(sdp, 0, sizeof(struct gfs2_sbd));
> >   	sdp->bsize = GFS2_DEFAULT_BSIZE;
> > @@ -335,16 +334,29 @@ main_grow(int argc, char *argv[])
> >   	
> >   	while ((argc - optind) > 0) {
> >   		int sane;
> > +		int isdev = 0;
> > +		struct mntent *mnt;
> >   		struct rgrp_tree *last_rgrp;
> >
> > -		strncpy(sdp->device_name, argv[optind], PATH_MAX - 1);
> > -		sdp->path_name = argv[optind++];
> > -
> > -		if ((!is_pathname_mounted(sdp->path_name, sdp->device_name, &ro_mnt))) {
> > -			perror(sdp->path_name);
> > +		if (lgfs2_find_mnt(argv[optind], &isdev, &mnt)) {
> > +			perror(argv[optind]);
> >   			exit(EXIT_FAILURE);
> >   		}
> >
> > +		if (mnt == NULL) {
> > +			fprintf(stderr, "%s: not a mounted gfs2 file system\n", argv[optind++]);
> > +			continue;
> > +		}
> > +
> > +		if (isdev) {
> > +			strncpy(sdp->device_name, argv[optind], PATH_MAX - 1);
> > +			sdp->path_name = mnt->mnt_dir;
> > +		} else {
> > +			strncpy(sdp->device_name, mnt->mnt_fsname, PATH_MAX - 1);
> > +			sdp->path_name = argv[optind];
> > +		}
> > +		optind++;
> > +
> >   		sdp->path_fd = open(sdp->path_name, O_RDONLY | O_CLOEXEC);
> >   		if (sdp->path_fd < 0){
> >   			perror(sdp->path_name);
> > diff --git a/gfs2/mkfs/main_jadd.c b/gfs2/mkfs/main_jadd.c
> > index b6cd8e4..9240838 100644
> > --- a/gfs2/mkfs/main_jadd.c
> > +++ b/gfs2/mkfs/main_jadd.c
> > @@ -490,8 +490,9 @@ add_j(struct gfs2_sbd *sdp)
> >   void main_jadd(int argc, char *argv[])
> >   {
> >   	struct gfs2_sbd sbd, *sdp = &sbd;
> > +	struct mntent *mnt;
> >   	unsigned int total;
> > -	int ro_mnt = 0;
> > +	int isdev;
> >
> >   	memset(sdp, 0, sizeof(struct gfs2_sbd));
> >   	sdp->jsize = GFS2_DEFAULT_JSIZE;
> > @@ -500,14 +501,26 @@ void main_jadd(int argc, char *argv[])
> >
> >   	decode_arguments(argc, argv, sdp);
> >   	verify_arguments(sdp);
> > -	
> > -	sdp->path_fd = open(sdp->path_name, O_RDONLY | O_CLOEXEC);
> > -	if (sdp->path_fd < 0){
> > +
> > +	if (lgfs2_find_mnt(sdp->path_name, &isdev, &mnt)) {
> >   		perror(sdp->path_name);
> >   		exit(EXIT_FAILURE);
> >   	}
> >
> > -	if (!is_pathname_mounted(sdp->path_name, sdp->device_name, &ro_mnt)) {
> > +	if (mnt == NULL) {
> > +		fprintf(stderr, "%s: not a mounted gfs2 file system\n", sdp->path_name);
> > +		exit(EXIT_FAILURE);
> > +	}
> > +
> > +	if (isdev) {
> > +		strncpy(sdp->device_name, sdp->path_name, PATH_MAX - 1);
> > +		sdp->path_name = mnt->mnt_dir;
> > +	} else {
> > +		strncpy(sdp->device_name, mnt->mnt_fsname, PATH_MAX - 1);
> > +	}
> > +
> > +	sdp->path_fd = open(sdp->path_name, O_RDONLY | O_CLOEXEC);
> > +	if (sdp->path_fd < 0){
> >   		perror(sdp->path_name);
> >   		exit(EXIT_FAILURE);
> >   	}
> >
> 



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