[Libguestfs] [PATCH v3 2/6] v2v: ova: don't detect compressed disks, read the OVF instead

[Richard W.M. Jones]

On Wed, Jan 04, 2017 at 10:12:13AM +0100, Pino Toscano wrote:
> On Saturday, 10 December 2016 13:50:08 CET Tomas Golembiovsky wrote:
> > On Fri, 09 Dec 2016 14:01:40 +0100
> > Pino Toscano <ptoscano at redhat.com> wrote:
> > 
> > > On Wednesday, 7 December 2016 17:13:06 CET Tomáš Golembiovský wrote:
> > > > The information whether the disk is gzip compressed or not is stored
> > > > in the OVF. There is no reason to do the detection.
> > > >
> > > > Signed-off-by: Tomáš Golembiovský <tgolembi at redhat.com>
> > > > ---
> > > >  v2v/input_ova.ml          | 28 +++++++++++++++++-----------
> > > >  v2v/test-v2v-i-ova-gz.ovf |  2 +-
> > > >  2 files changed, 18 insertions(+), 12 deletions(-)
> > > >
> > > > diff --git a/v2v/input_ova.ml b/v2v/input_ova.ml
> > > > index b283629..61930f0 100644
> > > > --- a/v2v/input_ova.ml
> > > > +++ b/v2v/input_ova.ml
> > > > @@ -275,6 +275,13 @@ object
> > > >              | None -> error (f_"no href in ovf:File (id=%s)") file_ref
> > > >              | Some s -> s in
> > > >
> > > > +          let expr = sprintf "/ovf:Envelope/ovf:References/ovf:File[@ovf:id='%s']/@ovf:compression" file_ref in
> > > > +          let compressed =
> > > > +            match xpath_string expr with
> > > > +            | None | Some "identity" -> false
> > > > +            | Some "gzip" -> true
> > > > +            | Some s -> error (f_"unsupported compression in OVF: %s") s in
> > >
> > > I'd still do the detection when no ovf:compression attribute is found
> > > (that is, the None case above).
> > > After all, right now we do the detection for any ova already, so a
> > > "broken ova" (with no compression attribute in the ovf but with a
> > > compressed image) is already handled by the current code.
> > 
> > I understand what you are trying to achieve here. Unfortunately
> > that's not possible, missing attribute means "no compression". As the
> > specification says:
> > 
> >     When a File element is compressed using gzip, the ovf:compression
> >     attribute shall be set to “gzip”. Otherwise, the ovf:compression
> >     attribute shall be set to “identity” or the entire attribute
> >     omitted.
> 
> Yep, I know this -- OTOH, we make extra efforts to support OVAs actually
> produced, even with non-standard features (example of this: all the
> extra formats -- tar.gz. tar.xz, zip -- instead of the uncompressed tar
> container).

I agree we need to try to work with OVAs that exist, which may not
match the specification.  Having said that I have no particular
knowledge about this subject.

But I do have a collection of OVAs from may different sources, so I
was able to look at the ovf:compression attribute and see if it
matches the file data.

* I found that in 13 OVF file references, ovf:compression="gzip" was
  used.  In all of those cases, the file reference was indeed to a
  gzip-compressed file.

* I did not find any other use of ovf:compression (neither "identity",
  not any other value except for "gzip").

* I did not find any case where there was a compressed disk, but the
  ovf:compression attribute was not used.

Therefore I conclude (from a rather small sample, unfortunately), it
does look as if the ovf:compressed attribute is always used in
accordance with the specification, and so Tomas's patch looks OK.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v