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

[Pino Toscano]

On Saturday, 12 November 2016 16:37:50 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          | 36 ++++++++++++++++++++----------------
>  v2v/test-v2v-i-ova-gz.ovf |  2 +-
>  2 files changed, 21 insertions(+), 17 deletions(-)
> 
> diff --git a/v2v/input_ova.ml b/v2v/input_ova.ml
> index b283629..db884d9 100644
> --- a/v2v/input_ova.ml
> +++ b/v2v/input_ova.ml
> @@ -275,25 +275,29 @@ 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 comprression in OVF: %s") s in

Typo here, "compression".

Also, the two "expr" and "compressed" let could be isolated in the
"let filename" block.

> +
> +          let filename = if compressed then (
> +            let new_filename = tmpdir // String.random8 () ^ ".vmdk" in
> +            let cmd =
> +              sprintf "zcat %s > %s" (quote ovf_folder // filename) (quote new_filename) in
> +            if shell_command cmd <> 0 then
> +              error (f_"error uncompressing %s, see earlier error messages")
> +                filename;
> +            new_filename
> +          )
> +          else
> +            ovf_folder // filename
> +          in

IMHO this code should be where the current code is: Unix.access is
currently done on the original file, while with your code it would be
done only on the uncompressed file (either the uncompressed original,
or the decompressed one). IMHO it is always better to do the existency
check on our side, and assume that the uncompressed file will exist
if zcat didn't fail.

Thanks,
-- 
Pino Toscano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20161121/c3e9af14/attachment.sig>