[PATCH] Revert "network: allow "modify" option for DNS-Srv records"

Adam Julis posted 1 patch 3 months, 2 weeks ago
src/conf/network_conf.c                       | 27 +++++--------------
.../srv-not-existing.xml                      |  1 -
.../srv-record-modify-few.xml                 |  1 -
.../nat-network-dns-srv-modify-few.xml        | 26 ------------------
tests/networkxml2xmlupdatetest.c              | 10 +------
5 files changed, 7 insertions(+), 58 deletions(-)
delete mode 100644 tests/networkxml2xmlupdatein/srv-not-existing.xml
delete mode 100644 tests/networkxml2xmlupdatein/srv-record-modify-few.xml
delete mode 100644 tests/networkxml2xmlupdateout/nat-network-dns-srv-modify-few.xml
[PATCH] Revert "network: allow "modify" option for DNS-Srv records"
Posted by Adam Julis 3 months, 2 weeks ago
This reverts commit cf934c87cca32149675020ea595712aad25978e6.

The matching logic is flawed and it would complicate support of
this command.

Signed-off-by: Adam Julis <ajulis@redhat.com>
---
See discussion:
https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/message/6VV6ZAFNGUYUIQZGNS3PZ2FN64NPHXBT/

 src/conf/network_conf.c                       | 27 +++++--------------
 .../srv-not-existing.xml                      |  1 -
 .../srv-record-modify-few.xml                 |  1 -
 .../nat-network-dns-srv-modify-few.xml        | 26 ------------------
 tests/networkxml2xmlupdatetest.c              | 10 +------
 5 files changed, 7 insertions(+), 58 deletions(-)
 delete mode 100644 tests/networkxml2xmlupdatein/srv-not-existing.xml
 delete mode 100644 tests/networkxml2xmlupdatein/srv-record-modify-few.xml
 delete mode 100644 tests/networkxml2xmlupdateout/nat-network-dns-srv-modify-few.xml

diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 68eee367c4..3af4e1d036 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -3257,6 +3257,12 @@ virNetworkDefUpdateDNSSrv(virNetworkDef *def,
                   command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST);
     int foundCt = 0;
 
+    if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY) {
+        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+                       _("DNS SRV records cannot be modified, only added or deleted"));
+        goto cleanup;
+    }
+
     if (virNetworkDefUpdateCheckElementName(def, ctxt->node, "srv") < 0)
         goto cleanup;
 
