On Fri, 09 Dec 2016 14:01:40 +0100
Pino Toscano <ptoscano(a)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(a)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.
Also the main reason for doing this check was to be able to skip the
detection. The detection would be problematic because we would have to
extract the header of each file, perform the check and clean the
leftovers. This seems like a lot of hassle to just handle some
potentially broken OVAs. Or do we know this really happened in the past?
Tomas
--
Tomáš Golembiovský <tgolembi(a)redhat.com>