Skip to contentSkip to conversation

Adding btrfs-progs host package.

PATCH New Robert J. Heywood 3 participants 7 messages Aug 21, 2018 – Aug 22, 2018

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/2 Adding btrfs-progs host package.

Adding a new option to build the btrfs toolset for the host.

---
 package/Config.in.host             | 1 +
 package/btrfs-progs/Config.in.host | 6 ++++++
 package/btrfs-progs/btrfs-progs.mk | 4 ++++
 3 files changed, 11 insertions(+)
 create mode 100644 package/btrfs-progs/Config.in.host
package/Config.in.host +1 -0
index 7838ffc219..432597ed31 100644
@@ -2,6 +2,7 @@ menu "Host utilities"
 
 	source "package/aespipe/Config.in.host"
 	source "package/android-tools/Config.in.host"
+	source "package/btrfs-progs/Config.in.host"
 	source "package/cargo/Config.in.host"
 	source "package/cbootimage/Config.in.host"
 	source "package/checkpolicy/Config.in.host"
package/btrfs-progs/Config.in.host new file +6 -0
new file mode 100644
index 0000000000..cdcd2e4ec4
@@ -0,0 +1,6 @@
+config BR2_PACKAGE_HOST_BTRFS_PROGS
+	bool "host btrfs-progs"
+	help
+	  Btrfs filesystem utilities
+
+	  https://btrfs.wiki.kernel.org/index.php/Main_Page
package/btrfs-progs/btrfs-progs.mk +4 -0
index b4cc839352..3129ecc714 100644
@@ -21,4 +21,8 @@ BTRFS_PROGS_INSTALL_TARGET_OPTS = DESTDIR=$(TARGET_DIR) install-static
 BTRFS_PROGS_INSTALL_STAGING_OPTS = DESTDIR=$(STAGING_DIR) install-static
 endif
 
+HOST_BTRFS_PROGS_DEPENDENCIES = host-e2fsprogs host-lzo host-zlib
+HOST_BTRFS_PROGS_CONF_OPTS = --disable-backtrace --disable-zstd --disable-python
+
 $(eval $(autotools-package))
+$(eval $(host-autotools-package))
Signature · 1 line
2.11.0
#1 lore raw

PATCH 2/2 Adding btrfs rootfs option.

This patch makes it possible to format the rootfs using btrfs. Introduces the option; BR2_TARGET_ROOTFS_BTRFS

When selected, the user is able to specify the filesystem size, label, options, and node and sector sizes.
The new files are based on fs/ext2/{Config.in,ext2.mk}

---
 fs/Config.in       |  1 +
 fs/btrfs/Config.in | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 fs/btrfs/btrfs.mk  | 45 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 94 insertions(+)
 create mode 100644 fs/btrfs/Config.in
 create mode 100644 fs/btrfs/btrfs.mk
fs/Config.in +1 -0
index c25b01c3de..24f22fd7e3 100644
@@ -1,6 +1,7 @@
 menu "Filesystem images"
 
 source "fs/axfs/Config.in"
+source "fs/btrfs/Config.in"
 source "fs/cloop/Config.in"
 source "fs/cpio/Config.in"
 source "fs/cramfs/Config.in"
fs/btrfs/Config.in new file +48 -0

48 lines not on this page · raw message

fs/btrfs/btrfs.mk new file +45 -0

45 lines not on this page · raw message

Signature · 1 line
2.11.0
#2 lore raw

PATCH 2/2 Adding btrfs rootfs option.

Robert, All,

On 2018-08-21 17:04 +0100, Robert J. Heywood spake thusly:

Quoted 6 lines

This patch makes it possible to format the rootfs using btrfs. Introduces the option; BR2_TARGET_ROOTFS_BTRFS

When selected, the user is able to specify the filesystem size, label, options, and node and sector sizes.
The new files are based on fs/ext2/{Config.in,ext2.mk}

Nice addition! :-)

Quoted 1 line

[--SNIP--]

fs/btrfs/Config.in line 28

