[PATCH 1/2] image-vm: Convert vmdk/vdi/qcow2/hdddirect images to IMAGE_CMD

[PATCH 1/2] image-vm: Convert vmdk/vdi/qcow2/hdddirect images to IMAGE_CMD

[Tom Rini]

The support for writing vmdk/vdi/qcow2 images has not been converted to make
use of the IMAGE_CMD infrastructure and instead relies on custom logic for
adding tasks in the right place.  Convert these images to making use of
IMAGE_CMD.  This mainly involves re-listing some of the depends to the new task
names.  We clarify that only hdddirect images depend on the value of
VM_ROOTFS_TYPE and then that vmdk/vdi/qcow2 depend on that.  We convert the old
do_bootdirectdisk task into a python function, boot_direct_disk_prep that we
call from build_boot_dd().  An important side-effect of this is that we now get
free compression on these image types.

Signed-off-by: Tom Rini <trini@konsulko.com>
---
 meta/classes/image-vm.bbclass                      | 74 ++++++++--------------
 meta/classes/image_types.bbclass                   |  2 +-
 .../images/build-appliance-image_15.0.0.bb         |  2 +-
 3 files changed, 27 insertions(+), 51 deletions(-)

diff --git a/meta/classes/image-vm.bbclass b/meta/classes/image-vm.bbclass
index b52df9fbf5a1..a888f0232caa 100644
--- a/meta/classes/image-vm.bbclass
+++ b/meta/classes/image-vm.bbclass
@@ -17,21 +17,26 @@
 
 inherit live-vm-common
 
-do_bootdirectdisk[depends] += "dosfstools-native:do_populate_sysroot \
-                               virtual/kernel:do_deploy \
-                               syslinux:do_populate_sysroot \
-                               syslinux-native:do_populate_sysroot \
-                               parted-native:do_populate_sysroot \
-                               mtools-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}"
+do_image_vmdk[depends] += "qemu-native:do_populate_sysroot"
+do_image_vdi[depends] += "qemu-native:do_populate_sysroot"
+do_image_qcow2[depends] += "qemu-native:do_populate_sysroot"
+do_image_hdddirect[depends] += "dosfstools-native:do_populate_sysroot \
+                                virtual/kernel:do_deploy \
+                                syslinux:do_populate_sysroot \
+                                syslinux-native:do_populate_sysroot \
+                                parted-native:do_populate_sysroot \
+                                mtools-native:do_populate_sysroot \
+                                ${PN}:do_image_${VM_ROOTFS_TYPE} \
+                                "
+
+IMAGE_CMD_vmdk = "run_qemu_img vmdk"
+IMAGE_TYPEDEP_vmdk = "hdddirect"
+IMAGE_CMD_vdi = "run_qemu_img vdi"
+IMAGE_TYPEDEP_vdi = "hdddirect"
+IMAGE_CMD_qcow2 = "run_qemu_img qcow2"
+IMAGE_TYPEDEP_qcow2 = "hdddirect"
+IMAGE_CMD_hdddirect = "build_boot_dd"
 IMAGE_TYPEDEP_hdddirect = "${VM_ROOTFS_TYPE}"
-IMAGE_TYPES_MASKED += "vmdk vdi qcow2 hdddirect"
-
 VM_ROOTFS_TYPE ?= "ext4"
 ROOTFS ?= "${IMGDEPLOYDIR}/${IMAGE_LINK_NAME}.${VM_ROOTFS_TYPE}"
 
