Libguestfs - Jan 2014 - [PATCH] sysprep: add --operations

On Friday 10 January 2014 10:09:19 Richard W.M. Jones
wrote:
> On Thu, Jan 09, 2014 at 03:45:54PM +0000, Richard W.M. Jones wrote:
> > On Thu, Jan 09, 2014 at 04:21:10PM +0100, Pino Toscano wrote:
> > > +  and set_operations op_string > > > +    let
currentopset > > > +      match (!operations) with
> > 
> > No need for parentheses around !operations.
Fixed.
> > > +        let n = ref op_name in
> > > +        let remove = string_prefix op_name "-" in
> > > +        if remove then
> > > +          n := String.sub op_name 1 (String.length op_name - 1);
> > > +        match !n with
> > 
> > This can be written a bit more naturally as ...
> > 
> >   let n, remove > >     if string_prefix op_name "-"
then
> >       String.sub op_name 1 (String.length op_name-1), true
> >     else
> >       op_name, false in
> >   match n with
> >   ...
This is nice even without the way below.
> An even more natural way to write this is:
Maybe for skilled OCaml programmers ;) not (yet) for me.
>   let op >     if string_prefix op_name "-" then
>       `Remove (String.sub op_name 1 (String.length op_name-1))
>     else
>       `Add op_name
>    match op with
> 
>    | `Add "" | `Remove "" -> (* error *)
>    | `Add "defaults" -> Sysprep_operation.add_defaults_to_set
opset
>    | `Remove "defaults" ->
Sysprep_operation.remove_defaults_from_set
>    | opset etc
> 
> The type of op will be [ `Add of string | `Remove of string ].
> Actually you never need to write that type out, as OCaml will infer it
> (and check it).  It will only appear in error messages if you get the
> code wrong.
Interesting way, made use of it, thanks.
> > +          let f = if remove then
> > Sysprep_operation.remove_defaults_from_set else
> > Sysprep_operation.add_defaults_to_set in
> > +          f opset
> 
> You can actually write:
> 
>   (if remove then Sysprep_operation.remove_defaults_from_set
>    else Sysprep_operation.add_defaults_to_set) opset
> 
> I don't know which way you think is clearer, but I would avoid the
>80
> character lines.
Yes, I knew about that, just thought it was clearer to split the actual 
call on an own line, making the argument a bit less "hidden" by the 
whole instruction.
> > +    "--operations", Arg.String set_operations, "
" ^
> > s_"Enable/disable specific operations";
> 
> I'd also add an alias "--operation" (it would just do the
same thing).
Added.
> > +let add_list_to_opset l s > > +  List.fold_left (
> > +    fun acc elem ->
> > +      OperationSet.add elem acc
> > +  ) s l
> > +
> > +let remove_list_from_opset l set > > +  OperationSet.fold (
> > +    fun elem opset ->
> > +      if List.exists (
> > +        fun li ->
> > +          li.name = elem.name
> > +      ) l then
> > +        opset
> > +      else
> > +        OperationSet.add elem opset
> > +  ) set empty_set
> 
> OCaml Set has subset, intersection, difference operations if you need
> them.  See /usr/lib64/ocaml/set.mli
True; I went with manual folding to avoid build new temporary sets when 
needed, but I guess doing this could be more natural.

-- 
Pino Toscano

Add a new --operation parameter which, similarly to --enable, can be
used to enable operations, but also to remove them, and to add/remove
the default operations and all the available ones.
---
 sysprep/main.ml               | 36 +++++++++++++++++++++++++++
 sysprep/sysprep_operation.ml  | 24 ++++++++++++++++++
 sysprep/sysprep_operation.mli | 21 ++++++++++++++++
 sysprep/virt-sysprep.pod      | 57 +++++++++++++++++++++++++++++++++++++------
 4 files changed, 131 insertions(+), 7 deletions(-)

diff --git a/sysprep/main.ml b/sysprep/main.ml
index 689a394..49750a9 100644
--- a/sysprep/main.ml
+++ b/sysprep/main.ml
@@ -87,6 +87,40 @@ let debug_gc, operations, g, selinux_relabel, quiet,
mount_opts            exit 1
     ) Sysprep_operation.empty_set ops in
     operations := Some opset
