From mboxrd@z Thu Jan 1 00:00:00 1970 From: Steven Whitehouse Date: Tue, 05 Nov 2013 11:46:00 +0000 Subject: [Cluster-devel] [PATCH] libgfs2: Return more context from is_pathname_mounted and rename it In-Reply-To: <5277BF5C.3040300@redhat.com> References: <1381616075-12740-1-git-send-email-anprice@redhat.com> <5277BF5C.3040300@redhat.com> Message-ID: <1383651960.2713.20.camel@menhir> List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 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 > > --- > > 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 > > #include > > #include > > +#include > > > > #include > > #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); > > } > > >