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

[Tomáš Golembiovský]

On Tue, 27 Feb 2018 11:47:38 +0000
"Richard W.M. Jones" <rjones at redhat.com> wrote:

> 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 at 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 ();

Maybe too long to my taste, but sure, why not.

> 
> 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;

Ah, right. The magical |> operator... I keep forgetting about it.

> 
> but your version may be clearer.

Agreed. I'll leave it as is then.

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

Not yet, but thanks for reminding me. I'll create a bug and add it to
the 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 at 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.
> http://people.redhat.com/~rjones/virt-df/


-- 
Tomáš Golembiovský <tgolembi at redhat.com>