Re: [Buildroot] [PATCH v3 3/3] fs/cpio: new option to use dracut tool

["Yann E. MORIN" <yann.morin.1998@free.fr>]

Thierry, All,

On 2021-12-23 12:13 +0100, Thierry Bultel spake thusly:
> Adds an option to invoke the dracut host tool, providing
> a configuration file, instead of having a full cpio archive
> of the whole target directory.
> 
> Signed-off-by: Thierry Bultel <thierry.bultel@linatsea.fr>
[--SNIP--]
> diff --git a/fs/cpio/Config.in b/fs/cpio/Config.in
> index c1151a2881..3d0f963a8d 100644
> --- a/fs/cpio/Config.in
> +++ b/fs/cpio/Config.in
> @@ -7,6 +7,33 @@ config BR2_TARGET_ROOTFS_CPIO
>  
>  if BR2_TARGET_ROOTFS_CPIO
>  
> +choice
> +	prompt "cpio type"

Missing empty line between the choice prompt and the first choice entry.

> +config BR2_TARGET_ROOTFS_CPIO_FULL
> +	bool "cpio the whole root filesystem (ie the content of 'target')"
> +	help
> +	  Build a cpio archive containing the whole the root filesystem.
> +
> +config BR2_TARGET_ROOTFS_CPIO_DRACUT
> +	bool "Invoke dracut to make an initramfs"
> +	select BR2_PACKAGE_HOST_DRACUT
> +	select BR2_PACKAGE_HOST_UBOOT_TOOLS
> +	help
> +	  Builds an additional initramfs using dracut.
> +	  This can be useful to create a recovery system,
> +	  for instance.
> +
> +if BR2_TARGET_ROOTFS_CPIO_DRACUT
> +config BR2_TARGET_ROOTFS_CPIO_DRACUT_CONF_FILE
> +	string "configuration file"
> +endif

Don't add the option in the choice, but outside of it.

> diff --git a/fs/cpio/cpio.mk b/fs/cpio/cpio.mk
> index 81f8c393d1..e0e0eb2b1e 100644
> --- a/fs/cpio/cpio.mk
> +++ b/fs/cpio/cpio.mk
> @@ -29,6 +29,41 @@ endif # BR2_ROOTFS_DEVICE_CREATION_STATIC
>  
>  ROOTFS_CPIO_PRE_GEN_HOOKS += ROOTFS_CPIO_ADD_INIT
>  
> +ifeq ($(BR2_TARGET_ROOTFS_CPIO_DRACUT),y)
> +
> +export TARGET_CROSS

Don't do such a global export. If you need it in the environment, then
do so when calling the utility, like:

    TARGET_CROSS="$(TARGET_CROSS)" \
    $(HOST_DIR)/bin/dracut \
        blabla other options...

