[Libguestfs] [PATCH v2 3/4] sysprep: Add a new operation to remove editor backup files (RHBZ#1401320).

[Pino Toscano]

On Wednesday, 14 December 2016 13:12:01 CET Richard W.M. Jones wrote:
> Remove editor backup files such as *~ and *.bak wherever
> they occur within the guest filesystem.
> 
> This also includes a test.
> ---

Looks nice -- I have few notes below.

Also, should this operation include also the swap files that editors
can leave while editing?

> +  let fses =
> +    match g#inspect_get_type root with
> +    | "hurd"
> +    | "linux"
> +    | "minix" -> unix_whitelist
> +    | f when String.is_suffix f "bsd" -> unix_whitelist
> +    | _ -> ["/"] in

There's a similar list also in customize/customize_run.ml for the
`SSHInject operation -- I guess an helper function could be added to
Common_utils.

> +  List.iter (
> +    fun fs ->
> +      if g#is_dir ~followsymlinks:false fs then (
> +        visit g#ocaml_handle fs (
> +          fun dir filename { G.st_mode = mode } _ ->
> +            match dir, filename, mode with
> +            (* Ignore root directory and non-regular files. *)
> +            | _, None, _ -> ()
> +            | _, Some _, mode when not (is_reg mode) -> ()
> +            | dir, Some filename, _ ->
> +               (* Check the filename against all of the globs, and if it
> +                * matches any then delete it.
> +                *)
> +               let matching glob = fnmatch glob filename [FNM_NOESCAPE] in
> +               if List.exists matching globs then (

Considering the current globs are basically filename suffixes, wouldn't
it be easier (and possibly faster too) to just check for them?

  let suffixes = [ ".bak"; "~" ] in
  ...
    if List.exists (String.is_suffix filename) suffixes then (
      ...

This would be easier to extend also for filename prefixes, in case we
need them, so for now the fnmatch usage can be avoided.

> +                 let path = full_path dir (Some filename) in
> +                 g#rm_f path
> +               )
> +        )
> +      )
> +  ) fses
> +
> +let op = {
> +  defaults with
> +    name = "backup-files";
> +    enabled_by_default = true;
> +    heading = s_"Remove editor backup files from the guest";
> +    pod_description = Some (
> +      sprintf (f_"\
> +The following files are removed from anywhere in the guest
> +filesystem:
> +
> +%s
> +
> +On Linux and Unix operating systems, only the following filesystems
> +will be examined:
> +
> +%s") globs_as_pod unix_whitelist_as_pod);
> +    perform_on_filesystems = Some backup_files_perform;
> +}
> +
> +let () = register_operation op
> diff --git a/sysprep/test-virt-sysprep-backup-files.sh b/sysprep/test-virt-sysprep-backup-files.sh
> new file mode 100755
> index 0000000..bd9a36c
> --- /dev/null
> +++ b/sysprep/test-virt-sysprep-backup-files.sh
> @@ -0,0 +1,64 @@
> [...]
> +guestfish -a test-backup-files.qcow2 -i <<'EOF'

Can you please specify --format qcow2 explicitly?

> +# Check the file list is the same as above.
> +guestfish -a test-backup-files.qcow2 -i find / > test-backup-files-after

Ditto.

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/20161214/57f87119/attachment.sig>