On Fri, Jul 11, 2014 at 08:34:24PM -0500, Eric Sandeen wrote:
> Both mountpoints and devices can be symlinks, so given a path
> to look for, and mountpoints/devices from the system, use
> realpath() on *everything* before making the comparison to see
> if our path is a match.
>
> So, with symlinks for mount points as well as for devices:
>
> # ls -l /dev/mapper/testvg-lvol0
> lrwxrwxrwx. 1 root root 7 Jul 11 19:24 /dev/mapper/testvg-lvol0 -> ../dm-3
> # ls -l /mnt/scratch2
> lrwxrwxrwx. 1 root root 12 Jul 11 19:57 /mnt/scratch2 -> /mnt/scratch
>
> this should all work, and does now:
>
> # xfs_quota -xc "report -h" /mnt/scratch2
> User quota on /mnt/scratch (/dev/mapper/testvg-lvol0)
> Blocks
> User ID Used Soft Hard Warn/Grace
> ---------- ---------------------------------
> root 0 0 0 00 [------]
>
> # xfs_quota -xc "report -h" /mnt/scratch
> User quota on /mnt/scratch (/dev/mapper/testvg-lvol0)
> Blocks
> User ID Used Soft Hard Warn/Grace
> ---------- ---------------------------------
> root 0 0 0 00 [------]
>
> # xfs_quota -xc "report -h" /dev/dm-3
> User quota on /mnt/scratch (/dev/mapper/testvg-lvol0)
> Blocks
> User ID Used Soft Hard Warn/Grace
> ---------- ---------------------------------
> root 0 0 0 00 [------]
>
> # xfs_quota -xc "report -h" /dev/mapper/testvg-lvol0
> User quota on /mnt/scratch (/dev/mapper/testvg-lvol0)
> Blocks
> User ID Used Soft Hard Warn/Grace
> ---------- ---------------------------------
> root 0 0 0 00 [------]
>
> The commit:
>
> 050a7f1 xfsprogs: handle symlinks etc in fs_table_initialise_mounts()
>
> tried to fix this earlier, but only worked one way;
> it compared the argument path in both given and realpath
> form to the paths in getmntent, but did not compare to
> the realpaths of the getmntent devices.
>
> If we reduce everything, everywhere, to a realpath(), we've
> got our best shot at finding the match.
>
> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>
> ---
>
> V2: remove the quota/report.c change which snuck in...
>
> diff --git a/libxcmd/paths.c b/libxcmd/paths.c
> index 7b0e434..8da3968 100644
> --- a/libxcmd/paths.c
> +++ b/libxcmd/paths.c
> @@ -269,6 +269,9 @@ out_nomem:
> /*
> * If *path is NULL, initialize the fs table with all xfs mount points in mtab
> * If *path is specified, search for that path in mtab
> + *
> + * Everything - path, devices, and mountpoints - are reduced to realpath()
> + * for comparison, but fs_table is populated with what comes from getmntent.
> */
> static int
> fs_table_initialise_mounts(
> @@ -278,7 +281,7 @@ fs_table_initialise_mounts(
> FILE *mtp;
> char *fslog, *fsrt;
> int error, found;
> - char *rpath = NULL;
> + char *rpath = NULL, *rmnt_fsname = NULL, *rmnt_dir = NULL;
>
> error = found = 0;
> fslog = fsrt = NULL;
> @@ -300,11 +303,16 @@ fs_table_initialise_mounts(
> while ((mnt = getmntent(mtp)) != NULL) {
> if (strcmp(mnt->mnt_type, "xfs") != 0)
> continue;
> + free(rmnt_dir);
> + if ((rmnt_dir = realpath(mnt->mnt_dir, NULL)) == NULL)
> + continue;
> + free(rmnt_fsname);
> + if ((rmnt_fsname = realpath(mnt->mnt_fsname, NULL)) == NULL)
> + continue;
> +
> if (path &&
> - ((strcmp(path, mnt->mnt_dir) != 0) &&
> - (strcmp(path, mnt->mnt_fsname) != 0) &&
> - (strcmp(rpath, mnt->mnt_dir) != 0) &&
> - (strcmp(rpath, mnt->mnt_fsname) != 0)))
> + ((strcmp(rpath, rmnt_dir) != 0) &&
> + (strcmp(rpath, rmnt_fsname) != 0)))
> continue;
> if (fs_extract_mount_options(mnt, &fslog, &fsrt))
> continue;
> @@ -317,6 +325,8 @@ fs_table_initialise_mounts(
> }
> endmntent(mtp);
> free(rpath);
> + free(rmnt_dir);
> + free(rmnt_fsname);
>
> if (path && !found)
> error = ENXIO;
> @@ -330,6 +340,9 @@ fs_table_initialise_mounts(
> /*
> * If *path is NULL, initialize the fs table with all xfs mount points in mtab
> * If *path is specified, search for that path in mtab
> + *
> + * Everything - path, devices, and mountpoints - are reduced to realpath()
> + * for comparison, but fs_table is populated with what comes from getmntinfo.
> */
> static int
> fs_table_initialise_mounts(
> @@ -337,7 +350,7 @@ fs_table_initialise_mounts(
> {
> struct statfs *stats;
> int i, count, error, found;
> - char *rpath = NULL;
> + char *rpath = NULL, *rmntfromname= NULL, *rmntonname= NULL;
A couple missing spaces before '=' here.
The fundamental change looks good, but the memory allocation handling
seems a little ugly to me. A 'next:' label in the loop that frees the
path buffers is cleaner IMO. Another option could be to put a couple
PATH_MAX buffers on the stack or allocate them directly to also
eliminate the realpath() return value assignment..?
Brian
>
> error = found = 0;
> if ((count = getmntinfo(&stats, 0)) < 0) {
> @@ -354,11 +367,16 @@ fs_table_initialise_mounts(
> for (i = 0; i < count; i++) {
> if (strcmp(stats[i].f_fstypename, "xfs") != 0)
> continue;
> + free(rmntfromname);
> + if ((rmntfromname = realpath(stats[i].f_mntfromname, NULL)) == NULL)
> + continue;
> + free(rmntonname);
> + if ((rmntfromname = realpath(stats[i].f_mntonname, NULL)) == NULL)
> + continue;
> +
> if (path &&
> - ((strcmp(path, stats[i].f_mntonname) != 0) &&
> - (strcmp(path, stats[i].f_mntfromname) != 0) &&
> - (strcmp(rpath, stats[i].f_mntonname) != 0) &&
> - (strcmp(rpath, stats[i].f_mntfromname) != 0)))
> + ((strcmp(rpath, rmntonname) != 0) &&
> + (strcmp(rpath, rmntfromname) != 0)))
> continue;
> /* TODO: external log and realtime device? */
> (void) fs_table_insert(stats[i].f_mntonname, 0,
> @@ -370,6 +388,8 @@ fs_table_initialise_mounts(
> }
> }
> free(rpath);
> + free(rmntfromname);
> + free(rmntonname);
> if (path && !found)
> error = ENXIO;
>
>
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
