Re: [OE-core] [PATCH v5 02/10] ufs image class: add

[Trevor Woerner <twoerner@gmail.com>]

On Tue 2026-02-24 @ 10:02:38 AM, Paul Barker wrote:
> On Mon, 2026-02-23 at 16:50 -0500, Trevor Woerner via
> lists.openembedded.org wrote:
> > Add an image class and wks file that demonstrates generating a wic image
> > with a 4096-byte sector size.
> > 
> > Signed-off-by: Mark Hatle <mark.hatle@kernel.crashing.org>
> > Signed-off-by: Trevor Woerner <twoerner@gmail.com>
> > ---
> > changes in v5:
> > - none
> > 
> > changes in v4:
> > - update the partition table format from mbr (msdos) to gpt
> > 
> > changes in v3:
> > - tested more scenarios and make sure to fix the warning from v1 in
> >   every case
> > 
> > changes in v2:
> > - add Mark as a co-creator (sorry for missing this the first time!)
> > - provide a fix for the following warning:
> > 	WARNING: core-image-minimal-1.0-r0 do_image_wic_ufs: Function do_image_wic_ufs doesn't exist
> > ---
> >  meta/classes-recipe/image.bbclass           |   2 +-
> >  meta/classes-recipe/image_types_ufs.bbclass | 221 ++++++++++++++++++++
> >  scripts/lib/wic/canned-wks/mkdisk-ufs.wks   |   5 +
> >  3 files changed, 227 insertions(+), 1 deletion(-)
> >  create mode 100644 meta/classes-recipe/image_types_ufs.bbclass
> >  create mode 100644 scripts/lib/wic/canned-wks/mkdisk-ufs.wks
> > 
> > diff --git a/meta/classes-recipe/image.bbclass b/meta/classes-recipe/image.bbclass
> > index 53f1a9dc45b0..97465836c14a 100644
> > --- a/meta/classes-recipe/image.bbclass
> > +++ b/meta/classes-recipe/image.bbclass
> > @@ -18,7 +18,7 @@ inherit populate_sdk_base
> >  IMGCLASSES += "${@['', 'populate_sdk_ext']['linux' in d.getVar("SDK_OS")]}"
> >  IMGCLASSES += "${@bb.utils.contains_any('IMAGE_FSTYPES', 'live iso hddimg', 'image-live', '', d)}"
> >  IMGCLASSES += "${@bb.utils.contains('IMAGE_FSTYPES', 'container', 'image-container', '', d)}"
> > -IMGCLASSES += "image_types_wic"
> > +IMGCLASSES += "image_types_wic image_types_ufs"
> >  IMGCLASSES += "rootfs-postcommands"
> >  IMGCLASSES += "image-postinst-intercepts"
> >  IMGCLASSES += "overlayfs-etc"
> > diff --git a/meta/classes-recipe/image_types_ufs.bbclass b/meta/classes-recipe/image_types_ufs.bbclass
> > new file mode 100644
> > index 000000000000..9a984f084289
> > --- /dev/null
> > +++ b/meta/classes-recipe/image_types_ufs.bbclass
> > @@ -0,0 +1,221 @@
> > +#
> > +# Copyright OpenEmbedded Contributors
> > +#
> > +# SPDX-License-Identifier: MIT
> > +#
> > +
> > +# The WICUFSVARS variable is used to define the base list of bitbake variables used in wic code
> > +# variables from this list are written to <image>.env file
> > +WICUFSVARS ?= "\
> > +	APPEND \
> > +	ASSUME_PROVIDED \
> > +	BBLAYERS \
> > +	DEPLOY_DIR_IMAGE \
> > +	FAKEROOTCMD \
> > +	HOSTTOOLS_DIR \
> > +	IMAGE_BASENAME \
> > +	IMAGE_BOOT_FILES \
> > +	IMAGE_CLASSES \
> > +	IMAGE_EFI_BOOT_FILES \
> > +	IMAGE_EXTRA_PARTITION_FILES \
> > +	IMAGE_LINK_NAME \
> > +	IMAGE_ROOTFS \
> > +	IMGDEPLOYDIR \
> > +	INITRAMFS_FSTYPES \
> > +	INITRAMFS_IMAGE \
> > +	INITRAMFS_IMAGE_BUNDLE \
> > +	INITRAMFS_LINK_NAME \
> > +	INITRD \
> > +	INITRD_LIVE \
> > +	ISODIR \
> > +	KERNEL_CONSOLE \
> > +	KERNEL_IMAGETYPE \
> > +	MACHINE \
> > +	PSEUDO_INCLUDE_PATHS \
> > +	RECIPE_SYSROOT_NATIVE \
> > +	ROOTFS_SIZE \
> > +	STAGING_DATADIR \
> > +	STAGING_DIR \
> > +	STAGING_DIR_HOST \
> > +	STAGING_LIBDIR \
> > +	TARGET_SYS \
> > +"
> > +
> > +inherit_defer ${@bb.utils.contains('INITRAMFS_IMAGE_BUNDLE', '1', 'kernel-artifact-names', '', d)}
> > +
> > +WKSUFS_FILE ??= "${WKS_FILE}"
> > +WKSUFS_FILES ?= "${WKSUFS_FILE} ${IMAGE_BASENAME}.wks"
> > +WKSUFS_SEARCH_PATH ?= "${THISDIR}:${@':'.join('%s/wic' % p for p in '${BBPATH}'.split(':'))}:${@':'.join('%s/scripts/lib/wic/canned-wks' % l for l in '${BBPATH}:${COREBASE}'.split(':'))}"
> > +WKSUFS_FULL_PATH = "${@wks_search(d.getVar('WKSUFS_FILES').split(), d.getVar('WKSUFS_SEARCH_PATH')) or ''}"
> > +
> > +def wks_search(files, search_path):
> > +    for f in files:
> > +        if os.path.isabs(f):
> > +            if os.path.exists(f):
> > +                return f
> > +        else:
> > +            searched = bb.utils.which(search_path, f)
> > +            if searched:
> > +                return searched
> > +
> > +def wks_checksums(files, search_path):
> > +    ret = ""
> > +    for f in files:
> > +        found, hist = bb.utils.which(search_path, f, history=True)
> > +        ret = ret + " " + " ".join(h + ":False" for h in hist[:-1])
> > +        if found:
> > +            ret = ret + " " + found + ":True"
> > +    return ret
> > +
> > +
> > +WICUFS_CREATE_EXTRA_ARGS ?= "${WIC_CREATE_EXTRA_ARGS}"
> > +
> > +IMAGE_CMD:wic.ufs () {
> > +	out="${IMGDEPLOYDIR}/${IMAGE_NAME}"
> > +	build_wic_ufs="${WORKDIR}/build-wic-ufs"
> > +	tmp_wic_ufs="${WORKDIR}/tmp-wic-ufs"
> > +	wks="${WKSUFS_FULL_PATH}"
> > +	if [ -e "$tmp_wic_ufs" ]; then
> > +		# Ensure we don't have any junk leftover from a previously interrupted
> > +		# do_image_wic_ufs execution
> > +		rm -rf "$tmp_wic_ufs"
> > +	fi
> > +	if [ -z "$wks" ]; then
> > +		bbfatal "No kickstart files from WKSUFS_FILES were found: ${WKSUFS_FILES}. Please set WKSUFS_FILE or WKSUFS_FILES appropriately."
> > +	fi
> > +	BUILDDIR="${TOPDIR}" PSEUDO_UNLOAD=1 wic create --debug "$wks" --sector-size 4096 --vars "${STAGING_DIR}/${MACHINE}/imgdata/" -e "${IMAGE_BASENAME}-ufs" -o "$build_wic_ufs/" -w "$tmp_wic_ufs" ${WICUFS_CREATE_EXTRA_ARGS}
> > +
> > +	# look to see if the user specifies a custom imager
> > +	IMAGER=direct
> > +	eval set -- "${WICUFS_CREATE_EXTRA_ARGS} --"
> > +	while [ 1 ]; do
> > +		case "$1" in
> > +			--imager|-i)
> > +				shift
> > +				IMAGER=$1
> > +				;;
> > +			--)
> > +				shift
> > +				break
> > +				;;
> > +		esac
> > +		shift
> > +	done
> > +	mv "$build_wic_ufs/$(basename "${wks%.wks}")"*.${IMAGER} "$out.wic.ufs"
> > +}
> > +IMAGE_CMD:wic.ufs[vardepsexclude] = "WKSUFS_FULL_PATH WKSUFS_FILES TOPDIR"
> > +SPDX_IMAGE_PURPOSE:wic.ufs = "diskImage"
> > +do_image_wic_ufs[cleandirs] = "${WORKDIR}/build-wic-ufs"
> > +
> > +# Rebuild when the wks file or vars in WICUFSVARS change
> > +USING_WIC_UFS = "${@bb.utils.contains_any('IMAGE_FSTYPES', 'wic.ufs ' + ' '.join('wic.ufs.%s' % c for c in '${CONVERSIONTYPES}'.split()), '1', '', d)}"
> > +WKSUFS_FILE_CHECKSUM = "${@wks_checksums(d.getVar('WKSUFS_FILES').split(), d.getVar('WKSUFS_SEARCH_PATH')) if '${USING_WIC_UFS}' else ''}"
> > +do_image_wic_ufs[file-checksums] += "${WKSUFS_FILE_CHECKSUM}"
> > +do_image_wic_ufs[depends] += "${@' '.join('%s-native:do_populate_sysroot' % r for r in ('parted', 'gptfdisk', 'dosfstools', 'mtools'))}"
> > +
> > +# We ensure all artfacts are deployed (e.g virtual/bootloader)
> > +do_image_wic_ufs[recrdeptask] += "do_deploy"
> > +do_image_wic_ufs[deptask] += "do_image_complete"
> > +
> > +WKSUFS_FILE_DEPENDS_DEFAULT = '${@bb.utils.contains_any("BUILD_ARCH", [ 'x86_64', 'i686' ], "syslinux-native", "",d)}'
> > +WKSUFS_FILE_DEPENDS_DEFAULT += "bmaptool-native cdrtools-native btrfs-tools-native squashfs-tools-native e2fsprogs-native erofs-utils-native"
> > +# Unified kernel images need objcopy
> > +WKSUFS_FILE_DEPENDS_DEFAULT += "virtual/cross-binutils"
> > +WKSUFS_FILE_DEPENDS_BOOTLOADERS = ""
> > +WKSUFS_FILE_DEPENDS_BOOTLOADERS:aarch64 = "grub-efi systemd-boot"
> > +WKSUFS_FILE_DEPENDS_BOOTLOADERS:arm = "systemd-boot"
> > +WKSUFS_FILE_DEPENDS_BOOTLOADERS:x86 = "syslinux grub-efi systemd-boot"
> > +WKSUFS_FILE_DEPENDS_BOOTLOADERS:x86-64 = "syslinux grub-efi systemd-boot"
> > +WKSUFS_FILE_DEPENDS_BOOTLOADERS:x86-x32 = "syslinux grub-efi"
> > +
> > +WKSUFS_FILE_DEPENDS ??= "${WKSUFS_FILE_DEPENDS_DEFAULT} ${WKSUFS_FILE_DEPENDS_BOOTLOADERS}"
> > +
> > +DEPENDS += "${@ '${WKSUFS_FILE_DEPENDS}' if d.getVar('USING_WIC_UFS') else '' }"
> > +
> > +python do_write_wksufs_template () {
> > +    """Write out expanded template contents to WKSUFS_FULL_PATH."""
> > +    import re
> > +
> > +    template_body = d.getVar('_WKSUFS_TEMPLATE')
> > +
> > +    # Remove any remnant variable references left behind by the expansion
> > +    # due to undefined variables
> > +    expand_var_regexp = re.compile(r"\${[^{}@\n\t :]+}")
> > +    while True:
> > +        new_body = re.sub(expand_var_regexp, '', template_body)
> > +        if new_body == template_body:
> > +            break
> > +        else:
> > +            template_body = new_body
> > +
> > +    wks_file = d.getVar('WKSUFS_FULL_PATH')
> > +    with open(wks_file, 'w') as f:
> > +        f.write(template_body)
> > +    f.close()
> > +    # Copy the finalized wks file to the deploy directory for later use
> > +    depdir = d.getVar('IMGDEPLOYDIR')
> > +    basename = d.getVar('IMAGE_BASENAME') + '-ufs'
> > +    bb.utils.copyfile(wks_file, "%s/%s" % (depdir, basename + '-' + os.path.basename(wks_file)))
> > +}
> > +
> > +do_flush_pseudodb() {
> > +	${FAKEROOTENV} ${FAKEROOTCMD} -S
> > +}
> > +
> > +python () {
> > +    if d.getVar('USING_WIC_UFS'):
> > +        wksufs_file_u = d.getVar('WKSUFS_FULL_PATH', False)
> > +        wksufs_file = d.expand(wksufs_file_u)
> > +        base, ext = os.path.splitext(wksufs_file)
> > +        if ext == '.in' and os.path.exists(wksufs_file):
> > +            wksufs_out_file = os.path.join(d.getVar('WORKDIR'), os.path.basename(base))
> > +            d.setVar('WKSUFS_FULL_PATH', wksufs_out_file)
> > +            d.setVar('WKSUFS_TEMPLATE_PATH', wksufs_file_u)
> > +            d.setVar('WKSUFS_FILE_CHECKSUM', '${WKSUFS_TEMPLATE_PATH}:True')
> > +
> > +            # We need to re-parse each time the file changes, and bitbake
> > +            # needs to be told about that explicitly.
> > +            bb.parse.mark_dependency(d, wksufs_file)
> > +
> > +            try:
> > +                with open(wksufs_file, 'r') as f:
> > +                    body = f.read()
> > +            except (IOError, OSError) as exc:
> > +                pass
> > +            else:
> > +                # Previously, I used expandWithRefs to get the dependency list
> > +                # and add it to WICUFSVARS, but there's no point re-parsing the
> > +                # file in process_wks_template as well, so just put it in
> > +                # a variable and let the metadata deal with the deps.
> > +                d.setVar('_WKSUFS_TEMPLATE', body)
> > +                bb.build.addtask('do_write_wksufs_template', 'do_image_wic_ufs', 'do_image', d)
> > +        bb.build.addtask('do_image_wic_ufs', 'do_image_complete', 'do_image_wic', d)
> > +}
> > +
> > +#
> > +# Write environment variables used by wic
> > +# to tmp/sysroots/<machine>/imgdata/<image>-ufs.env
> > +#
> > +python do_rootfs_wicufsenv () {
> > +    wicufsvars = d.getVar('WICUFSVARS')
> > +    if not wicufsvars:
> > +        return
> > +
> > +    stdir = d.getVar('STAGING_DIR')
> > +    outdir = os.path.join(stdir, d.getVar('MACHINE'), 'imgdata')
> > +    bb.utils.mkdirhier(outdir)
> > +    basename = d.getVar('IMAGE_BASENAME') + '-ufs'
> > +    with open(os.path.join(outdir, basename) + '.env', 'w') as envf:
> > +        for var in wicufsvars.split():
> > +            value = d.getVar(var)
> > +            if value:
> > +                envf.write('%s="%s"\n' % (var, value.strip()))
> > +    envf.close()
> > +    # Copy .env file to deploy directory for later use with stand alone wic
> > +    depdir = d.getVar('IMGDEPLOYDIR')
> > +    bb.utils.copyfile(os.path.join(outdir, basename) + '.env', os.path.join(depdir, basename) + '.env')
> > +}
> > +addtask do_flush_pseudodb after do_rootfs before do_image do_image_qa
> > +addtask do_rootfs_wicufsenv after do_image before do_image_wic_ufs
> > +do_rootfs_wicufsenv[vardeps] += "${WICUFSVARS}"
> > +do_rootfs_wicufsenv[prefuncs] = 'set_image_size'
> 
> This is mostly duplication of code from image_types_wic.bbclass with
> different variable names, which makes it difficult to review the
> meaningful differences. Why do we need a separate class here? If we do
> need one, can we reduce duplication?

At work we create sector-size=512 and sector-size=4k images in the same
build (the 4k images are for UFS-based flash devices). With a separate
class we can generate these images simultaneously rather than doing them
one after the other. But (I believe) the only way to generate them
simultaneously is to have separate classes with separate variable names,
otherwise they will clobber each other.

As UFS-based flash becomes more popular, I'm guessing more users of The
Yocto Project will be interested in having support for it readily
available. Therefore I wanted to provide it in oe-core.

The most significant difference between these two classes is the
"--sector-size 4096" option to the wic command.

Can we reduce duplication? Almost every line of both these classes has a
wic-related variable in it. If the variables need to be kept separate so
that parallel builds don't clobber each other, there aren't very many
lines in common!

If I am correct that separate classes with separate variable names are
needed to be able to generate both images simultaneously, would this
patch be okay as-is in a v6 patchset?