Quoted 5 lines
fs/btrfs/Config.in new file +5 -0
+config BR2_TARGET_ROOTFS_BTRFS_SIZE_NODE
+	string "btree node size"
+	help
+	  The tree block size in which btrfs stores metadata. The default
+	  value is 16KiB (16384) or the page size, whichever is bigger.

How is the page size detected? I'd like to be sure this is detecting the page size of the target, not that of the build machine.

Also, is it possible to indicate the size with a 'k' like for the fs size, or must it be only bytes?

fs/btrfs/Config.in line 39

Quoted 11 lines
fs/btrfs/Config.in new file +11 -0
+	  Must be a multiple of the sectorsize, but not larger than 64KiB
+	  (65536).
+
+config BR2_TARGET_ROOTFS_BTRFS_SIZE_SECTOR
+	string "sector size"
+	help
+	  The default value is the page size and is autodetected. If the
+	  sectorsize differs from the page size, the created filesystem
+	  may not be mountable by the kernel. Therefore it is not
+	  recommended to use this option unless you are going to mount it
+	  on a system with the appropriate page size.

Given that the filsystem is specifically made for the target that will mount it, does it make sense to ffer that option at all? I.e. if the target uses a 4KiB page size, does it make sense to generatea rootfs that has a different secotr size, and thus not mountable on the target at all?

My suggestion: don't add that option.

fs/btrfs/Config.in line 46

Quoted 6 lines
fs/btrfs/Config.in new file +6 -0
+config BR2_TARGET_ROOTFS_BTRFS_FEATURES
+	string "Filesystem Features"
+	help
+	  A comma separated string of features that can be enabled
+	  during creation time.
+	  For a list of available options, use; `mkfs.btrfs -O list-all`

Nice. I guess that has to be done using the host-btrfs we built, as its feature list may be different than the one from the build machine.

Quoted 21 lines
fs/btrfs/Config.in new file +1 -0
+endif # BR2_TARGET_ROOTFS_BTRFS
fs/btrfs/btrfs.mk new file +13 -0
new file mode 100644
index 0000000000..9f474edac3
@@ -0,0 +1,45 @@
+################################################################################
+#
+# Build the btrfs root filesystem image
+#
+################################################################################
+
+BTRFS_SIZE = $(call qstrip,$(BR2_TARGET_ROOTFS_BTRFS_SIZE))
+ifeq ($(BR2_TARGET_ROOTFS_BTRFS)-$(BTRFS_SIZE),y-)
+$(error BR2_TARGET_ROOTFS_BTRFS_SIZE cannot be empty)
+endif
+
+BTRFS_SIZE_NODE = $(call qstrip,$(BR2_TARGET_ROOTFS_BTRFS_SIZE_NODE))
+BTRFS_SIZE_NODE_FLAG = $(if $(BTRFS_SIZE_NODE),--nodesize "$(BTRFS_SIZE_NODE)",)
 

fs/btrfs/btrfs.mk line 24

