[PATCH 0/4] xfsprogs: even more fixes for 5.9

[PATCH 0/4] xfsprogs: even more fixes for 5.9

[Darrick J. Wong]

Hi all,

I found even more small problems involving realtime volumes, so fix
these too.

If you're going to start using this mess, you probably ought to just
pull from my git trees, which are linked below.

This is an extraordinary way to destroy everything.  Enjoy!
Comments and questions are, as always, welcome.

--D

xfsprogs git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=xfsprogs-5.9-fixes2
---
 libxfs/util.c   |   55 ++++++++++++++++++++++++++++++++-----------------------
 mkfs/proto.c    |    6 ++++++
 repair/dinode.c |   15 ++++++---------
 3 files changed, 44 insertions(+), 32 deletions(-)

[PATCH 1/4] libxfs: refactor inode flags propagation code

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Hoist the code that propagates di_flags from a parent to a new child
into a separate function.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 libxfs/util.c |   55 ++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 32 insertions(+), 23 deletions(-)


diff --git a/libxfs/util.c b/libxfs/util.c
index 7fb0a99596f4..78519872e8e8 100644
--- a/libxfs/util.c
+++ b/libxfs/util.c
@@ -207,6 +207,36 @@ xfs_flags2diflags2(
 	return di_flags2;
 }
 
+/* Propagate di_flags from a parent inode to a child inode. */
+static void
+xfs_inode_propagate_flags(
+	struct xfs_inode	*ip,
+	const struct xfs_inode	*pip)
+{
+	unsigned int		di_flags = 0;
+	umode_t			mode = VFS_I(ip)->i_mode;
+
+	if ((mode & S_IFMT) == S_IFDIR) {
+		if (pip->i_d.di_flags & XFS_DIFLAG_RTINHERIT)
+			di_flags |= XFS_DIFLAG_RTINHERIT;
+		if (pip->i_d.di_flags & XFS_DIFLAG_EXTSZINHERIT) {
+			di_flags |= XFS_DIFLAG_EXTSZINHERIT;
+			ip->i_d.di_extsize = pip->i_d.di_extsize;
+		}
+	} else {
+		if (pip->i_d.di_flags & XFS_DIFLAG_RTINHERIT) {
+			di_flags |= XFS_DIFLAG_REALTIME;
+		}
+		if (pip->i_d.di_flags & XFS_DIFLAG_EXTSZINHERIT) {
+			di_flags |= XFS_DIFLAG_EXTSIZE;
+			ip->i_d.di_extsize = pip->i_d.di_extsize;
+		}
+	}
+	if (pip->i_d.di_flags & XFS_DIFLAG_PROJINHERIT)
+		di_flags |= XFS_DIFLAG_PROJINHERIT;
+	ip->i_d.di_flags |= di_flags;
+}
+
 /*
  * Allocate an inode on disk and return a copy of its in-core version.
  * Set mode, nlink, and rdev appropriately within the inode.
@@ -299,29 +329,8 @@ libxfs_ialloc(
 		break;
 	case S_IFREG:
 	case S_IFDIR:
-		if (pip && (pip->i_d.di_flags & XFS_DIFLAG_ANY)) {
-			uint	di_flags = 0;
-
-			if ((mode & S_IFMT) == S_IFDIR) {
-				if (pip->i_d.di_flags & XFS_DIFLAG_RTINHERIT)
-					di_flags |= XFS_DIFLAG_RTINHERIT;
-				if (pip->i_d.di_flags & XFS_DIFLAG_EXTSZINHERIT) {
-					di_flags |= XFS_DIFLAG_EXTSZINHERIT;
-					ip->i_d.di_extsize = pip->i_d.di_extsize;
-				}
-			} else {
-				if (pip->i_d.di_flags & XFS_DIFLAG_RTINHERIT) {
-					di_flags |= XFS_DIFLAG_REALTIME;
-				}
-				if (pip->i_d.di_flags & XFS_DIFLAG_EXTSZINHERIT) {
-					di_flags |= XFS_DIFLAG_EXTSIZE;
-					ip->i_d.di_extsize = pip->i_d.di_extsize;
-				}
-			}
-			if (pip->i_d.di_flags & XFS_DIFLAG_PROJINHERIT)
-				di_flags |= XFS_DIFLAG_PROJINHERIT;
-			ip->i_d.di_flags |= di_flags;
-		}
+		if (pip && (pip->i_d.di_flags & XFS_DIFLAG_ANY))
+			xfs_inode_propagate_flags(ip, pip);
 		/* FALLTHROUGH */
 	case S_IFLNK:
 		ip->i_df.if_format = XFS_DINODE_FMT_EXTENTS;

[PATCH 2/4] libxfs: don't propagate RTINHERIT -> REALTIME when there is no rtdev

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

When creating a file inside a directory that has RTINHERIT set, only
propagate the REALTIME flag to the file if the filesystem actually has a
realtime volume configured.  Otherwise, we end up writing inodes that
trip the verifiers.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 libxfs/util.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)


