diff mbox series

[v4] package/criu: new package

Message ID 20231020060236.3725194-1-marcus.folkesson@gmail.com
State Superseded
Headers show
Series [v4] package/criu: new package | expand

Commit Message

Marcus Folkesson Oct. 20, 2023, 6:02 a.m. UTC
Checkpoint/Restore In Userspace (CRIU), is a software tool for the
Linux operating system to make it possible to freeze a running
application and checkpoint it to persistent storage as a collection of files.

Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
---

Notes:
    v2:
     - Addressed comments from Thomas.
     - Tested on ARM target and on x86_64 with qemu.
    
    v3:
     - set CONFIG_CHECKPOINT_RESTORE in kernel config
     - Only be available for ARMv6, ARMv7 and ARMv8
    
    v4:
    - set SUBARCH for armv6, armv7 and armv8
    - Use github download helper
    
    Result from test-pkg:
                        bootlin-armv5-uclibc [1/6]: SKIPPED
                         bootlin-armv7-glibc [2/6]: OK
                       bootlin-armv7m-uclibc [3/6]: SKIPPED
                         bootlin-x86-64-musl [4/6]: OK
                          br-arm-full-static [5/6]: SKIPPED
                                sourcery-arm [6/6]: SKIPPED
    6 builds, 4 skipped, 0 build failed, 0 legal-info failed, 0 show-info failed

 DEVELOPERS             |  1 +
 package/Config.in      |  1 +
 package/criu/Config.in | 48 +++++++++++++++++++++++++++++
 package/criu/criu.hash |  3 ++
 package/criu/criu.mk   | 70 ++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 123 insertions(+)
 create mode 100644 package/criu/Config.in
 create mode 100644 package/criu/criu.hash
 create mode 100644 package/criu/criu.mk

Comments

Julien Olivain Oct. 21, 2023, 11:15 a.m. UTC | #1
Hi Marcus,

Thanks for the updated patch!

I confirm the normal test-pkg is passing. Then, I ran a "test-pkg -a"
(which test more toolchains/arches) and observed few new minor
failures. I'll comment a bit each failures and provide hints for
improvements, when possible.

     cat >criu.config <<EOF
     BR2_PACKAGE_HOST_PYTHON3=y
     BR2_PACKAGE_CRIU=y
     EOF
     utils/test-pkg -a -c criu.config -p criu
                      arm-aarch64 [ 1/45]: OK
            bootlin-aarch64-glibc [ 2/45]: FAILED

criu uses restartable sequences, see:
https://github.com/checkpoint-restore/criu/blob/v3.18/criu/include/linux/rseq.h#L7

Criu seems to have a header detection on "sys/rseq.h" (i.e. on the
libc side). But the sys/rseq.h then includes linux/rseq.h (i.e. on the
kernel side), which is not checked. Some configurations like
bootlin-aarch64-glibc includes the glibc header, but also have older
kernel. Restartable sequences were introduced in kernel >= 4.18. So
this can be handled by adding a dependency on kernel.

        bootlin-arcle-hs38-uclibc [ 3/45]: SKIPPED
             bootlin-armv5-uclibc [ 4/45]: SKIPPED
              bootlin-armv7-glibc [ 5/45]: OK
            bootlin-armv7m-uclibc [ 6/45]: SKIPPED
               bootlin-armv7-musl [ 7/45]: OK
         bootlin-m68k-5208-uclibc [ 8/45]: SKIPPED
        bootlin-m68k-68040-uclibc [ 9/45]: SKIPPED
      bootlin-microblazeel-uclibc [10/45]: SKIPPED
         bootlin-mipsel32r6-glibc [11/45]: SKIPPED
            bootlin-mipsel-uclibc [12/45]: SKIPPED
              bootlin-nios2-glibc [13/45]: SKIPPED
          bootlin-openrisc-uclibc [14/45]: SKIPPED
bootlin-powerpc64le-power8-glibc [15/45]: SKIPPED
    bootlin-powerpc-e500mc-uclibc [16/45]: SKIPPED
            bootlin-riscv32-glibc [17/45]: SKIPPED
            bootlin-riscv64-glibc [18/45]: SKIPPED
             bootlin-riscv64-musl [19/45]: SKIPPED
          bootlin-s390x-z13-glibc [20/45]: FAILED

Build fails with output:

     Makefile:23: *** "The architecture s390x isn't supported".  Stop.

criu Makefile checks for "s390" arch (but "s390x" is passed
here). See:
https://github.com/checkpoint-restore/criu/blob/v3.18/Makefile#L22

               bootlin-sh4-uclibc [21/45]: SKIPPED
            bootlin-sparc64-glibc [22/45]: SKIPPED
             bootlin-sparc-uclibc [23/45]: SKIPPED
             bootlin-x86-64-glibc [24/45]: OK
              bootlin-x86-64-musl [25/45]: OK
            bootlin-x86-64-uclibc [26/45]: FAILED

Build fails with output:

     criu/fsnotify.c:18:10: fatal error: aio.h: No such file or directory
        18 | #include <aio.h>
           |          ^~~~~~~

            bootlin-xtensa-uclibc [27/45]: SKIPPED
                     br-arm-basic [28/45]: SKIPPED
             br-arm-full-nothread [29/45]: SKIPPED
               br-arm-full-static [30/45]: SKIPPED
            br-i386-pentium4-full [31/45]: FAILED

Build fails with output:

     Makefile:23: *** "The architecture i686 isn't supported".  Stop.

         br-i386-pentium-mmx-musl [32/45]: FAILED

Build fails with output:

     Makefile:23: *** "The architecture i586 isn't supported".  Stop.

               br-mips64-n64-full [33/45]: SKIPPED
          br-mips64r6-el-hf-glibc [34/45]: SKIPPED
        br-powerpc-603e-basic-cpp [35/45]: SKIPPED
        br-powerpc64-power7-glibc [36/45]: FAILED

