RE: [isar-cip-core][RFC v1 4/6] Add Delta update support with rdiff_image and delta handler

From: <Adithya.Balakumar@toshiba-tsip.com>
To: <felix.moessbauer@siemens.com>, <cip-dev@lists.cip-project.org>,
	<quirin.gylstorff@siemens.com>, <jan.kiszka@siemens.com>
Cc: <Shivanand.Kunijadar@toshiba-tsip.com>,
	<Sai.Sathujoda@toshiba-tsip.com>, <dinesh.kumar@toshiba-tsip.com>,
	<kazuhiro3.hayashi@toshiba.co.jp>
Subject: RE: [isar-cip-core][RFC v1 4/6] Add Delta update support with rdiff_image and delta handler
Date: Wed, 13 Mar 2024 06:11:46 +0000	[thread overview]
Message-ID: <TYCPR01MB966940F5431AB8457BD3204AC42A2@TYCPR01MB9669.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <0876d8dd533161214f2010cdbcd65d5084a1ddd3.camel@siemens.com>

From: MOESSBAUER, Felix <felix.moessbauer@siemens.com> 
Sent: Friday, March 8, 2024 4:22 PM
To: cip-dev@lists.cip-project.org; balakumar adithya(ＴＳＩＰ TEUR) <Adithya.Balakumar@toshiba-tsip.com>; quirin.gylstorff@siemens.com; Kiszka, Jan <jan.kiszka@siemens.com>
Cc: kunijadar shivanand(ＴＳＩＰ TMIEC ODG Porting) <Shivanand.Kunijadar@toshiba-tsip.com>; ashrith sai(ＴＳＩＰ) <Sai.Sathujoda@toshiba-tsip.com>; dinesh kumar(ＴＳＩＰ TMIEC ODG Porting) <dinesh.kumar@toshiba-tsip.com>; hayashi kazuhiro(林 和宏 ＤＭＥ ○ＤＩＧ□ＭＰＳ○ＭＰ４) <kazuhiro3.hayashi@toshiba.co.jp>
Subject: Re: [isar-cip-core][RFC v1 4/6] Add Delta update support with rdiff_image and delta handler

On Thu, 2024-03-07 at 13:16 +0530, Adithya Balakumar wrote:
> swupdate supports delta updates with rdiff_image and delta(zchunk) 
> handler. This change adds support to use either of the handler for 
> creating delta update artifacts. The type of handler can be set in the 
> delta-update.yml file along with additional variables needed for the 
> chosen handler.
> 
> Signed-off-by: Adithya Balakumar <Adithya.Balakumar@toshiba-tsip.com>
> ---
>  classes/delta_update.bbclass | 90

>Hi,

>please name this class delta-update.bbclass to keep the naming style similar with the rest of the project.

> ++++++++++++++++++++++++++++++++++++
>  kas/opt/delta-update.yml     | 21 +++++++++
>  2 files changed, 111 insertions(+)
>  create mode 100644 classes/delta_update.bbclass
>  create mode 100644 kas/opt/delta-update.yml
> 
> diff --git a/classes/delta_update.bbclass 
> b/classes/delta_update.bbclass new file mode 100644 index 
> 0000000..8d47081
> --- /dev/null
> +++ b/classes/delta_update.bbclass
> @@ -0,0 +1,90 @@
> +#
> +# CIP Core, generic profile
> +#
> +# Copyright (c) Toshiba Corporation 2024 # # Authors:
> +#  Adithya Balakumar <adithya.balakumar@toshiba-tsip.com>
> +#
> +# SPDX-License-Identifier: MIT
> +#
> +
> +IMAGER_INSTALL:delta_update += "fdisk zchunk rdiff"

>Conditionally appending with += overwrites already set weak defaults.
>You might want to use .= " fdisk zchunk rdiff" instead.

> +
> +FILESEXTRAPATHS:append = ":${LAYERDIR_cip-core}"
> +IMAGE_SRC_URI:delta_update += " ${@ "

>Same here.

> file://"+d.getVar('DELTA_RDIFF_REF_IMAGE') if

>I guess this could be simplified and made more portable by referencing the DELTA_RDIFF_REF_IMAGE directly (e.g. from ${DEPLOY_DIR_IMAGE}). The problem with the FILESEXTRAPATHS:append = ":${LAYERDIR_cip->core}" is that it only works if the delta image is also from cip-core, but usually cip-core is just referenced in a downstream layer and used there.

