[libvirt] [PATCH 3/3] test_driver: consider DHCP ranges in testDomainInterfaceAddresses
Ilias Stamatis
stamatis.iliass at gmail.com
Wed Jun 19 14:04:30 UTC 2019
On Wed, Jun 19, 2019 at 3:48 PM Michal Privoznik <mprivozn at redhat.com> wrote:
>
> 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 at gmail.com>
> > ---
> > 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;
Aah, right. I will remove this.
>
> 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 :-(
I managed to do it just a tiny bit simpler by using a virSocketAddr
instead of the 2 struct sockaddr_in ones.
But which part do you want me to separate exactly?
>
> Michal
More information about the libvir-list
mailing list