@@ -50,9 +55,11 @@ DISK_SIGNATURE ?= "${DISK_SIGNATURE_GENERATED}"
 DISK_SIGNATURE[vardepsexclude] = "DISK_SIGNATURE_GENERATED"
 
 build_boot_dd() {
+	${@boot_direct_disk_prep(d)}
+
 	HDDDIR="${S}/hdd/boot"
 	HDDIMG="${S}/hdd.image"
-	IMAGE=${IMGDEPLOYDIR}/${IMAGE_NAME}.hdddirect
+	IMAGE=${IMGDEPLOYDIR}/${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.hdddirect
 
 	populate_kernel $HDDDIR
 
@@ -103,21 +110,16 @@ build_boot_dd() {
 
 	dd if=$HDDIMG of=$IMAGE conv=notrunc seek=1 bs=512
 	dd if=${ROOTFS} of=$IMAGE conv=notrunc seek=$OFFSET bs=512
-
-	cd ${IMGDEPLOYDIR}
-
-	ln -sf ${IMAGE_NAME}.hdddirect ${IMGDEPLOYDIR}/${IMAGE_LINK_NAME}.hdddirect
 } 
 
-python do_bootdirectdisk() {
+def boot_direct_disk_prep(d):
     validate_disk_signature(d)
     set_live_vm_vars(d, 'VM')
     if d.getVar("PCBIOS") == "1":
         bb.build.exec_func('build_syslinux_cfg', d)
     if d.getVar("EFI") == "1":
         bb.build.exec_func('build_efi_cfg', d)
-    bb.build.exec_func('build_boot_dd', d)
-}
+    return ''
 
 def generate_disk_signature():
     import uuid
@@ -141,31 +143,5 @@ DISK_SIGNATURE_GENERATED := "${@generate_disk_signature()}"
 
 run_qemu_img (){
     type="$1"
-    qemu-img convert -O $type ${IMGDEPLOYDIR}/${IMAGE_LINK_NAME}.hdddirect ${IMGDEPLOYDIR}/${IMAGE_NAME}.$type
-
-    ln -sf ${IMAGE_NAME}.$type ${IMGDEPLOYDIR}/${IMAGE_LINK_NAME}.$type
-}
-create_vmdk_image () {
-    run_qemu_img vmdk
+    qemu-img convert -O $type ${IMGDEPLOYDIR}/${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.hdddirect ${IMGDEPLOYDIR}/${IMAGE_NAME}${IMAGE_NAME_SUFFIX}.$type
 }
-
-create_vdi_image () {
-    run_qemu_img vdi
-}
-
-create_qcow2_image () {
-    run_qemu_img qcow2
-}
-
-python do_vmimg() {
-    if 'vmdk' in d.getVar('IMAGE_FSTYPES'):
-        bb.build.exec_func('create_vmdk_image', d)
-    if 'vdi' in d.getVar('IMAGE_FSTYPES'):
-        bb.build.exec_func('create_vdi_image', d)
-    if 'qcow2' in d.getVar('IMAGE_FSTYPES'):
-        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"
diff --git a/meta/classes/image_types.bbclass b/meta/classes/image_types.bbclass
index 8f8d79cd3d70..059c0e73e785 100644
--- a/meta/classes/image_types.bbclass
+++ b/meta/classes/image_types.bbclass
@@ -302,5 +302,5 @@ RUNNABLE_MACHINE_PATTERNS ?= "qemu"
 DEPLOYABLE_IMAGE_TYPES ?= "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: hddimg, iso, etc.
 IMAGE_TYPES_MASKED ?= ""
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 f145b5e6568d..1a5d639f4f04 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
@@ -130,4 +130,4 @@ python do_bundle_files() {
     bb.build.exec_func('create_bundle_files', d)
 }
 
-addtask bundle_files after do_vmimg before do_image_complete
+addtask bundle_files after do_image_vmdk before do_image_complete
-- 
1.9.1

[PATCH 2/2] image-vm: Change default ROOT_VM to use PARTUUID

[Tom Rini]

From: Matt Porter <mporter@konsulko.com>

Support for finding the root partition via PARTUUID is a portable method to
determine the system root device as it does not depend on device ordering and
also abstracts the diference between IDE devices and SCSI/SATA/etc type
devices.

Signed-off-by: Matt Porter <mporter@konsulko.com>
Signed-off-by: Tom Rini <trini@konsulko.com>
---
 meta/classes/image-vm.bbclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/classes/image-vm.bbclass b/meta/classes/image-vm.bbclass
index a888f0232caa..a7cabae590c0 100644
--- a/meta/classes/image-vm.bbclass
+++ b/meta/classes/image-vm.bbclass
@@ -42,7 +42,7 @@ ROOTFS ?= "${IMGDEPLOYDIR}/${IMAGE_LINK_NAME}.${VM_ROOTFS_TYPE}"
 
 # Used by bootloader
 LABELS_VM ?= "boot"
-ROOT_VM ?= "root=/dev/sda2"
+ROOT_VM ?= "root=PARTUUID=${DISK_SIGNATURE}-02"
 # Using an initramfs is optional. Enable it by setting INITRD_IMAGE_VM.
 INITRD_IMAGE_VM ?= ""
 INITRD_VM ?= "${@'${IMGDEPLOYDIR}/${INITRD_IMAGE_VM}-${MACHINE}.cpio.gz' if '${INITRD_IMAGE_VM}' else ''}"
-- 
1.9.1

Re: [PATCH 1/2] image-vm: Convert vmdk/vdi/qcow2/hdddirect images to IMAGE_CMD

[Patrick Ohly]

On Thu, 2017-07-20 at 18:43 -0400, Tom Rini wrote:
> The support for writing vmdk/vdi/qcow2 images has not been converted to make
> use of the IMAGE_CMD infrastructure and instead relies on custom logic for
> adding tasks in the right place.  Convert these images to making use of
> IMAGE_CMD.

Thanks for working on this. I still have
https://bugzilla.yoctoproject.org/show_bug.cgi?id=10204 open for
enhancing vmdk/vdi/qcow2.

However, your patch doesn't go as far as described in that bug, does it?
Instead of allowing, for example, IMAGE_FSTYPES = "wic.vmdk", it merely
changes how IMAGE_FSTYPES = "vmdk" is implemented.

The current patch has its merits as it simplifies the implementation,
but I think it would be worthwhile to go all the way, even if it changes
how images are going to be named.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Re: [PATCH 1/2] image-vm: Convert vmdk/vdi/qcow2/hdddirect images to IMAGE_CMD

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1532 bytes --]

On Fri, Jul 21, 2017 at 09:47:21AM +0200, Patrick Ohly wrote:
> On Thu, 2017-07-20 at 18:43 -0400, Tom Rini wrote:
> > The support for writing vmdk/vdi/qcow2 images has not been converted to make
> > use of the IMAGE_CMD infrastructure and instead relies on custom logic for
> > adding tasks in the right place.  Convert these images to making use of
> > IMAGE_CMD.
> 
> Thanks for working on this. I still have
> https://bugzilla.yoctoproject.org/show_bug.cgi?id=10204 open for
> enhancing vmdk/vdi/qcow2.
> 
> However, your patch doesn't go as far as described in that bug, does it?
> Instead of allowing, for example, IMAGE_FSTYPES = "wic.vmdk", it merely
> changes how IMAGE_FSTYPES = "vmdk" is implemented.
> 
> The current patch has its merits as it simplifies the implementation,
> but I think it would be worthwhile to go all the way, even if it changes
> how images are going to be named.

Ah, interesting.  I guess the first thing I would argue against is
saying that hdddirect should be masked.  There are reasons to want to
have a raw image created.

I will take a look into using CONVERSIONCMD for vmdk/qcow2/vdi to allow
for wic.vmdk to work.  But we also must have vmdk.xz work (and fwiw with
my series you can get arbitrary compression that we support done, I did
vmdk.xz, qcow2.lzo and vdi.bz2 just for 'fun').

But I'm not 100% sure that I like changing the normal case from "vmdk"
to "wic.vmdk" or "hdddirect.vmdk" just to get a disk image to throw at
${VM_TECHNOLOGY}.

-- 
Tom

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/2] image-vm: Convert vmdk/vdi/qcow2/hdddirect images to IMAGE_CMD

[Patrick Ohly]

On Fri, 2017-07-21 at 07:21 -0400, Tom Rini wrote:
> On Fri, Jul 21, 2017 at 09:47:21AM +0200, Patrick Ohly wrote:
> > On Thu, 2017-07-20 at 18:43 -0400, Tom Rini wrote:
> > > The support for writing vmdk/vdi/qcow2 images has not been converted to make
> > > use of the IMAGE_CMD infrastructure and instead relies on custom logic for
> > > adding tasks in the right place.  Convert these images to making use of
> > > IMAGE_CMD.
> > 
> > Thanks for working on this. I still have
> > https://bugzilla.yoctoproject.org/show_bug.cgi?id=10204 open for
> > enhancing vmdk/vdi/qcow2.
> > 
> > However, your patch doesn't go as far as described in that bug, does it?
> > Instead of allowing, for example, IMAGE_FSTYPES = "wic.vmdk", it merely
> > changes how IMAGE_FSTYPES = "vmdk" is implemented.
> > 
> > The current patch has its merits as it simplifies the implementation,
> > but I think it would be worthwhile to go all the way, even if it changes
> > how images are going to be named.
> 
> Ah, interesting.  I guess the first thing I would argue against is
> saying that hdddirect should be masked.  There are reasons to want to
> have a raw image created.

I'm not sure I understand what you mean with "hdddirect should be
masked" - it's been a while that I looked at the code.

> I will take a look into using CONVERSIONCMD for vmdk/qcow2/vdi to allow
> for wic.vmdk to work.  But we also must have vmdk.xz work (and fwiw with
> my series you can get arbitrary compression that we support done, I did
> vmdk.xz, qcow2.lzo and vdi.bz2 just for 'fun').

The same is true with CONVERSIONCMD. They can be chained multiple times.

> But I'm not 100% sure that I like changing the normal case from "vmdk"
> to "wic.vmdk" or "hdddirect.vmdk" just to get a disk image to throw at
> ${VM_TECHNOLOGY}.

Yes, that's the biggest user-visible change. Basically the internal
implementation (multiple conversions chained together) becomes visible
in the end-result.

That is a good and a bad thing. The good thing is that it is visible how
the .vmdk image was created. The bad thing is that some users might not
care and get confused.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Re: [PATCH 1/2] image-vm: Convert vmdk/vdi/qcow2/hdddirect images to IMAGE_CMD

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 3146 bytes --]

On Fri, Jul 21, 2017 at 03:01:33PM +0200, Patrick Ohly wrote:
> On Fri, 2017-07-21 at 07:21 -0400, Tom Rini wrote:
> > On Fri, Jul 21, 2017 at 09:47:21AM +0200, Patrick Ohly wrote:
> > > On Thu, 2017-07-20 at 18:43 -0400, Tom Rini wrote:
> > > > The support for writing vmdk/vdi/qcow2 images has not been converted to make
> > > > use of the IMAGE_CMD infrastructure and instead relies on custom logic for
> > > > adding tasks in the right place.  Convert these images to making use of
> > > > IMAGE_CMD.
> > > 
> > > Thanks for working on this. I still have
> > > https://bugzilla.yoctoproject.org/show_bug.cgi?id=10204 open for
> > > enhancing vmdk/vdi/qcow2.
> > > 
> > > However, your patch doesn't go as far as described in that bug, does it?
> > > Instead of allowing, for example, IMAGE_FSTYPES = "wic.vmdk", it merely
> > > changes how IMAGE_FSTYPES = "vmdk" is implemented.
> > > 
> > > The current patch has its merits as it simplifies the implementation,
> > > but I think it would be worthwhile to go all the way, even if it changes
> > > how images are going to be named.
> > 
> > Ah, interesting.  I guess the first thing I would argue against is
> > saying that hdddirect should be masked.  There are reasons to want to
> > have a raw image created.
> 
> I'm not sure I understand what you mean with "hdddirect should be
> masked" - it's been a while that I looked at the code.
> 
> > I will take a look into using CONVERSIONCMD for vmdk/qcow2/vdi to allow
> > for wic.vmdk to work.  But we also must have vmdk.xz work (and fwiw with
> > my series you can get arbitrary compression that we support done, I did
> > vmdk.xz, qcow2.lzo and vdi.bz2 just for 'fun').
> 
> The same is true with CONVERSIONCMD. They can be chained multiple times.
> 
> > But I'm not 100% sure that I like changing the normal case from "vmdk"
> > to "wic.vmdk" or "hdddirect.vmdk" just to get a disk image to throw at
> > ${VM_TECHNOLOGY}.
> 
> Yes, that's the biggest user-visible change. Basically the internal
> implementation (multiple conversions chained together) becomes visible
> in the end-result.
> 
> That is a good and a bad thing. The good thing is that it is visible how
> the .vmdk image was created. The bad thing is that some users might not
> care and get confused.

So, Patrick, Ed and I talked on IRC a bit about this, and I think
without putting too many words in their mouths, I think it can be
summarized like this.

We all agree that vmdk/qcow2/vdi are a type of conversion.  It would be
quite handy to use these conversions on other things, such as wic.

And at least from my point of view, the contents of "hdddirect" doesn't
matter so much as the overall functionality.

So that here's where I'm at now.  I've found out when Patrick's chaining
compression code (so that you could do silly things like ext3.gz.bz2 or
_useful_ things like wic.vmdk.xz) was broken, and I see the best way to
fix that too.  So I'm going to v2 this series shortly which addresses
the problem of chaining compression types, and then brings the VM stuff
over to using that too.

-- 
Tom

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/2] image-vm: Convert vmdk/vdi/qcow2/hdddirect images to IMAGE_CMD