Build fails with output:

     In file included from compel/plugins/std/infect.c:14:
     compel/include/uapi/compel/asm/sigframe.h:27:2: error: #error Only 
supporting ABIv2.
        27 | #error Only supporting ABIv2.
           |  ^~~~~

                linaro-aarch64-be [37/45]: FAILED

Build fails with output:

     Makefile:23: *** "The architecture aarch64_be isn't supported".  
Stop.

                   linaro-aarch64 [38/45]: OK
                       linaro-arm [39/45]: FAILED

Build fails with output:

     arm-linux-gnueabihf-gcc: error: unrecognized argument in option 
'-march=armv7-a+fp'
     arm-linux-gnueabihf-gcc: note: valid arguments to '-march=' are: 
armv2 armv2a armv3 armv3m armv4 armv4t armv5 armv5e armv5t armv5te 
armv5tej armv6 armv6-m armv6j armv6k armv6kz armv6s-m armv6t2 armv6z 
armv6zk armv7 armv7-a armv7-m armv7-r armv7e-m armv7ve armv8-a armv8-a+ 
crc armv8-m.base armv8-m.main armv8-m.main+dsp armv8.1-a armv8.2-a 
armv8.2-a+dotprod armv8.2-a+fp16 armv8.2-a+fp16+dotprod iwmmxt iwmmxt2 
native; did you mean 'armv7-a'?

Gcc option "-march=armv7-a+fp" was introduced in gcc commit
https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=76d7d5334991a5646026e5aa8c3e7d23629f383a
first included in version 8.1.0.
So the package should have a dependency on this. linaro-arm fails
because it contains 7.3.1.

              sourcery-arm-armv4t [40/45]: SKIPPED
                     sourcery-arm [41/45]: SKIPPED
              sourcery-arm-thumb2 [42/45]: FAILED

Same story as linaro-arm toolchain.

                  sourcery-mips64 [43/45]: SKIPPED
                    sourcery-mips [44/45]: FAILED

Fails with output:

     compel/arch/mips/plugins/std/memcpy.S: Assembler messages:
     compel/arch/mips/plugins/std/memcpy.S:7: Error: opcode not supported 
on this processor: mips32r2 (mips32r2) 'dadd $2,$0,$4'
     compel/arch/mips/plugins/std/memcpy.S:8: Error: opcode not supported 
on this processor: mips32r2 (mips32r2) 'daddiu $13,$0,0'
     compel/arch/mips/plugins/std/memcpy.S:14: Error: opcode not 
supported on this processor: mips32r2 (mips32r2) 'daddiu $13,$13,1'
     compel/arch/mips/plugins/std/memcpy.S:15: Error: opcode not 
supported on this processor: mips32r2 (mips32r2) 'daddiu $4,$4,1'
     compel/arch/mips/plugins/std/memcpy.S:16: Error: opcode not 
supported on this processor: mips32r2 (mips32r2) 'daddiu $5,$5,1'

There is maybe few extra condtions missing in the _ARCH_SUPPORTS for 
mips.

                   sourcery-nios2 [45/45]: SKIPPED
45 builds, 29 skipped, 10 build failed, 0 legal-info failed, 0 show-info 
failed

To test thoroughly your package with a specific list of toolchains, you 
can use the
following commands:

     cp support/config-fragments/autobuild/toolchain-configs.csv 
criu-toolchains.csv
     # edit criu-toolchains.csv to keep your toolchains of interest.
     utils/test-pkg -a -t criu-toolchains.csv -c criu.config -p criu

This will retest only the toolchains kept in the csv.

On 20/10/2023 08:02, Marcus Folkesson wrote:
> Checkpoint/Restore In Userspace (CRIU), is a software tool for the
> Linux operating system to make it possible to freeze a running
> application and checkpoint it to persistent storage as a collection of 
> files.
> 
> Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
> ---
> 
> Notes:
>     v2:
>      - Addressed comments from Thomas.
>      - Tested on ARM target and on x86_64 with qemu.
> 
>     v3:
>      - set CONFIG_CHECKPOINT_RESTORE in kernel config
>      - Only be available for ARMv6, ARMv7 and ARMv8
> 
>     v4:
>     - set SUBARCH for armv6, armv7 and armv8
>     - Use github download helper
> 
>     Result from test-pkg:
>                         bootlin-armv5-uclibc [1/6]: SKIPPED
>                          bootlin-armv7-glibc [2/6]: OK
>                        bootlin-armv7m-uclibc [3/6]: SKIPPED
>                          bootlin-x86-64-musl [4/6]: OK
>                           br-arm-full-static [5/6]: SKIPPED
>                                 sourcery-arm [6/6]: SKIPPED
>     6 builds, 4 skipped, 0 build failed, 0 legal-info failed, 0 
> show-info failed
> 
>  DEVELOPERS             |  1 +
>  package/Config.in      |  1 +
>  package/criu/Config.in | 48 +++++++++++++++++++++++++++++
>  package/criu/criu.hash |  3 ++
>  package/criu/criu.mk   | 70 ++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 123 insertions(+)
>  create mode 100644 package/criu/Config.in
>  create mode 100644 package/criu/criu.hash
>  create mode 100644 package/criu/criu.mk
> 
> diff --git a/DEVELOPERS b/DEVELOPERS
> index 57015e245e..2047827bd9 100644
> --- a/DEVELOPERS
> +++ b/DEVELOPERS
> @@ -2007,6 +2007,7 @@ 
> F:	support/testing/tests/package/test_python_pytest.py
>  F:	support/testing/tests/package/test_python_pytest_asyncio.py
> 
>  N:	Marcus Folkesson <marcus.folkesson@gmail.com>
> +F:	package/criu/
>  F:	package/libcamera/
>  F:	package/libcamera-apps/
>  F:	package/libostree/
> diff --git a/package/Config.in b/package/Config.in
> index 4e489c4706..9e2099f6a5 100644
> --- a/package/Config.in
> +++ b/package/Config.in
> @@ -2678,6 +2678,7 @@ menu "System tools"
>  	source "package/coreutils/Config.in"
>  	source "package/cpulimit/Config.in"
>  	source "package/cpuload/Config.in"
> +	source "package/criu/Config.in"
>  	source "package/crun/Config.in"
>  	source "package/daemon/Config.in"
>  	source "package/dc3dd/Config.in"
> diff --git a/package/criu/Config.in b/package/criu/Config.in
> new file mode 100644
> index 0000000000..06e809fb83
> --- /dev/null
> +++ b/package/criu/Config.in
> @@ -0,0 +1,48 @@
> +# criu only builds on certain architectures
> +config BR2_PACKAGE_CRIU_ARCH_SUPPORTS
> +	bool
> +	default y if BR2_ARM_CPU_ARMV6
> +	default y if BR2_ARM_CPU_ARMV7A
> +	default y if BR2_ARM_CPU_ARMV7M
> +	default y if BR2_ARM_CPU_ARMV8A
> +	default y if BR2_ARM_CPU_ARMV8M
> +	default y if BR2_aarch64
> +	default y if BR2_i386
> +	default y if BR2_mips
> +	default y if BR2_x86_64
> +	default y if BR2_powerpc64
> +	default y if BR2_s390x

