Re: [Libguestfs] [PATCH] v2v: remove MAC address related information

Richard W.M. Jones
2018-02-27T11:47:38Z

On Tue, Feb 27, 2018 at 12:35:36PM +0100, Tomáš Golembiovský
wrote:
> Remove ties to MAC address because it is likely to change.
>
> The code is based on operations net-hwaddr and udev-persistent-net of
> virt-sysprep.
>
> Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
> ---
>  v2v/convert_linux.ml | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/v2v/convert_linux.ml b/v2v/convert_linux.ml
> index b273785e6..8bba74786 100644
> --- a/v2v/convert_linux.ml
> +++ b/v2v/convert_linux.ml
> @@ -88,6 +88,8 @@ let convert (g : G.guestfs) inspect source output rcaps
>      unconfigure_kudzu ();
>      unconfigure_prltools ();
>
> +    sysprep_networking();

The function name is a bit obscure.  How about this?

  remove_network_ifcfg_hwaddr_entries ();

Also needs a space between the function name and the parentheses.

>      let kernel = configure_kernel () in
>
>      if output#keep_serial_console then (
> @@ -452,6 +454,21 @@ let convert (g : G.guestfs) inspect source output
rcaps >              msg
>      )
>
> +  and sysprep_networking () > +    if family = `RHEL_family then
> +      (* Remove HWADDR=... entries from ifcfg-* files. *)
> +      let paths = g#aug_match
> +        "/files/etc/sysconfig/network-scripts/*[label() =~
glob('ifcfg-*')]/HWADDR" in
> +      let paths = Array.to_list paths in
> +      if paths <> [] then (
> +        List.iter (fun path -> ignore (g#aug_rm path)) paths;

It doesn't really matter but I think you could write this as:

  List.iter (g#aug_rm |> ignore) paths;

but your version may be clearer.

> +        g#aug_save ()
> +      );
> +    (* Remove persistent udev rules. *)
> +    try g#rm "/etc/udev/rules.d/70-persistent-net.rules"
> +    with G.Error _ -> ()

Rather than throwing away the error it's better to use 'g#rm_f' and
don't try to catch G.Error at all.  I guess the same problem exists in
virt-sysprep.

Is this a fix for any particular bug?  If so it needs to have
‘(RHBZ#xxxxxx)’ added to the end of the commit heading.

Patch looks OK otherwise.

Rich.

> +
> +
>    and configure_kernel () >      (* Previously this function would try
to install kernels, but we
>       * don't do that any longer.
> --
> 2.16.1
>
> _______________________________________________
> 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
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.