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

Abhiram Tilak posted 1 patch 6 months, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20240515100854.17602-2-atp.exp@gmail.com
There is a newer version of this series
docs/manpages/virsh.rst           |   5 +-
include/libvirt/libvirt-network.h |  12 +--
src/conf/network_conf.c           | 134 +++++++++++++++++++++---------
tools/virsh-network.c             |   6 +-
4 files changed, 110 insertions(+), 47 deletions(-)
[PATCH v2] network: add modify-or-add feature to net-update
Posted by Abhiram Tilak 6 months, 1 week ago
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.

Resolves: https://gitlab.com/libvirt/libvirt/-/issues/363
Signed-off-by: Abhiram Tilak <atp.exp@gmail.com>
---
Changes in v2: 
  - Removed the modify-or-add functionality for sections
    where modify is not applicable.
  - Changed the existing implementation of `UpdateIPDHCPHost`,
    to avoid code duplication.
  - Changed the implementation of modify-or-delete to reassign
    the `command` variable instead of using multiple nested conditions.

 docs/manpages/virsh.rst           |   5 +-
 include/libvirt/libvirt-network.h |  12 +--
 src/conf/network_conf.c           | 134 +++++++++++++++++++++---------
 tools/virsh-network.c             |   6 +-
 4 files changed, 110 insertions(+), 47 deletions(-)

diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index 115b802c45..0da3827f6b 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), or
+"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..bb4468b160 100644
--- a/include/libvirt/libvirt-network.h
+++ b/include/libvirt/libvirt-network.h
@@ -176,11 +176,13 @@ int                     virNetworkUndefine      (virNetworkPtr network);
  * Since: 0.10.2
  */
 typedef enum {
-    VIR_NETWORK_UPDATE_COMMAND_NONE      = 0, /* invalid (Since: 0.10.2) */
-    VIR_NETWORK_UPDATE_COMMAND_MODIFY    = 1, /* modify an existing element (Since: 0.10.2) */
-    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_NONE             = 0, /* invalid (Since: 0.10.2) */
+    VIR_NETWORK_UPDATE_COMMAND_MODIFY           = 1, /* modify an existing element (Since: 0.10.2) */
+    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_ADD_LAST  = 5, /* conditionally modify or add an element at end (Since: 10.4.0) */
+    VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_FIRST = 6, /* conditionally modify or add an element at start (Since: 10.4.0) */
 # 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..a7c3dea163 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -2720,6 +2720,7 @@ virNetworkDefUpdateIPDHCPHost(virNetworkDef *def,
     virNetworkIPDef *ipdef = virNetworkIPDefByIndex(def, parentIndex);
     virNetworkDHCPHostDef host = { 0 };
     bool partialOkay = (command == VIR_NETWORK_UPDATE_COMMAND_DELETE);
+    int foundMatchedEntry = -1, foundExactEntry = -1;
 
     if (virNetworkDefUpdateCheckElementName(def, ctxt->node, "host") < 0)
         goto cleanup;
@@ -2740,22 +2741,47 @@ virNetworkDefUpdateIPDHCPHost(virNetworkDef *def,
         goto cleanup;
     }
 
-    if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY) {
+    /* check if the entry already exsits */
+    for (i = 0; i < ipdef->nhosts; i++) {
 
-        /* search for the entry with this (ip|mac|name),
-         * and update the IP+(mac|name) */
-        for (i = 0; i < ipdef->nhosts; i++) {
-            if ((host.mac && ipdef->hosts[i].mac &&
-                 !virMacAddrCompare(host.mac, ipdef->hosts[i].mac)) ||
-                (VIR_SOCKET_ADDR_VALID(&host.ip) &&
-                 virSocketAddrEqual(&host.ip, &ipdef->hosts[i].ip)) ||
-                (host.name &&
-                 STREQ_NULLABLE(host.name, ipdef->hosts[i].name))) {
-                break;
-            }
+        /* try to match any of (ip|mac|name) attributes */
+        if ((host.mac && ipdef->hosts[i].mac &&
+             !virMacAddrCompare(host.mac, ipdef->hosts[i].mac)) ||
+            (VIR_SOCKET_ADDR_VALID(&host.ip) &&
+             virSocketAddrEqual(&host.ip, &ipdef->hosts[i].ip)) ||
+            (host.name &&
+             STREQ_NULLABLE(host.name, ipdef->hosts[i].name))) {
+            foundMatchedEntry = i;
+        }
+
+        /* find exact entry - all specified attributes must match */
+        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))) {
+            foundExactEntry = i;
+            break;
         }