[Patrick Ohly]

On Fri, 2017-07-21 at 16:52 -0400, Tom Rini wrote:
> On Fri, Jul 21, 2017 at 03:01:33PM +0200, Patrick Ohly wrote:
> > On Fri, 2017-07-21 at 07:21 -0400, Tom Rini wrote:
> > > On Fri, Jul 21, 2017 at 09:47:21AM +0200, Patrick Ohly wrote:
> > > > On Thu, 2017-07-20 at 18:43 -0400, Tom Rini wrote:
> > > > > The support for writing vmdk/vdi/qcow2 images has not been converted to make
> > > > > use of the IMAGE_CMD infrastructure and instead relies on custom logic for
> > > > > adding tasks in the right place.  Convert these images to making use of
> > > > > IMAGE_CMD.
> > > > 
> > > > Thanks for working on this. I still have
> > > > https://bugzilla.yoctoproject.org/show_bug.cgi?id=10204 open for
> > > > enhancing vmdk/vdi/qcow2.
> > > > 
> > > > However, your patch doesn't go as far as described in that bug, does it?
> > > > Instead of allowing, for example, IMAGE_FSTYPES = "wic.vmdk", it merely
> > > > changes how IMAGE_FSTYPES = "vmdk" is implemented.
> > > > 
> > > > The current patch has its merits as it simplifies the implementation,
> > > > but I think it would be worthwhile to go all the way, even if it changes
> > > > how images are going to be named.
> > > 
> > > Ah, interesting.  I guess the first thing I would argue against is
> > > saying that hdddirect should be masked.  There are reasons to want to
> > > have a raw image created.
> > 
> > I'm not sure I understand what you mean with "hdddirect should be
> > masked" - it's been a while that I looked at the code.
> > 
> > > I will take a look into using CONVERSIONCMD for vmdk/qcow2/vdi to allow
> > > for wic.vmdk to work.  But we also must have vmdk.xz work (and fwiw with
> > > my series you can get arbitrary compression that we support done, I did
> > > vmdk.xz, qcow2.lzo and vdi.bz2 just for 'fun').
> > 
> > The same is true with CONVERSIONCMD. They can be chained multiple times.
> > 
> > > But I'm not 100% sure that I like changing the normal case from "vmdk"
> > > to "wic.vmdk" or "hdddirect.vmdk" just to get a disk image to throw at
> > > ${VM_TECHNOLOGY}.
> > 
> > Yes, that's the biggest user-visible change. Basically the internal
> > implementation (multiple conversions chained together) becomes visible
> > in the end-result.
> > 
> > That is a good and a bad thing. The good thing is that it is visible how
> > the .vmdk image was created. The bad thing is that some users might not
> > care and get confused.
> 
> So, Patrick, Ed and I talked on IRC a bit about this, and I think
> without putting too many words in their mouths, I think it can be
> summarized like this.
>
> We all agree that vmdk/qcow2/vdi are a type of conversion.  It would be
> quite handy to use these conversions on other things, such as wic.
> 
> And at least from my point of view, the contents of "hdddirect" doesn't
> matter so much as the overall functionality.
> 
> So that here's where I'm at now.  I've found out when Patrick's chaining
> compression code (so that you could do silly things like ext3.gz.bz2 or
> _useful_ things like wic.vmdk.xz) was broken, and I see the best way to
> fix that too.  So I'm going to v2 this series shortly which addresses
> the problem of chaining compression types, and then brings the VM stuff
> over to using that too.

Thanks for the good summary and working on this functionality.

I'd like to add that you pointed out that a "generic" vmdk type is what
some developers ask for when they don't care about the exact way how the
resulting image is created (hdddirect vs. wic, for example). We agreed
that this is useful.

I didn't get around to mentioning it on IRC, but what I would find it
useful is if someone (BSP and/or distro?) can define which kind of raw
image is used as input for the conversion to vmdk. Hard-coding that to
just hdddirect always felt wrong to me.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.