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

[Richard W.M. Jones]

On Wed, Dec 14, 2016 at 05:32:20PM +0100, Pino Toscano wrote:
> 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.

Good idea.

> > +  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.

Emacs also uses .#* so I think it's better to leave the full
generality of wildcards available to us.  The main reason the
operation is slow is because of the roundtrips to the daemon.

> > +                 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?

OK.

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

OK.

Rich.


-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top