grub: xfs: Accept filesystem with sparse inodes

Message ID 20181213115250.17494-1-stefan.schantl@ipfire.org
State Accepted
Commit 848ac6900974f8ac3718c0ea4febec6e56954823
Headers
Series grub: xfs: Accept filesystem with sparse inodes |

Commit Message

Stefan Schantl Dec. 13, 2018, 10:52 p.m. UTC
  Signed-off-by: Stefan Schantl <stefan.schantl@ipfire.org>
Tested-by: Stefan Schantl <stefan.schantl@ipfire.org>
---
 lfs/grub                                      |  1 +
 ...accept-filesystem-with-sparse-inodes.patch | 60 +++++++++++++++++++
 2 files changed, 61 insertions(+)
 create mode 100644 src/patches/grub-2.02-xfs-accept-filesystem-with-sparse-inodes.patch
  

Comments

Michael Tremer Dec. 14, 2018, 12:40 a.m. UTC | #1
Hello,

Thank you very much for resolving this so quickly.

You did not reference the bug ticket in the commit message. Please think about that next time :)

  https://bugzilla.ipfire.org/show_bug.cgi?id=11951

I merged it and grub won’t be re-installed in the update.

-Michael

> On 13 Dec 2018, at 11:52, Stefan Schantl <stefan.schantl@ipfire.org> wrote:
> 
> Signed-off-by: Stefan Schantl <stefan.schantl@ipfire.org>
> Tested-by: Stefan Schantl <stefan.schantl@ipfire.org>
> ---
> lfs/grub                                      |  1 +
> ...accept-filesystem-with-sparse-inodes.patch | 60 +++++++++++++++++++
> 2 files changed, 61 insertions(+)
> create mode 100644 src/patches/grub-2.02-xfs-accept-filesystem-with-sparse-inodes.patch
> 
> diff --git a/lfs/grub b/lfs/grub
> index 1a10c2aa5..e6131f2f5 100644
> --- a/lfs/grub
> +++ b/lfs/grub
> @@ -99,6 +99,7 @@ $(TARGET) : $(patsubst %,$(DIR_DL)/%,$(objects))
> 	@rm -rf $(DIR_APP) $(DIR_APP_EFI) && cd $(DIR_SRC) && tar axf $(DIR_DL)/$(DL_FILE)
> 
> 	cd $(DIR_APP) && patch -Np1 < $(DIR_SRC)/src/patches/grub-2.02_disable_vga_fallback.patch
> +	cd $(DIR_APP) && patch -Np1 < $(DIR_SRC)/src/patches/grub-2.02-xfs-accept-filesystem-with-sparse-inodes.patch
> 
> 	# Install unifont
> 	cp -v $(DIR_DL)/unifont-7.0.03.pcf.gz $(DIR_APP)/unifont.pcf.gz
> diff --git a/src/patches/grub-2.02-xfs-accept-filesystem-with-sparse-inodes.patch b/src/patches/grub-2.02-xfs-accept-filesystem-with-sparse-inodes.patch
> new file mode 100644
> index 000000000..6c6a750b4
> --- /dev/null
> +++ b/src/patches/grub-2.02-xfs-accept-filesystem-with-sparse-inodes.patch
> @@ -0,0 +1,60 @@
> +From cda0a857dd7a27cd5d621747464bfe71e8727fff Mon Sep 17 00:00:00 2001
> +From: Daniel Kiper <daniel.kiper@oracle.com>
> +Date: Tue, 29 May 2018 16:16:02 +0200
> +Subject: xfs: Accept filesystem with sparse inodes
> +
> +The sparse inode metadata format became a mkfs.xfs default in
> +xfsprogs-4.16.0, and such filesystems are now rejected by grub as
> +containing an incompatible feature.
> +
> +In essence, this feature allows xfs to allocate inodes into fragmented
> +freespace.  (Without this feature, if xfs could not allocate contiguous
> +space for 64 new inodes, inode creation would fail.)
> +
> +In practice, the disk format change is restricted to the inode btree,
> +which as far as I can tell is not used by grub.  If all you're doing
> +today is parsing a directory, reading an inode number, and converting
> +that inode number to a disk location, then ignoring this feature
> +should be fine, so I've added it to XFS_SB_FEAT_INCOMPAT_SUPPORTED
> +
> +I did some brief testing of this patch by hacking up the regression
> +tests to completely fragment freespace on the test xfs filesystem, and
> +then write a large-ish number of inodes to consume any existing
> +contiguous 64-inode chunk.  This way any files the grub tests add and
> +traverse would be in such a fragmented inode allocation.  Tests passed,
> +but I'm not sure how to cleanly integrate that into the test harness.
> +
> +Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> +Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
> +Tested-by: Chris Murphy <lists@colorremedies.com>
> +---
> + grub-core/fs/xfs.c | 11 ++++++++++-
> + 1 file changed, 10 insertions(+), 1 deletion(-)
> +
> +diff --git a/grub-core/fs/xfs.c b/grub-core/fs/xfs.c
> +index c6031bd..3b00c74 100644
> +--- a/grub-core/fs/xfs.c
> ++++ b/grub-core/fs/xfs.c
> +@@ -79,9 +79,18 @@ GRUB_MOD_LICENSE ("GPLv3+");
> + #define XFS_SB_FEAT_INCOMPAT_SPINODES   (1 << 1)        /* sparse inode chunks */
> + #define XFS_SB_FEAT_INCOMPAT_META_UUID  (1 << 2)        /* metadata UUID */
> + 
> +-/* We do not currently verify metadata UUID so it is safe to read such filesystem */
> ++/*
> ++ * Directory entries with ftype are explicitly handled by GRUB code.
> ++ *
> ++ * We do not currently read the inode btrees, so it is safe to read filesystems
> ++ * with the XFS_SB_FEAT_INCOMPAT_SPINODES feature.
> ++ *
> ++ * We do not currently verify metadata UUID, so it is safe to read filesystems
> ++ * with the XFS_SB_FEAT_INCOMPAT_META_UUID feature.
> ++ */
> + #define XFS_SB_FEAT_INCOMPAT_SUPPORTED \
> + 	(XFS_SB_FEAT_INCOMPAT_FTYPE | \
> ++	 XFS_SB_FEAT_INCOMPAT_SPINODES | \
> + 	 XFS_SB_FEAT_INCOMPAT_META_UUID)
> + 
> + struct grub_xfs_sblock
> +-- 
> +cgit v1.0-41-gc330
> +
> -- 
> 2.19.1
>
  