There is possibly some rework needed here to fix architecture
related build failures described earlier.

> +
> +menuconfig BR2_PACKAGE_CRIU
> +	bool "criu"
> +	depends on BR2_PACKAGE_CRIU_ARCH_SUPPORTS
> +	depends on BR2_PACKAGE_HOST_PYTHON3 # host-python3-ssl
> +	depends on BR2_PACKAGE_HOST_PROTOBUF_ARCH_SUPPORTS # protobuf-c
> +	depends on BR2_PACKAGE_PROTOBUF_ARCH_SUPPORTS
> +	depends on BR2_PACKAGE_LIBBSD_ARCH_SUPPORTS
> +	depends on BR2_INSTALL_LIBSTDCPP # protobuf
> +	depends on BR2_TOOLCHAIN_HAS_THREADS # protobuf, libnl

A dependency on kernel header >= 4.18 should be added:

     depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_4_18 # rseq.h

> +	depends on BR2_TOOLCHAIN_GCC_AT_LEAST_4_8 # protobuf

The condition can be raised to:

     depends on BR2_HOST_GCC_AT_LEAST_8 # -march=armv7-a+fp

If gcc older than 8 for non-arm architecture are really needed, it can
also be refined as something like "gcc >= 4.8 && !arm || gcc >= 8 && 
arm".
For reference, gcc 8.1 was release on 2018-05-02.

> +	depends on !BR2_STATIC_LIBS # protobuf, libbsd
> +	depends on BR2_USE_WCHAR # libbsd
> +	depends on BR2_USE_MMU # libcap
> +	select BR2_PACKAGE_HOST_PYTHON3_SSL
> +	select BR2_PACKAGE_PROTOBUF
> +	select BR2_PACKAGE_PROTOBUF_C
> +	select BR2_PACKAGE_LIBAIO
> +	select BR2_PACKAGE_LIBBSD
> +	select BR2_PACKAGE_LIBCAP
> +	select BR2_PACKAGE_LIBNET
> +	select BR2_PACKAGE_LIBNL
> +	select BR2_PACKAGE_PYTHON3
> +	help
> +	  Checkpoint/Restore In Userspace (CRIU), is a software
> +	  tool for the Linux operating system to make it possible
> +	  to freeze a running application and checkpoint it to
> +	  persistent storage as a collection of files.
> +
> +	  https://criu.org/Main_Page
> +
> +comment "criu needs a toolchain w/ threads, dynamic library, wchar"
> +	depends on !BR2_TOOLCHAIN_HAS_THREADS || BR2_STATIC_LIBS \
> +		|| !BR2_USE_WCHAR

The new toolchain dependencies you will add will need to be reflected
here.

> diff --git a/package/criu/criu.hash b/package/criu/criu.hash
> new file mode 100644
> index 0000000000..2c4a07252b
> --- /dev/null
> +++ b/package/criu/criu.hash
> @@ -0,0 +1,3 @@
> +# Locally calculated
> +sha256  
> 6a9997981c9fe4730c848ce59346b3a22fad69b803607cb67a3f6ec0557fa474  
> criu-3.18.tar.gz
> +sha256  
> 568a1fa9d90e18a1a1a61ea58ec2eece16b56a5042cc72c1b4f8d4455ae6fcb7  
> COPYING
> diff --git a/package/criu/criu.mk b/package/criu/criu.mk
> new file mode 100644
> index 0000000000..a84c625696
> --- /dev/null
> +++ b/package/criu/criu.mk
> @@ -0,0 +1,70 @@
> +################################################################################
> +#
> +# CRIU
> +#
> +################################################################################
> +
> +CRIU_VERSION = 3.18
> +CRIU_SITE = $(call github,checkpoint-restore,criu,v$(CRIU_VERSION))
> +
> +CRIU_LICENSE = GPL-2.0
> +CRIU_LICENSE_FILES = COPYING
> +CRIU_DEPENDENCIES =\
> +	host-pkgconf \
> +	host-protobuf-c \
> +	host-python3 \
> +	host-python-pip \
> +	libaio \
> +	libbsd \
> +	libcap \
> +	libnet \
> +	libnl \
> +	protobuf \
> +	protobuf-c \
> +	python3
> +
> +CRIU_MAKE_ENV =\
> +	$(TARGET_MAKE_ENV) \
> +	$(TARGET_CONFIGURE_OPTS) \
> +	CROSS_COMPILE=$(TARGET_CROSS) \
> +	WERROR=0
> +
> +#x86_64 is treated as x86 in criu
> +#Also, powerpc64 is refered to as ppc64 in criu.

