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

[Richard Purdie <richard.purdie@linuxfoundation.org>]

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