[Adithya Balakumar] 
Hi Felix,

I understand the drawback of using FILESEXTRAPATHS:append = ":${LAYERDIR_cip-core}" that it only looks for the delta image placed in the isar-cip-core layer.
As an example you point to referencing the DELTA_RDIFF_REF_IMAGE directly from ${DEPLOY_DIR_IMAGE} but as per my understanding ${DEPLOY_DIR_IMAGE} points to the current build's image deploy directory. 

So, Could you please suggest a suitable way to provide the delta image in the case of rdiff.

> d.getVar('DELTA_UPDATE_TYPE') == 'rdiff' and
> d.getVar('DELTA_RDIFF_REF_IMAGE') else '' }"
> +
> +do_image_delta_update[cleandirs] +=
> "${WORKDIR}/delta_interim_artifacts"
> +
> +extract_root_fs() {
> +    INPUT_ARTIFACT=$1
> +    OUTPUT_ARTIFACT=$2
> +    SECTOR_SIZE=512
> +    ROOTFS_LABEL="primary"
> +    ROOTFS_START_SECTOR=$(${SUDO_CHROOT} /sbin/fdisk -l -o

>Can't we simply use sfdisk to query this information instead of all the awk stuff?

> Name,Start "${INPUT_ARTIFACT}" | awk -v name="${ROOTFS_LABEL}" '$0 ~ 
> name {print $2}' | head -n 1)
> +    SKIP_VALUE="$(expr $(expr $ROOTFS_START_SECTOR \* $SECTOR_SIZE)
> / $(expr 1024 \* 1024))"
> +    ${SUDO_CHROOT} /bin/dd if=${INPUT_ARTIFACT} bs=1M
> skip=$SKIP_VALUE count=1024 of=${OUTPUT_ARTIFACT} status=none
> +}

>Why copy exactly 1GB?

>Instead of extracting that here, it might be simpler to just store the .rdiff file (of the rootfs) with each image build and use that.[Adithya Balakumar] 

[Adithya Balakumar]  Here, I am copying 1GB from the wic image because that is the size of the rootfs partition. 

However, Quirin left a remark stating that the .squashfs file (or .verity file) could be used to create the delta artifact. Using the squashfs or verity file to create the delta will eliminate need for extracting the rootfs from the wic image and will simplify many things in this recipe. Would like to understand your opinion on this.

Thanks and Regards,
Adithya

> +
> +do_delta_update_sw_description_template() {
> +   # Since the sw-description file for delta update would not be
> very different from that of a
> +   # normal update, copy sw-description.tmpl as sw-description-
> delta.tmpl and add delta update
> +   # specific properties
> +   SWU_DESCRIPTION_DELTA_UPDATE="sw-description-delta"
> +   cp "${WORKDIR}/${SWU_DESCRIPTION_FILE}".tmpl
> "${WORKDIR}/${SWU_DESCRIPTION_DELTA_UPDATE}".tmpl
> +   sed -i 's/\SWU_ROOTFS_PARTITION_NAME/SWU_DELTA_UPDATE_ARTIFACT/g'
> ${WORKDIR}/${SWU_DESCRIPTION_DELTA_UPDATE}.tmpl
> +   sed -i '/configfilecheck =/a \
> \'$'{SWU_DELTA_UPDATE_ADDITIONAL_PROPERTIES}'
> ${WORKDIR}/${SWU_DESCRIPTION_DELTA_UPDATE}.tmpl
> +}

>@Quirin: IIRC you recently refactored the creation of the sw- description file. This might simplify things here.