There is possibly some rework needed here to fix architecture
related build failures described earlier.

> +ifeq ($(BR2_ARCH),"x86_64")
> +CRIU_MAKE_ENV += ARCH=x86
> +else ifeq ($(BR2_ARCH),"powerpc64")
> +CRIU_MAKE_ENV += ARCH=ppc64
> +else
> +CRIU_MAKE_ENV += ARCH=$(BR2_ARCH)
> +endif
> +
> +ifeq ($(BR2_ARM_CPU_ARMV6), y)
> +CRIU_MAKE_ENV += SUBARCH=armv6
> +else ifeq ($(BR2_ARM_CPU_ARMV7A), y)
> +CRIU_MAKE_ENV += SUBARCH=armv7
> +else ifeq ($(BR2_ARM_CPU_ARMV7M), y)
> +CRIU_MAKE_ENV += SUBARCH=armv7
> +else ifeq ($(BR2_ARM_CPU_ARMV8A), y)
> +CRIU_MAKE_ENV += SUBARCH=armv8
> +else ifeq ($(BR2_ARM_CPU_ARMV8M), y)
> +CRIU_MAKE_ENV += SUBARCH=armv8
> +endif
> +
> +# Criu needs Kernel Checkpoint/restore support which is not enabled
> +# by default.
> +define CRIU_LINUX_CONFIG_FIXUPS
> +	$(call KCONFIG_ENABLE_OPT,CONFIG_CHECKPOINT_RESTORE)
> +endef
> +
> +define CRIU_BUILD_CMDS
> +	rm -rf $(@D)/images/google/protobuf/descriptor.proto
> +	ln -s  $(STAGING_DIR)/usr/include/google/protobuf/descriptor.proto 
> $(@D)/images/google/protobuf/descriptor.proto
> +	$(CRIU_MAKE_ENV) $(MAKE) -C $(@D)
> +endef
> +
> +define CRIU_INSTALL_TARGET_CMDS
> +	$(CRIU_MAKE_ENV) $(MAKE) DESTDIR=$(TARGET_DIR) PREFIX=/usr -C $(@D) 
> install-criu install-lib install-compel
> +endef
> +
> +$(eval $(generic-package))
> --
> 2.41.0

Best regards,

Julien.
Marcus Folkesson Oct. 21, 2023, 3:25 p.m. UTC | #2
Hi Julien,

Thank you for your solid review!

I will apply all changes you suggested.

On Sat, Oct 21, 2023 at 01:15:10PM +0200, Julien Olivain wrote:
> Hi Marcus,
> 
> Thanks for the updated patch!
> 

[...]
>            bootlin-x86-64-uclibc [26/45]: FAILED
> 
> Build fails with output:
> 
>     criu/fsnotify.c:18:10: fatal error: aio.h: No such file or directory
>        18 | #include <aio.h>
>           |          ^~~~~~~

Not sure that to do about this. It already depends on libaio.
> 
>            bootlin-xtensa-uclibc [27/45]: SKIPPED
>                     br-arm-basic [28/45]: SKIPPED
>             br-arm-full-nothread [29/45]: SKIPPED
>               br-arm-full-static [30/45]: SKIPPED
>            br-i386-pentium4-full [31/45]: FAILED
> 

[...]

> 
>               br-mips64-n64-full [33/45]: SKIPPED
>          br-mips64r6-el-hf-glibc [34/45]: SKIPPED
>        br-powerpc-603e-basic-cpp [35/45]: SKIPPED
>        br-powerpc64-power7-glibc [36/45]: FAILED
> 
> Build fails with output:
> 
>     In file included from compel/plugins/std/infect.c:14:
>     compel/include/uapi/compel/asm/sigframe.h:27:2: error: #error Only
> supporting ABIv2.
>        27 | #error Only supporting ABIv2.
>           |  ^~~~~

I will only support PPC64 with LE. The GCC says that:
`
-mabi=elfv1
Change the current ABI to use the ELFv1 ABI. This is the default ABI for big-endian PowerPC 64-bit Linux. Overriding the default ABI requires special system support and is likely to fail in spectacular ways.

-mabi=elfv2
Change the current ABI to use the ELFv2 ABI. This is the default ABI for little-endian PowerPC 64-bit Linux. Overriding the default ABI requires special system support and is likely to fail in spectacular ways.
`

[...]
>                    sourcery-mips [44/45]: FAILED
> 
> Fails with output:
> 
>     compel/arch/mips/plugins/std/memcpy.S: Assembler messages:
>     compel/arch/mips/plugins/std/memcpy.S:7: Error: opcode not supported on
> this processor: mips32r2 (mips32r2) 'dadd $2,$0,$4'
>     compel/arch/mips/plugins/std/memcpy.S:8: Error: opcode not supported on
> this processor: mips32r2 (mips32r2) 'daddiu $13,$0,0'
>     compel/arch/mips/plugins/std/memcpy.S:14: Error: opcode not supported on
> this processor: mips32r2 (mips32r2) 'daddiu $13,$13,1'
>     compel/arch/mips/plugins/std/memcpy.S:15: Error: opcode not supported on
> this processor: mips32r2 (mips32r2) 'daddiu $4,$4,1'
>     compel/arch/mips/plugins/std/memcpy.S:16: Error: opcode not supported on
> this processor: mips32r2 (mips32r2) 'daddiu $5,$5,1'
> 
> There is maybe few extra condtions missing in the _ARCH_SUPPORTS for mips.

Hrm, will have a look. Thanks.
> 
>                   sourcery-nios2 [45/45]: SKIPPED
> 45 builds, 29 skipped, 10 build failed, 0 legal-info failed, 0 show-info
> failed
> 
> To test thoroughly your package with a specific list of toolchains, you can
> use the
> following commands:
> 
>     cp support/config-fragments/autobuild/toolchain-configs.csv
> criu-toolchains.csv
>     # edit criu-toolchains.csv to keep your toolchains of interest.
>     utils/test-pkg -a -t criu-toolchains.csv -c criu.config -p criu
> 
> This will retest only the toolchains kept in the csv.