@@ -3306,27 +3312,6 @@ virNetworkDefUpdateDNSSrv(virNetworkDef *def,
         virNetworkDNSSrvDefClear(&dns->srvs[foundIdx]);
         VIR_DELETE_ELEMENT(dns->srvs, foundIdx, dns->nsrvs);
 
-    } else if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY) {
-
-        if (foundCt == 0) {
-            virReportError(VIR_ERR_OPERATION_INVALID,
-                           _("couldn't locate a matching DNS SRV record in network %1$s"),
-                           def->name);
-            goto cleanup;
-        }
-
-        if (foundCt > 1) {
-            virReportError(VIR_ERR_OPERATION_INVALID,
-                           _("multiple DNS SRV records matching all specified fields were found in network %1$s"),
-                           def->name);
-            goto cleanup;
-        }
-
-        virNetworkDNSSrvDefClear(&dns->srvs[foundIdx]);
-
-        memcpy(&dns->srvs[foundIdx], &srv, sizeof(virNetworkDNSSrvDef));
-        memset(&srv, 0, sizeof(virNetworkDNSSrvDef));
-
     } else {
         virNetworkDefUpdateUnknownCommand(command);
         goto cleanup;
diff --git a/tests/networkxml2xmlupdatein/srv-not-existing.xml b/tests/networkxml2xmlupdatein/srv-not-existing.xml
deleted file mode 100644
index 401e14c616..0000000000
--- a/tests/networkxml2xmlupdatein/srv-not-existing.xml
+++ /dev/null
@@ -1 +0,0 @@
-<srv service='name' protocol='tcp' domain='unknown-domain' target='.' port='666' priority='99' weight='10'/>
diff --git a/tests/networkxml2xmlupdatein/srv-record-modify-few.xml b/tests/networkxml2xmlupdatein/srv-record-modify-few.xml
deleted file mode 100644
index 88ec1b97d9..0000000000
--- a/tests/networkxml2xmlupdatein/srv-record-modify-few.xml
+++ /dev/null
@@ -1 +0,0 @@
-<srv service='name' protocol='tcp' domain='test-domain-name' target='.' port='1221' priority='42' weight='69'/>
diff --git a/tests/networkxml2xmlupdateout/nat-network-dns-srv-modify-few.xml b/tests/networkxml2xmlupdateout/nat-network-dns-srv-modify-few.xml
deleted file mode 100644
index a7e5fcffa6..0000000000
--- a/tests/networkxml2xmlupdateout/nat-network-dns-srv-modify-few.xml
+++ /dev/null
@@ -1,26 +0,0 @@
-<network>
-  <name>default</name>
-  <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid>
-  <forward dev='eth1' mode='nat'>
-    <interface dev='eth1'/>
-  </forward>
-  <bridge name='virbr0' stp='on' delay='0'/>
-  <dns>
-    <srv service='name' protocol='tcp' domain='test-domain-name' target='.' port='1221' priority='42' weight='69'/>
-  </dns>
-  <ip address='192.168.122.1' netmask='255.255.255.0'>
-    <dhcp>
-      <range start='192.168.122.2' end='192.168.122.254'/>
-      <host mac='00:16:3e:77:e2:ed' name='a.example.com' ip='192.168.122.10'/>
-      <host mac='00:16:3e:3e:a9:1a' name='b.example.com' ip='192.168.122.11'/>
-    </dhcp>
-  </ip>
-  <ip family='ipv4' address='192.168.123.1' netmask='255.255.255.0'>
-  </ip>
-  <ip family='ipv6' address='2001:db8:ac10:fe01::1' prefix='64'>
-  </ip>
-  <ip family='ipv6' address='2001:db8:ac10:fd01::1' prefix='64'>
-  </ip>
-  <ip family='ipv4' address='10.24.10.1'>
-  </ip>
-</network>
diff --git a/tests/networkxml2xmlupdatetest.c b/tests/networkxml2xmlupdatetest.c
index 875cede035..60931a2eba 100644
--- a/tests/networkxml2xmlupdatetest.c
+++ b/tests/networkxml2xmlupdatetest.c
@@ -337,6 +337,7 @@ mymain(void)
             "nat-network-dns-srv-record",
             "nat-network-dns-srv-records",
             VIR_NETWORK_UPDATE_COMMAND_ADD_LAST);
+
     DO_TEST_FAIL("delete-missing-srv-record-service",
                  "srv-record-service",
                  "nat-network",
@@ -359,15 +360,6 @@ mymain(void)
             "nat-network-dns-srv-record",
             "nat-network",
             VIR_NETWORK_UPDATE_COMMAND_DELETE);
-    DO_TEST("modify-srv-record-protocol",
-            "srv-record-modify-few",
-            "nat-network-dns-srv-record",
-            "nat-network-dns-srv-modify-few",
-            VIR_NETWORK_UPDATE_COMMAND_MODIFY);
-    DO_TEST_FAIL("modify-not-existing-srv-record",
-                 "srv-not-existing",
-                 "nat-network-dns-srv-record",
-                 VIR_NETWORK_UPDATE_COMMAND_MODIFY);
 
 
     return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
-- 
2.45.2
Re: [PATCH] Revert "network: allow "modify" option for DNS-Srv records"
Posted by Martin Kletzander 3 months, 2 weeks ago
On Mon, Aug 05, 2024 at 10:50:43AM +0200, Adam Julis wrote:
>This reverts commit cf934c87cca32149675020ea595712aad25978e6.
>
>The matching logic is flawed and it would complicate support of
>this command.
>
>Signed-off-by: Adam Julis <ajulis@redhat.com>

Thanks.

Reviewed-by: Martin Kletzander <mkletzan@redhat.com>