diff --git a/libxfs/util.c b/libxfs/util.c
index 78519872e8e8..f1b4759728ec 100644
--- a/libxfs/util.c
+++ b/libxfs/util.c
@@ -224,9 +224,9 @@ xfs_inode_propagate_flags(
 			ip->i_d.di_extsize = pip->i_d.di_extsize;
 		}
 	} else {
-		if (pip->i_d.di_flags & XFS_DIFLAG_RTINHERIT) {
+		if ((pip->i_d.di_flags & XFS_DIFLAG_RTINHERIT) &&
+		    xfs_sb_version_hasrealtime(&ip->i_mount->m_sb))
 			di_flags |= XFS_DIFLAG_REALTIME;
-		}
 		if (pip->i_d.di_flags & XFS_DIFLAG_EXTSZINHERIT) {
 			di_flags |= XFS_DIFLAG_EXTSIZE;
 			ip->i_d.di_extsize = pip->i_d.di_extsize;

[PATCH 3/4] mkfs: don't allow creation of realtime files from a proto file

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

If someone runs mkfs with rtinherit=1, a realtime volume configured, and
a protofile that creates a regular file in the filesystem, mkfs will
error out with "Function not implemented" because userspace doesn't know
how to allocate extents from the rt bitmap.  Catch this specific case
and hand back a somewhat nicer explanation of what happened.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 mkfs/proto.c |    6 ++++++
 1 file changed, 6 insertions(+)


diff --git a/mkfs/proto.c b/mkfs/proto.c
index 9db8fe2d6447..20a7cc3bb5d5 100644
--- a/mkfs/proto.c
+++ b/mkfs/proto.c
@@ -244,6 +244,12 @@ newfile(
 		nb = XFS_B_TO_FSB(mp, len);
 		nmap = 1;
 		error = -libxfs_bmapi_write(tp, ip, 0, nb, 0, nb, &map, &nmap);
+		if (error == ENOSYS && XFS_IS_REALTIME_INODE(ip)) {
+			fprintf(stderr,
+	_("%s: creating realtime files from proto file not supported.\n"),
+					progname);
+			exit(1);
+		}
 		if (error) {
 			fail(_("error allocating space for a file"), error);
 		}

[PATCH 4/4] xfs_repair: don't flag RTINHERIT files when no rt volume

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Don't flag directories with the RTINHERIT flag set when the filesystem
doesn't have a realtime volume configured.  The kernel has let us set
RTINHERIT without a rt volume for ages, so it's not an invalid state.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 repair/dinode.c |   15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)


diff --git a/repair/dinode.c b/repair/dinode.c
index 48b75f7883c5..8aee9f9e8343 100644
--- a/repair/dinode.c
+++ b/repair/dinode.c
@@ -2393,17 +2393,14 @@ _("bad (negative) size %" PRId64 " on inode %" PRIu64 "\n"),
 			flags &= XFS_DIFLAG_ANY;
 		}
 
-		if (flags & (XFS_DIFLAG_REALTIME | XFS_DIFLAG_RTINHERIT)) {
-			/* need an rt-dev! */
-			if (!rt_name) {
-				if (!uncertain) {
-					do_warn(
+		/* need an rt-dev for the realtime flag! */
+		if ((flags & XFS_DIFLAG_REALTIME) && !rt_name) {
+			if (!uncertain) {
+				do_warn(
 	_("inode %" PRIu64 " has RT flag set but there is no RT device\n"),
-						lino);
-				}
-				flags &= ~(XFS_DIFLAG_REALTIME |
-						XFS_DIFLAG_RTINHERIT);
+					lino);
 			}
+			flags &= ~XFS_DIFLAG_REALTIME;
 		}
 		if (flags & XFS_DIFLAG_NEWRTBM) {
 			/* must be a rt bitmap inode */

Re: [PATCH 1/4] libxfs: refactor inode flags propagation code

[Christoph Hellwig]

On Mon, Sep 14, 2020 at 06:51:11PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Hoist the code that propagates di_flags from a parent to a new child
> into a separate function.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Looks good (except for the fact that this is duplicated and not shared
with the kernel..)

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 2/4] libxfs: don't propagate RTINHERIT -> REALTIME when there is no rtdev

[Christoph Hellwig]

On Mon, Sep 14, 2020 at 06:51:17PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> When creating a file inside a directory that has RTINHERIT set, only
> propagate the REALTIME flag to the file if the filesystem actually has a
> realtime volume configured.  Otherwise, we end up writing inodes that
> trip the verifiers.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 3/4] mkfs: don't allow creation of realtime files from a proto file

[Christoph Hellwig]

On Mon, Sep 14, 2020 at 06:51:23PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> If someone runs mkfs with rtinherit=1, a realtime volume configured, and
> a protofile that creates a regular file in the filesystem, mkfs will
> error out with "Function not implemented" because userspace doesn't know
> how to allocate extents from the rt bitmap.  Catch this specific case
> and hand back a somewhat nicer explanation of what happened.

Would this be so hard to fix?

Re: [PATCH 4/4] xfs_repair: don't flag RTINHERIT files when no rt volume

[Christoph Hellwig]

On Mon, Sep 14, 2020 at 06:51:30PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Don't flag directories with the RTINHERIT flag set when the filesystem
> doesn't have a realtime volume configured.  The kernel has let us set
> RTINHERIT without a rt volume for ages, so it's not an invalid state.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 1/4] libxfs: refactor inode flags propagation code

[Darrick J. Wong]

On Thu, Sep 17, 2020 at 09:01:59AM +0100, Christoph Hellwig wrote:
> On Mon, Sep 14, 2020 at 06:51:11PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Hoist the code that propagates di_flags from a parent to a new child
> > into a separate function.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Looks good (except for the fact that this is duplicated and not shared
> with the kernel..)

Yeah, I'm working towards that:

https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfs-linux.git/log/?h=inode-refactor

and

https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=inode-refactor

--D

> Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 3/4] mkfs: don't allow creation of realtime files from a proto file

[Darrick J. Wong]

On Thu, Sep 17, 2020 at 09:02:34AM +0100, Christoph Hellwig wrote:
> On Mon, Sep 14, 2020 at 06:51:23PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > If someone runs mkfs with rtinherit=1, a realtime volume configured, and
> > a protofile that creates a regular file in the filesystem, mkfs will
> > error out with "Function not implemented" because userspace doesn't know
> > how to allocate extents from the rt bitmap.  Catch this specific case
> > and hand back a somewhat nicer explanation of what happened.
> 
> Would this be so hard to fix?

Probably not, but I'd prefer to see users asking for this feature before
porting more code to userspace.

I guess I could take a look for 5.10, maybe it's not that difficult.

Though given the amount of bitrot and math errors I've seen recently, I
get the funny sense that my crash test box and TV are the only things
using rt support.

--D

Re: [PATCH 3/4] mkfs: don't allow creation of realtime files from a proto file

[Eric Sandeen]

On 9/14/20 8:51 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> If someone runs mkfs with rtinherit=1, a realtime volume configured, and
> a protofile that creates a regular file in the filesystem, mkfs will
> error out with "Function not implemented" because userspace doesn't know
> how to allocate extents from the rt bitmap.  Catch this specific case
> and hand back a somewhat nicer explanation of what happened.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

hch has a point about "maybe we should fix it" but it seems like it's
not somewhere we really need to spend development effort right now.

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

> ---
>  mkfs/proto.c |    6 ++++++
>  1 file changed, 6 insertions(+)
> 
> 
> diff --git a/mkfs/proto.c b/mkfs/proto.c
> index 9db8fe2d6447..20a7cc3bb5d5 100644
> --- a/mkfs/proto.c
> +++ b/mkfs/proto.c
> @@ -244,6 +244,12 @@ newfile(
>  		nb = XFS_B_TO_FSB(mp, len);
>  		nmap = 1;
>  		error = -libxfs_bmapi_write(tp, ip, 0, nb, 0, nb, &map, &nmap);
> +		if (error == ENOSYS && XFS_IS_REALTIME_INODE(ip)) {
> +			fprintf(stderr,
> +	_("%s: creating realtime files from proto file not supported.\n"),
> +					progname);
> +			exit(1);
> +		}
>  		if (error) {
>  			fail(_("error allocating space for a file"), error);
>  		}
> 

Re: [PATCH 2/4] libxfs: don't propagate RTINHERIT -> REALTIME when there is no rtdev

[Eric Sandeen]

On 9/14/20 8:51 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> When creating a file inside a directory that has RTINHERIT set, only
> propagate the REALTIME flag to the file if the filesystem actually has a
> realtime volume configured.  Otherwise, we end up writing inodes that
> trip the verifiers.

The "else" means this is only for non-directories; we still propagate
the inherit flag to dirs even without an RT subdir, because ....  *shrug*
kernel lets you set it explicitly, and repair (now) doesn't care either?

Ok, seems reasonable.

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

-Eric

> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  libxfs/util.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> 
> diff --git a/libxfs/util.c b/libxfs/util.c
> index 78519872e8e8..f1b4759728ec 100644
> --- a/libxfs/util.c
> +++ b/libxfs/util.c
> @@ -224,9 +224,9 @@ xfs_inode_propagate_flags(
>  			ip->i_d.di_extsize = pip->i_d.di_extsize;
>  		}
>  	} else {
> -		if (pip->i_d.di_flags & XFS_DIFLAG_RTINHERIT) {
> +		if ((pip->i_d.di_flags & XFS_DIFLAG_RTINHERIT) &&
> +		    xfs_sb_version_hasrealtime(&ip->i_mount->m_sb))
>  			di_flags |= XFS_DIFLAG_REALTIME;
> -		}
>  		if (pip->i_d.di_flags & XFS_DIFLAG_EXTSZINHERIT) {
>  			di_flags |= XFS_DIFLAG_EXTSIZE;
>  			ip->i_d.di_extsize = pip->i_d.di_extsize;
>