+  and set_operations op_string +    let currentopset +      match !operations
with
+      | Some x -> x
+      | None -> Sysprep_operation.empty_set
+    in
+    let ops = string_nsplit "," op_string in
+    let opset = List.fold_left (
+      fun opset op_name ->
+        let op +          if string_prefix op_name "-" then
+            `Remove (String.sub op_name 1 (String.length op_name - 1))
+          else
+            `Add op_name in
+        match op with
+        | `Add "" | `Remove "" ->
+          eprintf (f_"%s: --operations: empty operation name\n")
+            prog;
+          exit 1
+        | `Add "defaults" -> Sysprep_operation.add_defaults_to_set
opset
+        | `Remove "defaults" ->
Sysprep_operation.remove_defaults_from_set opset
+        | `Add "all" -> Sysprep_operation.add_all_to_set opset
+        | `Remove "all" -> Sysprep_operation.remove_all_from_set
opset
+        | `Add n | `Remove n ->
+          let f = match op with
+          | `Add n -> Sysprep_operation.add_to_set
+          | `Remove n -> Sysprep_operation.remove_from_set in
+          try f n opset with
+          | Not_found ->
+            eprintf (f_"%s: --operations: '%s' is not a known
operation\n")
+              prog n;
+            exit 1
+    ) currentopset ops in
+    operations := Some opset
   and force_selinux_relabel ()      selinux_relabel := `Force
   and no_force_selinux_relabel () @@ -114,6 +148,8 @@ let debug_gc, operations,
g, selinux_relabel, quiet, mount_opts      "--list-operations",
Arg.Unit list_operations, " " ^ s_"List supported
operations";
     "--long-options", Arg.Unit display_long_options, " " ^
s_"List long options";
     "--mount-options", Arg.Set_string mount_opts, s_"opts"
^ " " ^ s_"Set mount options (eg
/:noatime;/var:rw,noatime)";
+    "--operation",  Arg.String set_operations, " " ^
s_"Enable/disable specific operations";
+    "--operations", Arg.String set_operations, " " ^
s_"Enable/disable specific operations";
     "-q",        Arg.Set quiet,             " " ^
s_"Don't print log messages";
     "--quiet",   Arg.Set quiet,             " " ^
s_"Don't print log messages";
     "--selinux-relabel", Arg.Unit force_selinux_relabel, "
" ^ s_"Force SELinux relabel";
diff --git a/sysprep/sysprep_operation.ml b/sysprep/sysprep_operation.ml
index 1fb4f17..572a65f 100644
--- a/sysprep/sysprep_operation.ml
+++ b/sysprep/sysprep_operation.ml
@@ -68,10 +68,34 @@ type set = OperationSet.t
 
 let empty_set = OperationSet.empty
 
+let opset_of_oplist li +  List.fold_left (
+    fun acc elem ->
+      OperationSet.add elem acc
+  ) empty_set li
+
 let add_to_set name set    let op = List.find (fun { name = n } -> name = n)
!all_operations in
   OperationSet.add op set
 
+let add_defaults_to_set set +  OperationSet.union set (opset_of_oplist
!enabled_by_default_operations)
+
+let add_all_to_set set +  opset_of_oplist !all_operations
+
+let remove_from_set name set +  let name_filter = fun { name = n } -> name =
n in
+  if List.exists name_filter !all_operations <> true then
+    raise Not_found;
+  OperationSet.diff set (OperationSet.filter name_filter set)
+
+let remove_defaults_from_set set +  OperationSet.diff set (opset_of_oplist
!enabled_by_default_operations)
+
+let remove_all_from_set set +  empty_set
+
 let register_operation op    all_operations := op :: !all_operations;
   if op.enabled_by_default then
diff --git a/sysprep/sysprep_operation.mli b/sysprep/sysprep_operation.mli
index 16eee64..61dde72 100644
--- a/sysprep/sysprep_operation.mli
+++ b/sysprep/sysprep_operation.mli
@@ -128,6 +128,27 @@ val add_to_set : string -> set -> set
     Note that this will raise [Not_found] if [name] is not
     a valid operation name. *)
 
+val add_defaults_to_set : set -> set
+(** [add_defaults_to_set set] adds to [set] all the operations enabled
+    by default. *)
+
+val add_all_to_set : set -> set
+(** [add_all_to_set set] adds to [set] all the available operations. *)
+
+val remove_from_set : string -> set -> set
+(** [remove_from_set name set] remove the operation named [name] from [set].
+
+    Note that this will raise [Not_found] if [name] is not
+    a valid operation name. *)
+
+val remove_defaults_from_set : set -> set
+(** [remove_defaults_from_set set] removes from [set] all the operations
+    enabled by default. *)
+
+val remove_all_from_set : set -> set
+(** [remove_all_from_set set] removes from [set] all the available
+    operations. *)
+
 val perform_operations_on_filesystems : ?operations:set -> ?quiet:bool ->
Guestfs.guestfs -> string -> flag list
 (** Perform all operations, or the subset listed in the [operations] set. *)
 
diff --git a/sysprep/virt-sysprep.pod b/sysprep/virt-sysprep.pod
index afabcc9..a042db4 100755
--- a/sysprep/virt-sysprep.pod
+++ b/sysprep/virt-sysprep.pod
@@ -107,6 +107,47 @@ version of virt-sysprep.
 See L</OPERATIONS> below for a list and an explanation of each
 operation.
 
+=item B<--operation> operations
+
+=item B<--operations> operations
+
+Choose which sysprep operations to perform.  Give a comma-separated
+list of operations, for example:
+
+ --operations ssh-hostkeys,udev-persistent-net
+
+would enable ONLY C<ssh-hostkeys> and C<udev-persistent-net>
operations.
+
+I<--operations> allows you to enable and disable any operation, including
+the default ones (which would be tried when specifying neither
+I<--operations> nor I<--enable>) and all the available ones;
prepending
+a C<-> in front of an operation name removes it from the list of enabled
+operations, while the meta-names C<defaults> and C<all> represent
+respectively the operations enabled by default and all the available ones.
+For example:
+
+ --operations delete,defaults,-hostname
+
+would enable the C<delete> operation (regardless whether it is enabled by
+default), all the default ones, and disable the C<hostname> operation.
+
+I<--operations> can be specified multiple times; the first time the set
+of enabled operations is empty, while any further I<--operations> affects
+the operations enabled so far.
+
+If the I<--operations> option is not given, then we default to trying
most
+sysprep operations (see I<--list-operations> to show which are
+enabled).
+
+Regardless of the I<--operations> option, sysprep operations are skipped
+for some guest types.
+
+Use I<--list-operations> to list operations supported by a particular
+version of virt-sysprep.
+
+See L</OPERATIONS> below for a list and an explanation of each
+operation.
+
 =item B<--format> raw|qcow2|..
 
 =item B<--format> auto
@@ -209,24 +250,26 @@ __EXTRA_OPTIONS__
 
 =head1 OPERATIONS
 
-If the I<--enable> option is I<not> given, then most sysprep
-operations are enabled.
+If the I<--enable>/I<--operations> option is I<not> given,
+then most sysprep operations are enabled.
 
 Use C<virt-sysprep --list-operations> to list all operations for your
 virt-sysprep binary.  The ones which are enabled by default are marked
-with a C<*> character.  Regardless of the I<--enable> option,
sysprep
-operations are skipped for some guest types.
+with a C<*> character.  Regardless of the
I<--enable>/I<--operations>
+options, sysprep operations are skipped for some guest types.
 
-Operations can be individually enabled using the I<--enable> option.
+Operations can be individually enabled using the
+I<--enable>/I<--operations> options.
 Use a comma-separated list, for example:
 
- virt-sysprep --enable=ssh-hostkeys,udev-persistent-net [etc..]
+ virt-sysprep --operations=ssh-hostkeys,udev-persistent-net [etc..]
 
 Future versions of virt-sysprep may add more operations.  If you are
 using virt-sysprep and want predictable behaviour, specify only the
 operations that you want to have enabled.
 
-C<*> = enabled by default when no I<--enable> option is given.
+C<*> = enabled by default when no I<--enable>/I<--operations>
option
+is given.
 
 __OPERATIONS__
 
-- 
1.8.3.1

On Mon, Jan 13, 2014 at 02:39:24PM +0100, Pino Toscano
wrote:
> Add a new --operation parameter which, similarly to --enable, can be
> used to enable operations, but also to remove them, and to add/remove
> the default operations and all the available ones.
> ---
>  sysprep/main.ml               | 36 +++++++++++++++++++++++++++
>  sysprep/sysprep_operation.ml  | 24 ++++++++++++++++++
>  sysprep/sysprep_operation.mli | 21 ++++++++++++++++
>  sysprep/virt-sysprep.pod      | 57
+++++++++++++++++++++++++++++++++++++------
>  4 files changed, 131 insertions(+), 7 deletions(-)
> 
> diff --git a/sysprep/main.ml b/sysprep/main.ml
> index 689a394..49750a9 100644
> --- a/sysprep/main.ml
> +++ b/sysprep/main.ml
> @@ -87,6 +87,40 @@ let debug_gc, operations, g, selinux_relabel, quiet,
mount_opts >            exit 1
>      ) Sysprep_operation.empty_set ops in
>      operations := Some opset
> +  and set_operations op_string > +    let currentopset > +     
match !operations with
> +      | Some x -> x
> +      | None -> Sysprep_operation.empty_set
> +    in
> +    let ops = string_nsplit "," op_string in
> +    let opset = List.fold_left (
> +      fun opset op_name ->
> +        let op > +          if string_prefix op_name "-" then
> +            `Remove (String.sub op_name 1 (String.length op_name - 1))
> +          else
> +            `Add op_name in
> +        match op with
> +        | `Add "" | `Remove "" ->
> +          eprintf (f_"%s: --operations: empty operation name\n")
> +            prog;
> +          exit 1
> +        | `Add "defaults" ->
Sysprep_operation.add_defaults_to_set opset
> +        | `Remove "defaults" ->
Sysprep_operation.remove_defaults_from_set opset
> +        | `Add "all" -> Sysprep_operation.add_all_to_set
opset
> +        | `Remove "all" ->
Sysprep_operation.remove_all_from_set opset
> +        | `Add n | `Remove n ->
> +          let f = match op with
> +          | `Add n -> Sysprep_operation.add_to_set
> +          | `Remove n -> Sysprep_operation.remove_from_set in
> +          try f n opset with
> +          | Not_found ->
> +            eprintf (f_"%s: --operations: '%s' is not a known
operation\n")
> +              prog n;
> +            exit 1
> +    ) currentopset ops in
> +    operations := Some opset
>    and force_selinux_relabel () >      selinux_relabel := `Force
>    and no_force_selinux_relabel () > @@ -114,6 +148,8 @@ let debug_gc,
operations, g, selinux_relabel, quiet, mount_opts >     
"--list-operations", Arg.Unit list_operations, " " ^
s_"List supported operations";
>      "--long-options", Arg.Unit display_long_options, "
" ^ s_"List long options";
>      "--mount-options", Arg.Set_string mount_opts,
s_"opts" ^ " " ^ s_"Set mount options (eg
/:noatime;/var:rw,noatime)";
> +    "--operation",  Arg.String set_operations, " " ^
s_"Enable/disable specific operations";
> +    "--operations", Arg.String set_operations, " " ^
s_"Enable/disable specific operations";
>      "-q",        Arg.Set quiet,             " " ^
s_"Don't print log messages";
>      "--quiet",   Arg.Set quiet,             " " ^
s_"Don't print log messages";
>      "--selinux-relabel", Arg.Unit force_selinux_relabel, "
" ^ s_"Force SELinux relabel";
> diff --git a/sysprep/sysprep_operation.ml b/sysprep/sysprep_operation.ml
> index 1fb4f17..572a65f 100644
> --- a/sysprep/sysprep_operation.ml
> +++ b/sysprep/sysprep_operation.ml
> @@ -68,10 +68,34 @@ type set = OperationSet.t
>  
>  let empty_set = OperationSet.empty
>  
> +let opset_of_oplist li > +  List.fold_left (
> +    fun acc elem ->
> +      OperationSet.add elem acc
> +  ) empty_set li
> +
>  let add_to_set name set >    let op = List.find (fun { name = n } ->
name = n) !all_operations in
>    OperationSet.add op set
>  
> +let add_defaults_to_set set > +  OperationSet.union set
(opset_of_oplist !enabled_by_default_operations)
> +
> +let add_all_to_set set > +  opset_of_oplist !all_operations
> +
> +let remove_from_set name set > +  let name_filter = fun { name = n }
-> name = n in
> +  if List.exists name_filter !all_operations <> true then
> +    raise Not_found;
> +  OperationSet.diff set (OperationSet.filter name_filter set)
> +
> +let remove_defaults_from_set set > +  OperationSet.diff set
(opset_of_oplist !enabled_by_default_operations)
> +
> +let remove_all_from_set set > +  empty_set
> +
>  let register_operation op >    all_operations := op :: !all_operations;
>    if op.enabled_by_default then
> diff --git a/sysprep/sysprep_operation.mli b/sysprep/sysprep_operation.mli
> index 16eee64..61dde72 100644
> --- a/sysprep/sysprep_operation.mli
> +++ b/sysprep/sysprep_operation.mli
> @@ -128,6 +128,27 @@ val add_to_set : string -> set -> set
>      Note that this will raise [Not_found] if [name] is not
>      a valid operation name. *)
>  
> +val add_defaults_to_set : set -> set
> +(** [add_defaults_to_set set] adds to [set] all the operations enabled
> +    by default. *)
> +
> +val add_all_to_set : set -> set
> +(** [add_all_to_set set] adds to [set] all the available operations. *)
> +
> +val remove_from_set : string -> set -> set
> +(** [remove_from_set name set] remove the operation named [name] from
[set].
> +
> +    Note that this will raise [Not_found] if [name] is not
> +    a valid operation name. *)
> +
> +val remove_defaults_from_set : set -> set
> +(** [remove_defaults_from_set set] removes from [set] all the operations
> +    enabled by default. *)
> +
> +val remove_all_from_set : set -> set
> +(** [remove_all_from_set set] removes from [set] all the available
> +    operations. *)
> +
>  val perform_operations_on_filesystems : ?operations:set -> ?quiet:bool
-> Guestfs.guestfs -> string -> flag list
>  (** Perform all operations, or the subset listed in the [operations] set.
*)
>  
> diff --git a/sysprep/virt-sysprep.pod b/sysprep/virt-sysprep.pod
> index afabcc9..a042db4 100755
> --- a/sysprep/virt-sysprep.pod
> +++ b/sysprep/virt-sysprep.pod
> @@ -107,6 +107,47 @@ version of virt-sysprep.
>  See L</OPERATIONS> below for a list and an explanation of each
>  operation.
>  
> +=item B<--operation> operations
> +
> +=item B<--operations> operations
> +
> +Choose which sysprep operations to perform.  Give a comma-separated
> +list of operations, for example:
> +
> + --operations ssh-hostkeys,udev-persistent-net
> +
> +would enable ONLY C<ssh-hostkeys> and C<udev-persistent-net>
operations.
> +
> +I<--operations> allows you to enable and disable any operation,
including
> +the default ones (which would be tried when specifying neither
> +I<--operations> nor I<--enable>) and all the available ones;
prepending
> +a C<-> in front of an operation name removes it from the list of
enabled
> +operations, while the meta-names C<defaults> and C<all>
represent
> +respectively the operations enabled by default and all the available ones.
> +For example:
> +
> + --operations delete,defaults,-hostname
> +
> +would enable the C<delete> operation (regardless whether it is
enabled by
> +default), all the default ones, and disable the C<hostname>
operation.
> +
> +I<--operations> can be specified multiple times; the first time the
set
> +of enabled operations is empty, while any further I<--operations>
affects
> +the operations enabled so far.
> +
> +If the I<--operations> option is not given, then we default to
trying most
> +sysprep operations (see I<--list-operations> to show which are
> +enabled).
> +
> +Regardless of the I<--operations> option, sysprep operations are
skipped
> +for some guest types.
> +
> +Use I<--list-operations> to list operations supported by a
particular
> +version of virt-sysprep.
> +
> +See L</OPERATIONS> below for a list and an explanation of each
> +operation.
> +
>  =item B<--format> raw|qcow2|..
>  
>  =item B<--format> auto
> @@ -209,24 +250,26 @@ __EXTRA_OPTIONS__
>  
>  =head1 OPERATIONS
>  
> -If the I<--enable> option is I<not> given, then most sysprep
> -operations are enabled.
> +If the I<--enable>/I<--operations> option is I<not>
given,
> +then most sysprep operations are enabled.
>  
>  Use C<virt-sysprep --list-operations> to list all operations for
your
>  virt-sysprep binary.  The ones which are enabled by default are marked
> -with a C<*> character.  Regardless of the I<--enable> option,
sysprep
> -operations are skipped for some guest types.
> +with a C<*> character.  Regardless of the
I<--enable>/I<--operations>
> +options, sysprep operations are skipped for some guest types.
>  
> -Operations can be individually enabled using the I<--enable> option.
> +Operations can be individually enabled using the
> +I<--enable>/I<--operations> options.
>  Use a comma-separated list, for example:
>  
> - virt-sysprep --enable=ssh-hostkeys,udev-persistent-net [etc..]
> + virt-sysprep --operations=ssh-hostkeys,udev-persistent-net [etc..]
>  
>  Future versions of virt-sysprep may add more operations.  If you are
>  using virt-sysprep and want predictable behaviour, specify only the
>  operations that you want to have enabled.
>  
> -C<*> = enabled by default when no I<--enable> option is given.
> +C<*> = enabled by default when no
I<--enable>/I<--operations> option
> +is given.
ACK.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/