Bug 853763

Summary: virt-sparsify should use a more robust method to detect the input format
Product: Red Hat Enterprise Linux 6 Reporter: Richard W.M. Jones <rjones>
Component: libguestfsAssignee: Richard W.M. Jones <rjones>
Status: CLOSED ERRATA QA Contact: Virtualization Bugs <virt-bugs>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: 6.4CC: dyasny, leiwang, mbooth, moli, qguan, wshi
Target Milestone: rc   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: libguestfs-1.16.31-2.el6 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: 853762 Environment:
Last Closed: 2013-02-21 03:38:42 EST Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
Bug Depends On: 853762    
Bug Blocks:    

Description Richard W.M. Jones 2012-09-02 14:34:36 EDT
+++ This bug was initially created as a clone of Bug #853762 +++

Description of problem:

Currently virt-sparsify runs the 'file -bsL' command on the
input disk image to try to detect the format.  This is not
robust as 'file' is often broken or might not know about the
disk format.  'file' has also been known to print different
things for different versions of the command, which is why
virt-sparsify breaks in RHEL 6.3:

rhel$ file -bsL /tmp/disk.qcow2 
Qemu Image, Format: Qcow , Version: 2
fedora$ file -bsL /tmp/disk.qcow2 
QEMU QCOW Image (v2), 10485760 bytes

Version-Release number of selected component (if applicable):

libguestfs 1.16.19

How reproducible:

100%

Steps to Reproduce:
1. Use virt-sparsify on a file.
2. Don't specify the --convert argument.

Actual results:

Results will depend on the input format and the version of
'file' installed.  For example on RHEL 6.3 it will detect the
input format as raw, and so produce a raw output file.
Comment 2 Mohua Li 2012-11-07 01:40:21 EST
hi Rich,

i could found this commit in master branch, but didn't find in 1.16 branch, if we forget to backport this commit?



index 379ccf4..fa729f3 100644
--- a/sparsify/sparsify.ml
+++ b/sparsify/sparsify.ml
@@ -351,25 +351,10 @@ let output_format =
     | Some fmt -> fmt           (* user specified input format, use that *)
     | None ->
       (* Don't know, so we must autodetect. *)
-      let cmd = sprintf "file -bsL %s" (Filename.quote indisk) in
-      let chan = open_process_in cmd in
-      let line = input_line chan in
-      let stat = close_process_in chan in
-      (match stat with
-      | WEXITED 0 -> ()
-      | WEXITED _ ->
-        error (f_"external command failed: %s") cmd
-      | WSIGNALED i ->
-        error (f_"external command '%s' killed by signal %d") cmd i
-      | WSTOPPED i ->
-        error (f_"external command '%s' stopped by signal %d") cmd i
-      );
-      if string_prefix line "QEMU QCOW Image (v2)" then
-        "qcow2"
-      else if string_find line "VirtualBox" >= 0 then
-        "vdi"
-      else
-        "raw" (* XXX guess *)
+      match g#disk_format indisk  with
+      | "unknown" ->
+        error (f_"cannot detect input disk format; use the --format parameter")
+      | fmt -> fmt
 
 (* Now run qemu-img convert which copies the overlay to the
  * destination and automatically does sparsification.


but in 1.16, still get this, 


 *)
let output_format =
  match convert with
  | Some fmt -> fmt             (* user specified output conversion *)
  | None ->
    match format with
    | Some fmt -> fmt           (* user specified input format, use that *)
    | None ->
      (* Don't know, so we must autodetect. *)
      let cmd = sprintf "file -bsL %s" (Filename.quote indisk) in
      let chan = open_process_in cmd in
      let line = input_line chan in
      let stat = close_process_in chan in
      (match stat with
      | WEXITED 0 -> ()
      | WEXITED _ ->
        error "external command failed: %s" cmd
      | WSIGNALED i ->
        error "external command '%s' killed by signal %d" cmd i
      | WSTOPPED i ->
        error "external command '%s' stopped by signal %d" cmd i
      );
      if string_prefix line "QEMU QCOW Image (v2)" then
        "qcow2"
      else if string_find line "VirtualBox" >= 0 then
        "vdi"
      else
        "raw" (* XXX guess *)
Comment 3 Richard W.M. Jones 2012-11-07 02:14:09 EST
This is the patch that I added to RHEL 6.  Note it fixes
the problem (different 'file' command output), but it does
not use the guestfs_disk_format method.  The reason for not
using the guestfs_disk_format method is that it doesn't exist
in RHEL 6 / libguestfs 1.16.  Backporting that method would
be possible, but I opted for the simple (safer?) one-line
fix instead.


commit 9a772e90efef6b0313367600f3ff71916fee0ecc
Author: Richard W.M. Jones <rjones@redhat.com>
Date:   Thu Sep 27 16:36:56 2012 +0100

    RHEL 6: Change virt-sparsify to work with old 'file' command (RHBZ#853763).
    
    The old 'file' command in RHEL produces different output from
    upstream / Fedora:
    
      rhel$ file -bsL /tmp/disk.qcow2
      Qemu Image, Format: Qcow , Version: 2
      fedora$ file -bsL /tmp/disk.qcow2
      QEMU QCOW Image (v2), 10485760 bytes
    
    Upstream solves this in a more robust way, but that would involve
    difficult backporting of a new API.

diff --git a/sparsify/sparsify.ml b/sparsify/sparsify.ml
index 544c8ab..bf0d3d0 100644
--- a/sparsify/sparsify.ml
+++ b/sparsify/sparsify.ml
@@ -299,7 +299,7 @@ let output_format =
       | WSTOPPED i ->
         error "external command '%s' stopped by signal %d" cmd i
       );
-      if string_prefix line "QEMU QCOW Image (v2)" then
+      if string_prefix line "Qemu Image, Format: Qcow , Version: 2" then
         "qcow2"
       else if string_find line "VirtualBox" >= 0 then
         "vdi"
Comment 6 errata-xmlrpc 2013-02-21 03:38:42 EST
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

http://rhn.redhat.com/errata/RHBA-2013-0324.html