[PATCH v5 0/2] mage creation improvements

[PATCH v5 0/2] mage creation improvements

[Ed Bartosh]

Hi,

This patchset contains various improvements for the image creation functionality
made by Patchick Ohly during his work on bug #9076:
 - renamed COMPRESSION variables to CONVERSION as the term "compression" is no longer accurate
 - support converting masked types

Changes in v2: rebased on top of master-next
               returned image_getdepends back to image_types.bbclass
Changes in v3: rebased on top of recent master
Changes in v4: rebased on top of recent master
Changes in v5: removed 2 problematic patches

The following changes since commit 5b61fa04a335a4fdefd435dc25d4bac41ee21e39:

  bitbake: toaster-tests: fix URL given for Chromedriver download (2016-07-29 09:53:32 +0100)

are available in the git repository at:

  git://git.yoctoproject.org/poky-contrib ed/oe-core/image-type-9076.v5
  http://git.yoctoproject.org/cgit.cgi/poky-contrib/log/?h=ed/oe-core/image-type-9076.v5

Patrick Ohly (2):
  image creation: support converting masked types
  image.bbclass: rename COMPRESS(ION) to CONVERSION

 meta/classes/image-live.bbclass                    |  2 +-
 meta/classes/image-vm.bbclass                      | 33 ++-------
 meta/classes/image.bbclass                         | 82 +++++++++++++++-------
 meta/classes/image_types.bbclass                   | 66 +++++++++--------
 meta/classes/image_types_uboot.bbclass             | 18 ++---
 meta/conf/documentation.conf                       |  1 -
 .../images/build-appliance-image_15.0.0.bb         |  6 +-
 7 files changed, 112 insertions(+), 96 deletions(-)

--
Regards,
Ed

[PATCH v5 1/2] image creation: support converting masked types

[Ed Bartosh]

From: Patrick Ohly <patrick.ohly@intel.com>

Conversion to vmdk/vdi/qcow2 is also useful for other base images
types, not just for .hdddirect. This can be achieved by definining
them as conversion commands and relying on the conversion chaining
to convert arbitrary base images.

For this to work when the base image gets created by a masked image
type, the additional conversion commands now get executed in a
do_image_complete prefunc.

With all of that in place it becomes possible to remove the special
purpose code for vmdk/vdi/qcow2 types from image-vm.bbclass and
several other classes. This has (intentional!) implications on the
valid IMAGE_FSTYPES and the file suffices: now
"hdddirect.vmdk/vdi/qcow2" must be used as IMAGE_FSTYPES to select the
former special-case types "vmdk/vdi/qcow2", and the image files and
links will also have the extra .hdddirect suffix.

This is intentional because it makes it makes it possible to
distinguish between virtual machine images created from .hdddirect and
those created from other base images.

The new support for virtual machine images can also be combined with
compression, thus making it possible to create image files for
publication in compressed format, for example with:
  IMAGE_FSTYPES = "hdddirect.vdi.xz"

Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
---
 meta/classes/image-live.bbclass                    |  2 +-
 meta/classes/image-vm.bbclass                      | 33 ++--------
 meta/classes/image.bbclass                         | 73 +++++++++++++++-------
 meta/classes/image_types.bbclass                   | 20 ++++--
 meta/conf/documentation.conf                       |  1 -
 .../images/build-appliance-image_15.0.0.bb         |  6 +-
 6 files changed, 75 insertions(+), 60 deletions(-)

diff --git a/meta/classes/image-live.bbclass b/meta/classes/image-live.bbclass
index f0e6647..039a977 100644
--- a/meta/classes/image-live.bbclass
+++ b/meta/classes/image-live.bbclass
@@ -43,7 +43,7 @@ ROOT_LIVE ?= "root=/dev/ram0"
 INITRD_IMAGE_LIVE ?= "core-image-minimal-initramfs"
 INITRD_LIVE ?= "${DEPLOY_DIR_IMAGE}/${INITRD_IMAGE_LIVE}-${MACHINE}.cpio.gz"
 
-ROOTFS ?= "${DEPLOY_DIR_IMAGE}/${IMAGE_LINK_NAME}.ext4"
+ROOTFS ?= "${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.ext4"
 
 IMAGE_TYPEDEP_live = "ext4"
 IMAGE_TYPEDEP_iso = "ext4"
diff --git a/meta/classes/image-vm.bbclass b/meta/classes/image-vm.bbclass
index 72f7b4b..d37ce0a 100644
--- a/meta/classes/image-vm.bbclass
+++ b/meta/classes/image-vm.bbclass
@@ -26,14 +26,11 @@ do_bootdirectdisk[depends] += "dosfstools-native:do_populate_sysroot \
                                ${PN}:do_image_${VM_ROOTFS_TYPE} \
                                "
 
-IMAGE_TYPEDEP_vmdk = "${VM_ROOTFS_TYPE}"
-IMAGE_TYPEDEP_vdi = "${VM_ROOTFS_TYPE}"
-IMAGE_TYPEDEP_qcow2 = "${VM_ROOTFS_TYPE}"
+VM_ROOTFS_TYPE ?= "ext4"
 IMAGE_TYPEDEP_hdddirect = "${VM_ROOTFS_TYPE}"
-IMAGE_TYPES_MASKED += "vmdk vdi qcow2 hdddirect"
+IMAGE_TYPES_MASKED += "hdddirect"
 
-VM_ROOTFS_TYPE ?= "ext4"
-ROOTFS ?= "${DEPLOY_DIR_IMAGE}/${IMAGE_LINK_NAME}.${VM_ROOTFS_TYPE}"
+ROOTFS ?= "${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.ext4"
 
 # Used by bootloader
 LABELS_VM ?= "boot"
@@ -144,27 +141,5 @@ run_qemu_img (){
     qemu-img convert -O $type ${DEPLOY_DIR_IMAGE}/${IMAGE_LINK_NAME}.hdddirect ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.$type
     ln -sf ${IMAGE_NAME}.$type ${DEPLOY_DIR_IMAGE}/${IMAGE_LINK_NAME}.$type
 }
-create_vmdk_image () {
-    run_qemu_img vmdk
-}
-
-create_vdi_image () {
-    run_qemu_img vdi
-}
-
-create_qcow2_image () {
-    run_qemu_img qcow2
-}
-
-python do_vmimg() {
-    if 'vmdk' in d.getVar('IMAGE_FSTYPES', True):
-        bb.build.exec_func('create_vmdk_image', d)
-    if 'vdi' in d.getVar('IMAGE_FSTYPES', True):
-        bb.build.exec_func('create_vdi_image', d)
-    if 'qcow2' in d.getVar('IMAGE_FSTYPES', True):
-        bb.build.exec_func('create_qcow2_image', d)
-}
 
-addtask bootdirectdisk before do_vmimg
-addtask vmimg after do_bootdirectdisk before do_image_complete
-do_vmimg[depends] += "qemu-native:do_populate_sysroot"
+addtask bootdirectdisk before do_image_complete
diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
index af789f4..57e0744 100644
--- a/meta/classes/image.bbclass
+++ b/meta/classes/image.bbclass
@@ -127,6 +127,13 @@ do_rootfs[vardeps] += "${@rootfs_variables(d)}"
 
 do_build[depends] += "virtual/kernel:do_deploy"
 
+def image_type_active(type, d):
+    '''True if any entry in IMAGE_FSTYPES is type or depends on it.'''
+    for entry in d.getVar("IMAGE_FSTYPES", True).split():
+        if entry == type or entry.startswith(type + "."):
+            return True
+    return False
+
 def build_live(d):
     if bb.utils.contains("IMAGE_FSTYPES", "live", "live", "0", d) == "0": # live is not set but hob might set iso or hddimg
         d.setVar('NOISO', bb.utils.contains('IMAGE_FSTYPES', "iso", "0", "1", d))
@@ -139,7 +146,7 @@ def build_live(d):
 IMAGE_TYPE_live = "${@build_live(d)}"
 inherit ${IMAGE_TYPE_live}
 
