Re: [PATCH] network: add modify-or-add feature to net-update

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

 



ping, it's been a while since i have put this out. 

On Sat, 16 Mar 2024 at 01:51, Abhiram Tilak <atp.exp@xxxxxxxxx> wrote:
The current way of updating a network configuration uses `virsh
net-update` to add, delete or modify entries. But with such a mechansim
one should know if an entry with current info already exists. Adding
modify-or-add option automatically performs either modify or add
depending on the current state.

Fixes: https://gitlab.com/libvirt/libvirt/-/issues/363
Signed-off-by: Abhiram Tilak <atp.exp@xxxxxxxxx>
---
 docs/manpages/virsh.rst           |   5 +-
 include/libvirt/libvirt-network.h |   2 +
 src/conf/network_conf.c           | 148 ++++++++++++++++++++++++------
 tools/virsh-network.c             |   4 +-
 4 files changed, 126 insertions(+), 33 deletions(-)

diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index 115b802c45..dc91ba895c 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -5908,7 +5908,10 @@ changes optionally taking effect immediately, without needing to
 destroy and re-start the network.

 *command* is one of "add-first", "add-last", "add" (a synonym for
-add-last), "delete", or "modify".
+add-last), "delete", "modify", "modify-or-add" (modify + add-last),
+"modify-or-add-first". The 'modify-or-add' commands perform modify or
+add operation depending on the given state, and can be useful for
+scripting.

 *section* is one of "bridge", "domain", "ip", "ip-dhcp-host",
 "ip-dhcp-range", "forward", "forward-interface", "forward-pf",
