Pino Toscano
2015-Jul-01 13:12 UTC
[Libguestfs] [PATCH 1/3] mllib: add an optional filter for rm_rf_only_files
This way it is possible to use rm_rf_only_files, but not removing
specific files.
---
mllib/common_utils.ml | 8 +++++++-
mllib/common_utils.mli | 5 ++++-
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/mllib/common_utils.ml b/mllib/common_utils.ml
index 516cff3..3737b4c 100644
--- a/mllib/common_utils.ml
+++ b/mllib/common_utils.ml
@@ -640,13 +640,19 @@ let rmdir_on_exit * without removing the actual
directory structure. Also if 'dir' is
* not a directory or doesn't exist, ignore it.
*
+ * The optional filter is used to filter out files which will be
+ * removed: files returning true are not removed.
+ *
* XXX Could be faster with a specific API for doing this.
*)
-let rm_rf_only_files (g : Guestfs.guestfs) dir +let rm_rf_only_files (g :
Guestfs.guestfs) ?filter dir if g#is_dir dir then (
let files = Array.map (Filename.concat dir) (g#find dir) in
let files = Array.to_list files in
let files = List.filter g#is_file files in
+ let files = match filter with
+ | None -> files
+ | Some f -> List.filter (fun x -> not (f x)) files in
List.iter g#rm files
)
diff --git a/mllib/common_utils.mli b/mllib/common_utils.mli
index bf0cba6..a5b0a68 100644
--- a/mllib/common_utils.mli
+++ b/mllib/common_utils.mli
@@ -140,12 +140,15 @@ val unlink_on_exit : string -> unit
val rmdir_on_exit : string -> unit
(** Remove a temporary directory on exit (using [rm -rf]). *)
-val rm_rf_only_files : Guestfs.guestfs -> string -> unit
+val rm_rf_only_files : Guestfs.guestfs -> ?filter:(string -> bool) ->
string -> unit
(** Using the libguestfs API, recursively remove only files from the
given directory. Useful for cleaning [/var/cache] etc in sysprep
without removing the actual directory structure. Also if [dir] is
not a directory or doesn't exist, ignore it.
+ The optional [filter] is used to filter out files which will be
+ removed: files returning true are not removed.
+
XXX Could be faster with a specific API for doing this. *)
val truncate_recursive : Guestfs.guestfs -> string -> unit
--
2.1.0
Pino Toscano
2015-Jul-01 13:12 UTC
[Libguestfs] [PATCH 2/3] mllib: add and use last_part_of
Collect this small snippet to get the part of a string after the last
occurrency of a character; replace with it the current snippets doing
the same.
Should be just code motion.
---
customize/password.ml | 4 +---
mllib/common_utils.ml | 5 +++++
mllib/common_utils.mli | 3 +++
sysprep/sysprep_operation_user_account.ml | 4 +---
v2v/convert_linux.ml | 4 +---
v2v/utils.ml | 9 ++-------
6 files changed, 13 insertions(+), 16 deletions(-)
diff --git a/customize/password.ml b/customize/password.ml
index 25ce901..cb97804 100644
--- a/customize/password.ml
+++ b/customize/password.ml
@@ -97,9 +97,7 @@ let rec set_linux_passwords ?password_crypto (g :
Guestfs.guestfs) root password
let users = Array.to_list (g#aug_ls "/files/etc/shadow") in
List.iter (
fun userpath ->
- let user - let i = String.rindex userpath '/' in
- String.sub userpath (i+1) (String.length userpath -i-1) in
+ let user = last_part_of userpath '/' in
try
(* Each line is: "user:[!!]password:..."
* !! at the front of the password field means the account is locked.
diff --git a/mllib/common_utils.ml b/mllib/common_utils.ml
index 3737b4c..8601009 100644
--- a/mllib/common_utils.ml
+++ b/mllib/common_utils.ml
@@ -738,3 +738,8 @@ let guest_arch_compatible guest_arch | x, y when x = y
-> true
| "x86_64",
("i386"|"i486"|"i586"|"i686") -> true
| _ -> false
+
+(** Return the last part of a string, after the specified separator. *)
+let last_part_of str sep + let i = String.rindex str sep in
+ String.sub str (i+1) (String.length str - (i+1))
diff --git a/mllib/common_utils.mli b/mllib/common_utils.mli
index a5b0a68..7d0b38f 100644
--- a/mllib/common_utils.mli
+++ b/mllib/common_utils.mli
@@ -178,3 +178,6 @@ val mkdir_p : string -> int -> unit
val guest_arch_compatible : string -> bool
(** Are guest arch and host_cpu compatible, in terms of being able
to run commands in the libguestfs appliance? *)
+
+val last_part_of : string -> char -> string
+(** Return the last part of a string, after the specified separator. *)
diff --git a/sysprep/sysprep_operation_user_account.ml
b/sysprep/sysprep_operation_user_account.ml
index 3f3b142..fe460b8 100644
--- a/sysprep/sysprep_operation_user_account.ml
+++ b/sysprep/sysprep_operation_user_account.ml
@@ -68,9 +68,7 @@ let user_account_perform g root side_effects let uid
= userpath ^ "/uid" in
let uid = g#aug_get uid in
let uid = int_of_string uid in
- let username - let i = String.rindex userpath '/' in
- String.sub userpath (i+1) (String.length userpath -i-1) in
+ let username = last_part_of userpath '/' in
if uid >= uid_min && uid <= uid_max
&& check_remove_user username then (
changed := true;
diff --git a/v2v/convert_linux.ml b/v2v/convert_linux.ml
index 7967c0f..cadd515 100644
--- a/v2v/convert_linux.ml
+++ b/v2v/convert_linux.ml
@@ -795,10 +795,8 @@ let rec convert ~keep_serial_console (g : G.guestfs)
inspect source *)
let mkinitrd_kv let modpath = kernel.ki_modpath in
- let len = String.length modpath in
try
- let i = String.rindex modpath '/' in
- String.sub modpath (i+1) (len - (i+1))
+ last_part_of modpath '/'
with
Not_found ->
invalid_arg (sprintf "invalid module path: %s" modpath)
in
diff --git a/v2v/utils.ml b/v2v/utils.ml
index 1f8c357..4d6b08d 100644
--- a/v2v/utils.ml
+++ b/v2v/utils.ml
@@ -176,9 +176,7 @@ let find_virtio_win_drivers virtio_win let paths =
List.filter (g#is_file ~followsymlinks:false) paths in
List.map (
fun path ->
- let i = String.rindex path '/' in
- let len = String.length path in
- let basename = String.sub path (i+1) (len - (i+1)) in
+ let basename = last_part_of path '/' in
(path, sprintf "%s:%s" virtio_win path,
basename,
fun () -> g#read_file path)
@@ -198,10 +196,7 @@ let find_virtio_win_drivers virtio_win let
lc_path = String.lowercase path in
let lc_basename = String.lowercase basename in
- let extension - let i = String.rindex lc_basename
'.' in
- let len = String.length lc_basename in
- String.sub lc_basename (i+1) (len - (i+1)) in
+ let extension = last_part_of lc_basename '.' in
(* Skip files without specific extensions. *)
if extension <> "cat" && extension <>
"inf" &&
--
2.1.0
Pino Toscano
2015-Jul-01 13:12 UTC
[Libguestfs] [PATCH 3/3] sysprep: rework and fix cron-spool operation (RHBZ#1229305)
When cleaning the directories with cron/at jobs, remove only files
there, as subdirectories might be used by other systems; for example
in Debian under /var/spool/cron/ there is the atjobs subdirectory with
the actual at queue.
Make sure to not remove .SEQ files anymore, as they represent the at job
counter which is needed by the at daemon. Instead, reset these files to
0.
Furthermore, add also the path to the .SEQ location in Debian-based
systems.
---
sysprep/sysprep_operation_cron_spool.ml | 27 ++++++++++++++++++++-------
1 file changed, 20 insertions(+), 7 deletions(-)
diff --git a/sysprep/sysprep_operation_cron_spool.ml
b/sysprep/sysprep_operation_cron_spool.ml
index 687a7e9..888b97a 100644
--- a/sysprep/sysprep_operation_cron_spool.ml
+++ b/sysprep/sysprep_operation_cron_spool.ml
@@ -18,19 +18,32 @@
open Sysprep_operation
open Common_gettext.Gettext
+open Common_utils
module G = Guestfs
let cron_spool_perform (g : Guestfs.guestfs) root side_effects - Array.iter
g#rm_rf (g#glob_expand "/var/spool/cron/*");
+ let is_seq path + let basename + try last_part_of path '/'
+ with Not_found -> path in
+ basename = ".SEQ" in
+ let reset f + if g#is_file f then
+ (* This should overwrite the file in-place, as it's a very
+ * small buffer which will be handled using internal_write.
+ * This way, existing attributes like SELinux labels are
+ * preserved.
+ *)
+ g#write f "00000\n" in
+
+ rm_rf_only_files g ~filter:is_seq "/var/spool/cron/";
+ reset "/var/spool/cron/atjobs/.SEQ";
Array.iter g#rm (g#glob_expand "/var/spool/atjobs/*");
- Array.iter g#rm (g#glob_expand "/var/spool/atjobs/.SEQ");
+ reset "/var/spool/atjobs/.SEQ";
Array.iter g#rm (g#glob_expand "/var/spool/atspool/*");
- Array.iter
- (fun path -> if not (g#is_dir path) then g#rm path)
- (g#glob_expand "/var/spool/at/*");
- Array.iter g#rm (g#glob_expand "/var/spool/at/.SEQ");
- Array.iter g#rm (g#glob_expand "/var/spool/at/spool/*")
+ rm_rf_only_files g ~filter:is_seq "/var/spool/at/";
+ reset "/var/spool/at/.SEQ"
let op = {
defaults with
--
2.1.0
Richard W.M. Jones
2015-Jul-01 13:50 UTC
Re: [Libguestfs] [PATCH 2/3] mllib: add and use last_part_of
On Wed, Jul 01, 2015 at 03:12:54PM +0200, Pino Toscano wrote:> Collect this small snippet to get the part of a string after the last > occurrency of a character; replace with it the current snippets doing > the same. > > Should be just code motion.I see a future where we get a customer bug that some utility fails with a mysterious 'Not_found' exception and no stack trace. At the very least we should document that last_part_of can raise Not_found (because of String.rindex). Even better would be if last_part_of returns None | Some substring, which would force all the callers to check for this case and deal with it. I know the code may already have this bug, but best to kill it now. Rich.> customize/password.ml | 4 +--- > mllib/common_utils.ml | 5 +++++ > mllib/common_utils.mli | 3 +++ > sysprep/sysprep_operation_user_account.ml | 4 +--- > v2v/convert_linux.ml | 4 +--- > v2v/utils.ml | 9 ++------- > 6 files changed, 13 insertions(+), 16 deletions(-) > > diff --git a/customize/password.ml b/customize/password.ml > index 25ce901..cb97804 100644 > --- a/customize/password.ml > +++ b/customize/password.ml > @@ -97,9 +97,7 @@ let rec set_linux_passwords ?password_crypto (g : Guestfs.guestfs) root password > let users = Array.to_list (g#aug_ls "/files/etc/shadow") in > List.iter ( > fun userpath -> > - let user > - let i = String.rindex userpath '/' in > - String.sub userpath (i+1) (String.length userpath -i-1) in > + let user = last_part_of userpath '/' in > try > (* Each line is: "user:[!!]password:..." > * !! at the front of the password field means the account is locked. > diff --git a/mllib/common_utils.ml b/mllib/common_utils.ml > index 3737b4c..8601009 100644 > --- a/mllib/common_utils.ml > +++ b/mllib/common_utils.ml > @@ -738,3 +738,8 @@ let guest_arch_compatible guest_arch > | x, y when x = y -> true > | "x86_64", ("i386"|"i486"|"i586"|"i686") -> true > | _ -> false > + > +(** Return the last part of a string, after the specified separator. *) > +let last_part_of str sep > + let i = String.rindex str sep in > + String.sub str (i+1) (String.length str - (i+1)) > diff --git a/mllib/common_utils.mli b/mllib/common_utils.mli > index a5b0a68..7d0b38f 100644 > --- a/mllib/common_utils.mli > +++ b/mllib/common_utils.mli > @@ -178,3 +178,6 @@ val mkdir_p : string -> int -> unit > val guest_arch_compatible : string -> bool > (** Are guest arch and host_cpu compatible, in terms of being able > to run commands in the libguestfs appliance? *) > + > +val last_part_of : string -> char -> string > +(** Return the last part of a string, after the specified separator. *) > diff --git a/sysprep/sysprep_operation_user_account.ml b/sysprep/sysprep_operation_user_account.ml > index 3f3b142..fe460b8 100644 > --- a/sysprep/sysprep_operation_user_account.ml > +++ b/sysprep/sysprep_operation_user_account.ml > @@ -68,9 +68,7 @@ let user_account_perform g root side_effects > let uid = userpath ^ "/uid" in > let uid = g#aug_get uid in > let uid = int_of_string uid in > - let username > - let i = String.rindex userpath '/' in > - String.sub userpath (i+1) (String.length userpath -i-1) in > + let username = last_part_of userpath '/' in > if uid >= uid_min && uid <= uid_max > && check_remove_user username then ( > changed := true; > diff --git a/v2v/convert_linux.ml b/v2v/convert_linux.ml > index 7967c0f..cadd515 100644 > --- a/v2v/convert_linux.ml > +++ b/v2v/convert_linux.ml > @@ -795,10 +795,8 @@ let rec convert ~keep_serial_console (g : G.guestfs) inspect source > *) > let mkinitrd_kv > let modpath = kernel.ki_modpath in > - let len = String.length modpath in > try > - let i = String.rindex modpath '/' in > - String.sub modpath (i+1) (len - (i+1)) > + last_part_of modpath '/' > with > Not_found -> > invalid_arg (sprintf "invalid module path: %s" modpath) in > diff --git a/v2v/utils.ml b/v2v/utils.ml > index 1f8c357..4d6b08d 100644 > --- a/v2v/utils.ml > +++ b/v2v/utils.ml > @@ -176,9 +176,7 @@ let find_virtio_win_drivers virtio_win > let paths = List.filter (g#is_file ~followsymlinks:false) paths in > List.map ( > fun path -> > - let i = String.rindex path '/' in > - let len = String.length path in > - let basename = String.sub path (i+1) (len - (i+1)) in > + let basename = last_part_of path '/' in > (path, sprintf "%s:%s" virtio_win path, > basename, > fun () -> g#read_file path) > @@ -198,10 +196,7 @@ let find_virtio_win_drivers virtio_win > let lc_path = String.lowercase path in > let lc_basename = String.lowercase basename in > > - let extension > - let i = String.rindex lc_basename '.' in > - let len = String.length lc_basename in > - String.sub lc_basename (i+1) (len - (i+1)) in > + let extension = last_part_of lc_basename '.' in > > (* Skip files without specific extensions. *) > if extension <> "cat" && extension <> "inf" && > -- > 2.1.0 > > _______________________________________________ > Libguestfs mailing list > Libguestfs@redhat.com > https://www.redhat.com/mailman/listinfo/libguestfs-- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW
Richard W.M. Jones
2015-Jul-01 13:50 UTC
Re: [Libguestfs] [PATCH 1/3] mllib: add an optional filter for rm_rf_only_files
On Wed, Jul 01, 2015 at 03:12:53PM +0200, Pino Toscano wrote:> This way it is possible to use rm_rf_only_files, but not removing > specific files. > --- > mllib/common_utils.ml | 8 +++++++- > mllib/common_utils.mli | 5 ++++- > 2 files changed, 11 insertions(+), 2 deletions(-) > > diff --git a/mllib/common_utils.ml b/mllib/common_utils.ml > index 516cff3..3737b4c 100644 > --- a/mllib/common_utils.ml > +++ b/mllib/common_utils.ml > @@ -640,13 +640,19 @@ let rmdir_on_exit > * without removing the actual directory structure. Also if 'dir' is > * not a directory or doesn't exist, ignore it. > * > + * The optional filter is used to filter out files which will be > + * removed: files returning true are not removed. > + * > * XXX Could be faster with a specific API for doing this. > *) > -let rm_rf_only_files (g : Guestfs.guestfs) dir > +let rm_rf_only_files (g : Guestfs.guestfs) ?filter dir > if g#is_dir dir then ( > let files = Array.map (Filename.concat dir) (g#find dir) in > let files = Array.to_list files in > let files = List.filter g#is_file files in > + let files = match filter with > + | None -> files > + | Some f -> List.filter (fun x -> not (f x)) files in > List.iter g#rm files > ) > > diff --git a/mllib/common_utils.mli b/mllib/common_utils.mli > index bf0cba6..a5b0a68 100644 > --- a/mllib/common_utils.mli > +++ b/mllib/common_utils.mli > @@ -140,12 +140,15 @@ val unlink_on_exit : string -> unit > val rmdir_on_exit : string -> unit > (** Remove a temporary directory on exit (using [rm -rf]). *) > > -val rm_rf_only_files : Guestfs.guestfs -> string -> unit > +val rm_rf_only_files : Guestfs.guestfs -> ?filter:(string -> bool) -> string -> unit > (** Using the libguestfs API, recursively remove only files from the > given directory. Useful for cleaning [/var/cache] etc in sysprep > without removing the actual directory structure. Also if [dir] is > not a directory or doesn't exist, ignore it. > > + The optional [filter] is used to filter out files which will be > + removed: files returning true are not removed. > + > XXX Could be faster with a specific API for doing this. *) > > val truncate_recursive : Guestfs.guestfs -> string -> unit > -- > 2.1.0ACK. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org
Richard W.M. Jones
2015-Jul-01 13:51 UTC
Re: [Libguestfs] [PATCH 3/3] sysprep: rework and fix cron-spool operation (RHBZ#1229305)
On Wed, Jul 01, 2015 at 03:12:55PM +0200, Pino Toscano wrote:> When cleaning the directories with cron/at jobs, remove only files > there, as subdirectories might be used by other systems; for example > in Debian under /var/spool/cron/ there is the atjobs subdirectory with > the actual at queue. > > Make sure to not remove .SEQ files anymore, as they represent the at job > counter which is needed by the at daemon. Instead, reset these files to > 0. > > Furthermore, add also the path to the .SEQ location in Debian-based > systems. > --- > sysprep/sysprep_operation_cron_spool.ml | 27 ++++++++++++++++++++------- > 1 file changed, 20 insertions(+), 7 deletions(-) > > diff --git a/sysprep/sysprep_operation_cron_spool.ml b/sysprep/sysprep_operation_cron_spool.ml > index 687a7e9..888b97a 100644 > --- a/sysprep/sysprep_operation_cron_spool.ml > +++ b/sysprep/sysprep_operation_cron_spool.ml > @@ -18,19 +18,32 @@ > > open Sysprep_operation > open Common_gettext.Gettext > +open Common_utils > > module G = Guestfs > > let cron_spool_perform (g : Guestfs.guestfs) root side_effects > - Array.iter g#rm_rf (g#glob_expand "/var/spool/cron/*"); > + let is_seq path > + let basename > + try last_part_of path '/' > + with Not_found -> path in > + basename = ".SEQ" in > + let reset f > + if g#is_file f then > + (* This should overwrite the file in-place, as it's a very > + * small buffer which will be handled using internal_write. > + * This way, existing attributes like SELinux labels are > + * preserved. > + *) > + g#write f "00000\n" in > + > + rm_rf_only_files g ~filter:is_seq "/var/spool/cron/"; > + reset "/var/spool/cron/atjobs/.SEQ"; > Array.iter g#rm (g#glob_expand "/var/spool/atjobs/*"); > - Array.iter g#rm (g#glob_expand "/var/spool/atjobs/.SEQ"); > + reset "/var/spool/atjobs/.SEQ"; > Array.iter g#rm (g#glob_expand "/var/spool/atspool/*"); > - Array.iter > - (fun path -> if not (g#is_dir path) then g#rm path) > - (g#glob_expand "/var/spool/at/*"); > - Array.iter g#rm (g#glob_expand "/var/spool/at/.SEQ"); > - Array.iter g#rm (g#glob_expand "/var/spool/at/spool/*") > + rm_rf_only_files g ~filter:is_seq "/var/spool/at/"; > + reset "/var/spool/at/.SEQ"ACK. 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