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

[Ed Bartosh <ed.bartosh@linux.intel.com>]

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