-IMAGE_TYPE_vm = '${@bb.utils.contains_any("IMAGE_FSTYPES", ["vmdk", "vdi", "qcow2", "hdddirect"], "image-vm", "", d)}'
+IMAGE_TYPE_vm = '${@ "image-vm" if image_type_active("hdddirect", d) else ""}'
 inherit ${IMAGE_TYPE_vm}
 
 python () {
@@ -414,9 +421,7 @@ python () {
         cmds = []
         subimages = []
         realt = t
-
-        if t in maskedtypes:
-            continue
+        ismasked = t in maskedtypes
 
         localdata = bb.data.createCopy(d)
         debug = ""
@@ -434,12 +439,14 @@ python () {
         localdata.delVar('DATETIME')
         localdata.delVar('TMPDIR')
 
-        image_cmd = localdata.getVar("IMAGE_CMD", True)
-        vardeps.add('IMAGE_CMD_' + realt)
-        if image_cmd:
-            cmds.append("\t" + image_cmd)
-        else:
-            bb.fatal("No IMAGE_CMD defined for IMAGE_FSTYPES entry '%s' - possibly invalid type name or missing support class" % t)
+        if not ismasked:
+            image_cmd = localdata.getVar("IMAGE_CMD", True)
+            vardeps.add('IMAGE_CMD_' + realt)
+            if image_cmd:
+                cmds.append("\t" + image_cmd)
+            else:
+                bb.fatal("No IMAGE_CMD defined for IMAGE_FSTYPES entry '%s' - possibly invalid type name or missing support class" % t)
+
         cmds.append(localdata.expand("\tcd ${DEPLOY_DIR_IMAGE}"))
 
         # Since a copy of IMAGE_CMD_xxx will be inlined within do_image_xxx,
@@ -487,17 +494,41 @@ python () {
 
         t = t.replace("-", "_").replace(".", "_")
 
-        d.setVar('do_image_%s' % t, '\n'.join(cmds))
-        d.setVarFlag('do_image_%s' % t, 'func', '1')
-        d.setVarFlag('do_image_%s' % t, 'fakeroot', '1')
-        d.setVarFlag('do_image_%s' % t, 'prefuncs', debug + 'set_image_size')
-        d.setVarFlag('do_image_%s' % t, 'postfuncs', 'create_symlinks')
-        d.setVarFlag('do_image_%s' % t, 'subimages', ' '.join(subimages))
-        d.appendVarFlag('do_image_%s' % t, 'vardeps', ' '.join(vardeps))
-        d.appendVarFlag('do_image_%s' % t, 'vardepsexclude', 'DATETIME')
-
-        bb.debug(2, "Adding type %s before %s, after %s" % (t, 'do_image_complete', after))
-        bb.build.addtask('do_image_%s' % t, 'do_image_complete', after, d)
+        if ismasked:
+            # If the type is masked, then some unknown task (for example,
+            # do_bootdirectdisk in boot-directdisk.bbclass for IMAGE_FSTYPES hdddirect)
+            # will create the actual base image. All we know is that the files will
+            # be there right before do_image_complete. So in that case we put the
+            # conversion commands into a do_image_complete prefunc. The 'after'
+            # dependencies can be ignored because we are guaranteed to run after
+            # all of them, and the conversion dependencies are dealt with
+            # by making do_rootfs depend on them.
+            d.setVar('image_%s_conversion' % t, '\n'.join(cmds))
+            d.setVarFlag('image_%s_conversion' % t, 'func', '1')
+            d.setVarFlag('image_%s_conversion' % t, 'fakeroot', '1')
+            d.appendVarFlag('image_%s_conversion' % t, 'vardeps', ' '.join(vardeps))
+            d.appendVarFlag('image_%s_conversion' % t, 'vardepsexclude', 'DATETIME')
+            prefuncs = set((d.getVarFlag('do_image_complete', 'prefuncs', True) or '').split())
+            prefuncs.add('image_%s_conversion' % t)
+            # create_symlinks must run after the commands which create the real files
+            # because create_symlinks checks for them.
+            prefuncs.discard('create_symlinks')
+            d.setVarFlag('do_image_complete', 'prefuncs', ' '.join(sorted(prefuncs)) + ' create_symlinks')
+            d.appendVarFlag('do_image_complete', 'subimages', ' ' + ' '.join(subimages))
+        else:
+            # If not masked, we generate a new task which executes the image
+            # creation and the conversion commands.
+            d.setVar('do_image_%s' % t, '\n'.join(cmds))
+            d.setVarFlag('do_image_%s' % t, 'func', '1')
+            d.setVarFlag('do_image_%s' % t, 'fakeroot', '1')
+            d.setVarFlag('do_image_%s' % t, 'prefuncs', debug + 'set_image_size')
+            d.setVarFlag('do_image_%s' % t, 'postfuncs', 'create_symlinks')
+            d.setVarFlag('do_image_%s' % t, 'subimages', ' '.join(subimages))
+            d.appendVarFlag('do_image_%s' % t, 'vardeps', ' '.join(vardeps))
+            d.appendVarFlag('do_image_%s' % t, 'vardepsexclude', 'DATETIME')
+
+            bb.debug(2, "Adding type %s before %s, after %s" % (t, 'do_image_complete', after))
+            bb.build.addtask('do_image_%s' % t, 'do_image_complete', after, d)
 }
 
 #
diff --git a/meta/classes/image_types.bbclass b/meta/classes/image_types.bbclass
index 2b97397..c386bb6 100644
--- a/meta/classes/image_types.bbclass
+++ b/meta/classes/image_types.bbclass
@@ -284,6 +284,7 @@ IMAGE_DEPENDS_cramfs = "util-linux-native"
 IMAGE_DEPENDS_ext2 = "e2fsprogs-native"
 IMAGE_DEPENDS_ext3 = "e2fsprogs-native"
 IMAGE_DEPENDS_ext4 = "e2fsprogs-native"
+IMAGE_DEPENDS_hdddirect = "${IMAGE_DEPENDS_ext4}"
 IMAGE_DEPENDS_btrfs = "btrfs-tools-native"
 IMAGE_DEPENDS_squashfs = "squashfs-tools-native"
 IMAGE_DEPENDS_squashfs-xz = "squashfs-tools-native"
@@ -295,6 +296,9 @@ IMAGE_DEPENDS_multiubi = "mtd-utils-native"
 IMAGE_DEPENDS_wic = "parted-native"
 
 # This variable is available to request which values are suitable for IMAGE_FSTYPES
+# TODO: several other variations are now also possible, for example ext4.zip,
+# hdddirect.xz, hdddirect.vdi.xz. Which variations are supposed to be listed here
+# and which not? Generate all possible variations dynamically?
 IMAGE_TYPES = " \
     jffs2 jffs2.sum \
     cramfs \
@@ -308,15 +312,15 @@ IMAGE_TYPES = " \
     ubi ubifs multiubi \
     tar tar.gz tar.bz2 tar.xz tar.lz4 \
     cpio cpio.gz cpio.xz cpio.lzma cpio.lz4 \
-    vmdk \
-    vdi \
-    qcow2 \
+    hdddirect.vmdk \
+    hdddirect.vdi \
+    hdddirect.qcow2 \
     hdddirect \
     elf \
     wic wic.gz wic.bz2 wic.lzma \
 "
 
-COMPRESSIONTYPES = "gz bz2 lzma xz lz4 zip sum md5sum sha1sum sha224sum sha256sum sha384sum sha512sum bmap"
+COMPRESSIONTYPES = "gz bz2 lzma xz lz4 zip sum md5sum sha1sum sha224sum sha256sum sha384sum sha512sum bmap vmdk vdi qcow2"
 COMPRESS_CMD_lzma = "lzma -k -f -7 ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}"
 COMPRESS_CMD_gz = "gzip -f -9 -c ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type} > ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}.gz"
 COMPRESS_CMD_bz2 = "pbzip2 -f -k ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}"
@@ -324,6 +328,9 @@ COMPRESS_CMD_xz = "xz -f -k -c ${XZ_COMPRESSION_LEVEL} ${XZ_THREADS} --check=${X
 COMPRESS_CMD_lz4 = "lz4c -9 -c ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type} > ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}.lz4"
 COMPRESS_CMD_zip = "zip ${ZIP_COMPRESSION_LEVEL} ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}.zip ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}"
 COMPRESS_CMD_sum = "sumtool -i ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type} -o ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}.sum ${JFFS2_SUM_EXTRA_ARGS}"
+COMPRESS_CMD_vmdk = "qemu-img convert -O vmdk ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type} ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}.vmdk"
+COMPRESS_CMD_vdi = "qemu-img convert -O vdi ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type} ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}.vdi"
+COMPRESS_CMD_qcow2 = "qemu-img convert -O qcow2 ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type} ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}.qcow2"
 COMPRESS_CMD_md5sum = "md5sum ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type} > ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}.md5sum"
 COMPRESS_CMD_sha1sum = "sha1sum ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type} > ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}.sha1sum"
 COMPRESS_CMD_sha224sum = "sha224sum ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type} > ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}.sha224sum"
@@ -339,6 +346,9 @@ COMPRESS_DEPENDS_lz4 = "lz4-native"
 COMPRESS_DEPENDS_zip = "zip-native"
 COMPRESS_DEPENDS_sum = "mtd-utils-native"
 COMPRESS_DEPENDS_bmap = "bmap-tools-native"
+COMPRESS_DEPENDS_vmdk = "qemu-native"
+COMPRESS_DEPENDS_vdi = "qemu-native"
+COMPRESS_DEPENDS_qcow2 = "qemu-native"
 
 RUNNABLE_IMAGE_TYPES ?= "ext2 ext3 ext4"
 RUNNABLE_MACHINE_PATTERNS ?= "qemu"
@@ -349,7 +359,7 @@ DEPLOYABLE_IMAGE_TYPES ?= "hddimg iso"
 IMAGE_EXTENSION_live = "hddimg iso"
 
 # The IMAGE_TYPES_MASKED variable will be used to mask out from the IMAGE_FSTYPES,
-# images that will not be built at do_rootfs time: vmdk, vdi, qcow2, hdddirect, hddimg, iso, etc.
+# images that will not be built at do_rootfs time: hdddirect, hddimg, iso, etc.
 IMAGE_TYPES_MASKED ?= ""
 
 # The WICVARS variable is used to define list of bitbake variables used in wic code
diff --git a/meta/conf/documentation.conf b/meta/conf/documentation.conf
index 51c4116..35e487c 100644
--- a/meta/conf/documentation.conf
+++ b/meta/conf/documentation.conf
@@ -56,7 +56,6 @@ do_testsdk[doc] = "Installs an SDK and performs runtime tests on the tools insta
 do_uboot_mkimage[doc] = "Creates a uImage file from the kernel for the U-Boot bootloader"
 do_unpack[doc] = "Unpacks the source code into a working directory"
 do_validate_branches[doc] = "Ensures that the source/meta branches are on the locations specified by their SRCREV values for a linux-yocto style kernel"
-do_vmimg[doc] = "Creates an image for use with VMware or VirtualBox compatible virtual machine hosts (based on IMAGE_FSTYPES either .vmdk or .vdi)"
 
 # DESCRIPTIONS FOR VARIABLES #
 
diff --git a/meta/recipes-core/images/build-appliance-image_15.0.0.bb b/meta/recipes-core/images/build-appliance-image_15.0.0.bb
index dc621d6..957b8cf 100644
--- a/meta/recipes-core/images/build-appliance-image_15.0.0.bb
+++ b/meta/recipes-core/images/build-appliance-image_15.0.0.bb
@@ -18,7 +18,7 @@ IMAGE_ROOTFS_EXTRA_SPACE = "41943040"
 APPEND += "rootfstype=ext4 quiet"
 
 DEPENDS = "zip-native"
-IMAGE_FSTYPES = "vmdk"
+IMAGE_FSTYPES = "hdddirect.vmdk"
 
 inherit core-image module-base
 
@@ -101,7 +101,7 @@ create_bundle_files () {
 	cd ${WORKDIR}
 	mkdir -p Yocto_Build_Appliance
 	cp *.vmx* Yocto_Build_Appliance
-	ln -sf ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.vmdk Yocto_Build_Appliance/Yocto_Build_Appliance.vmdk
+	ln -sf ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}.hdddirect.vmdk Yocto_Build_Appliance/Yocto_Build_Appliance.vmdk
 	zip -r ${DEPLOY_DIR_IMAGE}/Yocto_Build_Appliance-${DATETIME}.zip Yocto_Build_Appliance
 	ln -sf Yocto_Build_Appliance-${DATETIME}.zip ${DEPLOY_DIR_IMAGE}/Yocto_Build_Appliance.zip 
 }
@@ -111,5 +111,5 @@ python do_bundle_files() {
     bb.build.exec_func('create_bundle_files', d)
 }
 
-addtask bundle_files after do_vmimg before do_build
+addtask bundle_files after do_image_complete before do_build
 do_bundle_files[nostamp] = "1"
-- 
2.1.4

Re: [PATCH v5 1/2] image creation: support converting masked types

[Richard Purdie]

On Fri, 2016-07-29 at 16:44 +0300, Ed Bartosh wrote:
> From: Patrick Ohly <patrick.ohly@intel.com>
> 
> Conversion to vmdk/vdi/qcow2 is also useful for other base images
> types, not just for .hdddirect. This can be achieved by definining
> them as conversion commands and relying on the conversion chaining
> to convert arbitrary base images.
> 
> For this to work when the base image gets created by a masked image
> type, the additional conversion commands now get executed in a
> do_image_complete prefunc.
> 
> With all of that in place it becomes possible to remove the special
> purpose code for vmdk/vdi/qcow2 types from image-vm.bbclass and
> several other classes. This has (intentional!) implications on the
> valid IMAGE_FSTYPES and the file suffices: now
> "hdddirect.vmdk/vdi/qcow2" must be used as IMAGE_FSTYPES to select
> the
> former special-case types "vmdk/vdi/qcow2", and the image files and
> links will also have the extra .hdddirect suffix.
> 
> This is intentional because it makes it makes it possible to
> distinguish between virtual machine images created from .hdddirect
> and
> those created from other base images.
> 
> The new support for virtual machine images can also be combined with
> compression, thus making it possible to create image files for
> publication in compressed format, for example with:
>   IMAGE_FSTYPES = "hdddirect.vdi.xz"

I'm afraid I really don't like this. The direction this code has taken
is to separate out the different steps into clearly identifiable tasks.
This was due to strong user feedback that nobody could figure out what
was going on. This change starts to merge them all back together,
hiding them in a prefunc of a task which is just horrible.

I haven't looked in detail at the problem thats being attempted to be
solved here but this doesn't look like a good approach at all and takes
us backwards rather than forwards.

So, sorry, but no.

Cheers,

Richard

Re: [PATCH v5 1/2] image creation: support converting masked types

[Richard Purdie]

On Fri, 2016-07-29 at 15:29 +0100, Richard Purdie wrote:
> On Fri, 2016-07-29 at 16:44 +0300, Ed Bartosh wrote:
> > From: Patrick Ohly <patrick.ohly@intel.com>
> > 
> > Conversion to vmdk/vdi/qcow2 is also useful for other base images
> > types, not just for .hdddirect. This can be achieved by definining
> > them as conversion commands and relying on the conversion chaining
> > to convert arbitrary base images.
> > 
> > For this to work when the base image gets created by a masked image
> > type, the additional conversion commands now get executed in a
> > do_image_complete prefunc.
> > 
> > With all of that in place it becomes possible to remove the special
> > purpose code for vmdk/vdi/qcow2 types from image-vm.bbclass and
> > several other classes. This has (intentional!) implications on the
> > valid IMAGE_FSTYPES and the file suffices: now
> > "hdddirect.vmdk/vdi/qcow2" must be used as IMAGE_FSTYPES to select
> > the
> > former special-case types "vmdk/vdi/qcow2", and the image files and
> > links will also have the extra .hdddirect suffix.
> > 
> > This is intentional because it makes it makes it possible to
> > distinguish between virtual machine images created from .hdddirect
> > and
> > those created from other base images.
> > 
> > The new support for virtual machine images can also be combined
> > with
> > compression, thus making it possible to create image files for
> > publication in compressed format, for example with:
> >   IMAGE_FSTYPES = "hdddirect.vdi.xz"
> 
> I'm afraid I really don't like this. The direction this code has
> taken
> is to separate out the different steps into clearly identifiable
> tasks.
> This was due to strong user feedback that nobody could figure out
> what
> was going on. This change starts to merge them all back together,
> hiding them in a prefunc of a task which is just horrible.
> 
> I haven't looked in detail at the problem thats being attempted to be
> solved here but this doesn't look like a good approach at all and
> takes
> us backwards rather than forwards.
> 
> So, sorry, but no.

Staring at and thinking about this some more, I think the description
doesn't do this patch any favours. I'd phrase this as:

"Make the vmdk/vdi/qcow2 image types behave more as compression type
post processes".

I'm more than fine with that and it makes sense. I don't however think
the "masked" bits of this patch look right. These should be able to
become compression types without the masked changes.

Its possible there is another step this patch is trying to make, such
as supporting chained compression. If that is the case it should be a
separate patch.

Cheers,

Richard

Re: [PATCH v5 1/2] image creation: support converting masked types

[Ed Bartosh]

On Fri, Jul 29, 2016 at 03:36:12PM +0100, Richard Purdie wrote:
> On Fri, 2016-07-29 at 15:29 +0100, Richard Purdie wrote:
> > On Fri, 2016-07-29 at 16:44 +0300, Ed Bartosh wrote:
> > > From: Patrick Ohly <patrick.ohly@intel.com>
> > > 
> > > Conversion to vmdk/vdi/qcow2 is also useful for other base images
> > > types, not just for .hdddirect. This can be achieved by definining
> > > them as conversion commands and relying on the conversion chaining
> > > to convert arbitrary base images.
> > > 
> > > For this to work when the base image gets created by a masked image
> > > type, the additional conversion commands now get executed in a
> > > do_image_complete prefunc.
> > > 
> > > With all of that in place it becomes possible to remove the special
> > > purpose code for vmdk/vdi/qcow2 types from image-vm.bbclass and
> > > several other classes. This has (intentional!) implications on the
> > > valid IMAGE_FSTYPES and the file suffices: now
> > > "hdddirect.vmdk/vdi/qcow2" must be used as IMAGE_FSTYPES to select
> > > the
> > > former special-case types "vmdk/vdi/qcow2", and the image files and
> > > links will also have the extra .hdddirect suffix.
> > > 
> > > This is intentional because it makes it makes it possible to
> > > distinguish between virtual machine images created from .hdddirect
> > > and
> > > those created from other base images.
> > > 
> > > The new support for virtual machine images can also be combined
> > > with
> > > compression, thus making it possible to create image files for
> > > publication in compressed format, for example with:
> > >   IMAGE_FSTYPES = "hdddirect.vdi.xz"
> > 
> > I'm afraid I really don't like this. The direction this code has
> > taken
> > is to separate out the different steps into clearly identifiable
> > tasks.
> > This was due to strong user feedback that nobody could figure out
> > what
> > was going on. This change starts to merge them all back together,
> > hiding them in a prefunc of a task which is just horrible.
> > 
> > I haven't looked in detail at the problem thats being attempted to be
> > solved here but this doesn't look like a good approach at all and
> > takes
> > us backwards rather than forwards.
> > 
> > So, sorry, but no.
> 
> Staring at and thinking about this some more, I think the description
> doesn't do this patch any favours. I'd phrase this as:
> 
> "Make the vmdk/vdi/qcow2 image types behave more as compression type
> post processes".
> 
> I'm more than fine with that and it makes sense. I don't however think
> the "masked" bits of this patch look right. These should be able to
> become compression types without the masked changes.
> 
> Its possible there is another step this patch is trying to make, such
> as supporting chained compression. If that is the case it should be a
> separate patch.
> 

Thank you for reviewing this.

OK, I'll drop this one too. I'll send only one COMPRESSION->CONVERSION
patch as it fixes 2.2M3 bug.

--
Regards,
Ed

[PATCH v5 2/2] image.bbclass: rename COMPRESS(ION) to CONVERSION

[Ed Bartosh]

From: Patrick Ohly <patrick.ohly@intel.com>

With the enhanced functionality, the term "compression" is no longer
accurate, because the mechanism also gets used for conversion
operations that do not actually compress data.

It is possible to remove this naming problem in a backward-compatible
manner by including COMPRESSIONTYPES in CONVERSIONTYPES and checking for
the old COMPRESS_CMD/DEPENDS as fallbacks.

[YOCTO #9346]

Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
---
 meta/classes/image.bbclass             |  9 ++---
 meta/classes/image_types.bbclass       | 60 +++++++++++++++++-----------------
 meta/classes/image_types_uboot.bbclass | 18 +++++-----
 3 files changed, 44 insertions(+), 43 deletions(-)

diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
index 57e0744..dfcad44 100644
--- a/meta/classes/image.bbclass
+++ b/meta/classes/image.bbclass
@@ -118,7 +118,7 @@ def rootfs_variables(d):
                  'IMAGE_ROOTFS_MAXSIZE','IMAGE_NAME','IMAGE_LINK_NAME','IMAGE_MANIFEST','DEPLOY_DIR_IMAGE','RM_OLD_IMAGE','IMAGE_FSTYPES','IMAGE_INSTALL_COMPLEMENTARY','IMAGE_LINGUAS',
                  'MULTILIBRE_ALLOW_REP','MULTILIB_TEMP_ROOTFS','MULTILIB_VARIANTS','MULTILIBS','ALL_MULTILIB_PACKAGE_ARCHS','MULTILIB_GLOBAL_VARIANTS','BAD_RECOMMENDATIONS','NO_RECOMMENDATIONS',
                  'PACKAGE_ARCHS','PACKAGE_CLASSES','TARGET_VENDOR','TARGET_ARCH','TARGET_OS','OVERRIDES','BBEXTENDVARIANT','FEED_DEPLOYDIR_BASE_URI','INTERCEPT_DIR','USE_DEVFS',
-                 'COMPRESSIONTYPES', 'IMAGE_GEN_DEBUGFS', 'ROOTFS_RO_UNNEEDED']
+                 'CONVERSIONTYPES', 'IMAGE_GEN_DEBUGFS', 'ROOTFS_RO_UNNEEDED']
     variables.extend(rootfs_command_variables(d))
     variables.extend(variable_depends(d))
     return " ".join(variables)
@@ -350,7 +350,7 @@ python setup_debugfs () {
 
 python () {
     vardeps = set()
-    # We allow COMPRESSIONTYPES to have duplicates. That avoids breaking
+    # We allow CONVERSIONTYPES to have duplicates. That avoids breaking
     # derived distros when OE-core or some other layer independently adds
     # the same type. There is still only one command for each type, but
     # presumably the commands will do the same when the type is the same,
@@ -358,7 +358,7 @@ python () {
     #
     # Without de-duplication, gen_conversion_cmds() below
     # would create the same compression command multiple times.
-    ctypes = set(d.getVar('COMPRESSIONTYPES', True).split())
+    ctypes = set(d.getVar('CONVERSIONTYPES', True).split())
     old_overrides = d.getVar('OVERRIDES', 0)
 
     def _image_base_type(type):
@@ -463,9 +463,10 @@ python () {
                     # Create input image first.
                     gen_conversion_cmds(type)
                     localdata.setVar('type', type)
-                    cmd = "\t" + localdata.getVar("COMPRESS_CMD_" + ctype, True)
+                    cmd = "\t" + (localdata.getVar("CONVERSION_CMD_" + ctype, True) or localdata.getVar("COMPRESS_CMD_" + ctype, True))
                     if cmd not in cmds:
                         cmds.append(cmd)
+                    vardeps.add('CONVERSION_CMD_' + ctype)
                     vardeps.add('COMPRESS_CMD_' + ctype)
                     subimage = type + "." + ctype
                     if subimage not in subimages:
diff --git a/meta/classes/image_types.bbclass b/meta/classes/image_types.bbclass
index c386bb6..491fd30 100644
--- a/meta/classes/image_types.bbclass
+++ b/meta/classes/image_types.bbclass
@@ -214,7 +214,7 @@ IMAGE_CMD_wic () {
 IMAGE_CMD_wic[vardepsexclude] = "WKS_FULL_PATH WKS_FILES"
 
 # Rebuild when the wks file or vars in WICVARS change
-USING_WIC = "${@bb.utils.contains_any('IMAGE_FSTYPES', 'wic ' + ' '.join('wic.%s' % c for c in '${COMPRESSIONTYPES}'.split()), '1', '', d)}"
+USING_WIC = "${@bb.utils.contains_any('IMAGE_FSTYPES', 'wic ' + ' '.join('wic.%s' % c for c in '${CONVERSIONTYPES}'.split()), '1', '', d)}"
 WKS_FILE_CHECKSUM = "${@'${WKS_FULL_PATH}:%s' % os.path.exists('${WKS_FULL_PATH}') if '${USING_WIC}' else ''}"
 do_image_wic[file-checksums] += "${WKS_FILE_CHECKSUM}"
 
@@ -320,35 +320,35 @@ IMAGE_TYPES = " \
     wic wic.gz wic.bz2 wic.lzma \
 "
 
-COMPRESSIONTYPES = "gz bz2 lzma xz lz4 zip sum md5sum sha1sum sha224sum sha256sum sha384sum sha512sum bmap vmdk vdi qcow2"
-COMPRESS_CMD_lzma = "lzma -k -f -7 ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}"
-COMPRESS_CMD_gz = "gzip -f -9 -c ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type} > ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}.gz"
-COMPRESS_CMD_bz2 = "pbzip2 -f -k ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}"
-COMPRESS_CMD_xz = "xz -f -k -c ${XZ_COMPRESSION_LEVEL} ${XZ_THREADS} --check=${XZ_INTEGRITY_CHECK} ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type} > ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}.xz"
-COMPRESS_CMD_lz4 = "lz4c -9 -c ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type} > ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}.lz4"
-COMPRESS_CMD_zip = "zip ${ZIP_COMPRESSION_LEVEL} ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}.zip ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}"
-COMPRESS_CMD_sum = "sumtool -i ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type} -o ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}.sum ${JFFS2_SUM_EXTRA_ARGS}"
-COMPRESS_CMD_vmdk = "qemu-img convert -O vmdk ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type} ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}.vmdk"
-COMPRESS_CMD_vdi = "qemu-img convert -O vdi ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type} ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}.vdi"
-COMPRESS_CMD_qcow2 = "qemu-img convert -O qcow2 ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type} ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}.qcow2"
-COMPRESS_CMD_md5sum = "md5sum ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type} > ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}.md5sum"
-COMPRESS_CMD_sha1sum = "sha1sum ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type} > ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}.sha1sum"
-COMPRESS_CMD_sha224sum = "sha224sum ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type} > ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}.sha224sum"
-COMPRESS_CMD_sha256sum = "sha256sum ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type} > ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}.sha256sum"
-COMPRESS_CMD_sha384sum = "sha384sum ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type} > ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}.sha384sum"
-COMPRESS_CMD_sha512sum = "sha512sum ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type} > ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}.sha512sum"
-COMPRESS_CMD_bmap = "bmaptool create ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type} -o ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}.bmap"
-COMPRESS_DEPENDS_lzma = "xz-native"
-COMPRESS_DEPENDS_gz = ""
-COMPRESS_DEPENDS_bz2 = "pbzip2-native"
-COMPRESS_DEPENDS_xz = "xz-native"
-COMPRESS_DEPENDS_lz4 = "lz4-native"
-COMPRESS_DEPENDS_zip = "zip-native"
-COMPRESS_DEPENDS_sum = "mtd-utils-native"
-COMPRESS_DEPENDS_bmap = "bmap-tools-native"
-COMPRESS_DEPENDS_vmdk = "qemu-native"
-COMPRESS_DEPENDS_vdi = "qemu-native"
-COMPRESS_DEPENDS_qcow2 = "qemu-native"
+CONVERSIONTYPES = "gz bz2 lzma xz lz4 zip sum md5sum sha1sum sha224sum sha256sum sha384sum sha512sum bmap vmdk vdi qcow2"
+CONVERSION_CMD_lzma = "lzma -k -f -7 ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}"
+CONVERSION_CMD_gz = "gzip -f -9 -c ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type} > ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}.gz"
+CONVERSION_CMD_bz2 = "pbzip2 -f -k ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}"
+CONVERSION_CMD_xz = "xz -f -k -c ${XZ_COMPRESSION_LEVEL} ${XZ_THREADS} --check=${XZ_INTEGRITY_CHECK} ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type} > ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}.xz"
+CONVERSION_CMD_lz4 = "lz4c -9 -c ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type} > ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}.lz4"
+CONVERSION_CMD_zip = "zip ${ZIP_COMPRESSION_LEVEL} ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}.zip ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}"
+CONVERSION_CMD_sum = "sumtool -i ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type} -o ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}.sum ${JFFS2_SUM_EXTRA_ARGS}"
+CONVERSION_CMD_vmdk = "qemu-img convert -O vmdk ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type} ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}.vmdk"
+CONVERSION_CMD_vdi = "qemu-img convert -O vdi ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type} ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}.vdi"
+CONVERSION_CMD_qcow2 = "qemu-img convert -O qcow2 ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type} ${DEPLOY_DIR_IMAGE}/${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}.qcow2"
+CONVERSION_CMD_md5sum = "md5sum ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type} > ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}.md5sum"
+CONVERSION_CMD_sha1sum = "sha1sum ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type} > ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}.sha1sum"
+CONVERSION_CMD_sha224sum = "sha224sum ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type} > ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}.sha224sum"
+CONVERSION_CMD_sha256sum = "sha256sum ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type} > ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}.sha256sum"
+CONVERSION_CMD_sha384sum = "sha384sum ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type} > ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}.sha384sum"
+CONVERSION_CMD_sha512sum = "sha512sum ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type} > ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}.sha512sum"
+CONVERSION_CMD_bmap = "bmaptool create ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type} -o ${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.${type}.bmap"
+CONVERSION_DEPENDS_lzma = "xz-native"
+CONVERSION_DEPENDS_gz = ""
+CONVERSION_DEPENDS_bz2 = "pbzip2-native"
+CONVERSION_DEPENDS_xz = "xz-native"
+CONVERSION_DEPENDS_lz4 = "lz4-native"
+CONVERSION_DEPENDS_zip = "zip-native"
+CONVERSION_DEPENDS_sum = "mtd-utils-native"
+CONVERSION_DEPENDS_bmap = "bmap-tools-native"
+CONVERSION_DEPENDS_vmdk = "qemu-native"
+CONVERSION_DEPENDS_vdi = "qemu-native"
+CONVERSION_DEPENDS_qcow2 = "qemu-native"
 
 RUNNABLE_IMAGE_TYPES ?= "ext2 ext3 ext4"
 RUNNABLE_MACHINE_PATTERNS ?= "qemu"
diff --git a/meta/classes/image_types_uboot.bbclass b/meta/classes/image_types_uboot.bbclass
index 19e4aa2..f72d9b2 100644
--- a/meta/classes/image_types_uboot.bbclass
+++ b/meta/classes/image_types_uboot.bbclass
@@ -8,19 +8,19 @@ oe_mkimage () {
     fi
 }
 
-COMPRESSIONTYPES += "gz.u-boot bz2.u-boot lzma.u-boot u-boot"
+CONVERSIONTYPES += "gz.u-boot bz2.u-boot lzma.u-boot u-boot"
 
-COMPRESS_DEPENDS_u-boot = "u-boot-mkimage-native"
-COMPRESS_CMD_u-boot      = "oe_mkimage ${IMAGE_NAME}.rootfs.${type} none"
+CONVERSION_DEPENDS_u-boot = "u-boot-mkimage-native"
+CONVERSION_CMD_u-boot      = "oe_mkimage ${IMAGE_NAME}.rootfs.${type} none"
 
-COMPRESS_DEPENDS_gz.u-boot = "u-boot-mkimage-native"
-COMPRESS_CMD_gz.u-boot      = "${COMPRESS_CMD_gz}; oe_mkimage ${IMAGE_NAME}.rootfs.${type}.gz gzip clean"
+CONVERSION_DEPENDS_gz.u-boot = "u-boot-mkimage-native"
+CONVERSION_CMD_gz.u-boot      = "${CONVERSION_CMD_gz}; oe_mkimage ${IMAGE_NAME}.rootfs.${type}.gz gzip clean"
 
-COMPRESS_DEPENDS_bz2.u-boot = "u-boot-mkimage-native"
-COMPRESS_CMD_bz2.u-boot      = "${COMPRESS_CMD_bz2}; oe_mkimage ${IMAGE_NAME}.rootfs.${type}.bz2 bzip2 clean"
+CONVERSION_DEPENDS_bz2.u-boot = "u-boot-mkimage-native"
+CONVERSION_CMD_bz2.u-boot      = "${CONVERSION_CMD_bz2}; oe_mkimage ${IMAGE_NAME}.rootfs.${type}.bz2 bzip2 clean"
 
-COMPRESS_DEPENDS_lzma.u-boot = "u-boot-mkimage-native"
-COMPRESS_CMD_lzma.u-boot      = "${COMPRESS_CMD_lzma}; oe_mkimage ${IMAGE_NAME}.rootfs.${type}.lzma lzma clean"
+CONVERSION_DEPENDS_lzma.u-boot = "u-boot-mkimage-native"
+CONVERSION_CMD_lzma.u-boot      = "${CONVERSION_CMD_lzma}; oe_mkimage ${IMAGE_NAME}.rootfs.${type}.lzma lzma clean"
 
 IMAGE_TYPES += "ext2.u-boot ext2.gz.u-boot ext2.bz2.u-boot ext2.lzma.u-boot ext3.gz.u-boot ext4.gz.u-boot cpio.gz.u-boot"
 
-- 
2.1.4