| Message ID | 20230908082741.409005-1-marcus.folkesson@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/2] package/criu: new package | expand |
Hello Marcus, Thanks for your contribution! See some suggestions below. On Fri, 8 Sep 2023 10:27:40 +0200 Marcus Folkesson <marcus.folkesson@gmail.com> 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. Commit log should be wrapped to ~80 columns. > > Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com> > --- > package/Config.in | 1 + > package/criu/Config.in | 28 ++++++++++++++++++++++++++++ > package/criu/criu.hash | 3 +++ > package/criu/criu.mk | 42 ++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 74 insertions(+) An entry in the DEVELOPERS file should be added in this commit (not as a separate patch). > diff --git a/package/criu/Config.in b/package/criu/Config.in > new file mode 100644 > index 0000000000..158e7ced57 > --- /dev/null > +++ b/package/criu/Config.in > @@ -0,0 +1,28 @@ > +menuconfig BR2_PACKAGE_CRIU > + bool "criu" > + 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_TOOLCHAIN_USES_MUSL Where does this dependency come from? > + depends on !BR2_STATIC_LIBS # protobuf, libbsd > + depends on BR2_USE_WCHAR #libbsd > + depends on BR2_USE_MMU #libcap Spaces after # sign. depends on BR2_PACKAGE_HOST_PROTOBUF_ARCH_SUPPORTS # protobuf-c is missing. > + 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 > + select BR2_PACKAGE_PYTHON_PIP It needs pip on the target? Seems odd. > + 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. This needs to be wrapped to the proper length. Run "make check-package" to get the details. Also we need the help text to end with a blank line, followed by the URL of the upstream project. > diff --git a/package/criu/criu.mk b/package/criu/criu.mk > new file mode 100644 > index 0000000000..01b07e3f3f > --- /dev/null > +++ b/package/criu/criu.mk > @@ -0,0 +1,42 @@ > +################################################################################ > +# > +# CRIU > +# > +################################################################################ > + > +CRIU_VERSION = 3.18 > +CRIU_SOURCE = criu-$(CRIU_VERSION).tar.gz > +CRIU_SITE = https://github.com/checkpoint-restore/criu/archive/refs/tags/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 You can split this long line: CRIU_DEPENDENCIES = \ foo \ bar \ baz > + > +CRIU_CFLAGS = $(TARGET_CFLAGS) > +CRIU_CFLAGS += -D__WORDSIZE -D__USE_GNU -D_GNU_SOURCE Can be: CRIU_CFLAGS = \ $(TARGET_CFLAGS) \ -D__WORDSIZE \ -D... However, this is odd. Why aren't those flags set by the package Makefile? > + > +CRIU_MAKE_ENV = $(TARGET_MAKE_ENV) > +CRIU_MAKE_ENV += WERROR=0 CRIU_MAKE_ENV = \ $(TARGET_MAKE_ENV) \ WERROR=0 > + > +#Needed as it adds strange paths to the tool otherwise. E.g. $CROSS_COMPILE/usr/bin/gcc > +CRIU_MAKE_ENV += HOSTLD=ld > +CRIU_MAKE_ENV += HOSTCC=gcc Meh. Not sure to understand here. Maybe you want to pass $(TARGET_CONFIGURE_OPTS), which does include HOSTCC, HOSTLD, and more. > + > +#x86_64 is treated as x86 in criu > +ifeq ($(BR2_ARCH),x86_64) > + CRIU_MAKE_ENV += ARCH=x86 Don't indent this line. > +else > + CRIU_MAKE_ENV += ARCH=$(BR2_ARCH) Ditto. In the Makefile: ifneq ($(filter-out x86 arm aarch64 ppc64 s390 mips loongarch64,$(ARCH)),) $(error "The architecture $(ARCH) isn't supported") endif So you need a BR2_PACKAGE_CRIU_ARCH_SUPPORTS option that lists those architectures only. > +endif > + > +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 Please indent those lines with one tab. > + $(CRIU_MAKE_ENV) $(MAKE) USERCFLAGS="$(CRIU_CFLAGS)" -C $(@D) I don't understand how this can know which cross-compiler to use, you are not passing it anywhere as far as I can see. Could you have a look at addressing those comments and sending an updated version? Thanks a lot! Thomas
Hi Thomas, Thank you for your solid review. Apparently I haven't done my homework very well. Sorry for that. On Fri, Sep 08, 2023 at 10:57:04PM +0200, Thomas Petazzoni wrote: > Hello Marcus, > > Thanks for your contribution! See some suggestions below. > > On Fri, 8 Sep 2023 10:27:40 +0200 > Marcus Folkesson <marcus.folkesson@gmail.com> wrote: > [...] > > + bool "criu" > > + 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_TOOLCHAIN_USES_MUSL > > Where does this dependency come from? I'm pretty sure that I read that CRIU does not compile with musl somewhere, but it seems like it does when I had a closer look. I will remove it. [...] > > > + 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 > > + select BR2_PACKAGE_PYTHON_PIP > > It needs pip on the target? Seems odd. What I actually need is host-python-pip as it is used during the installation step. Is there a way to only select the host-part from the PYTHON_PIP package? > > > + 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. > > This needs to be wrapped to the proper length. Run "make check-package" > to get the details. Thank you. [...] > > + > > +CRIU_CFLAGS = $(TARGET_CFLAGS) > > +CRIU_CFLAGS += -D__WORDSIZE -D__USE_GNU -D_GNU_SOURCE > > Can be: > > CRIU_CFLAGS = \ > $(TARGET_CFLAGS) \ > -D__WORDSIZE \ > -D... > > However, this is odd. Why aren't those flags set by the package > Makefile? I actually took those flags from the yocto recipe [1], but at least _GNU_SOURCE seems to be in the package Makefile anyway. I will remove __USE_GNU and _GNU-SOURCE. Do not know about __WORDSIZE though. [...] > > + > > +#Needed as it adds strange paths to the tool otherwise. E.g. $CROSS_COMPILE/usr/bin/gcc > > +CRIU_MAKE_ENV += HOSTLD=ld > > +CRIU_MAKE_ENV += HOSTCC=gcc > > Meh. Not sure to understand here. Maybe you want to pass > $(TARGET_CONFIGURE_OPTS), which does include HOSTCC, HOSTLD, and more. Sounds better. [...] > > Ditto. > > In the Makefile: > > ifneq ($(filter-out x86 arm aarch64 ppc64 s390 mips loongarch64,$(ARCH)),) > $(error "The architecture $(ARCH) isn't supported") > endif > > So you need a BR2_PACKAGE_CRIU_ARCH_SUPPORTS option that lists those > architectures only. Will do. [...] > > > +endif > > + > > +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 > > Please indent those lines with one tab. > > > + $(CRIU_MAKE_ENV) $(MAKE) USERCFLAGS="$(CRIU_CFLAGS)" -C $(@D) > > I don't understand how this can know which cross-compiler to use, you > are not passing it anywhere as far as I can see. Hrm, I had CROSS_COMPILE=$(TARGET_CROSS) at first, then I though I saw that $(TARGET_MAKE_ENV) had it set. Unfortunately, my last test was was on x86_64, so I did not notice it was wrong. I will put it back. > > Could you have a look at addressing those comments and sending an > updated version? > > Thanks a lot! > > Thomas > -- > Thomas Petazzoni, co-owner and CEO, Bootlin > Embedded Linux and Kernel engineering and training > https://bootlin.com [1] https://git.yoctoproject.org/meta-virtualization/tree/recipes-containers/criu/criu_git.bb Thanks, Marcus
On Sat, 9 Sep 2023 15:03:13 +0200 Marcus Folkesson <marcus.folkesson@gmail.com> wrote: > Thank you for your solid review. > Apparently I haven't done my homework very well. Sorry for that. Ah, ah, no worries! > > Where does this dependency come from? > > I'm pretty sure that I read that CRIU does not compile with musl > somewhere, but it seems like it does when I had a closer look. > I will remove it. Good. > > > + select BR2_PACKAGE_PYTHON_PIP > > > > It needs pip on the target? Seems odd. > > What I actually need is host-python-pip as it is used during the > installation step. Is there a way to only select the host-part from the > PYTHON_PIP package? For most packages, selecting them is useless, and actually most host packages don't even have a Config.in.host option to select in the first place. So having host-python-pip in your CRIU_DEPENDENCIES variable is enough. However, a package that needs pip during its build process kind of raises a red flag. Is that package using pip to download/install additional Python packages during its build procedure? If it's the case, that's not good, as this downloading of extra stuff by the package would work around the download logic and legal-info logic of Buildroot. > > CRIU_CFLAGS = \ > > $(TARGET_CFLAGS) \ > > -D__WORDSIZE \ > > -D... > > > > However, this is odd. Why aren't those flags set by the package > > Makefile? > > I actually took those flags from the yocto recipe [1], but at least > _GNU_SOURCE seems to be in the package Makefile anyway. > > I will remove __USE_GNU and _GNU-SOURCE. > Do not know about __WORDSIZE though. But what do you have it here, then? > > > + $(CRIU_MAKE_ENV) $(MAKE) USERCFLAGS="$(CRIU_CFLAGS)" -C $(@D) > > > > I don't understand how this can know which cross-compiler to use, you > > are not passing it anywhere as far as I can see. > > Hrm, I had CROSS_COMPILE=$(TARGET_CROSS) at first, then I though I saw > that $(TARGET_MAKE_ENV) had it set. TARGET_MAKE_ENV does not contain CROSS_COMPILE. TARGET_CONFIGURE_OPTS does, however. > Unfortunately, my last test was was on x86_64, so I did not notice it > was wrong. > > I will put it back. One good thing would be to have a runtime test for criu, in support/testing. Julien Olivain (in Cc) can probably provide some guidance here. Thanks a lot! Thomas
Hi Thomas, Marcus, On 09/09/2023 15:55, Thomas Petazzoni wrote: > On Sat, 9 Sep 2023 15:03:13 +0200 > Marcus Folkesson <marcus.folkesson@gmail.com> wrote: > >> Thank you for your solid review. [...] > One good thing would be to have a runtime test for criu, in > support/testing. Julien Olivain (in Cc) can probably provide some > guidance here. I should be able to easily write a runtime test once the package will be merged. I happen to be a criu user. I will probably do a test inspired from: https://criu.org/Simple_loop#Simplest_case While looking at it, I was not able to build criu for arm or aarch64, on branch master at commit 177170f. As Marcus said, it was apparently tested only on x8_64. It seems there issues with CFLAGS and libraries. Possibly mixups with host files. It's probably linked with the TARGET_MAKE_ENV and TARGET_CONFIGURE_OPTS mixups. I got error messages like: soccr/soccr.c:2:10: fatal error: libnet.h: No such file or directory 2 | #include <libnet.h> | ^~~~~~~~~~ ... compel/arch/aarch64/plugins/std/parasite-head.S: Assembler messages: compel/arch/aarch64/plugins/std/parasite-head.S:4: Error: unrecognized symbol type "" compel/arch/aarch64/plugins/std/parasite-head.S:5: Error: no such instruction: `bl parasite_service' compel/arch/aarch64/plugins/std/parasite-head.S:6: Error: no such instruction: `brk ' ... In file included from /usr/include/signal.h:301, from compel/include/uapi/compel/plugins/std/syscall-types.h:12, from compel/include/uapi/compel/plugins/std/syscall.h:4, from compel/include/uapi/compel/plugins/std.h:5, from compel/plugins/std/infect.c:1: /usr/include/bits/sigcontext.h:31:8: note: originally defined here 31 | struct _fpx_sw_bytes | ^~~~~~~~~~~~~ ... In file included from images/creds.pb-c.c:9: images/creds.pb-c.h:7:10: fatal error: protobuf-c/protobuf-c.h: No such file or directory 7 | #include <protobuf-c/protobuf-c.h> | ^~~~~~~~~~~~~~~~~~~~~~~~~ Marcus, you should be able to reproduce that with the command: utils/test-pkg -p criu Ideally, the package should have no failure (i.e. only OK or SKIPPED) when testing all toolchains, with: utils/test-pkg -a -p criu Then, you should be able to see the end of build logs with: tail ~/br-test-pkg/*/logfile For the details, see: https://nightly.buildroot.org/manual.html#testing-package Best regards, Julien.
Hi Julien, On Sat, Sep 09, 2023 at 11:16:27PM +0200, Julien Olivain wrote: > Hi Thomas, Marcus, > > On 09/09/2023 15:55, Thomas Petazzoni wrote: > > On Sat, 9 Sep 2023 15:03:13 +0200 > > Marcus Folkesson <marcus.folkesson@gmail.com> wrote: > > > > > Thank you for your solid review. > > [...] > > > One good thing would be to have a runtime test for criu, in > > support/testing. Julien Olivain (in Cc) can probably provide some > > guidance here. > > > Marcus, you should be able to reproduce that with the command: > > utils/test-pkg -p criu I've sent out a v2 where the comments from Thomas is addressed. But I am not able to reproduce the errors you got with test-pkg. I'm not sure why it skips my tests. Could you please provide some guidance? > > Ideally, the package should have no failure (i.e. only OK or SKIPPED) > when testing all toolchains, with: > > utils/test-pkg -a -p criu > > Then, you should be able to see the end of build logs with: > > tail ~/br-test-pkg/*/logfile > > For the details, see: > https://nightly.buildroot.org/manual.html#testing-package > > Best regards, > > Julien. Best regards Marcus Folkesson
Hi Marcus, On 10/09/2023 21:41, Marcus Folkesson wrote: > Hi Julien, > > On Sat, Sep 09, 2023 at 11:16:27PM +0200, Julien Olivain wrote: >> Hi Thomas, Marcus, >> >> On 09/09/2023 15:55, Thomas Petazzoni wrote: >> > On Sat, 9 Sep 2023 15:03:13 +0200 >> > Marcus Folkesson <marcus.folkesson@gmail.com> wrote: >> > >> > > Thank you for your solid review. >> >> [...] >> >> > One good thing would be to have a runtime test for criu, in >> > support/testing. Julien Olivain (in Cc) can probably provide some >> > guidance here. >> >> >> Marcus, you should be able to reproduce that with the command: >> >> utils/test-pkg -p criu > > I've sent out a v2 where the comments from Thomas is addressed. But I > am > not able to reproduce the errors you got with test-pkg. > > I'm not sure why it skips my tests. Could you please provide some > guidance? All test are skipped because of the "depends on BR2_PACKAGE_HOST_PYTHON3" not being set by default. I'm not sure is this "depends on" is really needed, as I expect it to be a build dependency. If it's the case, the dependency is only needed in the .mk file. I let the reviewers comment the v2 patch on that topic... Anyways, for the moment, this can be managed by manually creating a config setting this for test-pkg. For example: cat > criu.config <<EOF BR2_PACKAGE_HOST_PYTHON3=y BR2_PACKAGE_CRIU=y EOF Then, test-pkg will no longer skip the tests. On my side, I now see: utils/test-pkg -c criu.config -p criu bootlin-armv5-uclibc [1/6]: FAILED bootlin-armv7-glibc [2/6]: FAILED bootlin-armv7m-uclibc [3/6]: SKIPPED bootlin-x86-64-musl [4/6]: OK br-arm-full-static [5/6]: SKIPPED sourcery-arm [6/6]: FAILED 6 builds, 2 skipped, 3 build failed, 0 legal-info failed, 0 show-info failed I let you inspect the build log files ~/br-test-pkg/*/logfile for the details. With this v2, I am now able to build criu on some x86_64 and Aarch64 configurations, so I can start writing a runtime test ;) >> Ideally, the package should have no failure (i.e. only OK or SKIPPED) >> when testing all toolchains, with: >> >> utils/test-pkg -a -p criu Then, once the test-pkg (without "-a") will have no failure, it will be a good test to retry it _with_ the "-a" option: utils/test-pkg -c criu.config -p criu >> For the details, see: >> https://nightly.buildroot.org/manual.html#testing-package Thanks again! Julien.
Hi Julien, On Sun, Sep 10, 2023 at 11:05:19PM +0200, Julien Olivain wrote: > Hi Marcus, > > On 10/09/2023 21:41, Marcus Folkesson wrote: > > Hi Julien, > > > > On Sat, Sep 09, 2023 at 11:16:27PM +0200, Julien Olivain wrote: > > > Hi Thomas, Marcus, > > > > > > On 09/09/2023 15:55, Thomas Petazzoni wrote: > > > > On Sat, 9 Sep 2023 15:03:13 +0200 > > > > Marcus Folkesson <marcus.folkesson@gmail.com> wrote: > > > > > > > > > Thank you for your solid review. > > > > > > [...] > > > > > > > One good thing would be to have a runtime test for criu, in > > > > support/testing. Julien Olivain (in Cc) can probably provide some > > > > guidance here. > > > > > > > > > Marcus, you should be able to reproduce that with the command: > > > > > > utils/test-pkg -p criu > > > > I've sent out a v2 where the comments from Thomas is addressed. But I am > > not able to reproduce the errors you got with test-pkg. > > > > I'm not sure why it skips my tests. Could you please provide some > > guidance? > > All test are skipped because of the "depends on > BR2_PACKAGE_HOST_PYTHON3" not being set by default. > > I'm not sure is this "depends on" is really needed, as I expect it > to be a build dependency. If it's the case, the dependency is only > needed in the .mk file. I let the reviewers comment the v2 patch > on that topic... > > Anyways, for the moment, this can be managed by manually creating a > config setting this for test-pkg. For example: > > cat > criu.config <<EOF > BR2_PACKAGE_HOST_PYTHON3=y > BR2_PACKAGE_CRIU=y > EOF > > Then, test-pkg will no longer skip the tests. On my side, I now see: > > utils/test-pkg -c criu.config -p criu > bootlin-armv5-uclibc [1/6]: FAILED > bootlin-armv7-glibc [2/6]: FAILED > bootlin-armv7m-uclibc [3/6]: SKIPPED > bootlin-x86-64-musl [4/6]: OK > br-arm-full-static [5/6]: SKIPPED > sourcery-arm [6/6]: FAILED > 6 builds, 2 skipped, 3 build failed, 0 legal-info failed, 0 show-info > failed > > I let you inspect the build log files ~/br-test-pkg/*/logfile for > the details. The error seems to be related to host-libzlib rather than criu :-/ make[1]: Leaving directory '/home/marcus/mnt/encwork/git/br-test-pkg/bootlin-armv5-uclibc/build/host-libzlib-1.3' *** *** ERROR: package host-libzlib installs executables without proper RPATH: *** /home/marcus/mnt/encwork/git/br-test-pkg/bootlin-armv5-uclibc/host/bin/xmlwf *** /home/marcus/mnt/encwork/git/br-test-pkg/bootlin-armv5-uclibc/host/bin/pkgconf *** /home/marcus/mnt/encwork/git/br-test-pkg/bootlin-armv5-uclibc/host/bin/protoc *** /home/marcus/mnt/encwork/git/br-test-pkg/bootlin-armv5-uclibc/host/bin/protoc-gen-c *** /home/marcus/mnt/encwork/git/br-test-pkg/bootlin-armv5-uclibc/host/bin/python3.11 *** /home/marcus/mnt/encwork/git/br-test-pkg/bootlin-armv5-uclibc/host/bin/openssl make: *** [package/pkg-generic.mk:305: /home/marcus/mnt/encwork/git/br-test-pkg/bootlin-armv5-uclibc/build/host-libzlib-1.3/.stamp_host_installed] Error 1 > > With this v2, I am now able to build criu on some x86_64 and Aarch64 > configurations, so I can start writing a runtime test ;) Yeay! > Thanks again! > > Julien. /Marcus
On Tue, 12 Sep 2023 14:53:37 +0200 Marcus Folkesson <marcus.folkesson@gmail.com> wrote: > make[1]: Leaving directory '/home/marcus/mnt/encwork/git/br-test-pkg/bootlin-armv5-uclibc/build/host-libzlib-1.3' > *** > *** ERROR: package host-libzlib installs executables without proper RPATH: > *** /home/marcus/mnt/encwork/git/br-test-pkg/bootlin-armv5-uclibc/host/bin/xmlwf > *** /home/marcus/mnt/encwork/git/br-test-pkg/bootlin-armv5-uclibc/host/bin/pkgconf > *** /home/marcus/mnt/encwork/git/br-test-pkg/bootlin-armv5-uclibc/host/bin/protoc > *** /home/marcus/mnt/encwork/git/br-test-pkg/bootlin-armv5-uclibc/host/bin/protoc-gen-c > *** /home/marcus/mnt/encwork/git/br-test-pkg/bootlin-armv5-uclibc/host/bin/python3.11 > *** /home/marcus/mnt/encwork/git/br-test-pkg/bootlin-armv5-uclibc/host/bin/openssl This seems pretty broken as none of these files are installed by host-libzlib... Thomas
Hi Marcus, On 12/09/2023 14:53, Marcus Folkesson wrote: > Hi Julien, > > > On Sun, Sep 10, 2023 at 11:05:19PM +0200, Julien Olivain wrote: >> Hi Marcus, >> >> On 10/09/2023 21:41, Marcus Folkesson wrote: >> > Hi Julien, >> > >> > On Sat, Sep 09, 2023 at 11:16:27PM +0200, Julien Olivain wrote: >> > > Hi Thomas, Marcus, >> > > >> > > On 09/09/2023 15:55, Thomas Petazzoni wrote: >> > > > On Sat, 9 Sep 2023 15:03:13 +0200 >> > > > Marcus Folkesson <marcus.folkesson@gmail.com> wrote: >> > > > >> > > > > Thank you for your solid review. >> > > >> > > [...] >> > > >> > > > One good thing would be to have a runtime test for criu, in >> > > > support/testing. Julien Olivain (in Cc) can probably provide some >> > > > guidance here. >> > > >> > > >> > > Marcus, you should be able to reproduce that with the command: >> > > >> > > utils/test-pkg -p criu >> > >> > I've sent out a v2 where the comments from Thomas is addressed. But I am >> > not able to reproduce the errors you got with test-pkg. >> > >> > I'm not sure why it skips my tests. Could you please provide some >> > guidance? >> >> All test are skipped because of the "depends on >> BR2_PACKAGE_HOST_PYTHON3" not being set by default. >> >> I'm not sure is this "depends on" is really needed, as I expect it >> to be a build dependency. If it's the case, the dependency is only >> needed in the .mk file. I let the reviewers comment the v2 patch >> on that topic... >> >> Anyways, for the moment, this can be managed by manually creating a >> config setting this for test-pkg. For example: >> >> cat > criu.config <<EOF >> BR2_PACKAGE_HOST_PYTHON3=y >> BR2_PACKAGE_CRIU=y >> EOF >> >> Then, test-pkg will no longer skip the tests. On my side, I now see: >> >> utils/test-pkg -c criu.config -p criu >> bootlin-armv5-uclibc [1/6]: FAILED >> bootlin-armv7-glibc [2/6]: FAILED >> bootlin-armv7m-uclibc [3/6]: SKIPPED >> bootlin-x86-64-musl [4/6]: OK >> br-arm-full-static [5/6]: SKIPPED >> sourcery-arm [6/6]: FAILED >> 6 builds, 2 skipped, 3 build failed, 0 legal-info failed, 0 >> show-info >> failed >> >> I let you inspect the build log files ~/br-test-pkg/*/logfile for >> the details. > > > The error seems to be related to host-libzlib rather than criu :-/ > > > make[1]: Leaving directory > '/home/marcus/mnt/encwork/git/br-test-pkg/bootlin-armv5-uclibc/build/host-libzlib-1.3' > *** > *** ERROR: package host-libzlib installs executables without proper > RPATH: > *** > /home/marcus/mnt/encwork/git/br-test-pkg/bootlin-armv5-uclibc/host/bin/xmlwf > *** > /home/marcus/mnt/encwork/git/br-test-pkg/bootlin-armv5-uclibc/host/bin/pkgconf > *** > /home/marcus/mnt/encwork/git/br-test-pkg/bootlin-armv5-uclibc/host/bin/protoc > *** > /home/marcus/mnt/encwork/git/br-test-pkg/bootlin-armv5-uclibc/host/bin/protoc-gen-c > *** > /home/marcus/mnt/encwork/git/br-test-pkg/bootlin-armv5-uclibc/host/bin/python3.11 > *** > /home/marcus/mnt/encwork/git/br-test-pkg/bootlin-armv5-uclibc/host/bin/openssl > make: *** [package/pkg-generic.mk:305: > /home/marcus/mnt/encwork/git/br-test-pkg/bootlin-armv5-uclibc/build/host-libzlib-1.3/.stamp_host_installed] > Error 1 For information, on my side, I see the criu package build failing for various reasons. For example, toolchain bootlin-armv5-uclibc is failing with: compel/arch/arm/src/lib/infect.c: In function ‘compel_set_task_ext_regs’: compel/arch/arm/src/lib/infect.c:107:20: error: ‘PTRACE_SETVFPREGS’ undeclared (first use in this function); did you mean ‘PTRACE_SETFPREGS’? 107 | if (ptrace(PTRACE_SETVFPREGS, pid, NULL, ext_regs)) { | ^~~~~~~~~~~~~~~~~ | PTRACE_SETFPREGS Toolchain bootlin-armv7-glibc failing with: /br-test-pkg/bootlin-armv7-glibc/host/bin/arm-linux-ld: ./compel/plugins/std.lib.a(log.o): in function `std_vprint_num': log.c:(.text+0x2c0): undefined reference to `__aeabi_idivmod' /br-test-pkg/bootlin-armv7-glibc/host/bin/arm-linux-ld: ./compel/plugins/std.lib.a(log.o): in function `sbuf_printf': log.c:(.text+0x41c): undefined reference to `__aeabi_idivmod' /br-test-pkg/bootlin-armv7-glibc/host/bin/arm-linux-ld: ./compel/plugins/std.lib.a(string.o): in function `std_vdprintf': string.c:(.text+0x114): undefined reference to `__aeabi_idivmod' There is possibly others. I'll try to investigate how to resolve those issues. >> With this v2, I am now able to build criu on some x86_64 and Aarch64 >> configurations, so I can start writing a runtime test ;) > > Yeay! I have now a fully working test on Aarch64. I'll send it when criu will be merged. Best regards, Julien.
Hi Julien, On Tue, Sep 12, 2023 at 11:53:32PM +0200, Julien Olivain wrote: > Hi Marcus, > > On 12/09/2023 14:53, Marcus Folkesson wrote: > > Hi Julien, > > > > > > On Sun, Sep 10, 2023 at 11:05:19PM +0200, Julien Olivain wrote: > > > Hi Marcus, > > > > > > On 10/09/2023 21:41, Marcus Folkesson wrote: > > > > Hi Julien, > > > > > > > > On Sat, Sep 09, 2023 at 11:16:27PM +0200, Julien Olivain wrote: > > > > > Hi Thomas, Marcus, > > > > > > > > > > On 09/09/2023 15:55, Thomas Petazzoni wrote: > > > > > > On Sat, 9 Sep 2023 15:03:13 +0200 > > > > > > Marcus Folkesson <marcus.folkesson@gmail.com> wrote: > > > > > > > > > > > > > Thank you for your solid review. > > > > > > > > > > [...] > > > > > > > > > > > One good thing would be to have a runtime test for criu, in > > > > > > support/testing. Julien Olivain (in Cc) can probably provide some > > > > > > guidance here. > > > > > > > > > > > > > > > Marcus, you should be able to reproduce that with the command: > > > > > > > > > > utils/test-pkg -p criu > > > > > > > > I've sent out a v2 where the comments from Thomas is addressed. But I am > > > > not able to reproduce the errors you got with test-pkg. > > > > > > > > I'm not sure why it skips my tests. Could you please provide some > > > > guidance? > > > > > > All test are skipped because of the "depends on > > > BR2_PACKAGE_HOST_PYTHON3" not being set by default. > > > > > > I'm not sure is this "depends on" is really needed, as I expect it > > > to be a build dependency. If it's the case, the dependency is only > > > needed in the .mk file. I let the reviewers comment the v2 patch > > > on that topic... > > > > > > Anyways, for the moment, this can be managed by manually creating a > > > config setting this for test-pkg. For example: > > > > > > cat > criu.config <<EOF > > > BR2_PACKAGE_HOST_PYTHON3=y > > > BR2_PACKAGE_CRIU=y > > > EOF > > > > > > Then, test-pkg will no longer skip the tests. On my side, I now see: > > > > > > utils/test-pkg -c criu.config -p criu > > > bootlin-armv5-uclibc [1/6]: FAILED > > > bootlin-armv7-glibc [2/6]: FAILED > > > bootlin-armv7m-uclibc [3/6]: SKIPPED > > > bootlin-x86-64-musl [4/6]: OK > > > br-arm-full-static [5/6]: SKIPPED > > > sourcery-arm [6/6]: FAILED > > > 6 builds, 2 skipped, 3 build failed, 0 legal-info failed, 0 > > > show-info > > > failed > > > > > > I let you inspect the build log files ~/br-test-pkg/*/logfile for > > > the details. > > > > > > The error seems to be related to host-libzlib rather than criu :-/ > > > > > > make[1]: Leaving directory '/home/marcus/mnt/encwork/git/br-test-pkg/bootlin-armv5-uclibc/build/host-libzlib-1.3' > > *** > > *** ERROR: package host-libzlib installs executables without proper > > RPATH: > > *** /home/marcus/mnt/encwork/git/br-test-pkg/bootlin-armv5-uclibc/host/bin/xmlwf > > *** /home/marcus/mnt/encwork/git/br-test-pkg/bootlin-armv5-uclibc/host/bin/pkgconf > > *** /home/marcus/mnt/encwork/git/br-test-pkg/bootlin-armv5-uclibc/host/bin/protoc > > *** /home/marcus/mnt/encwork/git/br-test-pkg/bootlin-armv5-uclibc/host/bin/protoc-gen-c > > *** /home/marcus/mnt/encwork/git/br-test-pkg/bootlin-armv5-uclibc/host/bin/python3.11 > > *** /home/marcus/mnt/encwork/git/br-test-pkg/bootlin-armv5-uclibc/host/bin/openssl > > make: *** [package/pkg-generic.mk:305: /home/marcus/mnt/encwork/git/br-test-pkg/bootlin-armv5-uclibc/build/host-libzlib-1.3/.stamp_host_installed] > > Error 1 > > For information, on my side, I see the criu package build failing > for various reasons. > > For example, toolchain bootlin-armv5-uclibc is failing with: > > compel/arch/arm/src/lib/infect.c: In function ‘compel_set_task_ext_regs’: > compel/arch/arm/src/lib/infect.c:107:20: error: ‘PTRACE_SETVFPREGS’ > undeclared (first use in this function); did you mean ‘PTRACE_SETFPREGS’? > 107 | if (ptrace(PTRACE_SETVFPREGS, pid, NULL, ext_regs)) { > | ^~~~~~~~~~~~~~~~~ > | PTRACE_SETFPREGS I've replaced config BR2_PACKAGE_CRIU_ARCH_SUPPORTS bool default y if BR2_arm ... With: 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 Due to this comment from the yocto recipe: # # CRIU just can be built on ARMv7 and ARMv6, so the Makefile check # if the ARCH is ARMv7 or ARMv6. # So the toolchain bootlin-armv5-uclibc will be skipped. > > Toolchain bootlin-armv7-glibc failing with: > > /br-test-pkg/bootlin-armv7-glibc/host/bin/arm-linux-ld: > ./compel/plugins/std.lib.a(log.o): in function `std_vprint_num': > log.c:(.text+0x2c0): undefined reference to `__aeabi_idivmod' > /br-test-pkg/bootlin-armv7-glibc/host/bin/arm-linux-ld: > ./compel/plugins/std.lib.a(log.o): in function `sbuf_printf': > log.c:(.text+0x41c): undefined reference to `__aeabi_idivmod' > /br-test-pkg/bootlin-armv7-glibc/host/bin/arm-linux-ld: > ./compel/plugins/std.lib.a(string.o): in function `std_vdprintf': > string.c:(.text+0x114): undefined reference to `__aeabi_idivmod' > > There is possibly others. I'll try to investigate how to resolve those > issues. > > > > With this v2, I am now able to build criu on some x86_64 and Aarch64 > > > configurations, so I can start writing a runtime test ;) > > > > Yeay! > > I have now a fully working test on Aarch64. I'll send it when criu > will be merged. > > Best regards, > > Julien. /Marcus
diff --git a/package/Config.in b/package/Config.in index b21a2f8c65..c6c809f59c 100644 --- a/package/Config.in +++ b/package/Config.in @@ -2665,6 +2665,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..158e7ced57 --- /dev/null +++ b/package/criu/Config.in @@ -0,0 +1,28 @@ +menuconfig BR2_PACKAGE_CRIU + bool "criu" + 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_TOOLCHAIN_USES_MUSL + 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 + select BR2_PACKAGE_PYTHON_PIP + 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. + +comment "criu needs a uClibc or glibc toolchain w/ threads, dynamic library, wchar" + depends on !BR2_TOOLCHAIN_HAS_THREADS || BR2_STATIC_LIBS \ + || !BR2_USE_WCHAR || BR2_TOOLCHAIN_USES_MUSL 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..01b07e3f3f --- /dev/null +++ b/package/criu/criu.mk @@ -0,0 +1,42 @@ +################################################################################ +# +# CRIU +# +################################################################################ + +CRIU_VERSION = 3.18 +CRIU_SOURCE = criu-$(CRIU_VERSION).tar.gz +CRIU_SITE = https://github.com/checkpoint-restore/criu/archive/refs/tags/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_CFLAGS = $(TARGET_CFLAGS) +CRIU_CFLAGS += -D__WORDSIZE -D__USE_GNU -D_GNU_SOURCE + +CRIU_MAKE_ENV = $(TARGET_MAKE_ENV) +CRIU_MAKE_ENV += WERROR=0 + +#Needed as it adds strange paths to the tool otherwise. E.g. $CROSS_COMPILE/usr/bin/gcc +CRIU_MAKE_ENV += HOSTLD=ld +CRIU_MAKE_ENV += HOSTCC=gcc + +#x86_64 is treated as x86 in criu +ifeq ($(BR2_ARCH),x86_64) + CRIU_MAKE_ENV += ARCH=x86 +else + CRIU_MAKE_ENV += ARCH=$(BR2_ARCH) +endif + +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) USERCFLAGS="$(CRIU_CFLAGS)" -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))
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> --- package/Config.in | 1 + package/criu/Config.in | 28 ++++++++++++++++++++++++++++ package/criu/criu.hash | 3 +++ package/criu/criu.mk | 42 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 74 insertions(+) create mode 100644 package/criu/Config.in create mode 100644 package/criu/criu.hash create mode 100644 package/criu/criu.mk