Quoted 10 lines
fs/btrfs/btrfs.mk new file +10 -0
+BTRFS_SIZE_SECTOR = $(call qstrip,$(BR2_TARGET_ROOTFS_BTRFS_SIZE_SECTOR))
+BTRFS_SIZE_SECTOR_FLAG = $(if $(BTRFS_SIZE_SECTOR),--sectorsize "$(BTRFS_SIZE_SECTOR)",)
+
+BTRFS_FEATURES = $(call qstrip,$(BR2_TARGET_ROOTFS_BTRFS_FEATURES))
+BTRFS_FEATURES_FLAG = $(if $(BTRFS_FEATURES),--features "$(BTRFS_FEATURES)",)
+
+# qstrip results in stripping consecutive spaces into a single one. So the
+# variable is not qstrip-ed to preserve the integrity of the string value.
+BTRFS_LABEL := $(subst ",,$(BR2_TARGET_ROOTFS_BTRFS_LABEL))
+## ")

Why double comment here?

fs/btrfs/btrfs.mk line 31

Quoted 6 lines
fs/btrfs/btrfs.mk new file +6 -0
+BTRFS_OPTS = \
+	-r $(TARGET_DIR) \
+	-L "$(BTRFS_LABEL)" \
+	$(BTRFS_SIZE_NODE_FLAG) \
+	$(BTRFS_SIZE_SECTOR_FLAG) \
+	$(BTRFS_FEATURES_FLAG)

It is a bit annoying that you have to qstrip the variables, just to re-add double quotes right after. However, I prefer the way you do it, because then it is obvious the strings are quotted when passed to the shell.

Furhtermore, I would quote with single quotes, to avoid shell expansion.

Also, no need for the trailing comma when there is no 'else' clause.

No need for intermediate variables for each option; just directly set the BTRFS_OPTS variable.

Finally, the label case is a bit annyoing, but here is no better solution, and is exactly what we do in ext2.mk.

    BTRFS_SIZE_NODE = $(call qstrip,$(BR2_TARGET_ROOTFS_BTRFS_SIZE_NODE))
    BTRFS_SIZE_SECTOR = $(call qstrip,$(BR2_TARGET_ROOTFS_BTRFS_SIZE_SECTOR))
    BTRFS_FEATURES = $(call qstrip,$(BR2_TARGET_ROOTFS_BTRFS_FEATURES))
    # qstrip results in stripping consecutive spaces into a single one. So the
    # variable is not qstrip-ed to preserve the integrity of the string value.
    BTRFS_LABEL := $(subst ",,$(BR2_TARGET_ROOTFS_BTRFS_LABEL))
    # ")

    BTRFS_OPTS = \
        -r '$(TARGET_DIR)' \
        -L '$(BTRFS_LABEL)' \
        $(if $(BTRFS_SIZE_NODE),--nodesize '$(BTRFS_SIZE_NODE)') \
        $(if $(BTRFS_SIZE_SECTOR),--sectorsize '$(BTRFS_SIZE_SECTOR)') \
        $(if $(BTRFS_FEATURES),--features '$(BTRFS_FEATURES)')

fs/btrfs/btrfs.mk line 37

Quoted 6 lines
fs/btrfs/btrfs.mk new file +6 -0
+
+ROOTFS_BTRFS_DEPENDENCIES = host-btrfs-progs
+
+define ROOTFS_BTRFS_CMD
+	rm -f $@
+	fallocate -l $(BTRFS_SIZE) $@

Is fallocate really necessary here? If the host's filesystem is too small to accomodate the image, then the mkfs.btrfs below fill fail; we don't need fallocate to fail.

Besides, is fallocate readilly available on all distros?

Usually, we use truncate(1) to that effect; see for example:
support/testing/tests/fs/test_ubi.py at 29

Regards,
Yann E. MORIN.

Quoted 15 lines
fs/btrfs/btrfs.mk new file +8 -0
+	$(HOST_DIR)/bin/mkfs.btrfs $(BTRFS_OPTS) $@ \
+	|| { ret=$$?; \
+	     echo "*** Maybe you need to increase the filesystem size (BR2_TARGET_ROOTFS_BTRFS_SIZE)" 1>&2; \
+	     exit $$ret; \
+	}
+endef
+
+$(eval $(rootfs))
Signature · 6 lines
2.11.0

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
#3 lore raw

PATCH 2/2 Adding btrfs rootfs option.

Hi Yann,
thanks for the through review :)
Comments inline.

Quoted 23 lines · Yann E. MORIN

Robert, All,

On 2018-08-21 17:04 +0100, Robert J. Heywood spake thusly:

Quoted 6 lines

This patch makes it possible to format the rootfs using btrfs. Introduces the option; BR2_TARGET_ROOTFS_BTRFS

When selected, the user is able to specify the filesystem size, label, options, and node and sector sizes.
The new files are based on fs/ext2/{Config.in,ext2.mk}

Nice addition! :-)

Quoted 1 line

[--SNIP--]

fs/btrfs/Config.in line 28

Quoted 5 lines
fs/btrfs/Config.in new file +5 -0
+config BR2_TARGET_ROOTFS_BTRFS_SIZE_NODE
+	string "btree node size"
+	help
+	  The tree block size in which btrfs stores metadata. The default
+	  value is 16KiB (16384) or the page size, whichever is bigger.

How is the page size detected? I'd like to be sure this is detecting the page size of the target, not that of the build machine.

Quoted 3 lines

Also, is it possible to indicate the size with a 'k' like for the fs size, or must it be only bytes?

Good question.
I guess because the makefile creates a sparse file with fallocate which mkfs.btrfs then formats, it won't be able to detect any page size (host or otherwise) and so will always be set to a default value of 16384. I'll set that in the Config file explicitly.

It's possible to specify the size in kilobytes, ie 4k for 4096. I can change the help text to reflect this.

Quoted 18 lines

fs/btrfs/Config.in line 39

Quoted 11 lines
fs/btrfs/Config.in new file +11 -0
+	  Must be a multiple of the sectorsize, but not larger than 64KiB
+	  (65536).
+
+config BR2_TARGET_ROOTFS_BTRFS_SIZE_SECTOR
+	string "sector size"
+	help
+	  The default value is the page size and is autodetected. If the
+	  sectorsize differs from the page size, the created filesystem
+	  may not be mountable by the kernel. Therefore it is not
+	  recommended to use this option unless you are going to mount it
+	  on a system with the appropriate page size.

Given that the filsystem is specifically made for the target that will mount it, does it make sense to ffer that option at all? I.e. if the target uses a 4KiB page size, does it make sense to generatea rootfs that has a different secotr size, and thus not mountable on the target at all?

Quoted 2 lines

My suggestion: don't add that option.

On my system (building for a raspi0) it autodetects the sector size to be 4k, and the resulting image is bootable. However I'm worried the host won't be able to detect the correct value for the target. In such a case I can see why it would be useful to set the sector size explicitly.

Is there another place where the sector size is specified? If so, it might be better to use the existing value instead.

Quoted 10 lines

fs/btrfs/Config.in line 46

Quoted 6 lines
fs/btrfs/Config.in new file +6 -0
+config BR2_TARGET_ROOTFS_BTRFS_FEATURES
+	string "Filesystem Features"
+	help
+	  A comma separated string of features that can be enabled
+	  during creation time.
+	  For a list of available options, use; `mkfs.btrfs -O list-all`

Nice. I guess that has to be done using the host-btrfs we built, as its feature list may be different than the one from the build machine.

I can change the help text to include the full path to the binary.

Quoted 21 lines
Quoted 21 lines
fs/btrfs/Config.in new file +1 -0
+endif # BR2_TARGET_ROOTFS_BTRFS
fs/btrfs/btrfs.mk new file +13 -0
new file mode 100644
index 0000000000..9f474edac3
@@ -0,0 +1,45 @@
+################################################################################
+#
+# Build the btrfs root filesystem image
+#
+################################################################################
+
+BTRFS_SIZE = $(call qstrip,$(BR2_TARGET_ROOTFS_BTRFS_SIZE))
+ifeq ($(BR2_TARGET_ROOTFS_BTRFS)-$(BTRFS_SIZE),y-)
+$(error BR2_TARGET_ROOTFS_BTRFS_SIZE cannot be empty)
+endif
+
+BTRFS_SIZE_NODE = $(call qstrip,$(BR2_TARGET_ROOTFS_BTRFS_SIZE_NODE))
+BTRFS_SIZE_NODE_FLAG = $(if $(BTRFS_SIZE_NODE),--nodesize "$(BTRFS_SIZE_NODE)",)
Quoted 13 lines

fs/btrfs/btrfs.mk line 24

