Re: [libvirt] [PATCH 22/28] util: provide default destination IP in alternate virNetDevIPRouteAdd()
On 06/24/2016 09:08 AM, John Ferlan wrote:
On 06/22/2016 01:37 PM, Laine Stump wrote:
> The version of virNetDevIPRouteAdd() has a bit of code to create the
> appropriate "0.0.0.0" or "::" virSocketAddr when the addr passed
in is
> NULL or invalid, but the alternate implementation (used on platforms
> that don't support libnl) had no such code, making the two
> implementations semantically diferent. This patch corrects that
> oversight.
> ---
> src/util/virnetdevip.c | 20 +++++++++++++++++++-
> 1 file changed, 19 insertions(+), 1 deletion(-)
>
Seems like a reasonable and nice thing to do ... although you could
have written a helper routine to return the result of
virSocketAddrFormat based on actualAddr... Avoids duplicated code and
the chance that someone only changes one in the future.
Interesting side note - can gateway be invalid and family doesn't have
AF_INET, thus you create an IPv6 default address?
ACK - with the helper routine...
I don't like the idea of a helper function that may or may not allocate
memory (makes it too easy for the callers to misunderstand), and I'm
thinking it may be better to just make a platform-agnostic
virNetDevIPRouteAdd() that has the common code followed by a call to
virNetDevIPRouteAddInternal() which would be defined different for each
platform. I don't feel like dealing with that right now though (I'd
rather get the rest of these pushed), so I'll drop this patch for now
and come back to it later.