> +addtask do_delta_update_sw_description_template before
> do_transform_template after do_generate_image_uuid
> +
> +IMAGE_CMD:delta_update() {
> +    if [ "${DELTA_UPDATE_TYPE}" != "zchunk" ] && [
> "${DELTA_UPDATE_TYPE}" != "rdiff" ]; then
> +        bbfatal "You must set a valid DELTA_UPDATE_TYPE
> (rdiff/zchunk)"
> +    fi
> +    
> +    DELTA_ARTIFACT_SWU=""
> +
> +    if [ "${DELTA_UPDATE_TYPE}" = "zchunk" ]; then
> +       extract_root_fs "${PP_DEPLOY}/${IMAGE_FULLNAME}.wic"
> "${WORKDIR}/delta_interim_artifacts/${IMAGE_FULLNAME}.raw"
> +
> +       # Create .zck file
> +       ${SUDO_CHROOT} /bin/zck \
> +               --output ${PP_DEPLOY}/${IMAGE_FULLNAME}.zck \
> +               -u --chunk-hash-type sha256 \
> +               ${WORKDIR}/delta_interim_artifacts/${IMAGE_FULLNAME}.
> raw
> +
> +       # Calculate size of zck header
> +       HSIZE="$(${SUDO_CHROOT} /bin/zck_read_header -v
> ${PP_DEPLOY}/${IMAGE_FULLNAME}.zck | grep "Header size" | cut -d ':'
> -f 2)"
> +    
> +       # Extract the zck header
> +       ${SUDO_CHROOT} /bin/dd
> if="${PP_DEPLOY}/${IMAGE_FULLNAME}".zck
> of="${PP_DEPLOY}/${IMAGE_FULLNAME}".zck.header bs=1 count="$HSIZE"
> status=none
> +
> +        DELTA_ARTIFACT_SWU=${IMAGE_FULLNAME}.zck.header
> +
> +    elif [ "${DELTA_UPDATE_TYPE}" = "rdiff" ]; then
> +        if [ -z "${DELTA_RDIFF_REF_IMAGE}" ]; then
> +            bbfatal "You must set DELTA_RDIFF_REF_IMAGE and provide
> the required files as artifacts to this recipe"
> +        fi
> +
> +       # Extracting root partition from image1
> +       extract_root_fs "${WORKDIR}/${DELTA_RDIFF_REF_IMAGE}"
> "${WORKDIR}/delta_interim_artifacts/old-image-rootfs.raw"

>We only need that to compute the rdiff signature, right?
>As these images might be quite big, better remove the old one before
>extracting the new one. By that we save ~half of temporary space.

> +       
> +       # Extracting root partition from image2
> +       extract_root_fs "${PP_DEPLOY}/${IMAGE_FULLNAME}.wic" 
> "${WORKDIR}/delta_interim_artifacts/new-image-rootfs.raw" 
> +
> +       rm -f ${DEPLOY_DIR_IMAGE}/${IMAGE_FULLNAME}.delta
> +       # create signature file with rdiff
> +       ${SUDO_CHROOT} /usr/bin/rdiff signature
> ${WORKDIR}/delta_interim_artifacts/old-image-rootfs.raw \
> +               ${WORKDIR}/delta_interim_artifacts/old-image-
> rootfs.sig
> +
> +        # create delta file with the signature file
> +       ${SUDO_CHROOT} /usr/bin/rdiff delta
> ${WORKDIR}/delta_interim_artifacts/old-image-rootfs.sig \
> +               ${WORKDIR}/delta_interim_artifacts/new-image-
> rootfs.raw ${PP_DEPLOY}/${IMAGE_FULLNAME}.delta
> +
> +       DELTA_ARTIFACT_SWU=${IMAGE_FULLNAME}.delta
> +    fi
> +        # create a symbolic link as IMAGE_CMD expects a
> *.delta_update file in deploy image directory
> +        ln -sf ${DELTA_ARTIFACT_SWU}
> ${DEPLOY_DIR_IMAGE}/${IMAGE_FULLNAME}.delta_update
> +}
> +
> +addtask do_image_delta_update before do_image_swu after do_image_wic
> diff --git a/kas/opt/delta-update.yml b/kas/opt/delta-update.yml
> new file mode 100644
> index 0000000..c7a3893
> --- /dev/null
> +++ b/kas/opt/delta-update.yml
> @@ -0,0 +1,21 @@
> +#
> +# CIP Core, generic profile
> +#
> +# Copyright (c) Toshiba Corporation 2024
> +#
> +# Authors:
> +#  Adithya Balakumar <adithya.balakumar@toshiba-tsip.com>
> +#
> +# SPDX-License-Identifier: MIT
> +#
> +
> +header:
> +  version: 14
> +
> +local_conf_header:
> +  delta-update: |
> +    IMAGE_CLASSES:append = " delta_update"
> +    IMAGE_FSTYPES:append = " delta_update"
> +    DELTA_UPDATE_TYPE = ""
> +    DELTA_ZCK_URL = ""
> +    DELTA_RDIFF_REF_IMAGE = "" 

>Please also extra-weak define the DELTA_* variables in the
>delta_update.bbclass (to always have them defined and for documentation
>reasons).

>Best regards,
>Felix

-- 
>Siemens AG, Technology
>Linux Expert Center