Quoted 10 lines
fs/btrfs/btrfs.mk new file +10 -0
+BTRFS_SIZE_SECTOR = $(call qstrip,$(BR2_TARGET_ROOTFS_BTRFS_SIZE_SECTOR))
+BTRFS_SIZE_SECTOR_FLAG = $(if $(BTRFS_SIZE_SECTOR),--sectorsize "$(BTRFS_SIZE_SECTOR)",)
+
+BTRFS_FEATURES = $(call qstrip,$(BR2_TARGET_ROOTFS_BTRFS_FEATURES))
+BTRFS_FEATURES_FLAG = $(if $(BTRFS_FEATURES),--features "$(BTRFS_FEATURES)",)
+
+# qstrip results in stripping consecutive spaces into a single one. So the
+# variable is not qstrip-ed to preserve the integrity of the string value.
+BTRFS_LABEL := $(subst ",,$(BR2_TARGET_ROOTFS_BTRFS_LABEL))
+## ")

Why double comment here?

There is no reason for that. It was added when I first copied the ext2.mk file and commented out everything.
I'll remove one of them.

Quoted 12 lines

fs/btrfs/btrfs.mk line 31

Quoted 6 lines
fs/btrfs/btrfs.mk new file +6 -0
+BTRFS_OPTS = \
+	-r $(TARGET_DIR) \
+	-L "$(BTRFS_LABEL)" \
+	$(BTRFS_SIZE_NODE_FLAG) \
+	$(BTRFS_SIZE_SECTOR_FLAG) \
+	$(BTRFS_FEATURES_FLAG)

It is a bit annoying that you have to qstrip the variables, just to re-add double quotes right after. However, I prefer the way you do it, because then it is obvious the strings are quotted when passed to the shell.

Quoted 1 line

Furhtermore, I would quote with single quotes, to avoid shell expansion.

Sure thing.

Quoted 2 lines

Also, no need for the trailing comma when there is no 'else' clause.

I didn't know that! :)

Quoted 7 lines

No need for intermediate variables for each option; just directly set the BTRFS_OPTS variable.

Finally, the label case is a bit annyoing, but here is no better solution, and is exactly what we do in ext2.mk.

Quoted 15 lines
     BTRFS_SIZE_NODE = $(call qstrip,$(BR2_TARGET_ROOTFS_BTRFS_SIZE_NODE))
     BTRFS_SIZE_SECTOR = $(call qstrip,$(BR2_TARGET_ROOTFS_BTRFS_SIZE_SECTOR))
     BTRFS_FEATURES = $(call qstrip,$(BR2_TARGET_ROOTFS_BTRFS_FEATURES))
     # qstrip results in stripping consecutive spaces into a single one. So the
     # variable is not qstrip-ed to preserve the integrity of the string value.
     BTRFS_LABEL := $(subst ",,$(BR2_TARGET_ROOTFS_BTRFS_LABEL))
     # ")

     BTRFS_OPTS = \
         -r '$(TARGET_DIR)' \
         -L '$(BTRFS_LABEL)' \
         $(if $(BTRFS_SIZE_NODE),--nodesize '$(BTRFS_SIZE_NODE)') \
         $(if $(BTRFS_SIZE_SECTOR),--sectorsize '$(BTRFS_SIZE_SECTOR)') \
         $(if $(BTRFS_FEATURES),--features '$(BTRFS_FEATURES)')

Good suggestions.

Quoted 11 lines

fs/btrfs/btrfs.mk line 37

Quoted 6 lines
fs/btrfs/btrfs.mk new file +6 -0
+
+ROOTFS_BTRFS_DEPENDENCIES = host-btrfs-progs
+
+define ROOTFS_BTRFS_CMD
+	rm -f $@
+	fallocate -l $(BTRFS_SIZE) $@

Is fallocate really necessary here? If the host's filesystem is too small to accomodate the image, then the mkfs.btrfs below fill fail; we don't need fallocate to fail.

Quoted 5 lines

Besides, is fallocate readilly available on all distros?

Usually, we use truncate(1) to that effect; see for example:
support/testing/tests/fs/test_ubi.py at 29

fallocate is used here because (unlike other mkfs programs) you can't specify a size and have mkfs.btrfs create an image file for you. It needs a sparse file to work with.

I'll be happy to change this to truncate, as per your example.

Quoted 14 lines

Regards,
Yann E. MORIN.

Quoted 11 lines
fs/btrfs/btrfs.mk new file +8 -0
+	$(HOST_DIR)/bin/mkfs.btrfs $(BTRFS_OPTS) $@ \
+	|| { ret=$$?; \
+	     echo "*** Maybe you need to increase the filesystem size (BR2_TARGET_ROOTFS_BTRFS_SIZE)" 1>&2; \
+	     exit $$ret; \
+	}
+endef
+
+$(eval $(rootfs))
Signature · 1 line
2.11.0

regards, rob

#4 lore raw

PATCH 2/2 Adding btrfs rootfs option.

Robert, All,

On 2018-08-22 14:10 +0100, Robert Heywood spake thusly:

Quoted 21 lines · Yann E. MORIN
Quoted 12 lines

On 2018-08-21 17:04 +0100, Robert J. Heywood spake thusly:

Quoted 6 lines

This patch makes it possible to format the rootfs using btrfs. Introduces the option; BR2_TARGET_ROOTFS_BTRFS

When selected, the user is able to specify the filesystem size, label, options, and node and sector sizes.
The new files are based on fs/ext2/{Config.in,ext2.mk}

Nice addition! :-)

Quoted 1 line

[--SNIP--]

Quoted 8 lines

fs/btrfs/Config.in line 28

Quoted 5 lines
fs/btrfs/Config.in new file +5 -0
+config BR2_TARGET_ROOTFS_BTRFS_SIZE_NODE
+	string "btree node size"
+	help
+	  The tree block size in which btrfs stores metadata. The default
+	  value is 16KiB (16384) or the page size, whichever is bigger.

How is the page size detected? I'd like to be sure this is detecting the page size of the target, not that of the build machine.

Quoted 5 lines

Good question.
I guess because the makefile creates a sparse file with fallocate which mkfs.btrfs then formats, it won't be able to detect any page size (host or otherwise) and so will always be set to a default value of 16384. I'll set that in the Config file explicitly.

So, the mkfs.btrfs is a host tool; it has no knowledge of the target at all, so it can't possibly know the target page size. I believe the node size should default to 16384, so that we get an explicit sane default.

Users who would change that value would know what they would be doing.

Quoted 15 lines
Quoted 15 lines

fs/btrfs/Config.in line 39

Quoted 8 lines
fs/btrfs/Config.in new file +8 -0
+config BR2_TARGET_ROOTFS_BTRFS_SIZE_SECTOR
+	string "sector size"
+	help
+	  The default value is the page size and is autodetected. If the
+	  sectorsize differs from the page size, the created filesystem
+	  may not be mountable by the kernel. Therefore it is not
+	  recommended to use this option unless you are going to mount it
+	  on a system with the appropriate page size.

Given that the filsystem is specifically made for the target that will mount it, does it make sense to ffer that option at all? I.e. if the target uses a 4KiB page size, does it make sense to generatea rootfs that has a different secotr size, and thus not mountable on the target at all?

Quoted 7 lines
Quoted 2 lines

My suggestion: don't add that option.

On my system (building for a raspi0) it autodetects the sector size to be 4k, and the resulting image is bootable. However I'm worried the host won't be able to detect the correct value for the target. In such a case I can see why it would be useful to set the sector size explicitly.

Well, basically all architectures have a 4K page size, so does your x86_64 and so does the rpi (0/1/2/3), so the default for your host is valid for your target.

What architectures have a page-size that is not 4K?

Quoted 2 lines

Is there another place where the sector size is specified? If so, it might be better to use the existing value instead.

I would also add a default here, to 4096. Users who want to change that value would know what they would be doing.

Quoted 11 lines
Quoted 9 lines

fs/btrfs/Config.in line 46

Quoted 6 lines
fs/btrfs/Config.in new file +6 -0
+config BR2_TARGET_ROOTFS_BTRFS_FEATURES
+	string "Filesystem Features"
+	help
+	  A comma separated string of features that can be enabled
+	  during creation time.
+	  For a list of available options, use; `mkfs.btrfs -O list-all`

Nice. I guess that has to be done using the host-btrfs we built, as its feature list may be different than the one from the build machine.

I can change the help text to include the full path to the binary.

For a list of available options, see:
.../host/sbin/mkfs.btrfs -O list-all

[--SNIP--]

Quoted 4 lines
Quoted 4 lines

Is fallocate really necessary here? If the host's filesystem is too small to accomodate the image, then the mkfs.btrfs below fill fail; we don't need fallocate to fail.

Quoted 8 lines
Quoted 5 lines

Besides, is fallocate readilly available on all distros?

Usually, we use truncate(1) to that effect; see for example:
support/testing/tests/fs/test_ubi.py at 29

fallocate is used here because (unlike other mkfs programs) you can't specify a size and have mkfs.btrfs create an image file for you.

Yeah, I know. I was just questionning the use of fallocate for that, rather than the usual truncate we use everywhere else.

Quoted 2 lines

It needs a
sparse file to work with.

Except fallocate will not create a sparse file at all; it will really allocate all blocks the file needs.

Quoted 1 line

I'll be happy to change this to truncate, as per your example.

Yes, please, for consistency with the other parts of the code in Buildroot.

Thanks for the feedback; eager to read your v2. ;-)