Patch

diff --git a/lfs/grub b/lfs/grub
index 1a10c2aa5..e6131f2f5 100644
--- a/lfs/grub
+++ b/lfs/grub
@@ -99,6 +99,7 @@  $(TARGET) : $(patsubst %,$(DIR_DL)/%,$(objects))
 	@rm -rf $(DIR_APP) $(DIR_APP_EFI) && cd $(DIR_SRC) && tar axf $(DIR_DL)/$(DL_FILE)
 
 	cd $(DIR_APP) && patch -Np1 < $(DIR_SRC)/src/patches/grub-2.02_disable_vga_fallback.patch
+	cd $(DIR_APP) && patch -Np1 < $(DIR_SRC)/src/patches/grub-2.02-xfs-accept-filesystem-with-sparse-inodes.patch
 
 	# Install unifont
 	cp -v $(DIR_DL)/unifont-7.0.03.pcf.gz $(DIR_APP)/unifont.pcf.gz
diff --git a/src/patches/grub-2.02-xfs-accept-filesystem-with-sparse-inodes.patch b/src/patches/grub-2.02-xfs-accept-filesystem-with-sparse-inodes.patch
new file mode 100644
index 000000000..6c6a750b4
--- /dev/null
+++ b/src/patches/grub-2.02-xfs-accept-filesystem-with-sparse-inodes.patch
@@ -0,0 +1,60 @@ 
+From cda0a857dd7a27cd5d621747464bfe71e8727fff Mon Sep 17 00:00:00 2001
+From: Daniel Kiper <daniel.kiper@oracle.com>
+Date: Tue, 29 May 2018 16:16:02 +0200
+Subject: xfs: Accept filesystem with sparse inodes
+
+The sparse inode metadata format became a mkfs.xfs default in
+xfsprogs-4.16.0, and such filesystems are now rejected by grub as
+containing an incompatible feature.
+
+In essence, this feature allows xfs to allocate inodes into fragmented
+freespace.  (Without this feature, if xfs could not allocate contiguous
+space for 64 new inodes, inode creation would fail.)
+
+In practice, the disk format change is restricted to the inode btree,
+which as far as I can tell is not used by grub.  If all you're doing
+today is parsing a directory, reading an inode number, and converting
+that inode number to a disk location, then ignoring this feature
+should be fine, so I've added it to XFS_SB_FEAT_INCOMPAT_SUPPORTED
+
+I did some brief testing of this patch by hacking up the regression
+tests to completely fragment freespace on the test xfs filesystem, and
+then write a large-ish number of inodes to consume any existing
+contiguous 64-inode chunk.  This way any files the grub tests add and
+traverse would be in such a fragmented inode allocation.  Tests passed,
+but I'm not sure how to cleanly integrate that into the test harness.
+
+Signed-off-by: Eric Sandeen <sandeen@redhat.com>
+Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
+Tested-by: Chris Murphy <lists@colorremedies.com>
+---
+ grub-core/fs/xfs.c | 11 ++++++++++-
+ 1 file changed, 10 insertions(+), 1 deletion(-)
+
+diff --git a/grub-core/fs/xfs.c b/grub-core/fs/xfs.c
+index c6031bd..3b00c74 100644
+--- a/grub-core/fs/xfs.c
++++ b/grub-core/fs/xfs.c
+@@ -79,9 +79,18 @@ GRUB_MOD_LICENSE ("GPLv3+");
+ #define XFS_SB_FEAT_INCOMPAT_SPINODES   (1 << 1)        /* sparse inode chunks */
+ #define XFS_SB_FEAT_INCOMPAT_META_UUID  (1 << 2)        /* metadata UUID */
+ 
+-/* We do not currently verify metadata UUID so it is safe to read such filesystem */
++/*
++ * Directory entries with ftype are explicitly handled by GRUB code.
++ *
++ * We do not currently read the inode btrees, so it is safe to read filesystems
++ * with the XFS_SB_FEAT_INCOMPAT_SPINODES feature.
++ *
++ * We do not currently verify metadata UUID, so it is safe to read filesystems
++ * with the XFS_SB_FEAT_INCOMPAT_META_UUID feature.
++ */
+ #define XFS_SB_FEAT_INCOMPAT_SUPPORTED \
+ 	(XFS_SB_FEAT_INCOMPAT_FTYPE | \
++	 XFS_SB_FEAT_INCOMPAT_SPINODES | \
+ 	 XFS_SB_FEAT_INCOMPAT_META_UUID)
+ 
+ struct grub_xfs_sblock
+-- 
+cgit v1.0-41-gc330
+