Re: [PATCH] test_driver: implement virNetworkGetDHCPLeases

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

 



On 6/17/19 10:09 PM, Ilias Stamatis wrote:
On Sat, Jun 15, 2019 at 3:25 PM Michal Prívozník <mprivozn@xxxxxxxxxx> wrote:

On 6/8/19 12:00 PM, Ilias Stamatis wrote:
Return infinite time DHCP leases for fixed IPV4 addresses.

Signed-off-by: Ilias Stamatis <stamatis.iliass@xxxxxxxxx>
---
  src/test/test_driver.c | 118 +++++++++++++++++++++++++++++++++++++++++
  1 file changed, 118 insertions(+)

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 1aa79ce898..b7f8f6ecf2 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -3831,6 +3831,123 @@ testNetworkSetAutostart(virNetworkPtr net,
  }


+static int
+testNetworkGetDHCPLeases(virNetworkPtr net,
+                         const char *mac,
+                         virNetworkDHCPLeasePtr **leases,
+                         unsigned int flags)
+{
+    int ret = -1;
+    int ndomains = 0;
+    size_t i, j;
+    size_t nleases = 0;
+    bool need_results = !!leases;
+    char *hostname = NULL;
+    char mac_str[VIR_MAC_STRING_BUFLEN];
+    virMacAddr mac_addr;
+    virDomainObjPtr vm = NULL;
+    virDomainPtr *domains = NULL;
+    virDomainNetDefPtr inf = NULL;
+    virNetworkObjPtr obj = NULL;
+    virNetworkDefPtr obj_def = NULL;
+    virNetworkDHCPLeasePtr lease = NULL;
+    virNetworkDHCPLeasePtr *leases_ret = NULL;
+    testDriverPtr privconn = net->conn->privateData;
+
+    virCheckFlags(0, -1);
+
+    if (mac && virMacAddrParse(mac, &mac_addr) < 0) {
+        virReportError(VIR_ERR_INVALID_MAC, "%s", mac);
+        return -1;
+    }
+
+    if (!(obj = testNetworkObjFindByName(privconn, net->name)))
+        goto cleanup;
+    obj_def = virNetworkObjGetDef(obj);
+
+    ndomains = virDomainObjListExport(privconn->domains, net->conn, &domains,
+                                      NULL, VIR_CONNECT_LIST_DOMAINS_ACTIVE);
+    if (ndomains < 0)
+        goto cleanup;
+
+    for (i = 0; i < ndomains; i++) {
+        /* the following must be called before testDomObjFromDomain */
+        hostname = testDomainGetHostname(domains[i], 0);
+
+        if (!(vm = testDomObjFromDomain(domains[i])))
+            continue;
+
+        for (j = 0; j < vm->def->nnets; j++) {
+            inf = vm->def->nets[j];
+            if (STRNEQ(inf->data.network.name, obj_def->name))
+                continue;
+
+            virMacAddrFormat(&inf->mac, mac_str);
+            if (mac && virMacAddrCompare(mac, mac_str))
+                continue;
+
+            if (!need_results) {
+                nleases++;
+                continue;
+            }
+
+            if (VIR_ALLOC(lease) < 0)
+                goto error;
+
+            /* the lease is for infinite time */
+            lease->expirytime = 0;
+
+            if ((VIR_STRDUP(lease->mac, mac_str) < 0) ||
+                (VIR_STRDUP(lease->iface, obj_def->bridge) < 0) ||
+                (VIR_STRDUP(lease->hostname, hostname) < 0))
+                goto error;
+
+            lease->prefix = 24;
+            lease->type = VIR_IP_ADDR_TYPE_IPV4;
+            if (virAsprintf(&lease->ipaddr, "192.168.0.%zu", 1 + j) < 0)
+                goto error;

This doesn't look right. What is the nestwork has a different range defined?

Right. We had a longer discussion about it with Erik and Pavel, sorry
I didn't include any thoughts about this.

So, as you already noticed testDomainInterfaceAddresses returns some
hard-coded IP addresses at the moment, ignoring any defined DHCP
ranges. We could make it such as it returns the first N addresses
starting from dhcp_range_start. And then from within
testNetworkGetDHCPLeases we can call testDomainInterfaceAddresses and
filter out those that belong to different networks based on the subnet
mask.

However, if we have for example 2 domains using the same network with
1 address each, then testNetworkGetDHCPLeases will return the same IP
lease twice and we already have an inconsistency.

Maybe one idea would be for testDomainInterfaceAddresses to also take
into account the domain id in such a way that it will return different
addresses for different domains. But many of the things mentioned
before would make the implementations more complex.

Since there was a recent discussion about the scope of the test
driver, we started wondering if it worths the effort. The conclusion
of the "scope of the test driver" discussion was that it should only
serve as a proof, that your app integrates with the API well and can
process the info. Having everything interconnected and return some
sane data would lead to full stack integration testing, which was
never the intention. So we thought that for testing purposes returning
hardcoded values is probably good enough.

However, now that I'm thinking about it again, maybe it wouldn't take
a huge effort to make it return some saner values so I can probably
re-write it.

Any additional thoughts would be useful.

I think returning the same IP address fro two distinct domains is a good trade off between code complexity and sane API behaviour. IOW, making the API return an IP address from the network range is a matter of a few lines of code but making sure it's unique across all domains would be much bigger task and probably not worth it.

I can cook the patch if you want me to. After all, I should have raised this sooner.

Michal

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux