On Friday 13 June 2014 13:49:35 Richard W.M. Jones wrote: I'd just reindent it like the other is_* functions right above this one, mostly for readability. In any case, LGTM. -- Pino Toscano

Add the virt-sparsify --tmp option. This works in two ways. Either you can specify a temporary directory, in which case it's just the same as setting $TMPDIR before: virt-sparsify indisk outdisk --tmp /var/tmp Or, as a new feature, you can specify a block device which is directly used (and OVERWRITTEN): virt-sparsify indisk outdisk --tmp /dev/sdX This is useful for oVirt nodes, where there is limited temporary space, but a block device can be assigned to the node. In both cases it is only used in copying mode. In-place sparsification doesn't require large amounts of temporary space. --- sparsify/cmdline.ml | 12 +++++- sparsify/copying.ml | 98 +++++++++++++++++++++++++++++----------------- sparsify/sparsify.ml | 4 +- sparsify/virt-sparsify.pod | 23 ++++++++++- 4 files changed, 95 insertions(+), 42 deletions(-) diff --git a/sparsify/cmdline.ml b/sparsify/cmdline.ml index c96de57..01f66f1 100644 --- a/sparsify/cmdline.ml +++ b/sparsify/cmdline.ml @@ -27,7 +27,8 @@ let prog = Filename.basename Sys.executable_name let error ?exit_code fs = error ~prog ?exit_code fs type mode_t = -| Mode_copying of string * check_t * bool * string option * string option +| Mode_copying of string * check_t * bool * string option * string option * + string option | Mode_in_place and check_t = [`Ignore|`Continue|`Warn|`Fail] @@ -59,6 +60,7 @@ let parse_cmdline () = let machine_readable = ref false in let option = ref "" in let quiet = ref false in + let tmp = ref "" in let verbose = ref false in let trace = ref false in let zeroes = ref [] in @@ -78,6 +80,7 @@ let parse_cmdline () = "-o", Arg.Set_string option, s_"option" ^ " " ^ s_"Add qemu-img options"; "-q", Arg.Set quiet, " " ^ s_"Quiet output"; "--quiet", Arg.Set quiet, ditto; + "--tmp", Arg.Set_string tmp, s_"block|dir" ^ " " ^ s_"Set temporary block device or directory"; "-v", Arg.Set verbose, " " ^ s_"Enable debugging messages"; "--verbose", Arg.Set verbose, ditto; "-V", Arg.Unit display_version, " " ^ s_"Display version and exit"; @@ -113,6 +116,7 @@ read the man page virt-sparsify(1). let machine_readable = !machine_readable in let option = match !option with "" -> None | str -> Some str in let quiet = !quiet in + let tmp = match !tmp with "" -> None | str -> Some str in let verbose = !verbose in let trace = !trace in let zeroes = List.rev !zeroes in @@ -126,6 +130,7 @@ read the man page virt-sparsify(1). printf "zero\n"; printf "check-tmpdir\n"; printf "in-place\n"; + printf "tmp-option\n"; let g = new G.guestfs () in g#add_drive "/dev/null"; g#launch (); @@ -186,12 +191,15 @@ read the man page virt-sparsify(1). if option <> None then error (f_"you cannot use --in-place and -o options together"); + if tmp <> None then + error (f_"you cannot use --in-place and --tmp options together"); + indisk ) in let mode = if not in_place then - Mode_copying (outdisk, check_tmpdir, compress, convert, option) + Mode_copying (outdisk, check_tmpdir, compress, convert, option, tmp) else Mode_in_place in diff --git a/sparsify/copying.ml b/sparsify/copying.ml index 5123bc2..5afb22f 100644 --- a/sparsify/copying.ml +++ b/sparsify/copying.ml @@ -33,8 +33,11 @@ open Cmdline external statvfs_free_space : string -> int64 = "virt_sparsify_statvfs_free_space" +type tmp_place = Directory of string | Block_device of string + let run indisk outdisk check_tmpdir compress convert - format ignores machine_readable option quiet verbose trace zeroes = + format ignores machine_readable option tmp_param + quiet verbose trace zeroes = (* Once we have got past argument parsing and start to create * temporary files (including the potentially massive overlay file), we @@ -65,20 +68,30 @@ let run indisk outdisk check_tmpdir compress convert if output_format = "raw" && compress then error (f_"--compress cannot be used for raw output. Remove this option or use --convert qcow2."); - (* Get virtual size of the input disk. *) - let virtual_size = (new G.guestfs ())#disk_virtual_size indisk in - if not quiet then - printf (f_"Input disk virtual size = %Ld bytes (%s)\n%!") - virtual_size (human_size virtual_size); + (* Use TMPDIR or --tmp parameter? *) + let tmp_place = + match tmp_param with + | None -> Directory Filename.temp_dir_name (* $TMPDIR or /tmp *) + | Some dir when is_directory dir -> Directory dir + | Some dev when is_block_device dev -> Block_device dev + | Some path -> + error (f_"--tmp parameter must point to a directory or a block device") in - (* Check there is enough space in $TMPDIR. *) - let tmpdir = Filename.temp_dir_name in + (* Check there is enough space in temporary directory. *) + (match tmp_place with + | Block_device _ -> () + | Directory tmpdir -> + (* Get virtual size of the input disk. *) + let virtual_size = (new G.guestfs ())#disk_virtual_size indisk in + if not quiet then + printf (f_"Input disk virtual size = %Ld bytes (%s)\n%!") + virtual_size (human_size virtual_size); - let print_warning () = - let free_space = statvfs_free_space tmpdir in - let extra_needed = virtual_size -^ free_space in - if extra_needed > 0L then ( - eprintf (f_"\ + let print_warning () = + let free_space = statvfs_free_space tmpdir in + let extra_needed = virtual_size -^ free_space in + if extra_needed > 0L then ( + eprintf (f_"\ WARNING: There may not be enough free space on %s. You may need to set TMPDIR to point to a directory with more free space. @@ -92,34 +105,47 @@ You can ignore this warning or change it to a hard failure using the --check-tmpdir=(ignore|continue|warn|fail) option. See virt-sparsify(1). %!") - tmpdir (human_size virtual_size) - (human_size free_space) (human_size extra_needed); - true - ) else false - in + tmpdir (human_size virtual_size) + (human_size free_space) (human_size extra_needed); + true + ) else false + in - (match check_tmpdir with - | `Ignore -> () - | `Continue -> ignore (print_warning ()) - | `Warn -> - if print_warning () then ( - eprintf "Press RETURN to continue or ^C to quit.\n%!"; - ignore (read_line ()) - ); - | `Fail -> - if print_warning () then ( - eprintf "Exiting because --check-tmpdir=fail was set.\n%!"; - exit 2 - ) + match check_tmpdir with + | `Ignore -> () + | `Continue -> ignore (print_warning ()) + | `Warn -> + if print_warning () then ( + eprintf "Press RETURN to continue or ^C to quit.\n%!"; + ignore (read_line ()) + ); + | `Fail -> + if print_warning () then ( + eprintf "Exiting because --check-tmpdir=fail was set.\n%!"; + exit 2 + ) ); - if not quiet then - printf (f_"Create overlay file in %s to protect source disk ...\n%!") tmpdir; - (* Create the temporary overlay file. *) let overlaydisk = - let tmp = Filename.temp_file "sparsify" ".qcow2" in - unlink_on_exit tmp; + if not quiet then ( + match tmp_place with + | Directory tmpdir -> + printf (f_"Create overlay file in %s to protect source disk ...\n%!") + tmpdir + | Block_device device -> + printf (f_"Create overlay device %s to protect source disk ...\n%!") + device + ); + + let tmp = + match tmp_place with + | Directory temp_dir -> + let tmp = Filename.temp_file ~temp_dir "sparsify" ".qcow2" in + unlink_on_exit tmp; + tmp + + | Block_device device -> device in (* Create it with the indisk as the backing file. *) (* XXX Old code used to: diff --git a/sparsify/sparsify.ml b/sparsify/sparsify.ml index f148296..529a054 100644 --- a/sparsify/sparsify.ml +++ b/sparsify/sparsify.ml @@ -34,9 +34,9 @@ let rec main () = parse_cmdline () in (match mode with - | Mode_copying (outdisk, check_tmpdir, compress, convert, option) -> + | Mode_copying (outdisk, check_tmpdir, compress, convert, option, tmp) -> Copying.run indisk outdisk check_tmpdir compress convert - format ignores machine_readable option quiet verbose trace zeroes + format ignores machine_readable option tmp quiet verbose trace zeroes | Mode_in_place -> In_place.run indisk format ignores machine_readable quiet verbose trace zeroes diff --git a/sparsify/virt-sparsify.pod b/sparsify/virt-sparsify.pod index 5573c95..3aac096 100644 --- a/sparsify/virt-sparsify.pod +++ b/sparsify/virt-sparsify.pod @@ -127,8 +127,8 @@ Display help. =item B<--check-tmpdir=fail> -Check if L</TMPDIR> has enough space to complete the operation. This -is just an estimate. +Check if L</TMPDIR> or I<--tmp> directory has enough space to complete +the operation. This is just an estimate. If the check indicates a problem, then you can either: @@ -246,6 +246,22 @@ You cannot use this option and I<--in-place> together. This disables progress bars and other unnecessary output. +=item B<--tmp> block_device + +=item B<--tmp> dir + +In copying mode only, use the named device, directory or filename as +the location of the temporary overlay (see also L</TMPDIR> below). + +If the parameter given is a block device, then the block device is +written to directly. +B<Note this erases the existing contents of the block device>. + +If the parameter is a directory, then this is the same as setting the +L</TMPDIR> environment variable. + +You cannot use this option and I<--in-place> together. + =item B<-v> =item B<--verbose> @@ -358,6 +374,9 @@ See L<guestfs(3)/WINDOWS HIBERNATION AND WINDOWS 8 FAST STARTUP>. Location of the temporary directory used for the potentially large temporary overlay file. +In virt-sparsify E<ge> 1.28, you can override this environment +variable using the I<--tmp> option. + You should ensure there is enough free space in the worst case for a full copy of the source disk (I<virtual> size), or else set C<$TMPDIR> to point to another directory that has enough space. -- 1.9.0