Re: [Libguestfs] [v2v PATCH] convert: warn about "--mac" options that don't match any source NICs

On Sun, Apr 10, 2022 at 03:49:51PM +0200, Laszlo Ersek wrote: > Just before we call "Networks.map", collect all "--mac" references (from > "options.network_map" and "options.static_ips"), and check each against > the list of MACs available on the source ("source.s_nics"). Collect and > report unresolved references. > > The algorithm added here has quadratic time complexity, kind of undoing > the sophistication of the "Networks.t" type -- but I don't think we expect > a huge number of NICs / MACs. > > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1685809 > Signed-off-by: Laszlo Ersek <lersek(a)redhat.com&gt; Looking at this properly, I think the whole bug is dubious to me. If the user specified a --mac option then we should probably assume they know more about the source or target than we do. Or maybe they are adding a group of --mac command line parameters to every virt-v2v invocation, which each apply to a subset of the guests being converted. Personally I would be inclined close this one as NOTABUG, but as it's just a warning (and has no effect on anything) I guess it's OK. Be nice if it wasn't quadratic though. I was looking at /usr/lib64/ocaml/map.mli to see if there's any operation we can use to intersect the map keys with a List or Set, but I cannot see anything ... Lukewarm ACK.

Rich. > lib/networks.mli | 3 +++ > convert/convert.ml | 19 +++++++++++++++++++ > lib/networks.ml | 2 ++ > 3 files changed, 24 insertions(+) > > diff --git a/lib/networks.mli b/lib/networks.mli > index 8800e21cbcf0..d1a62bf0cf2e 100644 > --- a/lib/networks.mli > +++ b/lib/networks.mli > @@ -55,3 +55,6 @@ val map : t -> Types.source_nic -> Types.source_nic > MAC address mappings take precedence, followed by network > and bridge mappings if no MAC address mapping for the NIC can > be found. *) > + > +val macs : t -> string list > +(** Return all MAC addresses for which address mappings have been added. *) > diff --git a/convert/convert.ml b/convert/convert.ml > index 87fca7252ba3..1e3db6b99780 100644 > --- a/convert/convert.ml > +++ b/convert/convert.ml > @@ -47,6 +47,25 @@ type mpstat = { > } > > let rec convert dir options source = > + let nic_macs = List.filter_map > + (fun { s_mac } -> > + match s_mac with > + | None -> None > + | Some mac -> Some (String.lowercase_ascii mac)) > + source.s_nics > + and mac_refs1 = Networks.macs options.network_map > + and mac_refs2 = List.map > + (fun { if_mac_addr } -> String.lowercase_ascii if_mac_addr) > + options.static_ips in > + let unresolved_mac_refs = > + List.filter (fun mac_ref -> not (List.mem mac_ref nic_macs)) > + (mac_refs1 @ mac_refs2) in > + if unresolved_mac_refs <> [] then ( > + let mac_list = String.concat ", " unresolved_mac_refs in > + warning (f_"The following --mac addresses do not match any NICs from the \ > + source: %s") mac_list > + ); > + > let target_nics = List.map (Networks.map options.network_map) source.s_nics in > > message (f_"Opening the source"); > diff --git a/lib/networks.ml b/lib/networks.ml > index 93250fe40ab0..52646166e496 100644 > --- a/lib/networks.ml > +++ b/lib/networks.ml > @@ -103,3 +103,5 @@ let add_default_bridge t o = > if t.default_bridge <> None then > error (f_"duplicate -b/--bridge parameter. Only one default mapping is allowed."); > t.default_bridge <- Some o > + > +let macs t = StringMap.keys t.macs > -- > 2.19.1.3.g30247aa5d201 > > _______________________________________________ > Libguestfs mailing list > Libguestfs(a)redhat.com > https://listman.redhat.com/mailman/listinfo/libguestfs