Libguestfs - Apr 2016 - [PATCH 1/2] sparsify: Refactor handling of checks of copying mode / --in-place.

Just refactoring, no change.
---
 sparsify/cmdline.ml | 49 +++++++++++++++++++++++--------------------------
 1 file changed, 23 insertions(+), 26 deletions(-)

diff --git a/sparsify/cmdline.ml b/sparsify/cmdline.ml
index ce2b913..bd49e71 100644
--- a/sparsify/cmdline.ml
+++ b/sparsify/cmdline.ml
@@ -98,6 +98,7 @@ read the man page virt-sparsify(1).
   let check_tmpdir = !check_tmpdir in
   let compress = !compress in
   let convert = match !convert with "" -> None | str -> Some
str in
+  let disks = List.rev !disks in
   let format = match !format with "" -> None | str -> Some str
in
   let ignores = List.rev !ignores in
   let in_place = !in_place in
@@ -109,7 +110,7 @@ read the man page virt-sparsify(1).
   (* No arguments and machine-readable mode?  Print out some facts
    * about what this binary supports.
    *)
-  if !disks = [] && machine_readable then (
+  if disks = [] && machine_readable then (
     printf "virt-sparsify\n";
     printf "linux-swap\n";
     printf "zero\n";
@@ -126,23 +127,19 @@ read the man page virt-sparsify(1).
     exit 0
   );
 
-  (* Verify we got exactly 1 or 2 disks, depending on the mode. *)
-  let indisk, outdisk -    match in_place, List.rev !disks with
-    | false, [indisk; outdisk] -> indisk, outdisk
-    | true, [disk] -> disk, ""
-    | _ ->
-      error "usage is: %s [--options] indisk outdisk OR %s --in-place
disk"
-        prog prog in
+  let indisk, mode +    if not in_place then (      (* copying mode checks *)
+      let indisk, outdisk +        match disks with
+        | [indisk; outdisk] -> indisk, outdisk
+        | _ -> error (f_"usage: %s [--options] indisk outdisk")
prog in
 
-  (* Simple-minded check that the user isn't trying to use the
-   * same disk for input and output.
-   *)
-  if indisk = outdisk then
-    error (f_"you cannot use the same disk image for input and
output");
+      (* Simple-minded check that the user isn't trying to use the
+       * same disk for input and output.
+       *)
+      if indisk = outdisk then
+        error (f_"you cannot use the same disk image for input and
output");
 
-  let indisk -    if not in_place then (
       (* The input disk must be an absolute path, so we can store the name
        * in the overlay disk.
        *)
@@ -155,11 +152,17 @@ read the man page virt-sparsify(1).
       (* Check the output is not a char special (RHBZ#1056290). *)
       if is_char_device outdisk then
         error (f_"output '%s' cannot be a character device, it
must be a regular file")
-          outdisk;
+              outdisk;
 
-      indisk
+      indisk,
+      Mode_copying (outdisk, check_tmpdir, compress, convert, option, tmp)
     )
-    else (                              (* --in-place checks *)
+    else (                      (* --in-place checks *)
+      let indisk +        match disks with
+        | [indisk] -> indisk
+        | _ -> error "usage: %s --in-place [--options] indisk"
prog in
+
       if check_tmpdir <> `Warn then
         error (f_"you cannot use --in-place and --check-tmpdir options
together");
 
@@ -175,15 +178,9 @@ read the man page virt-sparsify(1).
       if tmp <> None then
         error (f_"you cannot use --in-place and --tmp options
together");
 
-      indisk
+      indisk, Mode_in_place
     ) in
 
-  let mode -    if not in_place then
-      Mode_copying (outdisk, check_tmpdir, compress, convert, option, tmp)
-    else
-      Mode_in_place in
-
   { indisk = indisk;
     format = format;
     ignores = ignores;
-- 
2.7.4

No functional change, just various improvements to the safety of match
statements.
---
 customize/customize_main.ml                        | 4 ++--
 dib/dib.ml                                         | 2 +-
 sysprep/main.ml                                    | 2 +-
 sysprep/sysprep_operation_package_manager_cache.ml | 2 +-
 v2v/convert_linux.ml                               | 2 +-
 v2v/input_ova.ml                                   | 3 +--
 v2v/output_libvirt.ml                              | 8 ++++----
 7 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/customize/customize_main.ml b/customize/customize_main.ml
index 55cff2d..5b7712e 100644
--- a/customize/customize_main.ml
+++ b/customize/customize_main.ml
@@ -135,9 +135,9 @@ read the man page virt-customize(1).
                   ~readonly ?discard
                   ?libvirturi ~allowuuid ~readonlydisk
                   dom)
-    | _, Some _ ->
+    | _::_, Some _ ->
       error (f_"you cannot give -a and -d options together. Read
virt-customize(1) man page for further information.")
-    | files, None ->
+    | (_::_) as files, None ->
       fun g readonly ->
         List.iter (
           fun (uri, format) ->
diff --git a/dib/dib.ml b/dib/dib.ml
index 35ae6b7..534a072 100644
--- a/dib/dib.ml
+++ b/dib/dib.ml
@@ -365,7 +365,7 @@ let run_parts ~debug ~sysroot ~blockdev ~log_file ?(new_wd =
"")
   let new_wd      match sysroot, new_wd with
     | (Out|Subroot), "" -> "''"
-    | _, dir -> dir in
+    | (In|Out|Subroot), dir -> dir in
   List.iter (
     fun x ->
       message (f_"Running: %s/%s") hook_name x;
diff --git a/sysprep/main.ml b/sysprep/main.ml
index 1441619..6f331b5 100644
--- a/sysprep/main.ml
+++ b/sysprep/main.ml
@@ -173,7 +173,7 @@ read the man page virt-sysprep(1).
                     ~readonly ?discard
                     ?libvirturi ~allowuuid ~readonlydisk
                     dom)
-      | _, Some _ ->
+      | _::_, Some _ ->
         error (f_"you cannot give -a and -d options together.  Read
virt-sysprep(1) man page for further information.")
       | files, None ->
         fun g readonly ->
diff --git a/sysprep/sysprep_operation_package_manager_cache.ml
b/sysprep/sysprep_operation_package_manager_cache.ml
index f35764a..428352d 100644
--- a/sysprep/sysprep_operation_package_manager_cache.ml
+++ b/sysprep/sysprep_operation_package_manager_cache.ml
@@ -37,7 +37,7 @@ let package_manager_cache_perform (g : Guestfs.guestfs) root
side_effects      | _ -> None in
   match cache_dirs with
   | Some dirs -> List.iter (rm_rf_only_files g) dirs
-  | _ -> ()
+  | None -> ()
 
 let op = {
   defaults with
diff --git a/v2v/convert_linux.ml b/v2v/convert_linux.ml
index 951aa32..e5778ef 100644
--- a/v2v/convert_linux.ml
+++ b/v2v/convert_linux.ml
@@ -1140,7 +1140,7 @@ let rec convert ~keep_serial_console (g : G.guestfs)
inspect source rcaps          match block_type with
         | Virtio_blk -> "virtio_blk"
         | Virtio_SCSI -> "virtio_scsi"
-        | _ -> assert false in
+        | IDE -> assert false in
 
       if paths <> [] then (
         (* There's only 1 scsi controller in the converted guest.
diff --git a/v2v/input_ova.ml b/v2v/input_ova.ml
index db0588d..1aba662 100644
--- a/v2v/input_ova.ml
+++ b/v2v/input_ova.ml
@@ -85,8 +85,7 @@ object
           let zcat, tar_fmt              match format with
             | `GZip -> "zcat", "z"
-            | `XZ -> "xzcat", "J"
-            | _ -> assert false in
+            | `XZ -> "xzcat", "J" in
           let tmpfile = uncompress_head zcat ova in
           let tmpfiletype = detect_file_type tmpfile in
           (* Remove tmpfile from tmpdir, to leave it empty. *)
diff --git a/v2v/output_libvirt.ml b/v2v/output_libvirt.ml
index aedde61..bcb9db2 100644
--- a/v2v/output_libvirt.ml
+++ b/v2v/output_libvirt.ml
@@ -242,10 +242,10 @@ let create_libvirt_xml ?pool source target_buses guestcaps
 
   (match source.s_display with
    | Some { s_keymap = Some km } -> append_attr ("keymap", km)
graphics
-   | _ -> ());
+   | Some { s_keymap = None } | None -> ());
   (match source.s_display with
    | Some { s_password = Some pw } -> append_attr ("passwd", pw)
graphics
-   | _ -> ());
+   | Some { s_password = None } | None -> ());
   (match source.s_display with
    | Some { s_listen = listen } ->
       (match listen with
@@ -256,12 +256,12 @@ let create_libvirt_xml ?pool source target_buses guestcaps
           let sub = e "listen" [ "type",
"network"; "network", n ] [] in
           append_child sub graphics
        | LNone -> ())
-   | _ -> ());
+   | None -> ());
   (match source.s_display with
    | Some { s_port = Some p } ->
       append_attr ("autoport", "no") graphics;
       append_attr ("port", string_of_int p) graphics
-   | _ ->
+   | Some { s_port = None } | None ->
       append_attr ("autoport", "yes") graphics;
       append_attr ("port", "-1") graphics);
 
-- 
2.7.4

On Thursday 21 April 2016 12:52:43 Richard W.M. Jones
wrote:
> Just refactoring, no change.
> ---
Both LGTM.

Thanks,
-- 
Pino Toscano