+    }
+
+    /* modify-or-add: convert command to add or modify based on foundEntry */
+    if (foundEntry == -1) {
+        if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_FIRST)
+            command = VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST;
+        else if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_LAST)
+            command = VIR_NETWORK_UPDATE_COMMAND_ADD_LAST;
+    } else if (foundEntry >= 0) {
+        if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_FIRST ||
+            command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_LAST)
+            command = VIR_NETWORK_UPDATE_COMMAND_MODIFY;
+    }
+
+    if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY) {
 
-        if (i == ipdef->nhosts) {
+        /* log error if no such entry exists to be modified */
+        if (foundMatchedEntry == -1) {
             g_autofree char *ip = virSocketAddrFormat(&host.ip);
             virReportError(VIR_ERR_OPERATION_INVALID,
                            _("couldn't locate an existing dhcp host entry with \"mac='%1$s'\" \"name='%2$s'\" \"ip='%3$s'\" in network '%4$s'"),
@@ -2779,21 +2805,13 @@ virNetworkDefUpdateIPDHCPHost(virNetworkDef *def,
             goto cleanup;
 
         /* log error if an entry with same name/address/ip already exists */
-        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))) {
-                g_autofree char *ip = virSocketAddrFormat(&host.ip);
+        if (foundMatchedEntry >= 0) {
+            virReportError(VIR_ERR_OPERATION_INVALID,
+                           _("there is an existing dhcp host entry in network '%1$s' that matches \"<host mac='%2$s' name='%3$s' ip='%4$s'/>\""),
+                           def->name, host.mac ? host.mac : _("unknown"),
+                           host.name, ip ? ip : _("unknown"));
+            goto cleanup;
 
-                virReportError(VIR_ERR_OPERATION_INVALID,
-                               _("there is an existing dhcp host entry in network '%1$s' that matches \"<host mac='%2$s' name='%3$s' ip='%4$s'/>\""),
-                               def->name, host.mac ? host.mac : _("unknown"),
-                               host.name, ip ? ip : _("unknown"));
-                goto cleanup;
-            }
         }
 
         /* add to beginning/end of list */
@@ -2804,18 +2822,8 @@ virNetworkDefUpdateIPDHCPHost(virNetworkDef *def,
             goto cleanup;
     } else if (command == VIR_NETWORK_UPDATE_COMMAND_DELETE) {
 
-        /* find matching entry - all specified attributes must match */
-        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))) {
-                break;
-            }
-        }
-        if (i == ipdef->nhosts) {
+        /* log error if there is no entry with exact match*/
+        if (foundExactEntry == -1) {
             virReportError(VIR_ERR_OPERATION_INVALID,
                            _("couldn't locate a matching dhcp host entry in network '%1$s'"),
                            def->name);
@@ -2865,6 +2873,14 @@ virNetworkDefUpdateIPDHCPRange(virNetworkDef *def,
         return -1;
     }
 
+    if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_FIRST ||
+        command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_LAST) {
+
+        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+                       _("dhcp ranges cannot be modified, use add command instead of modify-or-add"));
+        return -1;
+    }
+
     if (virNetworkDHCPRangeDefParseXML(def->name, ipdef, ctxt->node, &range) < 0)
         return -1;
 
@@ -2964,6 +2980,13 @@ virNetworkDefUpdateForwardInterface(virNetworkDef *def,
         goto cleanup;
     }
 
+    if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_FIRST ||
+        command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_LAST) {
+        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+                       _("forward interface entries cannot be modified, use add command instead of modify-or-add"));
+        goto cleanup;
+    }
+
     /* parsing this is so simple that it doesn't have its own function */
     iface.type = VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV;
     if (!(iface.device.dev = virXMLPropString(ctxt->node, "dev"))) {
@@ -3085,6 +3108,18 @@ virNetworkDefUpdatePortGroup(virNetworkDef *def,
         goto cleanup;
     }
 
+    /* modify-or-add: convert command to add or modify based on foundName */
+    if (foundName == -1) {
+        if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_FIRST)
+            command = VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST;
+        else if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_LAST)
+            command = VIR_NETWORK_UPDATE_COMMAND_ADD_LAST;
+    } else if (foundName >= 0) {
+        if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_FIRST ||
+            command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_LAST)
+            command = VIR_NETWORK_UPDATE_COMMAND_MODIFY;
+    }
+
     /* if there is already a different default, we can't make this
      * one the default.
      */