Thanks for the tip!

[...]
> > +
> > +menuconfig BR2_PACKAGE_CRIU
> > +	bool "criu"
> > +	depends on BR2_PACKAGE_CRIU_ARCH_SUPPORTS
> > +	depends on BR2_PACKAGE_HOST_PYTHON3 # host-python3-ssl
> > +	depends on BR2_PACKAGE_HOST_PROTOBUF_ARCH_SUPPORTS # protobuf-c
> > +	depends on BR2_PACKAGE_PROTOBUF_ARCH_SUPPORTS
> > +	depends on BR2_PACKAGE_LIBBSD_ARCH_SUPPORTS
> > +	depends on BR2_INSTALL_LIBSTDCPP # protobuf
> > +	depends on BR2_TOOLCHAIN_HAS_THREADS # protobuf, libnl
> 
> A dependency on kernel header >= 4.18 should be added:
> 
>     depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_4_18 # rseq.h


This makes the bootlin-x86-64-musl test to be skipped as it has
BR2_TOOLCHAIN_HEADERS_AT_LEAST_4_14 at highest.

The build used to succeed though :-/

[...]
> > +################################################################################
> > +#
> > +# CRIU
> > +#
> > +################################################################################
> > +
> > +CRIU_VERSION = 3.18
> > +CRIU_SITE = $(call github,checkpoint-restore,criu,v$(CRIU_VERSION))
> > +
> > +CRIU_LICENSE = GPL-2.0
> > +CRIU_LICENSE_FILES = COPYING
> > +CRIU_DEPENDENCIES =\
> > +	host-pkgconf \
> > +	host-protobuf-c \
> > +	host-python3 \
> > +	host-python-pip \
> > +	libaio \
> > +	libbsd \
> > +	libcap \
> > +	libnet \
> > +	libnl \
> > +	protobuf \
> > +	protobuf-c \
> > +	python3
> > +
> > +CRIU_MAKE_ENV =\
> > +	$(TARGET_MAKE_ENV) \
> > +	$(TARGET_CONFIGURE_OPTS) \
> > +	CROSS_COMPILE=$(TARGET_CROSS) \
> > +	WERROR=0
> > +
> > +#x86_64 is treated as x86 in criu
> > +#Also, powerpc64 is refered to as ppc64 in criu.
> 
> There is possibly some rework needed here to fix architecture
> related build failures described earlier.
> 

I will go for $(BR2_NORMALIZED_ARCH) instead of $(BR2_ARCH), it will
solve the most of these type of errors.

> 
> Best regards,
> 
> Julien.
Marcus Folkesson Oct. 21, 2023, 4:27 p.m. UTC | #3
Hi again,

On Sat, Oct 21, 2023 at 01:15:10PM +0200, Julien Olivain wrote:
> Hi Marcus,
> 
> Thanks for the updated patch!
> 
> I confirm the normal test-pkg is passing. Then, I ran a "test-pkg -a"
> (which test more toolchains/arches) and observed few new minor
> failures. I'll comment a bit each failures and provide hints for
> improvements, when possible.
> 
>     cat >criu.config <<EOF
>     BR2_PACKAGE_HOST_PYTHON3=y
>     BR2_PACKAGE_CRIU=y
>     EOF
>     utils/test-pkg -a -c criu.config -p criu
>                      arm-aarch64 [ 1/45]: OK
>            bootlin-aarch64-glibc [ 2/45]: FAILED
> 
> criu uses restartable sequences, see:
> https://github.com/checkpoint-restore/criu/blob/v3.18/criu/include/linux/rseq.h#L7
> 
> Criu seems to have a header detection on "sys/rseq.h" (i.e. on the
> libc side). But the sys/rseq.h then includes linux/rseq.h (i.e. on the
> kernel side), which is not checked. Some configurations like
> bootlin-aarch64-glibc includes the glibc header, but also have older
> kernel. Restartable sequences were introduced in kernel >= 4.18. So
> this can be handled by adding a dependency on kernel.
> 
>        bootlin-arcle-hs38-uclibc [ 3/45]: SKIPPED
>             bootlin-armv5-uclibc [ 4/45]: SKIPPED
>              bootlin-armv7-glibc [ 5/45]: OK
>            bootlin-armv7m-uclibc [ 6/45]: SKIPPED
>               bootlin-armv7-musl [ 7/45]: OK
>         bootlin-m68k-5208-uclibc [ 8/45]: SKIPPED
>        bootlin-m68k-68040-uclibc [ 9/45]: SKIPPED
>      bootlin-microblazeel-uclibc [10/45]: SKIPPED
>         bootlin-mipsel32r6-glibc [11/45]: SKIPPED
>            bootlin-mipsel-uclibc [12/45]: SKIPPED
>              bootlin-nios2-glibc [13/45]: SKIPPED
>          bootlin-openrisc-uclibc [14/45]: SKIPPED
> bootlin-powerpc64le-power8-glibc [15/45]: SKIPPED
>    bootlin-powerpc-e500mc-uclibc [16/45]: SKIPPED
>            bootlin-riscv32-glibc [17/45]: SKIPPED
>            bootlin-riscv64-glibc [18/45]: SKIPPED
>             bootlin-riscv64-musl [19/45]: SKIPPED
>          bootlin-s390x-z13-glibc [20/45]: FAILED
> 
> Build fails with output:
> 
>     Makefile:23: *** "The architecture s390x isn't supported".  Stop.

Another problem that occured...
After I use $(BR2_NORMALIZED_ARCH) instead of $(BR2_ARCH), the arch
is at least supported. But it fails with

Error (compel/src/lib/handle-elf-host.c:161): Unsupported header detected
make[3]: *** [criu/pie/Makefile:58: criu/pie/parasite-blob.h] Error 255

After some more investigation, it ended up here:

static bool is_header_supported(Elf_Ehdr *hdr)
{
	if (!arch_is_machine_supported(hdr->e_machine))
		return false;
	if ((hdr->e_type != ET_REL
#ifdef NO_RELOCS
	     && hdr->e_type != ET_EXEC
#endif
	     ) ||
	    hdr->e_version != EV_CURRENT)
		return false;
	return true;
}

I've added debug trace to see why it fails and got this:
Error (compel/src/lib/handle-elf-host.c:155): 	type 0x100 machine 0x1600 version 0x1000000
Error (compel/src/lib/handle-elf-host.c:158): 	ET_REL 0x1 EM_s390 0x16, EV_CURRENT 0x1

It is a tool that runs on host (little endian), but read data of an elf
compiled for s390 (big endian), so bytes is read swapped.

Right now I thinking of not list s390 as a compatible architecture for
this package.

Do you think it is reasonable?


/Marcus
Julien Olivain Oct. 22, 2023, 11:30 a.m. UTC | #4
Hi Marcus,

On 21/10/2023 17:25, Marcus Folkesson wrote:
> Hi Julien,
> 
> Thank you for your solid review!
> 
> I will apply all changes you suggested.
> 
> On Sat, Oct 21, 2023 at 01:15:10PM +0200, Julien Olivain wrote:
>> Hi Marcus,
>> 
>> Thanks for the updated patch!
>> 
> 
> [...]
>>            bootlin-x86-64-uclibc [26/45]: FAILED
>> 
>> Build fails with output:
>> 
>>     criu/fsnotify.c:18:10: fatal error: aio.h: No such file or 
>> directory
>>        18 | #include <aio.h>
>>           |          ^~~~~~~
> 
> Not sure that to do about this. It already depends on libaio.

There is something strange here, in the criu code:

libaio (Linux AIO) normally provides the "libaio.h" header file. See:
https://pagure.io/libaio/blob/master/f/src/libaio.h

This header file is used exactly once in a criu test, here:
https://github.com/checkpoint-restore/criu/blob/v3.18/test/zdtm/static/aio00.c#L1

The rest of the code uses "aio.h" and <aio.h> (Posix AIO).

criu code also includes its own "aio.h" file:
https://github.com/checkpoint-restore/criu/blob/v3.18/criu/include/aio.h

The <aio.h> header file is normally provided by the libc, if
implemented. See glibc for example:
https://sourceware.org/git/?p=glibc.git;a=blob;f=include/aio.h

musl does also provide aio.h, see for example:
https://git.musl-libc.org/cgit/musl/tree/include/aio.h?h=v1.2.4

uclibc-ng does not currently provide aio.h.

So maybe this means criu requires a libc with aio.h?
In that case, that would mean this packages need a dependency on:

     depends on !BR2_TOOLCHAIN_USES_UCLIBC # no aio.h

Were you ever able to do a successful build with uclibc?

Best regards,

Julien.
Julien Olivain Oct. 22, 2023, 11:37 a.m. UTC | #5
Hi Marcus,

On 21/10/2023 17:25, Marcus Folkesson wrote:
> Hi Julien,
> 

[...]

>> > +
>> > +menuconfig BR2_PACKAGE_CRIU
>> > +	bool "criu"
>> > +	depends on BR2_PACKAGE_CRIU_ARCH_SUPPORTS
>> > +	depends on BR2_PACKAGE_HOST_PYTHON3 # host-python3-ssl
>> > +	depends on BR2_PACKAGE_HOST_PROTOBUF_ARCH_SUPPORTS # protobuf-c
>> > +	depends on BR2_PACKAGE_PROTOBUF_ARCH_SUPPORTS
>> > +	depends on BR2_PACKAGE_LIBBSD_ARCH_SUPPORTS
>> > +	depends on BR2_INSTALL_LIBSTDCPP # protobuf
>> > +	depends on BR2_TOOLCHAIN_HAS_THREADS # protobuf, libnl
>> 
>> A dependency on kernel header >= 4.18 should be added:
>> 
>>     depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_4_18 # rseq.h
> 
> 
> This makes the bootlin-x86-64-musl test to be skipped as it has
> BR2_TOOLCHAIN_HEADERS_AT_LEAST_4_14 at highest.
> 
> The build used to succeed though :-/

Ah, good point. Adding this kernel >= 4.18 requirement would
indeed exclude some working cases...

criu has some logic to detect if a libc provides rseq.h. See:
https://github.com/checkpoint-restore/criu/blob/v3.18/scripts/feature-tests.mak#L187
which define CONFIG_HAS_NO_LIBC_RSEQ_DEFS.

In the case of the working bootlin-x86-64-musl toolchain, we have a
4.14 kernel without linux/rseq.h AND a libc also not providing
sys/rseq.h.

In the failing bootlin-aarch64-glibc toolchain, we have a kernel
headers without linux/rseq.h, but a glibc with sys/rseq.h.

I did not investigated any further.

Maybe there is a corner case in the criu detection logic and it could
be patched to support it?

Best regards,

Julien.
Julien Olivain Oct. 22, 2023, 11:46 a.m. UTC | #6
Hi Marcus,

