[PATCH 0/4] xfs: online relabeling [RFC]

[PATCH 0/4] xfs: online relabeling [RFC]

[Eric Sandeen]

Ok, this more or less works; not really up to snuff
for submission or merging, just sketching it out, but some
questions first:

1) Is there really any point to this? :) We did have one
   request, and btrfs can do it ...

2) Is using m_growlock horrible?  growfs is the only other
   thing that writes all supers, so I grabbed it.  We don't
   want multiple relabels stepping on each other.

3) Is there some way to actually force the primary to disk?
   Right now the label change isn't actually visible on the
   primary until unmount, which defeats the purpose.  I'm not
   sure if there's a straightforward/safe way to make it
   visible...

Thanks,
-Eric

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

[PATCH 1/4] fs: hoist [GS]ET_FSLABEL to vfs

[Eric Sandeen]

Move the btrfs label ioctls up to the vfs for general use.

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

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 053e677..b23be4e 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -5431,6 +5431,10 @@ long btrfs_ioctl(struct file *file, unsigned int
 		return btrfs_ioctl_setflags(file, argp);
 	case FS_IOC_GETVERSION:
 		return btrfs_ioctl_getversion(file, argp);
+	case FS_IOC_GET_FSLABEL:
+		return btrfs_ioctl_get_fslabel(file, argp);
+	case FS_IOC_SET_FSLABEL:
+		return btrfs_ioctl_set_fslabel(file, argp);
 	case FITRIM:
 		return btrfs_ioctl_fitrim(file, argp);
 	case BTRFS_IOC_SNAP_CREATE:
@@ -5538,10 +5542,6 @@ long btrfs_ioctl(struct file *file, unsigned int
 		return btrfs_ioctl_quota_rescan_wait(file, argp);
 	case BTRFS_IOC_DEV_REPLACE:
 		return btrfs_ioctl_dev_replace(root, argp);
-	case BTRFS_IOC_GET_FSLABEL:
-		return btrfs_ioctl_get_fslabel(file, argp);
-	case BTRFS_IOC_SET_FSLABEL:
-		return btrfs_ioctl_set_fslabel(file, argp);
 	case BTRFS_IOC_GET_SUPPORTED_FEATURES:
 		return btrfs_ioctl_get_supported_features(argp);
 	case BTRFS_IOC_GET_FEATURES:
diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
index a079d50..fca64f2 100644
--- a/include/uapi/linux/fs.h
+++ b/include/uapi/linux/fs.h
@@ -234,6 +234,8 @@ struct fsxattr {
 #define FICLONERANGE	_IOW(0x94, 13, struct file_clone_range)
 #define FIDEDUPERANGE	_IOWR(0x94, 54, struct file_dedupe_range)
 
+#define FSLABEL_MAX 256	/* Max chars for the interface; each fs may differ */
+
 #define	FS_IOC_GETFLAGS			_IOR('f', 1, long)
 #define	FS_IOC_SETFLAGS			_IOW('f', 2, long)
 #define	FS_IOC_GETVERSION		_IOR('v', 1, long)
@@ -243,8 +245,10 @@ struct fsxattr {
 #define FS_IOC32_SETFLAGS		_IOW('f', 2, int)
 #define FS_IOC32_GETVERSION		_IOR('v', 1, int)
 #define FS_IOC32_SETVERSION		_IOW('v', 2, int)
-#define FS_IOC_FSGETXATTR		_IOR ('X', 31, struct fsxattr)
-#define FS_IOC_FSSETXATTR		_IOW ('X', 32, struct fsxattr)
+#define FS_IOC_FSGETXATTR		_IOR('X', 31, struct fsxattr)
+#define FS_IOC_FSSETXATTR		_IOW('X', 32, struct fsxattr)
+#define FS_IOC_GET_FSLABEL		_IOR(0x94, 49, char[FSLABEL_MAX])
+#define FS_IOC_SET_FSLABEL		_IOW(0x94, 50, char[FSLABEL_MAX])
 
 /*
  * File system encryption support

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

[PATCH 2/4] xfs: factor out secondary superblock updates

[Eric Sandeen]

growfs rewrites all secondary supers, and relabel must
do so as well; factor out the code which does this for
use by both operations.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 fs/xfs/xfs_fsops.c |  131 +++++++++++++++++++++++++++++++---------------------
 fs/xfs/xfs_fsops.h |    2 +
 2 files changed, 80 insertions(+), 53 deletions(-)

diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
index b4d7582..b49b4e4 100644
--- a/fs/xfs/xfs_fsops.c
+++ b/fs/xfs/xfs_fsops.c
@@ -140,6 +140,79 @@ xfs_growfs_get_hdr_buf(
 	return bp;
 }
 
+/*
+ * Copy the contents of the primary super to all backup supers.
+ * %agcount is current number of ags in the filesystem.
+ * %nagcount is used during growfs when we may have new secondaries.
+ * If %nagcount is 0 (no growfs), we use %agcount in its place.
+ */
+int
+xfs_update_secondary_supers(
+	xfs_mount_t		*mp,
+	xfs_agnumber_t		agcount,
+	xfs_agnumber_t		nagcount)
+{
+	int			error, saved_error;
+	xfs_agnumber_t		agno;
+	xfs_buf_t		*bp;
+
+	error = saved_error = 0;
+
+	if (nagcount == 0)
+		nagcount = agcount;
+
+	for (agno = 1; agno < nagcount; agno++) {
+		error = 0;
+		if (agno < agcount) {
+			error = xfs_trans_read_buf(mp, NULL, mp->m_ddev_targp,
+				  XFS_AGB_TO_DADDR(mp, agno, XFS_SB_BLOCK(mp)),
+				  XFS_FSS_TO_BB(mp, 1), 0, &bp,
+				  &xfs_sb_buf_ops);
+		} else {
+			/*
+			 * new secondary superblocks need to be zeroed, not read
+			 * from disk as the contents of the new area we are
+			 * growing into is completely unknown.
+			 */
+			bp = xfs_trans_get_buf(NULL, mp->m_ddev_targp,
+				  XFS_AGB_TO_DADDR(mp, agno, XFS_SB_BLOCK(mp)),
+				  XFS_FSS_TO_BB(mp, 1), 0);
+			if (bp) {
+				bp->b_ops = &xfs_sb_buf_ops;
+				xfs_buf_zero(bp, 0, BBTOB(bp->b_length));
+			} else
+				error = -ENOMEM;
+		}
+
+		/*
+		 * If we get an error reading or writing alternate superblocks,
+		 * continue.  xfs_repair chooses the "best" superblock based
+		 * on most matches; if we break early, we'll leave more
+		 * superblocks un-updated than updated, and xfs_repair may
+		 * pick them over the properly-updated primary.
+		 */
+		if (error) {
+			xfs_warn(mp,
+		"error %d reading secondary superblock for ag %d",
+				error, agno);
+			saved_error = error;
+			continue;
+		}
+		xfs_sb_to_disk(XFS_BUF_TO_SBP(bp), &mp->m_sb);
+
+		error = xfs_bwrite(bp);
+		xfs_buf_relse(bp);
+		if (error) {
+			xfs_warn(mp,
+		"write error %d updating secondary superblock for ag %d",
+				error, agno);
+			saved_error = error;
+			continue;
+		}
+	}
+	return saved_error ? saved_error : error;
+}
+
 static int
 xfs_growfs_data_private(
 	xfs_mount_t		*mp,		/* mount point for filesystem */
@@ -155,7 +228,7 @@ xfs_growfs_data_private(
 	xfs_buf_t		*bp;
 	int			bucket;
 	int			dpct;
-	int			error, saved_error = 0;
+	int			error;
 	xfs_agnumber_t		nagcount;
 	xfs_agnumber_t		nagimax = 0;
 	xfs_rfsblock_t		nb, nb_mod;
@@ -502,59 +575,11 @@ xfs_growfs_data_private(
 		mp->m_maxicount = 0;
 	xfs_set_low_space_thresholds(mp);
 
-	/* update secondary superblocks. */
-	for (agno = 1; agno < nagcount; agno++) {
-		error = 0;
-		/*
-		 * new secondary superblocks need to be zeroed, not read from
-		 * disk as the contents of the new area we are growing into is
-		 * completely unknown.
-		 */
-		if (agno < oagcount) {
-			error = xfs_trans_read_buf(mp, NULL, mp->m_ddev_targp,
-				  XFS_AGB_TO_DADDR(mp, agno, XFS_SB_BLOCK(mp)),
-				  XFS_FSS_TO_BB(mp, 1), 0, &bp,
-				  &xfs_sb_buf_ops);
-		} else {
-			bp = xfs_trans_get_buf(NULL, mp->m_ddev_targp,
-				  XFS_AGB_TO_DADDR(mp, agno, XFS_SB_BLOCK(mp)),
-				  XFS_FSS_TO_BB(mp, 1), 0);
-			if (bp) {
-				bp->b_ops = &xfs_sb_buf_ops;
-				xfs_buf_zero(bp, 0, BBTOB(bp->b_length));
-			} else
-				error = -ENOMEM;
-		}
-
-		/*
-		 * If we get an error reading or writing alternate superblocks,
-		 * continue.  xfs_repair chooses the "best" superblock based
-		 * on most matches; if we break early, we'll leave more
-		 * superblocks un-updated than updated, and xfs_repair may
-		 * pick them over the properly-updated primary.
-		 */
-		if (error) {
-			xfs_warn(mp,
-		"error %d reading secondary superblock for ag %d",
-				error, agno);
-			saved_error = error;
-			continue;
-		}
-		xfs_sb_to_disk(XFS_BUF_TO_SBP(bp), &mp->m_sb);
-
-		error = xfs_bwrite(bp);
-		xfs_buf_relse(bp);
-		if (error) {
-			xfs_warn(mp,
-		"write error %d updating secondary superblock for ag %d",
-				error, agno);
-			saved_error = error;
-			continue;
-		}
-	}
-	return saved_error ? saved_error : error;
+	/* Copy new geometry to all backup superblocks */
+	error = xfs_update_secondary_supers(mp, oagcount, nagcount);
+	return error;
 
- error0:
+error0:
 	xfs_trans_cancel(tp);
 	return error;
 }
diff --git a/fs/xfs/xfs_fsops.h b/fs/xfs/xfs_fsops.h
index f32713f..82e8dc7 100644
--- a/fs/xfs/xfs_fsops.h
+++ b/fs/xfs/xfs_fsops.h
@@ -19,6 +19,8 @@
 #define	__XFS_FSOPS_H__
 
 extern int xfs_fs_geometry(xfs_mount_t *mp, xfs_fsop_geom_t *geo, int nversion);
+extern int xfs_update_secondary_supers(xfs_mount_t *mp, xfs_agnumber_t aogcount,
+					xfs_agnumber_t nagcount);
 extern int xfs_growfs_data(xfs_mount_t *mp, xfs_growfs_data_t *in);
 extern int xfs_growfs_log(xfs_mount_t *mp, xfs_growfs_log_t *in);
 extern int xfs_fs_counts(xfs_mount_t *mp, xfs_fsop_counts_t *cnt);
-- 
1.7.1


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

[PATCH 3/4] xfs: implement online get/set fs label

[Eric Sandeen]

Wire up label ioctls for XFS.

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

This is where the implementation questions come in;
is using growlock an abomination?  How can I make the
primary super change immediately visible?

 fs/xfs/xfs_ioctl.c |   62 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 62 insertions(+), 0 deletions(-)

diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index dbca737..ab59213 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -41,6 +41,8 @@
 #include "xfs_trans.h"
 #include "xfs_pnfs.h"
 #include "xfs_acl.h"
+#include "xfs_log.h"
+#include "xfs_sb.h"
 
 #include <linux/capability.h>
 #include <linux/dcache.h>
@@ -1603,6 +1605,62 @@ xfs_ioc_swapext(
 	return error;
 }
 
+static int
+xfs_ioc_getlabel(
+	struct xfs_mount	*mp,
+	char			__user *label)
+{
+	int			error = 0;
+	struct xfs_sb		*sbp = &mp->m_sb;
+
+	if (!mutex_trylock(&mp->m_growlock))
+		return -EWOULDBLOCK;
+	if (copy_to_user(label, sbp->sb_fname, sizeof(sbp->sb_fname)))
+		error = -EFAULT;
+	mutex_unlock(&mp->m_growlock);
+	return error;
+}
+
+static int
+xfs_ioc_setlabel(
+	struct file		*filp,
+	struct xfs_mount	*mp,
+	char			__user *newlabel)
+{
+	int			error;
+	struct xfs_sb		*sbp = &mp->m_sb;
+	char			sb_fname[12];
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
+	if (copy_from_user(sb_fname, newlabel, sizeof(sb_fname)))
+		return -EFAULT;
+
+	error = mnt_want_write_file(filp);
+	if (error)
+		return error;
+
+	/* growfs & label both muck w/ the super directly... */
+	if (!mutex_trylock(&mp->m_growlock))
+		return -EWOULDBLOCK;
+	memset(sbp->sb_fname, 0, sizeof(sbp->sb_fname));
+	strncpy(sbp->sb_fname, sb_fname, sizeof(sbp->sb_fname));
+
+	error = xfs_sync_sb(mp, true);
+	if (error)
+		goto out;
+	/*
+	 * Most kernelspace superblock updates only update sb 0.
+	 * Userspace relabel has always updated all, though, so:
+	 */
+	error = xfs_update_secondary_supers(mp, sbp->sb_agcount, 0);
+out:
+	mutex_unlock(&mp->m_growlock);
+	mnt_drop_write_file(filp);
+	return error;
+}
+
 /*
  * Note: some of the ioctl's return positive numbers as a
  * byte count indicating success, such as readlink_by_handle.
@@ -1630,6 +1688,10 @@ xfs_file_ioctl(
 	switch (cmd) {
 	case FITRIM:
 		return xfs_ioc_trim(mp, arg);
+	case FS_IOC_GET_FSLABEL:
+		return xfs_ioc_getlabel(mp, arg);
+	case FS_IOC_SET_FSLABEL:
+		return xfs_ioc_setlabel(filp, mp, arg);
 	case XFS_IOC_ALLOCSP:
 	case XFS_IOC_FREESP:
 	case XFS_IOC_RESVSP:
-- 
1.7.1

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

[PATCH 4/4] xfsprogs: add online relabel commands

[Eric Sandeen]

This adds a get/set label command to xfs_io, and updates
the xfs_admin script to use it rather than xfs_db if
the filesystem is mounted.

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

Still needs a man page update.
The new xfs_io command is:

# xfs_io> label [newlabel]

The hack to flush the bdev is maybe not right, I'm
not sure xfs has actually gotten the change out to sector 0
yet at this point...

diff --git a/db/xfs_admin.sh b/db/xfs_admin.sh
index d42446b..02485b0 100755
--- a/db/xfs_admin.sh
+++ b/db/xfs_admin.sh
@@ -16,7 +16,7 @@ do
 	f)	DB_OPTS=$DB_OPTS" -f";;
 	j)	DB_OPTS=$DB_OPTS" -c 'version log2'";;
 	l)	DB_OPTS=$DB_OPTS" -r -c label";;
-	L)	DB_OPTS=$DB_OPTS" -c 'label "$OPTARG"'";;
+	L)	LABEL_OPT=$OPTARG;;
 	p)	DB_OPTS=$DB_OPTS" -c 'version projid32bit'";;
 	u)	DB_OPTS=$DB_OPTS" -r -c uuid";;
 	U)	DB_OPTS=$DB_OPTS" -c 'uuid "$OPTARG"'";;
@@ -32,10 +32,18 @@ done
 set -- extra $@
 shift $OPTIND
 case $# in
-	1)	if [ -n "$DB_OPTS" ]
+	1)	DIR=`grep -w $1 /proc/mounts | awk '{print $2}'`
+		if [ -n "$LABEL_OPT" -a -n "$DIR" ]
 		then
-			eval xfs_db -x -p xfs_admin $DB_OPTS $1
+			eval io/xfs_io -c "'label $LABEL_OPT'" $DIR
 			status=$?
+		else
+			DB_OPTS=$DB_OPTS" -c 'label "$LABEL_OPT"'"
+		fi
+		if [ -n "$DB_OPTS" ]
+		then
+			eval db/xfs_db -x -p xfs_admin $DB_OPTS $1
+			status=`expr $? + $status`
 		fi
 		if [ -n "$REPAIR_OPTS" ]
 		then
diff --git a/io/Makefile b/io/Makefile
index 0b53f41..8f46340 100644
--- a/io/Makefile
+++ b/io/Makefile
@@ -9,9 +9,9 @@ LTCOMMAND = xfs_io
 LSRCFILES = xfs_bmap.sh xfs_freeze.sh xfs_mkfile.sh
 HFILES = init.h io.h
 CFILES = init.c \
-	attr.c bmap.c file.c freeze.c fsync.c getrusage.c imap.c link.c \
-	mmap.c open.c parent.c pread.c prealloc.c pwrite.c seek.c shutdown.c \
-	sync.c truncate.c reflink.c
+	attr.c bmap.c file.c freeze.c fsync.c getrusage.c imap.c label.c \
+	link.c mmap.c open.c parent.c pread.c prealloc.c pwrite.c seek.c \
+	shutdown.c sync.c truncate.c reflink.c
 
 LLDLIBS = $(LIBXCMD) $(LIBHANDLE)
 LTDEPENDENCIES = $(LIBXCMD) $(LIBHANDLE)
diff --git a/io/init.c b/io/init.c
index 51f1f5c..0c08a57 100644
--- a/io/init.c
+++ b/io/init.c
@@ -65,6 +65,7 @@ init_commands(void)
 	help_init();
 	imap_init();
 	inject_init();
+	label_init();
 	seek_init();
 	madvise_init();
 	mincore_init();
diff --git a/io/io.h b/io/io.h
index 172b1f8..560ad52 100644
--- a/io/io.h
+++ b/io/io.h
@@ -102,6 +102,7 @@ extern void		getrusage_init(void);
 extern void		help_init(void);
 extern void		imap_init(void);
 extern void		inject_init(void);
+extern void		label_init(void);
 extern void		mmap_init(void);
 extern void		open_init(void);
 extern void		parent_init(void);
diff --git a/io/label.c b/io/label.c
new file mode 100644
index 0000000..721f27b
--- /dev/null
+++ b/io/label.c
@@ -0,0 +1,87 @@
+/*
+ * Copyright (c) 2016 Red Hat, Inc.
+ * All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it would be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write the Free Software Foundation,
+ * Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+
+#include <sys/ioctl.h>
+#include <sys/mount.h>
+#include "platform_defs.h"
+#include "libxfs.h"
+#include "path.h"
+#include "command.h"
+#include "init.h"
+#include "io.h"
+
+#ifndef FS_IOC_GET_FSLABEL
+#define FSLABEL_MAX 256	/* Max chars for the interface; each fs may differ */
+#define FS_IOC_GET_FSLABEL		_IOR(0x94, 49, char[FSLABEL_MAX])
+#define FS_IOC_SET_FSLABEL		_IOW(0x94, 50, char[FSLABEL_MAX])
+#endif
+
+static cmdinfo_t label_cmd;
+
+static int
+label_f(
+	int		argc,
+	char		**argv)
+{
+	int		error;
+	char		label[FSLABEL_MAX];
+
+	if (argc == 1) {
+		memset(&label, 0, sizeof(label));
+		error = ioctl(file->fd, FS_IOC_GET_FSLABEL, &label);
+	} else {
+		fs_path_t	*fs;	/* mount point information */
+		int 		fd;
+
+		strncpy(label, argv[1], sizeof(label));
+		error = ioctl(file->fd, FS_IOC_SET_FSLABEL, &label);
+		/* best effort to flush bdev cache */
+		fs_table_initialise(0, NULL, 0, NULL);
+		fs = fs_table_lookup(file->name, FS_MOUNT_POINT);
+		if (fs) {
+			fd = open(fs->fs_name, O_RDONLY);
+			if (fd >= 0) {
+				ioctl(fd, BLKFLSBUF, 0);
+				close(fd);
+			}
+		}
+	}
+
+	if (error)
+		perror("label:");
+	else
+		printf("label = \"%s\"\n", label);
+
+	return error;
+
+}
+
+void
+label_init(void)
+{
+	label_cmd.name = "label";
+	label_cmd.cfunc = label_f;
+	label_cmd.argmin = 0;
+	label_cmd.argmax = 1;
+	label_cmd.args = _("[label]");
+	label_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
+	label_cmd.oneline =
+		_("queries or sets the filesystem label while mounted");
+
+	add_command(&label_cmd);
+}


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

Re: [PATCH 0/4] xfs: online relabeling [RFC]

[Eric Sandeen]

On 6/9/16 11:36 AM, Eric Sandeen wrote:
> Ok, this more or less works; not really up to snuff
> for submission or merging, just sketching it out, but some
> questions first:
> 
> 1) Is there really any point to this? :) We did have one
>    request, and btrfs can do it ...
> 
> 2) Is using m_growlock horrible?  growfs is the only other
>    thing that writes all supers, so I grabbed it.  We don't
>    want multiple relabels stepping on each other.
> 
> 3) Is there some way to actually force the primary to disk?
>    Right now the label change isn't actually visible on the
>    primary until unmount, which defeats the purpose.  I'm not
>    sure if there's a straightforward/safe way to make it
>    visible...

Oh, sorry - I guess it is getting written out, but it's only
available via an O_DIRECT read from userspace; it's not
invalidating the cache.

# io/xfs_io -c "label derp" /mnt/test
label = "derp"

# dd if=/dev/sdb2 bs=512 count=1 | hexdump -C
...
00000060  00 00 0a 00 b4 e5 02 00  02 00 00 08 66 6f 6f 00  |............foo.|
...

# dd if=/dev/sdb2 iflag=direct bs=512 count=1 | hexdump -C
...
00000060  00 00 0a 00 b4 e5 02 00  02 00 00 08 64 65 72 70  |............derp|
...

# dd if=/dev/sdb2 bs=512 count=1 | hexdump -C
...
00000060  00 00 0a 00 b4 e5 02 00  02 00 00 08 66 6f 6f 00  |............foo.|
...

Guess I need to think about this some more.

-Eric

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

Re: [PATCH 3/4] xfs: implement online get/set fs label

[Brian Foster]

On Thu, Jun 09, 2016 at 11:41:07AM -0500, Eric Sandeen wrote:
> Wire up label ioctls for XFS.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> This is where the implementation questions come in;
> is using growlock an abomination?  How can I make the
> primary super change immediately visible?
> 
>  fs/xfs/xfs_ioctl.c |   62 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 62 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index dbca737..ab59213 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -41,6 +41,8 @@
>  #include "xfs_trans.h"
>  #include "xfs_pnfs.h"
>  #include "xfs_acl.h"
> +#include "xfs_log.h"
> +#include "xfs_sb.h"
>  
>  #include <linux/capability.h>
>  #include <linux/dcache.h>
> @@ -1603,6 +1605,62 @@ xfs_ioc_swapext(
>  	return error;
>  }
>  
> +static int
> +xfs_ioc_getlabel(
> +	struct xfs_mount	*mp,
> +	char			__user *label)
> +{
> +	int			error = 0;
> +	struct xfs_sb		*sbp = &mp->m_sb;
> +
> +	if (!mutex_trylock(&mp->m_growlock))
> +		return -EWOULDBLOCK;
> +	if (copy_to_user(label, sbp->sb_fname, sizeof(sbp->sb_fname)))
> +		error = -EFAULT;
> +	mutex_unlock(&mp->m_growlock);
> +	return error;
> +}
> +
> +static int
> +xfs_ioc_setlabel(
> +	struct file		*filp,
> +	struct xfs_mount	*mp,
> +	char			__user *newlabel)
> +{
> +	int			error;
> +	struct xfs_sb		*sbp = &mp->m_sb;
> +	char			sb_fname[12];
> +
> +	if (!capable(CAP_SYS_ADMIN))
> +		return -EPERM;
> +
> +	if (copy_from_user(sb_fname, newlabel, sizeof(sb_fname)))
> +		return -EFAULT;
> +
> +	error = mnt_want_write_file(filp);
> +	if (error)
> +		return error;
> +
> +	/* growfs & label both muck w/ the super directly... */
> +	if (!mutex_trylock(&mp->m_growlock))
> +		return -EWOULDBLOCK;

Why the trylock here? It seems like we can still block in other places
(e.g., mnt_want_write_file() above).

> +	memset(sbp->sb_fname, 0, sizeof(sbp->sb_fname));
> +	strncpy(sbp->sb_fname, sb_fname, sizeof(sbp->sb_fname));
> +

So m_growlock excludes grow and nothing else looks like it mucks with
sb_fname, but what about any other invocations of xfs_log_sb()? For
example, is there a risk here of somebody else logging the superblock
buffer based on a transiently zeroed mp->m_sb.sb_fname? (In fact,
xfs_log_sb() looks kind of racy to me, but maybe I'm missing something.)

Perhaps we need an xfs_trans_getsb() somewhere in here..?

Brian

> +	error = xfs_sync_sb(mp, true);
> +	if (error)
> +		goto out;
> +	/*
> +	 * Most kernelspace superblock updates only update sb 0.
> +	 * Userspace relabel has always updated all, though, so:
> +	 */
> +	error = xfs_update_secondary_supers(mp, sbp->sb_agcount, 0);
> +out:
> +	mutex_unlock(&mp->m_growlock);
> +	mnt_drop_write_file(filp);
> +	return error;
> +}
> +
>  /*
>   * Note: some of the ioctl's return positive numbers as a
>   * byte count indicating success, such as readlink_by_handle.
> @@ -1630,6 +1688,10 @@ xfs_file_ioctl(
>  	switch (cmd) {
>  	case FITRIM:
>  		return xfs_ioc_trim(mp, arg);
> +	case FS_IOC_GET_FSLABEL:
> +		return xfs_ioc_getlabel(mp, arg);
> +	case FS_IOC_SET_FSLABEL:
> +		return xfs_ioc_setlabel(filp, mp, arg);
>  	case XFS_IOC_ALLOCSP:
>  	case XFS_IOC_FREESP:
>  	case XFS_IOC_RESVSP:
> -- 
> 1.7.1
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

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

Re: [PATCH 0/4] xfs: online relabeling [RFC]

[Brian Foster]

On Thu, Jun 09, 2016 at 11:51:12AM -0500, Eric Sandeen wrote:
> 
> 
> On 6/9/16 11:36 AM, Eric Sandeen wrote:
> > Ok, this more or less works; not really up to snuff
> > for submission or merging, just sketching it out, but some
> > questions first:
> > 
> > 1) Is there really any point to this? :) We did have one
> >    request, and btrfs can do it ...
> > 

Seems reasonable to me. Any details on the use case for the request?

> > 2) Is using m_growlock horrible?  growfs is the only other
> >    thing that writes all supers, so I grabbed it.  We don't
> >    want multiple relabels stepping on each other.
> > 
> > 3) Is there some way to actually force the primary to disk?
> >    Right now the label change isn't actually visible on the
> >    primary until unmount, which defeats the purpose.  I'm not
> >    sure if there's a straightforward/safe way to make it
> >    visible...
> 

> Oh, sorry - I guess it is getting written out, but it's only
> available via an O_DIRECT read from userspace; it's not
> invalidating the cache.
> 
> # io/xfs_io -c "label derp" /mnt/test
> label = "derp"
> 
> # dd if=/dev/sdb2 bs=512 count=1 | hexdump -C
> ...
> 00000060  00 00 0a 00 b4 e5 02 00  02 00 00 08 66 6f 6f 00  |............foo.|
> ...
> 
> # dd if=/dev/sdb2 iflag=direct bs=512 count=1 | hexdump -C
> ...
> 00000060  00 00 0a 00 b4 e5 02 00  02 00 00 08 64 65 72 70  |............derp|
> ...
> 
> # dd if=/dev/sdb2 bs=512 count=1 | hexdump -C
> ...
> 00000060  00 00 0a 00 b4 e5 02 00  02 00 00 08 66 6f 6f 00  |............foo.|
> ...
> 
> Guess I need to think about this some more.
> 

Isn't this to be expected? You're directly accessing the block device of
a mounted filesystem. I would think this is expected behavior, so long
as the set/get interfaces through the fs are consistent.

Brian

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

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

Re: [PATCH 0/4] xfs: online relabeling [RFC]

[Darrick J. Wong]

On Fri, Jun 10, 2016 at 08:19:36AM -0400, Brian Foster wrote:
> On Thu, Jun 09, 2016 at 11:51:12AM -0500, Eric Sandeen wrote:
> > 
> > 
> > On 6/9/16 11:36 AM, Eric Sandeen wrote:
> > > Ok, this more or less works; not really up to snuff
> > > for submission or merging, just sketching it out, but some
> > > questions first:
> > > 
> > > 1) Is there really any point to this? :) We did have one
> > >    request, and btrfs can do it ...
> > > 
> 
> Seems reasonable to me. Any details on the use case for the request?
> 
> > > 2) Is using m_growlock horrible?  growfs is the only other
> > >    thing that writes all supers, so I grabbed it.  We don't
> > >    want multiple relabels stepping on each other.
> > > 
> > > 3) Is there some way to actually force the primary to disk?
> > >    Right now the label change isn't actually visible on the
> > >    primary until unmount, which defeats the purpose.  I'm not
> > >    sure if there's a straightforward/safe way to make it
> > >    visible...
> > 
> 
> > Oh, sorry - I guess it is getting written out, but it's only
> > available via an O_DIRECT read from userspace; it's not
> > invalidating the cache.
> > 
> > # io/xfs_io -c "label derp" /mnt/test
> > label = "derp"
> > 
> > # dd if=/dev/sdb2 bs=512 count=1 | hexdump -C
> > ...
> > 00000060  00 00 0a 00 b4 e5 02 00  02 00 00 08 66 6f 6f 00  |............foo.|
> > ...
> > 
> > # dd if=/dev/sdb2 iflag=direct bs=512 count=1 | hexdump -C
> > ...
> > 00000060  00 00 0a 00 b4 e5 02 00  02 00 00 08 64 65 72 70  |............derp|
> > ...
> > 
> > # dd if=/dev/sdb2 bs=512 count=1 | hexdump -C
> > ...
> > 00000060  00 00 0a 00 b4 e5 02 00  02 00 00 08 66 6f 6f 00  |............foo.|
> > ...
> > 
> > Guess I need to think about this some more.
> > 
> 
> Isn't this to be expected? You're directly accessing the block device of
> a mounted filesystem. I would think this is expected behavior, so long
> as the set/get interfaces through the fs are consistent.

Trouble is, I bet blkid prints LABEL=foo here and not LABEL=derp as
the admin is probably expecting.

/me wonders if invalidate_inode_pages2_range here would help?

--D

> 
> Brian
> 
> > -Eric
> > 
> > _______________________________________________
> > xfs mailing list
> > xfs@oss.sgi.com
> > http://oss.sgi.com/mailman/listinfo/xfs
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

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

Re: [PATCH 0/4] xfs: online relabeling [RFC]

[Eric Sandeen]

On 6/10/16 11:41 AM, Darrick J. Wong wrote:
> On Fri, Jun 10, 2016 at 08:19:36AM -0400, Brian Foster wrote:
>> On Thu, Jun 09, 2016 at 11:51:12AM -0500, Eric Sandeen wrote:
>>>
>>>
>>> On 6/9/16 11:36 AM, Eric Sandeen wrote:
>>>> Ok, this more or less works; not really up to snuff
>>>> for submission or merging, just sketching it out, but some
>>>> questions first:
>>>>
>>>> 1) Is there really any point to this? :) We did have one
>>>>    request, and btrfs can do it ...
>>>>
>>
>> Seems reasonable to me. Any details on the use case for the request?
>>
>>>> 2) Is using m_growlock horrible?  growfs is the only other
>>>>    thing that writes all supers, so I grabbed it.  We don't
>>>>    want multiple relabels stepping on each other.
>>>>
>>>> 3) Is there some way to actually force the primary to disk?
>>>>    Right now the label change isn't actually visible on the
>>>>    primary until unmount, which defeats the purpose.  I'm not
>>>>    sure if there's a straightforward/safe way to make it
>>>>    visible...
>>>
>>
>>> Oh, sorry - I guess it is getting written out, but it's only
>>> available via an O_DIRECT read from userspace; it's not
>>> invalidating the cache.
>>>
>>> # io/xfs_io -c "label derp" /mnt/test
>>> label = "derp"
>>>
>>> # dd if=/dev/sdb2 bs=512 count=1 | hexdump -C
>>> ...
>>> 00000060  00 00 0a 00 b4 e5 02 00  02 00 00 08 66 6f 6f 00  |............foo.|
>>> ...
>>>
>>> # dd if=/dev/sdb2 iflag=direct bs=512 count=1 | hexdump -C
>>> ...
>>> 00000060  00 00 0a 00 b4 e5 02 00  02 00 00 08 64 65 72 70  |............derp|
>>> ...
>>>
>>> # dd if=/dev/sdb2 bs=512 count=1 | hexdump -C
>>> ...
>>> 00000060  00 00 0a 00 b4 e5 02 00  02 00 00 08 66 6f 6f 00  |............foo.|
>>> ...
>>>
>>> Guess I need to think about this some more.
>>>
>>
>> Isn't this to be expected? You're directly accessing the block device of
>> a mounted filesystem. I would think this is expected behavior, so long
>> as the set/get interfaces through the fs are consistent.
> 
> Trouble is, I bet blkid prints LABEL=foo here and not LABEL=derp as
> the admin is probably expecting.

exactly.

And FWIW, when btrfs does it, it *is* visible.

> /me wonders if invalidate_inode_pages2_range here would help?

Um, not sure.  Calling that against metadata from xfs feels a bit
bizarre...

-Eric

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