@@ -3153,6 +3188,13 @@ virNetworkDefUpdateDNSHost(virNetworkDef *def,
         goto cleanup;
     }
 
+    if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_FIRST ||
+        command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_LAST) {
+        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+                       _("DNS HOST records cannot be modified, use add instead of modify-or-add"));
+        goto cleanup;
+    }
+
     if (virNetworkDefUpdateCheckElementName(def, ctxt->node, "host") < 0)
         goto cleanup;
 
@@ -3249,6 +3291,13 @@ virNetworkDefUpdateDNSSrv(virNetworkDef *def,
         goto cleanup;
     }
 
+    if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_FIRST ||
+        command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_LAST) {
+        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+                       _("DNS SRV records cannot be modified, use add command instead of modify-or-add"));
+        goto cleanup;
+    }
+
     if (virNetworkDefUpdateCheckElementName(def, ctxt->node, "srv") < 0)
         goto cleanup;
 
@@ -3330,6 +3379,13 @@ virNetworkDefUpdateDNSTxt(virNetworkDef *def,
         goto cleanup;
     }
 
+    if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_FIRST ||
+        command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_LAST) {
+        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+                       _("DNS TXT records cannot be modified, use add command instead of modify-or-add"));
+        goto cleanup;
+    }
+
     if (virNetworkDefUpdateCheckElementName(def, ctxt->node, "txt") < 0)
         goto cleanup;
 
diff --git a/tools/virsh-network.c b/tools/virsh-network.c
index 597e3d4530..d7dc5df5a8 100644
--- a/tools/virsh-network.c
+++ b/tools/virsh-network.c
@@ -1231,7 +1231,8 @@ 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 +1261,8 @@ 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.42.1
Re: [PATCH v2] network: add modify-or-add feature to net-update
Posted by atp exp 6 months, 1 week ago
Please ignore this, forgot to add v2 in `git-send-email`

On Wed, 15 May 2024 at 15:41, Abhiram Tilak <atp.exp@gmail.com> 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.
>
> Resolves: https://gitlab.com/libvirt/libvirt/-/issues/363
> Signed-off-by: Abhiram Tilak <atp.exp@gmail.com>
> ---
> Changes in v2:
>   - Removed the modify-or-add functionality for sections
>     where modify is not applicable.
>   - Changed the existing implementation of `UpdateIPDHCPHost`,
>     to avoid code duplication.
>   - Changed the implementation of modify-or-delete to reassign
>     the `command` variable instead of using multiple nested conditions.
>
>  docs/manpages/virsh.rst           |   5 +-
>  include/libvirt/libvirt-network.h |  12 +--
>  src/conf/network_conf.c           | 134 +++++++++++++++++++++---------
>  tools/virsh-network.c             |   6 +-
>  4 files changed, 110 insertions(+), 47 deletions(-)
>
> diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
> index 115b802c45..0da3827f6b 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), or
> +"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..bb4468b160 100644
> --- a/include/libvirt/libvirt-network.h
> +++ b/include/libvirt/libvirt-network.h
> @@ -176,11 +176,13 @@ int                     virNetworkUndefine
> (virNetworkPtr network);
>   * Since: 0.10.2
>   */
>  typedef enum {
> -    VIR_NETWORK_UPDATE_COMMAND_NONE      = 0, /* invalid (Since: 0.10.2)
> */
> -    VIR_NETWORK_UPDATE_COMMAND_MODIFY    = 1, /* modify an existing
> element (Since: 0.10.2) */
> -    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_NONE             = 0, /* invalid (Since:
> 0.10.2) */
> +    VIR_NETWORK_UPDATE_COMMAND_MODIFY           = 1, /* modify an
> existing element (Since: 0.10.2) */
> +    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_ADD_LAST  = 5, /* conditionally
> modify or add an element at end (Since: 10.4.0) */
> +    VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_FIRST = 6, /* conditionally
> modify or add an element at start (Since: 10.4.0) */
>  # 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..a7c3dea163 100644
> --- a/src/conf/network_conf.c
> +++ b/src/conf/network_conf.c
> @@ -2720,6 +2720,7 @@ virNetworkDefUpdateIPDHCPHost(virNetworkDef *def,
>      virNetworkIPDef *ipdef = virNetworkIPDefByIndex(def, parentIndex);
>      virNetworkDHCPHostDef host = { 0 };
>      bool partialOkay = (command == VIR_NETWORK_UPDATE_COMMAND_DELETE);
> +    int foundMatchedEntry = -1, foundExactEntry = -1;
>
>      if (virNetworkDefUpdateCheckElementName(def, ctxt->node, "host") < 0)
>          goto cleanup;
> @@ -2740,22 +2741,47 @@ virNetworkDefUpdateIPDHCPHost(virNetworkDef *def,
>          goto cleanup;
>      }
>
> -    if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY) {
> +    /* check if the entry already exsits */
> +    for (i = 0; i < ipdef->nhosts; i++) {
>
> -        /* search for the entry with this (ip|mac|name),
> -         * and update the IP+(mac|name) */
> -        for (i = 0; i < ipdef->nhosts; i++) {
> -            if ((host.mac && ipdef->hosts[i].mac &&
> -                 !virMacAddrCompare(host.mac, ipdef->hosts[i].mac)) ||
> -                (VIR_SOCKET_ADDR_VALID(&host.ip) &&
> -                 virSocketAddrEqual(&host.ip, &ipdef->hosts[i].ip)) ||
> -                (host.name &&
> -                 STREQ_NULLABLE(host.name, ipdef->hosts[i].name))) {
> -                break;
> -            }
> +        /* try to match any of (ip|mac|name) attributes */
> +        if ((host.mac && ipdef->hosts[i].mac &&
> +             !virMacAddrCompare(host.mac, ipdef->hosts[i].mac)) ||
> +            (VIR_SOCKET_ADDR_VALID(&host.ip) &&
> +             virSocketAddrEqual(&host.ip, &ipdef->hosts[i].ip)) ||
> +            (host.name &&
> +             STREQ_NULLABLE(host.name, ipdef->hosts[i].name))) {
> +            foundMatchedEntry = i;
> +        }
> +
> +        /* find exact entry - all specified attributes must match */
> +        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))) {
> +            foundExactEntry = i;
> +            break;
>          }
> +    }
> +
> +    /* modify-or-add: convert command to add or modify based on
> foundEntry */
> +    if (foundEntry == -1) {
> +        if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_FIRST)
> +            command = VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST;
> +        else if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_LAST)
> +            command = VIR_NETWORK_UPDATE_COMMAND_ADD_LAST;
> +    } else if (foundEntry >= 0) {
> +        if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_FIRST ||
> +            command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_LAST)
> +            command = VIR_NETWORK_UPDATE_COMMAND_MODIFY;
> +    }
> +
> +    if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY) {
>
> -        if (i == ipdef->nhosts) {
> +        /* log error if no such entry exists to be modified */
> +        if (foundMatchedEntry == -1) {
>              g_autofree char *ip = virSocketAddrFormat(&host.ip);
>              virReportError(VIR_ERR_OPERATION_INVALID,
>                             _("couldn't locate an existing dhcp host entry
> with \"mac='%1$s'\" \"name='%2$s'\" \"ip='%3$s'\" in network '%4$s'"),
> @@ -2779,21 +2805,13 @@ virNetworkDefUpdateIPDHCPHost(virNetworkDef *def,
>              goto cleanup;
>
>          /* log error if an entry with same name/address/ip already exists
> */
> -        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))) {
> -                g_autofree char *ip = virSocketAddrFormat(&host.ip);
> +        if (foundMatchedEntry >= 0) {
> +            virReportError(VIR_ERR_OPERATION_INVALID,
> +                           _("there is an existing dhcp host entry in
> network '%1$s' that matches \"<host mac='%2$s' name='%3$s' ip='%4$s'/>\""),
> +                           def->name, host.mac ? host.mac : _("unknown"),
> +                           host.name, ip ? ip : _("unknown"));
> +            goto cleanup;
>
> -                virReportError(VIR_ERR_OPERATION_INVALID,
> -                               _("there is an existing dhcp host entry in
> network '%1$s' that matches \"<host mac='%2$s' name='%3$s' ip='%4$s'/>\""),
> -                               def->name, host.mac ? host.mac :
> _("unknown"),
> -                               host.name, ip ? ip : _("unknown"));
> -                goto cleanup;
> -            }
>          }
>
>          /* add to beginning/end of list */
> @@ -2804,18 +2822,8 @@ virNetworkDefUpdateIPDHCPHost(virNetworkDef *def,
>              goto cleanup;
>      } else if (command == VIR_NETWORK_UPDATE_COMMAND_DELETE) {
>
> -        /* find matching entry - all specified attributes must match */
> -        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))) {
> -                break;
> -            }
> -        }
> -        if (i == ipdef->nhosts) {
> +        /* log error if there is no entry with exact match*/
> +        if (foundExactEntry == -1) {
>              virReportError(VIR_ERR_OPERATION_INVALID,
>                             _("couldn't locate a matching dhcp host entry
> in network '%1$s'"),
>                             def->name);
> @@ -2865,6 +2873,14 @@ virNetworkDefUpdateIPDHCPRange(virNetworkDef *def,
>          return -1;
>      }
>
> +    if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_FIRST ||
> +        command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_LAST) {
> +
> +        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> +                       _("dhcp ranges cannot be modified, use add command
> instead of modify-or-add"));
> +        return -1;
> +    }
> +
>      if (virNetworkDHCPRangeDefParseXML(def->name, ipdef, ctxt->node,
> &range) < 0)
>          return -1;
>
> @@ -2964,6 +2980,13 @@ virNetworkDefUpdateForwardInterface(virNetworkDef
> *def,
>          goto cleanup;
>      }
>
> +    if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_FIRST ||
> +        command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_LAST) {
> +        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> +                       _("forward interface entries cannot be modified,
> use add command instead of modify-or-add"));
> +        goto cleanup;
> +    }
> +
>      /* parsing this is so simple that it doesn't have its own function */
>      iface.type = VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV;
>      if (!(iface.device.dev = virXMLPropString(ctxt->node, "dev"))) {
> @@ -3085,6 +3108,18 @@ virNetworkDefUpdatePortGroup(virNetworkDef *def,
>          goto cleanup;
>      }
>
> +    /* modify-or-add: convert command to add or modify based on foundName
> */
> +    if (foundName == -1) {
> +        if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_FIRST)
> +            command = VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST;
> +        else if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_LAST)
> +            command = VIR_NETWORK_UPDATE_COMMAND_ADD_LAST;
> +    } else if (foundName >= 0) {
> +        if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_FIRST ||
> +            command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_LAST)
> +            command = VIR_NETWORK_UPDATE_COMMAND_MODIFY;
> +    }
> +
>      /* if there is already a different default, we can't make this
>       * one the default.
>       */
> @@ -3153,6 +3188,13 @@ virNetworkDefUpdateDNSHost(virNetworkDef *def,
>          goto cleanup;
>      }
>
> +    if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_FIRST ||
> +        command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_LAST) {
> +        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> +                       _("DNS HOST records cannot be modified, use add
> instead of modify-or-add"));
> +        goto cleanup;
> +    }
> +
>      if (virNetworkDefUpdateCheckElementName(def, ctxt->node, "host") < 0)
>          goto cleanup;
>
> @@ -3249,6 +3291,13 @@ virNetworkDefUpdateDNSSrv(virNetworkDef *def,
>          goto cleanup;
>      }
>
> +    if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_FIRST ||
> +        command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_LAST) {
> +        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> +                       _("DNS SRV records cannot be modified, use add
> command instead of modify-or-add"));
> +        goto cleanup;
> +    }
> +
>      if (virNetworkDefUpdateCheckElementName(def, ctxt->node, "srv") < 0)
>          goto cleanup;
>
> @@ -3330,6 +3379,13 @@ virNetworkDefUpdateDNSTxt(virNetworkDef *def,
>          goto cleanup;
>      }
>
> +    if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_FIRST ||
> +        command == VIR_NETWORK_UPDATE_COMMAND_MODIFY_ADD_LAST) {
> +        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> +                       _("DNS TXT records cannot be modified, use add
> command instead of modify-or-add"));
> +        goto cleanup;
> +    }
> +
>      if (virNetworkDefUpdateCheckElementName(def, ctxt->node, "txt") < 0)
>          goto cleanup;
>
> diff --git a/tools/virsh-network.c b/tools/virsh-network.c
> index 597e3d4530..d7dc5df5a8 100644
> --- a/tools/virsh-network.c
> +++ b/tools/virsh-network.c
> @@ -1231,7 +1231,8 @@ 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 +1261,8 @@ 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.42.1
>
>

-- 
Abhiram