Re: [PATCH 1/1] add zchunk compression support

["MOESSBAUER, Felix" <felix.moessbauer@siemens.com>]

On Wed, 2023-05-24 at 06:38 +0000, Schmidt, Adriaan (T CED SES-DE)
wrote:
> Uladzimir Bely <ubely@ilbers.de>, Wednesday, May 24, 2023 6:06 AM:
> > On Thu, 2023-04-27 at 06:37 +0000, 'Moessbauer, Felix' via isar-
> > users
> > wrote:
> > > On Thu, 2023-04-27 at 06:18 +0000, Schmidt, Adriaan (T CED SES-
> > > DE)
> > > wrote:
> > > > 'Felix Moessbauer' via isar-users
> > > > <isar-users@googlegroups.com>,
> > > > Donnerstag, 27. April 2023 07:51:
> > > > > This patch adds support to compress images with zchunk. The
> > > > > compression
> > > > > format is optimized for delta transfers, by ensuring that
> > > > > small
> > > > > changes
> > > > > on the input also only invalidate some blocks of the output.
> > > > > 
> > > > > There are not yet many configuration options available, but
> > > > > once
> > > > > available, they can be added to the ZCK_DEFAULTS variable.
> > > > > 
> > > > > Signed-off-by: Felix Moessbauer
> > > > > <felix.moessbauer@siemens.com>
> > > > > ---
> > > > >  meta/classes/imagetypes.bbclass | 5 ++++-
> > > > >  meta/conf/bitbake.conf          | 3 +++
> > > > >  2 files changed, 7 insertions(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/meta/classes/imagetypes.bbclass
> > > > > b/meta/classes/imagetypes.bbclass
> > > > > index a80a6da5..3639662b 100644
> > > > > --- a/meta/classes/imagetypes.bbclass
> > > > > +++ b/meta/classes/imagetypes.bbclass
> > > > > @@ -93,7 +93,7 @@ IMAGE_CMD:ubi() {
> > > > >  IMAGE_CMD:ubi[depends] = "${PN}:do_transform_template"
> > > > > 
> > > > >  # image conversions
> > > > > -IMAGE_CONVERSIONS = "gz xz zst"
> > > > > +IMAGE_CONVERSIONS = "gz xz zst zck"
> > > > > 
> > > > >  CONVERSION_CMD:gz = "${SUDO_CHROOT} sh -c 'gzip -f -9 -n -c
> > > > > --
> > > > > rsyncable
> > > > > ${IMAGE_FILE_CHROOT} > ${IMAGE_FILE_CHROOT}.gz'"
> > > > >  CONVERSION_DEPS:gz = "gzip"
> > > > > @@ -103,3 +103,6 @@ CONVERSION_DEPS:xz = "xz-utils"
> > > > > 
> > > > >  CONVERSION_CMD:zst = "${SUDO_CHROOT} sh -c 'zstd -c --sparse
> > > > > ${ZSTD_DEFAULTS} ${IMAGE_FILE_CHROOT} >
> > > > > ${IMAGE_FILE_CHROOT}.zst'"
> > > > >  CONVERSION_DEPS:zst = "zstd"
> > > > > +
> > > > > +CONVERSION_CMD:zck = "${SUDO_CHROOT} sh -c 'cd $(dirname
> > > > > ${IMAGE_FILE_CHROOT}); zck ${ZCK_DEFAULTS}
> > > > > ${IMAGE_FILE_CHROOT}'"
> > > > 
> > > > Hi Felix,
> > > > 
> > > > This looks like it compresses in-place, i.e., removes the
> > > > original
> > > > file.
> > > 
> > > It looks like that, but there zchunk behaves differently from the
> > > other
> > > compressors. It actually just adds the compressed file next to
> > > the
> > > input file (version 1.2.3). Also, it is not possible to tell zck
> > > to
> > > write to stdout, hence the changedir logic.
> > > 
> > > I already thought about adding an issue in the upstream project
> > > [1]
> > > about the CLI api. It is pretty non-standard. Another issue is,
> > > that
> > > the underlying zstd compression level cannot be set and
> > > compression
> > > is
> > > done sequentially. Also the documentation is really sparse.
> > > 
> > > Anyways, while the tool is in a pretty early state, it is still
> > > good
> > > to
> > > have it to get some experience on delta updates.
> > > 
> > > [1] https://github.com/zchunk/zchunk
> > > 
> > > Felix
> > > 
> > > > That would break the imagetypes/conversion logic, where it is
> > > > assumed
> > > > that the original file still exists after conversion. That lets
> > > > us
> > > > define
> > > > things like IMAGE_FSYTPES="wic wic.zck" if we want both files
> > > > in
> > > > the
> > > > output.
> > > > In case the uncompressed one is not needed (it's not in
> > > > IMAGE_FSTYPES), it will
> > > > be deleted explicitly by the imagetypes class.
> > > > 
> > > > Adriaan
> > > > 
> > > > > +CONVERSION_DEPS:zck = "zchunk"
> > > > > diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf
> > > > > index c660660a..f73e4838 100644
> > > > > --- a/meta/conf/bitbake.conf
> > > > > +++ b/meta/conf/bitbake.conf
> > > > > @@ -144,6 +144,9 @@ ZSTD_LEVEL ?= "19"
> > > > >  ZSTD_DEFAULTS ?= "--rsyncable -${ZSTD_LEVEL} --
> > > > > threads=${ZSTD_THREADS}"
> > > > >  ZSTD_DEFAULTS[vardepsexclude] += "ZSTD_LEVEL ZSTD_THREADS"
> > > > > 
> > > > > +# Default compression settings for zchunk
> > > > > +ZCK_DEFAULTS ?= ""
> > > > > +
> > > > >  BBINCLUDELOGS ??= "yes"
> > > > > 
> > > > >  # Add event handlers for bitbake
> > > > > --
> > > > > 2.34.1
> > > > > 
> > > > > --
> > > > > You received this message because you are subscribed to the
> > > > > Google
> > > > > Groups
> > > > > "isar-users" group.
> > > > > To unsubscribe from this group and stop receiving emails from
> > > > > it,
> > > > > send an
> > > > > email to isar-users+unsubscribe@googlegroups.com.
> > > > > To view this discussion on the web visit
> > > > > https://groups.google.com/d/msgid/isar-users/20230427055118.1993072-1
> > > > > -
> > > > > felix.moessbauer%40siemens.com.
> > > 
> > 
> > Bumping the topic to make a decision on merge.
> > 
> > Patch itself passes fast/full CI, so if there are no other reasons
> > against (except a bit non-standard compression behaviour), we could
> > merge it.
> 
> Yes, I think we can merge.
> Felix' implementation already compensates for the strangeness in the
> invocation of the zck compression.

Agree. I also created an issue regarding this in the upstream project:
https://github.com/zchunk/zchunk/issues/97

Felix

> 
> Adriaan
>