On 21/10/2023 18:27, Marcus Folkesson wrote:
> Hi again,
> 
> On Sat, Oct 21, 2023 at 01:15:10PM +0200, Julien Olivain wrote:
>> Hi Marcus,
>> 
>> Thanks for the updated patch!
>> 
>> I confirm the normal test-pkg is passing. Then, I ran a "test-pkg -a"
>> (which test more toolchains/arches) and observed few new minor
>> failures. I'll comment a bit each failures and provide hints for
>> improvements, when possible.
>> 
>>     cat >criu.config <<EOF
>>     BR2_PACKAGE_HOST_PYTHON3=y
>>     BR2_PACKAGE_CRIU=y
>>     EOF
>>     utils/test-pkg -a -c criu.config -p criu
>>                      arm-aarch64 [ 1/45]: OK
>>            bootlin-aarch64-glibc [ 2/45]: FAILED
>> 
>> criu uses restartable sequences, see:
>> https://github.com/checkpoint-restore/criu/blob/v3.18/criu/include/linux/rseq.h#L7
>> 
>> Criu seems to have a header detection on "sys/rseq.h" (i.e. on the
>> libc side). But the sys/rseq.h then includes linux/rseq.h (i.e. on the
>> kernel side), which is not checked. Some configurations like
>> bootlin-aarch64-glibc includes the glibc header, but also have older
>> kernel. Restartable sequences were introduced in kernel >= 4.18. So
>> this can be handled by adding a dependency on kernel.
>> 
>>        bootlin-arcle-hs38-uclibc [ 3/45]: SKIPPED
>>             bootlin-armv5-uclibc [ 4/45]: SKIPPED
>>              bootlin-armv7-glibc [ 5/45]: OK
>>            bootlin-armv7m-uclibc [ 6/45]: SKIPPED
>>               bootlin-armv7-musl [ 7/45]: OK
>>         bootlin-m68k-5208-uclibc [ 8/45]: SKIPPED
>>        bootlin-m68k-68040-uclibc [ 9/45]: SKIPPED
>>      bootlin-microblazeel-uclibc [10/45]: SKIPPED
>>         bootlin-mipsel32r6-glibc [11/45]: SKIPPED
>>            bootlin-mipsel-uclibc [12/45]: SKIPPED
>>              bootlin-nios2-glibc [13/45]: SKIPPED
>>          bootlin-openrisc-uclibc [14/45]: SKIPPED
>> bootlin-powerpc64le-power8-glibc [15/45]: SKIPPED
>>    bootlin-powerpc-e500mc-uclibc [16/45]: SKIPPED
>>            bootlin-riscv32-glibc [17/45]: SKIPPED
>>            bootlin-riscv64-glibc [18/45]: SKIPPED
>>             bootlin-riscv64-musl [19/45]: SKIPPED
>>          bootlin-s390x-z13-glibc [20/45]: FAILED
>> 
>> Build fails with output:
>> 
>>     Makefile:23: *** "The architecture s390x isn't supported".  Stop.
> 
> Another problem that occured...
> After I use $(BR2_NORMALIZED_ARCH) instead of $(BR2_ARCH), the arch
> is at least supported. But it fails with
> 
> Error (compel/src/lib/handle-elf-host.c:161): Unsupported header 
> detected
> make[3]: *** [criu/pie/Makefile:58: criu/pie/parasite-blob.h] Error 255
> 
> After some more investigation, it ended up here:
> 
> static bool is_header_supported(Elf_Ehdr *hdr)
> {
> 	if (!arch_is_machine_supported(hdr->e_machine))
> 		return false;
> 	if ((hdr->e_type != ET_REL
> #ifdef NO_RELOCS
> 	     && hdr->e_type != ET_EXEC
> #endif
> 	     ) ||
> 	    hdr->e_version != EV_CURRENT)
> 		return false;
> 	return true;
> }
> 
> I've added debug trace to see why it fails and got this:
> Error (compel/src/lib/handle-elf-host.c:155): 	type 0x100 machine 
> 0x1600 version 0x1000000
> Error (compel/src/lib/handle-elf-host.c:158): 	ET_REL 0x1 EM_s390 0x16, 
> EV_CURRENT 0x1
> 
> It is a tool that runs on host (little endian), but read data of an elf
> compiled for s390 (big endian), so bytes is read swapped.
> 
> Right now I thinking of not list s390 as a compatible architecture for
> this package.
> 
> Do you think it is reasonable?

Yes, I think it is reasonable to exclude s390 for now.

You can also add a comment about that in the _ARCH_SUPPORTS, that
criu does have "some" support for s390 but it is not included due
to this endianess issue.

This can be improved later, if there is Buildroot criu users on s390.

Best regards,

Julien.
diff mbox series

Patch

diff --git a/DEVELOPERS b/DEVELOPERS
index 57015e245e..2047827bd9 100644
--- a/DEVELOPERS
+++ b/DEVELOPERS
@@ -2007,6 +2007,7 @@  F:	support/testing/tests/package/test_python_pytest.py
 F:	support/testing/tests/package/test_python_pytest_asyncio.py
 
 N:	Marcus Folkesson <marcus.folkesson@gmail.com>
+F:	package/criu/
 F:	package/libcamera/
 F:	package/libcamera-apps/
 F:	package/libostree/
diff --git a/package/Config.in b/package/Config.in
index 4e489c4706..9e2099f6a5 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -2678,6 +2678,7 @@  menu "System tools"
 	source "package/coreutils/Config.in"
 	source "package/cpulimit/Config.in"
 	source "package/cpuload/Config.in"
+	source "package/criu/Config.in"
 	source "package/crun/Config.in"
 	source "package/daemon/Config.in"
 	source "package/dc3dd/Config.in"
