Re: [PATCH] Share the code and schemas for domain and network route definitions

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

 



On 09.01.2015 17:47, Cédric Bosdonnat wrote:
Made the network configuration schemas and codes for the route element reusable.
Created networkcommon_conf.[ch] files containing pieces to be used in both domain
and network configurations.

Replaced the brand new domain route configuration by the network one. Note that it
now forces the destination address to be defined, even if the user wants to define
the default gateway.
---
  docs/formatdomain.html.in                         |  14 +-
  docs/schemas/basictypes.rng                       |   2 +-
  docs/schemas/domaincommon.rng                     |  29 +-
  docs/schemas/network.rng                          |  20 +-
  docs/schemas/networkcommon.rng                    |  22 ++
  po/POTFILES.in                                    |   1 +
  src/Makefile.am                                   |   3 +-
  src/conf/domain_conf.c                            | 117 ++------
  src/conf/domain_conf.h                            |  14 +-
  src/conf/network_conf.c                           | 284 +-----------------
  src/conf/network_conf.h                           |  20 +-
  src/conf/networkcommon_conf.c                     | 337 ++++++++++++++++++++++
  src/conf/networkcommon_conf.h                     |  76 +++++
  src/libvirt_private.syms                          |   7 +
  src/lxc/lxc_container.c                           |  22 +-
  src/lxc/lxc_native.c                              |  26 +-
  tests/lxcconf2xmldata/lxcconf2xml-physnetwork.xml |   4 +-
  tests/lxcconf2xmldata/lxcconf2xml-simple.xml      |   4 +-
  tests/lxcxml2xmldata/lxc-hostdev.xml              |   4 +-
  tests/lxcxml2xmldata/lxc-idmap.xml                |   4 +-
  20 files changed, 542 insertions(+), 468 deletions(-)
  create mode 100644 src/conf/networkcommon_conf.c
  create mode 100644 src/conf/networkcommon_conf.h


I've always felt that what's shared in RNG should share parser/formatter in the code too.

diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng
index 9ddd92b..efc9da4 100644
--- a/docs/schemas/basictypes.rng
+++ b/docs/schemas/basictypes.rng
@@ -174,7 +174,7 @@
    <define name="ipv6Addr">
      <data type="string">
        <!-- To understand this better, take apart the toplevel "|"s -->
-<param name="pattern">(([0-9A-Fa-f]{1,4}:){7}[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){6}:[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){5}:([0-9A-Fa-f]{1,4}:)?[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){4}:([0-9A-Fa-f]{1,4}:){0,2}[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){3}:([0-9A-Fa-f]{1,4}:){0,3}[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){2}:([0-9A-Fa-f]{1,4}:){0,4}[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){6}(((25[0-5])|(2[0-4][0-9])|(1[0-9]{2})|([1-9][0-9])|([0-9]))\.){3}((25[0-5])|(2[0-4][0-9])|(1[0-9]{2})|([1-9][0-9])|([0-9])))|(([0-9A-Fa-f]{1,4}:){0,5}:(((25[0-5])|(2[0-4][0-9])|(1[0-9]{2})|([1-9][0-9])|([0-9]))\.){3}((25[0-5])|(2[0-4][0-9])|(1[0-9]{2})|([1-9][0-9])|([0-9])))|(::([0-9A-Fa-f]{1,4}:){0,5}(((25[0-5])|(2[0-4][0-9])|(1[0-9]{2})|([1-9][0-9])|([0-9]))\.){3}((25[0-5])|(2[0-4][0-9])|(1[0-9]{2})|([1-9][0-9])|([0-9])))|([0-9A-Fa-f]{1,4}::([0-9A-Fa-f]{1,4}:){0,5}[0-9A-Fa-f]{1,4})|(::([0-9A-Fa-f]{1,4}:){0,6}[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){1,7}:)</param>
+<param name="pattern">(([0-9A-Fa-f]{1,4}:){7}[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){6}:[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){5}:([0-9A-Fa-f]{1,4}:)?[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){4}:([0-9A-Fa-f]{1,4}:){0,2}[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){3}:([0-9A-Fa-f]{1,4}:){0,3}[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){2}:([0-9A-Fa-f]{1,4}:){0,4}[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){6}(((25[0-5])|(2[0-4][0-9])|(1[0-9]{2})|([1-9][0-9])|([0-9]))\.){3}((25[0-5])|(2[0-4][0-9])|(1[0-9]{2})|([1-9][0-9])|([0-9])))|(([0-9A-Fa-f]{1,4}:){0,5}:(((25[0-5])|(2[0-4][0-9])|(1[0-9]{2})|([1-9][0-9])|([0-9]))\.){3}((25[0-5])|(2[0-4][0-9])|(1[0-9]{2})|([1-9][0-9])|([0-9])))|(::([0-9A-Fa-f]{1,4}:){0,5}(((25[0-5])|(2[0-4][0-9])|(1[0-9]{2})|([1-9][0-9])|([0-9]))\.){3}((25[0-5])|(2[0-4][0-9])|(1[0-9]{2})|([1-9][0-9])|([0-9])))|([0-9A-Fa-f]{1,4}::([0-9A-Fa-f]{1,4}:){0,5}[0-9A-Fa-f]{1,4})|(::([0-9A-Fa-f]{1,4}:){0,6}[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){1,7}:)|(::)</param>