diff --git a/include/libvirt/libvirt-network.h b/include/libvirt/libvirt-network.h
index 58591be7ac..a6e132f407 100644
--- a/include/libvirt/libvirt-network.h
+++ b/include/libvirt/libvirt-network.h
@@ -181,6 +181,8 @@ typedef enum {
     VIR_NETWORK_UPDATE_COMMAND_DELETE    = 2, /* delete an existing element (Since: 0.10.2) */
     VIR_NETWORK_UPDATE_COMMAND_ADD_LAST  = 3, /* add an element at end of list (Since: 0.10.2) */
     VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST = 4, /* add an element at start of list (Since: 0.10.2) */
+    VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_LAST = 5, /* if exists modify or add an element at end of list (Since: 0.10.2) */
+    VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_FIRST = 6, /* if exists modify or add an element at start of list (Since: 0.10.2) */
 # ifdef VIR_ENUM_SENTINELS
     VIR_NETWORK_UPDATE_COMMAND_LAST /* (Since: 0.10.2) */
 # endif
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index cc92ed0b03..2835395385 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -2721,6 +2721,9 @@ virNetworkDefUpdateIPDHCPHost(virNetworkDef *def,
     virNetworkDHCPHostDef host = { 0 };
     bool partialOkay = (command == VIR_NETWORK_UPDATE_COMMAND_DELETE);

+    /* added for modify-or-add feature */
+    bool modified = false;
+
     if (virNetworkDefUpdateCheckElementName(def, ctxt->node, "host") < 0)
         goto cleanup;

@@ -2826,7 +2829,34 @@ virNetworkDefUpdateIPDHCPHost(virNetworkDef *def,
         virNetworkDHCPHostDefClear(&ipdef->hosts[i]);
         VIR_DELETE_ELEMENT(ipdef->hosts, i, ipdef->nhosts);

-    } else {
+    } else if ((command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_LAST) ||
+               (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_FIRST)) {
+
+        /* find entries with matching name/address/ip */
+        for (i = 0; i < ipdef->nhosts; i++) {
+            if ((host.mac && ipdef->hosts[i].mac &&
+                 !virMacAddrCompare(host.mac, ipdef->hosts[i].mac)) ||
+                (host.name &&
+                 STREQ_NULLABLE(host.name, ipdef->hosts[i].name)) ||
+                (VIR_SOCKET_ADDR_VALID(&host.ip) &&
+                 virSocketAddrEqual(&host.ip, &ipdef->hosts[i].ip))) {
+
+                modified = true;
+                break;
+            }
+        }
+
+        /* if element is found then modify, or else add to beginning/end of list */
+        if (modified) {
+            virNetworkDHCPHostDefClear(&ipdef->hosts[i]);
+            ipdef->hosts[i] = host;
+            memset(&host, 0, sizeof(host));
+        } else if (VIR_INSERT_ELEMENT(ipdef->hosts,
+                        command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_FIRST
+                        ? 0 : ipdef->nhosts,
+                        ipdef->nhosts, host) < 0)
+            goto cleanup;
+    } else  {
         virNetworkDefUpdateUnknownCommand(command);
         goto cleanup;
     }
@@ -2885,7 +2915,9 @@ virNetworkDefUpdateIPDHCPRange(virNetworkDef *def,
     }

     if ((command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST) ||
-        (command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST)) {
+        (command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST) ||
+        (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_LAST) ||
+        (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_FIRST)) {

         if (virNetworkDefUpdateCheckMultiDHCP(def, ipdef) < 0)
             return -1;
@@ -2894,17 +2926,24 @@ virNetworkDefUpdateIPDHCPRange(virNetworkDef *def,
             g_autofree char *startip = virSocketAddrFormat(&range.addr.start);
             g_autofree char *endip = virSocketAddrFormat(&range.addr.end);

-            virReportError(VIR_ERR_OPERATION_INVALID,
-                           _("there is an existing dhcp range entry in network '%1$s' that matches \"<range start='%2$s' end='%3$s'/>\""),
-                           def->name,
-                           startip ? startip : "unknown",
-                           endip ? endip : "unknown");
+
+            if ((command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST) ||
+                (command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST))
+                virReportError(VIR_ERR_OPERATION_INVALID,
+                               _("there is an existing dhcp range entry in network '%1$s' that matches \"<range start='%2$s' end='%3$s'/>\""),
+                               def->name,
+                               startip ? startip : "unknown",
+                               endip ? endip : "unknown");
+            else
+                virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+                               _("dhcp ranges cannot be modified, only added or deleted"));
             return -1;
         }

         /* add to beginning/end of list */
         if (VIR_INSERT_ELEMENT(ipdef->ranges,
-                               command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST
+                               (command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST ||
+                                command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_FIRST)
                                ? 0 : ipdef->nranges,
                                ipdef->nranges, range) < 0)
             return -1;
@@ -2981,18 +3020,26 @@ virNetworkDefUpdateForwardInterface(virNetworkDef *def,
     }

     if ((command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST) ||
-        (command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST)) {
+        (command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST) ||
+        (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_LAST) ||
+        (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_FIRST)) {

         if (i < def->forward.nifs) {
-            virReportError(VIR_ERR_OPERATION_INVALID,
-                           _("there is an existing interface entry in network '%1$s' that matches \"<interface dev='%2$s'>\""),
-                           def->name, iface.device.dev);
+            if ((command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST) ||
+                command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST)
+                virReportError(VIR_ERR_OPERATION_INVALID,
+                               _("there is an existing interface entry in network '%1$s' that matches \"<interface dev='%2$s'>\""),
+                               def->name, iface.device.dev);
+            else
+                virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+                               _("forward interface entries cannot be modified, only added or deleted"));
             goto cleanup;
         }

         /* add to beginning/end of list */
         if (VIR_INSERT_ELEMENT(def->forward.ifs,
-                               command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST
+                               (command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST ||
+                                command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_FIRST)
                                ? 0 : def->forward.nifs,
                                def->forward.nifs, iface) < 0)
             goto cleanup;
@@ -3056,6 +3103,9 @@ virNetworkDefUpdatePortGroup(virNetworkDef *def,
     int ret = -1;
     virPortGroupDef portgroup = { 0 };

+    /* added for modify-or-add feature */
+    bool modified = false;
+
     if (virNetworkDefUpdateCheckElementName(def, ctxt->node, "portgroup") < 0)
         goto cleanup;

@@ -3097,6 +3147,17 @@ virNetworkDefUpdatePortGroup(virNetworkDef *def,
         goto cleanup;
     }

+    /* modify found entries for modify-or-add command */
+    if (foundName >= 0 && (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_LAST ||
+        command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_FIRST)) {
+
+        /* replace existing entry */
+        virPortGroupDefClear(&def->portGroups[foundName]);
+        def->portGroups[foundName] = portgroup;
+        memset(&portgroup, 0, sizeof(portgroup));
+        modified = true;
+    }
+
     if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY) {

         /* replace existing entry */
@@ -3105,11 +3166,14 @@ virNetworkDefUpdatePortGroup(virNetworkDef *def,
         memset(&portgroup, 0, sizeof(portgroup));

     } else if ((command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST) ||
-        (command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST)) {
+        (command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST) ||
+        (!modified && command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_LAST) ||
+        (!modified && command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_FIRST)) {

         /* add to beginning/end of list */
         if (VIR_INSERT_ELEMENT(def->portGroups,
-                               command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST
+                               (command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST ||
+                               command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_FIRST)
                                ? 0 : def->nPortGroups,
                                def->nPortGroups, portgroup) < 0)
             goto cleanup;
@@ -3144,7 +3208,9 @@ virNetworkDefUpdateDNSHost(virNetworkDef *def,
     virNetworkDNSDef *dns = &def->dns;
     virNetworkDNSHostDef host = { 0 };
     bool isAdd = (command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST ||
-                  command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST);
+                  command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST ||
+                  command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_FIRST ||
+                  command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_LAST);
     int foundCt = 0;

     if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY) {
@@ -3185,15 +3251,21 @@ virNetworkDefUpdateDNSHost(virNetworkDef *def,
     if (isAdd) {

         if (foundCt > 0) {
-            virReportError(VIR_ERR_OPERATION_INVALID,
-                           _("there is already at least one DNS HOST record with a matching field in network %1$s"),
-                           def->name);
+            if ((command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST) ||
+                command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST)
+                virReportError(VIR_ERR_OPERATION_INVALID,
+                               _("there is already at least one DNS HOST record with a matching field in network %1$s"),
+                               def->name);
+            else
+                virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+                               _("DNS HOST records cannot be modified, only added or deleted"));
             goto cleanup;
         }

         /* add to beginning/end of list */
         if (VIR_INSERT_ELEMENT(dns->hosts,
-                               command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST
+                               (command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST ||
+                                command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_FIRST)
                                ? 0 : dns->nhosts, dns->nhosts, host) < 0)
             goto cleanup;
     } else if (command == VIR_NETWORK_UPDATE_COMMAND_DELETE) {
@@ -3240,7 +3312,9 @@ virNetworkDefUpdateDNSSrv(virNetworkDef *def,
     virNetworkDNSDef *dns = &def->dns;
     virNetworkDNSSrvDef srv = { 0 };
     bool isAdd = (command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST ||
-                  command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST);
+                  command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST ||
+                  command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_FIRST ||
+                  command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_LAST);
     int foundCt = 0;

     if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY) {
@@ -3268,15 +3342,21 @@ virNetworkDefUpdateDNSSrv(virNetworkDef *def,
     if (isAdd) {

         if (foundCt > 0) {
-            virReportError(VIR_ERR_OPERATION_INVALID,
-                           _("there is already at least one DNS SRV record matching all specified fields in network %1$s"),
-                           def->name);
+            if ((command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST) ||
+                (command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST))
+                virReportError(VIR_ERR_OPERATION_INVALID,
+                               _("there is already at least one DNS SRV record matching all specified fields in network %1$s"),
+                               def->name);
+            else
+                virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+                               _("DNS SRV records cannot be modified, only added or deleted"));
             goto cleanup;
         }

         /* add to beginning/end of list */
         if (VIR_INSERT_ELEMENT(dns->srvs,
-                               command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST
+                               (command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST ||
+                                command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_FIRST)
                                ? 0 : dns->nsrvs, dns->nsrvs, srv) < 0)
             goto cleanup;
     } else if (command == VIR_NETWORK_UPDATE_COMMAND_DELETE) {
@@ -3322,7 +3402,9 @@ virNetworkDefUpdateDNSTxt(virNetworkDef *def,
     virNetworkDNSDef *dns = &def->dns;
     virNetworkDNSTxtDef txt = { 0 };
     bool isAdd = (command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST ||
-                  command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST);
+                  command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST ||
+                  command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_FIRST ||
+                  command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_LAST);

     if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY) {
         virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
@@ -3344,15 +3426,21 @@ virNetworkDefUpdateDNSTxt(virNetworkDef *def,
     if (isAdd) {

         if (foundIdx < dns->ntxts) {
-            virReportError(VIR_ERR_OPERATION_INVALID,
-                           _("there is already a DNS TXT record with name '%1$s' in network %2$s"),
-                           txt.name, def->name);
+            if ((command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST) ||
+                (command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST))
+                virReportError(VIR_ERR_OPERATION_INVALID,
+                               _("there is already a DNS TXT record with name '%1$s' in network %2$s"),
+                               txt.name, def->name);
+            else
+                virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+                               _("DNS TXT records cannot be modified, only added or deleted"));
             goto cleanup;
         }

         /* add to beginning/end of list */
         if (VIR_INSERT_ELEMENT(dns->txts,
-                               command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST
+                               (command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST ||
+                                command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_OR_ADD_FIRST)
                                ? 0 : dns->ntxts, dns->ntxts, txt) < 0)
             goto cleanup;
     } else if (command == VIR_NETWORK_UPDATE_COMMAND_DELETE) {
diff --git a/tools/virsh-network.c b/tools/virsh-network.c
index 597e3d4530..c30305ac50 100644
--- a/tools/virsh-network.c
+++ b/tools/virsh-network.c
@@ -1231,7 +1231,7 @@ static const vshCmdOptDef opts_network_update[] = {
      .positional = true,
      .required = true,
      .completer = virshNetworkUpdateCommandCompleter,
-     .help = N_("type of update (add-first, add-last (add), delete, or modify)")
+     .help = N_("type of update (add-first, add-last (add), delete, modify, modify-or-add, or modify-or-add-first)")
     },
     {.name = "section",
      .type = VSH_OT_STRING,
@@ -1260,7 +1260,7 @@ static const vshCmdOptDef opts_network_update[] = {

 VIR_ENUM_IMPL(virshNetworkUpdateCommand,
               VIR_NETWORK_UPDATE_COMMAND_LAST,
-              "none", "modify", "delete", "add-last", "add-first");
+              "none", "modify", "delete", "add-last", "add-first", "modify-or-add", "modify-or-add-first");

 VIR_ENUM_IMPL(virshNetworkSection,
               VIR_NETWORK_SECTION_LAST,
--
2.44.0



--
Abhiram
_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx

[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