Re: [libvirt] [PATCH 3/3] test_driver: consider DHCP ranges in testDomainInterfaceAddresses

On 6/19/19 1:18 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, return IPv6 addresses too when needed. > > Signed-off-by: Ilias Stamatis <stamatis.iliass(a)gmail.com&gt; > --- > src/test/test_driver.c | 78 +++++++++++++++++++++++++++++++++++++++--- > 1 file changed, 73 insertions(+), 5 deletions(-) > > diff --git a/src/test/test_driver.c b/src/test/test_driver.c > index 0c2cfdd2f7..96142b549c 100644 > --- a/src/test/test_driver.c > +++ b/src/test/test_driver.c > @@ -27,6 +27,7 @@ > #include <sys/stat.h> > #include <libxml/xmlsave.h> > #include <libxml/xpathInternals.h> > +#include <arpa/inet.h> > > > #include "virerror.h" > @@ -3380,6 +3381,11 @@ static int testDomainBlockStats(virDomainPtr domain, > return ret; > } > > + > +static virNetworkObjPtr testNetworkObjFindByName(testDriverPtr privconn, const char *name); > +static virNetworkPtr testNetworkLookupByName(virConnectPtr conn, const char *name); > + > + > static int > testDomainInterfaceAddresses(virDomainPtr dom, > virDomainInterfacePtr **ifaces, > @@ -3388,11 +3394,18 @@ testDomainInterfaceAddresses(virDomainPtr dom, > { > size_t i; > size_t ifaces_count = 0; > + size_t addr_offset; > int ret = -1; > char macaddr[VIR_MAC_STRING_BUFLEN]; > + char ip_str[INET6_ADDRSTRLEN]; > + struct sockaddr_in ipv4_addr; > + struct sockaddr_in6 ipv6_addr; > virDomainObjPtr vm = NULL; > virDomainInterfacePtr iface = NULL; > virDomainInterfacePtr *ifaces_ret = NULL; > + virNetworkPtr net = NULL; > + virNetworkObjPtr net_obj = NULL; > + virNetworkDefPtr net_obj_def = NULL; > > virCheckFlags(0, -1); > > @@ -3413,6 +3426,16 @@ testDomainInterfaceAddresses(virDomainPtr dom, > if (vm->def->nets[i]->type != VIR_DOMAIN_NET_TYPE_NETWORK) > continue; > > + virObjectUnref(net); > + if (!(net = testNetworkLookupByName(dom->conn, > + vm->def->nets[i]->data.network.name))) > + goto cleanup; > + > + if (!(net_obj = testNetworkObjFindByName(net->conn->privateData, net->name))) > + goto cleanup; This is needless IMO. I mean, @net variable looks redundant to me, why not look up the network object directly? For instance: if (!(net_obj = testNetworkObjFindByName(dom->conn->privateData, vm->def->nets[i]->data.network.name))) goto cleanup;

But looking further at next hunk, I wonder if it makes sense to separate it into a separate function. Otherwise the code looks good. A bit complicated, but that was expected, since dealing with IP addresses is never a few lines of code :-(

Michal