diff --git a/package/criu/Config.in b/package/criu/Config.in
new file mode 100644
index 0000000000..06e809fb83
--- /dev/null
+++ b/package/criu/Config.in
@@ -0,0 +1,48 @@ 
+# criu only builds on certain architectures
+config BR2_PACKAGE_CRIU_ARCH_SUPPORTS
+	bool
+	default y if BR2_ARM_CPU_ARMV6
+	default y if BR2_ARM_CPU_ARMV7A
+	default y if BR2_ARM_CPU_ARMV7M
+	default y if BR2_ARM_CPU_ARMV8A
+	default y if BR2_ARM_CPU_ARMV8M
+	default y if BR2_aarch64
+	default y if BR2_i386
+	default y if BR2_mips
+	default y if BR2_x86_64
+	default y if BR2_powerpc64
+	default y if BR2_s390x
+
+menuconfig BR2_PACKAGE_CRIU
+	bool "criu"
+	depends on BR2_PACKAGE_CRIU_ARCH_SUPPORTS
+	depends on BR2_PACKAGE_HOST_PYTHON3 # host-python3-ssl
+	depends on BR2_PACKAGE_HOST_PROTOBUF_ARCH_SUPPORTS # protobuf-c
+	depends on BR2_PACKAGE_PROTOBUF_ARCH_SUPPORTS
+	depends on BR2_PACKAGE_LIBBSD_ARCH_SUPPORTS
+	depends on BR2_INSTALL_LIBSTDCPP # protobuf
+	depends on BR2_TOOLCHAIN_HAS_THREADS # protobuf, libnl
+	depends on BR2_TOOLCHAIN_GCC_AT_LEAST_4_8 # protobuf
+	depends on !BR2_STATIC_LIBS # protobuf, libbsd
+	depends on BR2_USE_WCHAR # libbsd
+	depends on BR2_USE_MMU # libcap
+	select BR2_PACKAGE_HOST_PYTHON3_SSL
+	select BR2_PACKAGE_PROTOBUF
+	select BR2_PACKAGE_PROTOBUF_C
+	select BR2_PACKAGE_LIBAIO
+	select BR2_PACKAGE_LIBBSD
+	select BR2_PACKAGE_LIBCAP
+	select BR2_PACKAGE_LIBNET
+	select BR2_PACKAGE_LIBNL
+	select BR2_PACKAGE_PYTHON3
+	help
+	  Checkpoint/Restore In Userspace (CRIU), is a software
+	  tool for the Linux operating system to make it possible
+	  to freeze a running application and checkpoint it to
+	  persistent storage as a collection of files.
+
+	  https://criu.org/Main_Page
+
+comment "criu needs a toolchain w/ threads, dynamic library, wchar"
+	depends on !BR2_TOOLCHAIN_HAS_THREADS || BR2_STATIC_LIBS \
+		|| !BR2_USE_WCHAR
diff --git a/package/criu/criu.hash b/package/criu/criu.hash
new file mode 100644
index 0000000000..2c4a07252b
--- /dev/null
+++ b/package/criu/criu.hash
@@ -0,0 +1,3 @@ 
+# Locally calculated
+sha256  6a9997981c9fe4730c848ce59346b3a22fad69b803607cb67a3f6ec0557fa474  criu-3.18.tar.gz
+sha256  568a1fa9d90e18a1a1a61ea58ec2eece16b56a5042cc72c1b4f8d4455ae6fcb7  COPYING
diff --git a/package/criu/criu.mk b/package/criu/criu.mk
new file mode 100644
index 0000000000..a84c625696
--- /dev/null
+++ b/package/criu/criu.mk
@@ -0,0 +1,70 @@ 
+################################################################################
+#
+# CRIU
+#
+################################################################################
+
+CRIU_VERSION = 3.18
+CRIU_SITE = $(call github,checkpoint-restore,criu,v$(CRIU_VERSION))
+
+CRIU_LICENSE = GPL-2.0
+CRIU_LICENSE_FILES = COPYING
+CRIU_DEPENDENCIES =\
+	host-pkgconf \
+	host-protobuf-c \
+	host-python3 \
+	host-python-pip \
+	libaio \
+	libbsd \
+	libcap \
+	libnet \
+	libnl \
+	protobuf \
+	protobuf-c \
+	python3
+
+CRIU_MAKE_ENV =\
+	$(TARGET_MAKE_ENV) \
+	$(TARGET_CONFIGURE_OPTS) \
+	CROSS_COMPILE=$(TARGET_CROSS) \
+	WERROR=0
+
+#x86_64 is treated as x86 in criu
+#Also, powerpc64 is refered to as ppc64 in criu.
+ifeq ($(BR2_ARCH),"x86_64")
+CRIU_MAKE_ENV += ARCH=x86
+else ifeq ($(BR2_ARCH),"powerpc64")
+CRIU_MAKE_ENV += ARCH=ppc64
+else
+CRIU_MAKE_ENV += ARCH=$(BR2_ARCH)
+endif
+
+ifeq ($(BR2_ARM_CPU_ARMV6), y)
+CRIU_MAKE_ENV += SUBARCH=armv6
+else ifeq ($(BR2_ARM_CPU_ARMV7A), y)
+CRIU_MAKE_ENV += SUBARCH=armv7
+else ifeq ($(BR2_ARM_CPU_ARMV7M), y)
+CRIU_MAKE_ENV += SUBARCH=armv7
+else ifeq ($(BR2_ARM_CPU_ARMV8A), y)
+CRIU_MAKE_ENV += SUBARCH=armv8
+else ifeq ($(BR2_ARM_CPU_ARMV8M), y)
+CRIU_MAKE_ENV += SUBARCH=armv8
+endif
+
+# Criu needs Kernel Checkpoint/restore support which is not enabled
+# by default.
+define CRIU_LINUX_CONFIG_FIXUPS
+	$(call KCONFIG_ENABLE_OPT,CONFIG_CHECKPOINT_RESTORE)
+endef
+
+define CRIU_BUILD_CMDS
+	rm -rf $(@D)/images/google/protobuf/descriptor.proto
+	ln -s  $(STAGING_DIR)/usr/include/google/protobuf/descriptor.proto $(@D)/images/google/protobuf/descriptor.proto
+	$(CRIU_MAKE_ENV) $(MAKE) -C $(@D)
+endef
+
+define CRIU_INSTALL_TARGET_CMDS
+	$(CRIU_MAKE_ENV) $(MAKE) DESTDIR=$(TARGET_DIR) PREFIX=/usr -C $(@D) install-criu install-lib install-compel
+endef
+
+$(eval $(generic-package))