Regards,
Yann E. MORIN.

Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
#5 lore raw

PATCH 2/2 Adding btrfs rootfs option.

Robert, All,

On 2018-08-22 18:57 +0200, Yann E. MORIN spake thusly:

Quoted 2 lines

On 2018-08-22 14:10 +0100, Robert Heywood spake thusly:

Quoted 1 line · Yann E. MORIN

[--SNIP--]

Quoted 13 lines
Quoted 6 lines
Quoted 6 lines

Given that the filsystem is specifically made for the target that will mount it, does it make sense to ffer that option at all? I.e. if the target uses a 4KiB page size, does it make sense to generatea rootfs that has a different secotr size, and thus not mountable on the target at all?

Quoted 6 lines
Quoted 1 line

My suggestion: don't add that option.

On my system (building for a raspi0) it autodetects the sector size to be 4k, and the resulting image is bootable. However I'm worried the host won't be able to detect the correct value for the target. In such a case I can see why it would be useful to set the sector size explicitly.

Quoted 5 lines

Well, basically all architectures have a 4K page size, so does your x86_64 and so does the rpi (0/1/2/3), so the default for your host is valid for your target.

What architectures have a page-size that is not 4K?

After discussing on IRC with Paul (Cc), it turns out that quite a few architectures can have a page size different from 4K, like mips, arc and a few others. Sometimes it is even configrable. Thanks Paul.

4K just happens to be the most common, but definitely not the only one.

Quoted 4 lines
Quoted 2 lines

Is there another place where the sector size is specified? If so, it might be better to use the existing value instead.

I would also add a default here, to 4096. Users who want to change that value would know what they would be doing.

So yeah, just keep that option, and let set the default to 4K. People who have a non 4K arch or kernel option will know to change that value.

Regards,
Yann E. MORIN.

Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
#6 lore raw

PATCH 1/2 Adding btrfs-progs host package.

In reply to Robert J. Heywood

Hello Robert,

Thanks for your contribution!

Quoted 9 lines · Robert J. Heywood

Adding a new option to build the btrfs toolset for the host.

---
 package/Config.in.host             | 1 +
 package/btrfs-progs/Config.in.host | 6 ++++++
 package/btrfs-progs/btrfs-progs.mk | 4 ++++
 3 files changed, 11 insertions(+)
 create mode 100644 package/btrfs-progs/Config.in.host

Applied to next after fixing the commit title to:

btrfs-progs: add host package variant

Indeed, we want all our commit title to look like this:

<package>: <description>

This should also be fixed for your PATCH 2/2, along with the other comments made by Yann E. Morin.

Thanks!

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com