> +ROOTFS_CPIO_DEPENDENCIES += host-dracut
> +
> +ifeq ($(BR2_LINUX_KERNEL),y)
> +ROOTFS_CPIO_DEPENDENCIES += linux
> +endif
> +
> +ifeq ($(BR_BUILDING).$(BR2_TARGET_ROOTFS_CPIO_DRACUT_CONF_FILE),y.)
> +$(error No dracut config file name specified, check your BR2_TARGET_ROOTFS_CPIO_DRACUT_CONF_FILE setting)
> +endif
> +
> +ifeq ($(BR2_LINUX_KERNEL),y)
> +ROOTFS_CPIO_DRACUT_CMD_OPTS += --kver $(LINUX_VERSION_PROBED)
> +else
> +ROOTFS_CPIO_DRACUT_CMD_OPTS += --no-kernel
> +endif
> +
> +define ROOTFS_CPIO_CMD
> +	mkdir -p $(@D)/tmp
> +	rm -rf $(@D)/tmp/*

The $(@D) for ROOTFS_*_CMD is $(BINARIES_DIR), i.e. output/images/ and
so creating a te,mporary directory in there is not so nice. You should
use $(ROOTFS_CPIO_DIR)/tmp

Also, if I understand correctly, you are doing this mkdir+rm trick to
ensure you have an empty tmp directory to start with. By using
$(ROOTFS_CPIO_DIR) as base, this will be guaranteed, because
$(ROOTFS_CPIO_DIR) is removed and recreated before running the fs
commands.

> +	$(HOST_DIR)/usr/bin/dracut_wrapper.sh \
> +		$(ROOTFS_CPIO_DRACUT_CMD_OPTS) \
> +		-c $(BR2_TARGET_ROOTFS_CPIO_DRACUT_CONF_FILE) \
> +		--tmpdir $(@D)/tmp \
> +		-M \
> +		--force \
> +		--keep \
> +		$@
> +endef
> +
> +else ifeq ($(BR2_TARGET_ROOTFS_CPIO_FULL),y)

Please, keep the original, plain cpio first in the code, and add the
dracut specificities after.

>  # --reproducible option was introduced in cpio v2.12, which may not be
>  # available in some old distributions, so we build host-cpio
>  ifeq ($(BR2_REPRODUCIBLE),y)
> @@ -53,4 +88,6 @@ endef
>  ROOTFS_CPIO_POST_GEN_HOOKS += ROOTFS_CPIO_UBOOT_MKIMAGE
>  endif
>  
> +endif #BR2_TARGET_ROOTFS_CPIO_DRACUT
> +
>  $(eval $(rootfs))
> diff --git a/support/testing/conf/dracut.conf b/support/testing/conf/dracut.conf
> new file mode 100644
> index 0000000000..eb793a3562
> --- /dev/null
> +++ b/support/testing/conf/dracut.conf
> @@ -0,0 +1,93 @@
> +#Yuco dracut config

"Yuco"?

> +#Dracut configuration
> +
> +show_modules=yes
> +i18n_install_all=no
> +lvmconf=no
> +mdadmconf=no
> +early_microcode=no
> +hostonly=no
> +hostonly_cmdline=no
> +use_fstab=no
> +kernel_cmdline="rd.break=initqueue"
> +do_strip=no
> +
> +# Dracut modules need
> +add_dracutmodules+=" \
> +busybox-buildroot \
> +bash
> +"
> +
> +# Modules to ignore
> +omit_dracutmodules+=" \
[--SNIP big list of stuff--]
> +"

Rather than remove select stuff, can't we just start from an empty set
and add just what is needed?

Regards,
Yann E. MORIN.

> +
> diff --git a/support/testing/tests/fs/test_cpio.py b/support/testing/tests/fs/test_cpio.py
> new file mode 100644
> index 0000000000..5570693dc2
> --- /dev/null
> +++ b/support/testing/tests/fs/test_cpio.py
> @@ -0,0 +1,44 @@
> +import os
> +import infra.basetest
> +
> +CHECK_FS_CMD = "mount | grep 'rootfs on / type rootfs'"
> +
> +def boot_img(emulator, builddir):
> +    img = os.path.join(builddir, "images", "rootfs.cpio")
> +    emulator.boot(arch="armv7",
> +                  kernel="builtin",
> +                  options=["-initrd", "{}".format(img)])
> +    emulator.login()
> +    _, exit_code = emulator.run(CHECK_FS_CMD)
> +    return exit_code
> +
> +class TestCpioDracut(infra.basetest.BRTest):
> +    config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + \
> +        """
> +        BR2_INIT_BUSYBOX=y
> +        BR2_TARGET_ROOTFS_CPIO=y
> +        BR2_TARGET_ROOTFS_CPIO_DRACUT=y
> +        BR2_TARGET_ROOTFS_CPIO_DRACUT_CONF_FILE="{}"
> +        """.format(infra.filepath("conf/dracut.conf"))
> +
> +    def test_run(self):
> +
> +        exit_code = boot_img(self.emulator,
> +                             self.builddir)
> +        self.assertEqual(exit_code, 0)
> +
> +class TestCpioFull(infra.basetest.BRTest):
> +    config = infra.basetest.BASIC_TOOLCHAIN_CONFIG + \
> +        """
> +        BR2_INIT_BUSYBOX=y
> +        BR2_TARGET_ROOTFS_CPIO=y
> +        BR2_TARGET_ROOTFS_CPIO_FULL=y
> +        """
> +
> +    def test_run(self):
> +
> +        exit_code = boot_img(self.emulator,
> +                             self.builddir)
> +        self.assertEqual(exit_code, 0)
> +
> +
> -- 
> 2.25.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot