[libvirt] [PATCH v2] test_driver: properly handle DHCP ranges and IPv6 networks in testDomainInterfaceAddresses

[Michal Privoznik]

On 6/19/19 6:45 PM, Ilias Stamatis wrote:
> testDomainInterfaceAddresses always returns the same hard-coded
> addresses. Change the behavior such as if there is a DHCP range defined,
> addresses are returned from that pool.
> 
> The specific address returned depends on both the domain id and the
> specific guest interface in an attempt to return unique addresses *most
> of the time*.
> 
> Additionally, properly handle IPv6 networks which were previously
> ignored completely.
> 
> Signed-off-by: Ilias Stamatis <stamatis.iliass at gmail.com>
> ---
>   src/test/test_driver.c | 44 +++++++++++++++++++++++++++++++++++++-----
>   1 file changed, 39 insertions(+), 5 deletions(-)
> 
> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> index 2a0ffbc6c5..21bd95941e 100755
> --- a/src/test/test_driver.c
> +++ b/src/test/test_driver.c
> @@ -3414,6 +3414,10 @@ static int testDomainBlockStats(virDomainPtr domain,
>       return ret;
>   }
> 
> +
> +static virNetworkObjPtr testNetworkObjFindByName(testDriverPtr privconn, const char *name);
> +
> +
>   static int
>   testDomainInterfaceAddresses(virDomainPtr dom,
>                                virDomainInterfacePtr **ifaces,
> @@ -3422,11 +3426,15 @@ testDomainInterfaceAddresses(virDomainPtr dom,
>   {
>       size_t i;
>       size_t ifaces_count = 0;
> +    size_t addr_offset;
>       int ret = -1;
>       char macaddr[VIR_MAC_STRING_BUFLEN];
>       virDomainObjPtr vm = NULL;
>       virDomainInterfacePtr iface = NULL;
>       virDomainInterfacePtr *ifaces_ret = NULL;
> +    virSocketAddr addr;
> +    virNetworkObjPtr net = NULL;
> +    virNetworkDefPtr net_def = NULL;
> 
>       virCheckFlags(0, -1);
> 
> @@ -3447,6 +3455,12 @@ testDomainInterfaceAddresses(virDomainPtr dom,
>           goto cleanup;
> 
>       for (i = 0; i < vm->def->nnets; i++) {
> +        if (!(net = testNetworkObjFindByName(dom->conn->privateData,
> +                                             vm->def->nets[i]->data.network.name)))

This is unsafe. We can access ->data.network iff type is NETWORK.

> +            goto cleanup;
> +
> +        net_def = virNetworkObjGetDef(net);
> +
>           if (VIR_ALLOC(iface) < 0)
>               goto cleanup;
> 
> @@ -3460,14 +3474,33 @@ testDomainInterfaceAddresses(virDomainPtr dom,
>           if (VIR_ALLOC(iface->addrs) < 0)
>               goto cleanup;
> 
> -        iface->addrs[0].type = VIR_IP_ADDR_TYPE_IPV4;
> -        iface->addrs[0].prefix = 24;
> -        if (virAsprintf(&iface->addrs[0].addr, "192.168.0.%zu", 1 + i) < 0)
> -            goto cleanup;
> -

Instead of removing, we can use this for !NETWORK types.

>           iface->naddrs = 1;
> +        iface->addrs[0].prefix = virSocketAddrGetIPPrefix(&net_def->ips->address,
> +                                                          &net_def->ips->netmask,
> +                                                          net_def->ips->prefix);
> +
> +        if (net_def->ips->nranges > 0)
> +            addr = net_def->ips->ranges[0].start;
> +        else
> +            addr = net_def->ips->address;
> +
> +        /* try using different addresses per different inf and domain */
> +        addr_offset = 20 * (vm->def->id - 1) + i + 1;
> +
> +        if (net_def->ips->family && STREQ(net_def->ips->family, "ipv6")) {
> +            iface->addrs[0].type = VIR_IP_ADDR_TYPE_IPV6;
> +            addr.data.inet6.sin6_addr.s6_addr[15] += addr_offset;
> +        } else {
> +            iface->addrs[0].type = VIR_IP_ADDR_TYPE_IPV4;
> +            addr.data.inet4.sin_addr.s_addr = \
> +                htonl(ntohl(addr.data.inet4.sin_addr.s_addr) + addr_offset);
> +        }
> +
> +        if (!(iface->addrs[0].addr = virSocketAddrFormat(&addr)))
> +            goto cleanup;
> 
>           VIR_APPEND_ELEMENT_INPLACE(ifaces_ret, ifaces_count, iface);
> +        virNetworkObjEndAPI(&net);

This should be moved into a separate function.

>       }
> 
>       VIR_STEAL_PTR(*ifaces, ifaces_ret);
> @@ -3475,6 +3508,7 @@ testDomainInterfaceAddresses(virDomainPtr dom,
> 
>    cleanup:
>       virDomainObjEndAPI(&vm);
> +    virNetworkObjEndAPI(&net);
> 
>       if (ifaces_ret) {
>           for (i = 0; i < ifaces_count; i++)


With all that fixed, I've ACKed and pushed this patch. Thank you for 
taking care of this.

Michal