My brain is just too small to go through this on Monday morning.

      </data>
    </define>


diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 57e99e6..8e34fd6 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3,6 +3,7 @@
   *
   * Copyright (C) 2006-2014 Red Hat, Inc.
   * Copyright (C) 2006-2008 Daniel P. Berrange
+ * Copyright (c) 2015 SUSE LINUX Products GmbH, Nuernberg, Germany.
   *
   * This library is free software; you can redistribute it and/or
   * modify it under the terms of the GNU Lesser General Public
@@ -1475,11 +1476,13 @@ void virDomainNetDefFree(virDomainNetDefPtr def)
          VIR_FREE(def->ips[i]);
      VIR_FREE(def->ips);

-    for (i = 0; i < def->nroutes; i++)
+    for (i = 0; i < def->nroutes; i++) {
+        virNetworkRouteDefClear(def->routes[i]);
          VIR_FREE(def->routes[i]);

This code snippet repeats itself in a few places too. Looks like virNetworkRouteDefFree() which encapsulates the two comes handy.

+    }
      VIR_FREE(def->routes);

-        virDomainDeviceInfoClear(&def->info);
+    virDomainDeviceInfoClear(&def->info);

      VIR_FREE(def->filter);
      virNWFilterHashTableFree(def->filterparams);

  static void
  virDomainNetRoutesFormat(virBufferPtr buf,
-                         virDomainNetRouteDefPtr *routes,
+                         virNetworkRouteDefPtr *routes,
                           size_t nroutes)
  {
      size_t i;

      for (i = 0; i < nroutes; i++) {
-        virDomainNetRouteDefPtr route = routes[i];
-        const char *familyStr = NULL;
-        char *via = virSocketAddrFormat(&route->via);
-        char *to = NULL;
-
-        if (VIR_SOCKET_ADDR_IS_FAMILY(&route->via, AF_INET6))
-            familyStr = "ipv6";
-        else if (VIR_SOCKET_ADDR_IS_FAMILY(&route->via, AF_INET))
-            familyStr = "ipv4";
-        virBufferAsprintf(buf, "<route family='%s' via='%s'", familyStr, via);
-
-        if (VIR_SOCKET_ADDR_VALID(&route->to)) {
-            to = virSocketAddrFormat(&route->to);
-            virBufferAsprintf(buf, " address='%s'", to);
-        }
-
-        if (route->prefix > 0)
-            virBufferAsprintf(buf, " prefix='%d'", route->prefix);
-
-        virBufferAddLit(buf, "/>\n");
+        virNetworkRouteDefPtr route = routes[i];
+        virNetworkRouteDefFormat(buf, route);

Or just user routes[i] directly.

      }
  }


diff --git a/src/conf/networkcommon_conf.c b/src/conf/networkcommon_conf.c
new file mode 100644
index 0000000..da80c0f
--- /dev/null
+++ b/src/conf/networkcommon_conf.c

+int
+virNetworkRouteDefFormat(virBufferPtr buf,
+                         const virNetworkRouteDef *def)
+{
+    int result = -1;
+
+    virBufferAddLit(buf, "<route");
+
+    if (def->family)
+        virBufferAsprintf(buf, " family='%s'", def->family);
+    if (VIR_SOCKET_ADDR_VALID(&def->address)) {
+        char *addr = virSocketAddrFormat(&def->address);
+
+        if (!addr)
+            goto error;
+        virBufferAsprintf(buf, " address='%s'", addr);
+        VIR_FREE(addr);
+    }
+    if (VIR_SOCKET_ADDR_VALID(&def->netmask)) {
+        char *addr = virSocketAddrFormat(&def->netmask);
+
+        if (!addr)
+            goto error;
+        virBufferAsprintf(buf, " netmask='%s'", addr);
+        VIR_FREE(addr);
+    }
+    if (def->has_prefix)
+        virBufferAsprintf(buf, " prefix='%u'", def->prefix);
+    if (VIR_SOCKET_ADDR_VALID(&def->gateway)) {

I know you've copied the preexisting code, but since this kind of check is done in virNetworkRouteDefCreate() - is it needed here too? It doesn't hurt anything (but performance, which we don't care about that much). I'm just curious.

+        char *addr = virSocketAddrFormat(&def->gateway);
+        if (!addr)
+            goto error;
+        virBufferAsprintf(buf, " gateway='%s'", addr);
+        VIR_FREE(addr);
+    }
+    if (def->has_metric && def->metric > 0)
+        virBufferAsprintf(buf, " metric='%u'", def->metric);
+    virBufferAddLit(buf, "/>\n");
+
+    result = 0;
+ error:
+    return result;
+}

diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c
index d7cd1d5..dd99bbb 100644
--- a/src/lxc/lxc_native.c
+++ b/src/lxc/lxc_native.c
@@ -2,7 +2,7 @@
   * lxc_native.c: LXC native configuration import
   *
   * Copyright (c) 2014 Red Hat, Inc.
- * Copyright (c) 2013 SUSE LINUX Products GmbH, Nuernberg, Germany.
+ * Copyright (c) 2013-2015 SUSE LINUX Products GmbH, Nuernberg, Germany.

Technically speaking this should be 2013,2015 but who cares, right? :)

   *
   * This library is free software; you can redistribute it and/or
   * modify it under the terms of the GNU Lesser General Public


ACK

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]