[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] v2:Support for adding a static route to a bridge



On 03/15/2013 02:10 PM, Gene Czarcinski wrote:
> diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
> index 296871c..c90b3d2 100644
> --- a/src/util/virnetdev.c
> +++ b/src/util/virnetdev.c
> @@ -729,6 +729,7 @@ int virNetDevGetVLanID(const char *ifname ATTRIBUTE_UNUSED,
>   * Add an IP address to an interface. This function *does not* remove
>   * any previously added IP addresses - that must be done separately with
>   * brDelInetAddress.
> + * TODO: what is "brDelInetAddress"?

That is a left-over comment from long ago. The function has been renamed
to virNetDevClearIPv4Address.

(A very useful separate patch would be one to rename
virNetDev(Set|Clear)IPv4Address to virNetDev(Set|Clear)IPAddress (since
they really *can* be (and are being) used to set both IPv4 and IPv6
addresses), then reimplement them using netlink/libnl calls.

Likewise, it would be much nicer if virNetDevSetGateway() was
implemented using netlink/libnl.


>   *
>   * Returns 0 in case of success or -1 in case of error.
>   */
> @@ -769,6 +770,52 @@ cleanup:
>  }
>  
>  /**
> + * virNetDevSetGateway:

I'm thinking maybe this would be better named "virNetDevAddRoute", since
the thing it's adding is a route (of which the gateway is one attribute).

> + * @ifname: the interface name
> + * @addr: the IP network address (IPv4 or IPv6)
> + * @prefix: number of 1 bits in the netmask
> + * @gateway: via address for route (same as @addr)
> + *
> + * Add a route for a network IP address to an interface. This function
> + * *does not* remove any previously added IP static routes.
> + *
> + * Returns 0 in case of success or -1 in case of error.
> + */
> +
> +int virNetDevSetGateway(const char *ifname,
> +                        virSocketAddr *addr,
> +                        unsigned int prefix,
> +                        virSocketAddr *gateway)
> +{
> +    virCommandPtr cmd = NULL;
> +    char *addrstr = NULL, *gatewaystr = NULL;
> +    int ret = -1;
> +
> +    if (!(addrstr = virSocketAddrFormat(addr)))
> +        goto cleanup;
> +    if (!(gatewaystr = virSocketAddrFormat(gateway)))
> +        goto cleanup;
> +    cmd = virCommandNew(IP_PATH);
> +    virCommandAddArgList(cmd, "route", "add", NULL);
> +    virCommandAddArgFormat(cmd, "%s/%u", addrstr, prefix);
> +    virCommandAddArgList(cmd, "via", NULL);
That one could have been a simple virCommandAddArg().

> +    virCommandAddArgFormat(cmd, "%s", gatewaystr);
> +    virCommandAddArgList(cmd, "dev", ifname, NULL);
> +    virCommandAddArgList(cmd, "proto", "static", "metric", NULL);
> +    virCommandAddArgFormat(cmd, "%u", 1);
Are all of those necessary?

Partial answer to myself: "proto static" is needed because otherwise
"proto boot" is assumed, and if a routing daemon is started on the host,
any route added with "proto boot" will be purged, which *isn't* what we
want.

I'm not sure about metric...

> +
> +    if (virCommandRun(cmd, NULL) < 0)
> +        goto cleanup;
> +
> +    ret = 0;
> +cleanup:
> +    VIR_FREE(addrstr);
> +    VIR_FREE(gatewaystr);
> +    virCommandFree(cmd);
> +    return ret;
> +}
> +
> +/**
>   * virNetDevClearIPv4Address:
>   * @ifname: the interface name
>   * @addr: the IP address (IPv4 or IPv6)
> diff --git a/src/util/virnetdev.h b/src/util/virnetdev.h
> index 06d0650..8b94ea8 100644
> --- a/src/util/virnetdev.h
> +++ b/src/util/virnetdev.h
> @@ -42,6 +42,11 @@ int virNetDevSetIPv4Address(const char *ifname,
>                              virSocketAddr *addr,
>                              unsigned int prefix)
>      ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
> +int virNetDevSetGateway(const char *ifname,
> +                        virSocketAddr *addr,
> +                        unsigned int prefix,
> +                        virSocketAddr *gateway)
> +    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
>  int virNetDevClearIPv4Address(const char *ifname,
>                                virSocketAddr *addr,
>                                unsigned int prefix)
> diff --git a/tests/networkxml2xmlin/dhcp6host-routed-network.xml b/tests/networkxml2xmlin/dhcp6host-routed-network.xml
> index 2693d87..dcad62d 100644
> --- a/tests/networkxml2xmlin/dhcp6host-routed-network.xml
> +++ b/tests/networkxml2xmlin/dhcp6host-routed-network.xml
> @@ -19,4 +19,8 @@
>        <host id='0:1:0:1:18:aa:62:fe:0:16:3e:44:55:66' name='badbob' ip='2001:db8:ac10:fd01::1:24' />
>      </dhcp>
>    </ip>
> +  <ip address="192.168.222.0" netmask="255.255.255.0" via="192.168.122.10">
> +  </ip>
> +  <ip family="ipv6" address="2001:db8:ac10:fc00::" prefix="64" via="2001:db8:ac10:fd01::1:24">
> +  </ip>
>  </network>
> diff --git a/tests/networkxml2xmlout/dhcp6host-routed-network.xml b/tests/networkxml2xmlout/dhcp6host-routed-network.xml
> index 7305043..880c2dd 100644
> --- a/tests/networkxml2xmlout/dhcp6host-routed-network.xml
> +++ b/tests/networkxml2xmlout/dhcp6host-routed-network.xml
> @@ -21,4 +21,8 @@
>        <host id='0:1:0:1:18:aa:62:fe:0:16:3e:44:55:66' name='badbob' ip='2001:db8:ac10:fd01::1:24' />
>      </dhcp>
>    </ip>
> +  <ip address='192.168.222.0' netmask='255.255.255.0' via='192.168.122.10'>
> +  </ip>
> +  <ip family='ipv6' address='2001:db8:ac10:fc00::' prefix='64' via='2001:db8:ac10:fd01::1:24'>
> +  </ip>
>  </network>


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]