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

Adam Julis posted 1 patch 3 months, 1 week ago
Failed in applying to current master (apply log)
src/conf/network_conf.c                       | 18 ++++++++-----
.../dns-txt-record-modify-fail.xml            |  1 +
.../dns-txt-record-modify-success.xml         |  1 +
.../nat-network-dns-txt-modify-ok.xml         | 26 +++++++++++++++++++
tests/networkxml2xmlupdatetest.c              |  9 +++++++
5 files changed, 49 insertions(+), 6 deletions(-)
create mode 100644 tests/networkxml2xmlupdatein/dns-txt-record-modify-fail.xml
create mode 100644 tests/networkxml2xmlupdatein/dns-txt-record-modify-success.xml
create mode 100644 tests/networkxml2xmlupdateout/nat-network-dns-txt-modify-ok.xml
[PATCH] network: allow "modify" option for DNS-Txt records
Posted by Adam Julis 3 months, 1 week ago
The "modify" command allows to replace an existing record (its
text value). The primary key is the name of the record. If
duplicity or missing record detected, throw error.

Tests in networkxml2xmlupdatetest.c contain replacements of an
existing DNS-text record and failure due to non-existing record.

Resolves: https://gitlab.com/libvirt/libvirt/-/issues/639
Signed-off-by: Adam Julis <ajulis@redhat.com>
---
 src/conf/network_conf.c                       | 18 ++++++++-----
 .../dns-txt-record-modify-fail.xml            |  1 +
 .../dns-txt-record-modify-success.xml         |  1 +
 .../nat-network-dns-txt-modify-ok.xml         | 26 +++++++++++++++++++
 tests/networkxml2xmlupdatetest.c              |  9 +++++++
 5 files changed, 49 insertions(+), 6 deletions(-)
 create mode 100644 tests/networkxml2xmlupdatein/dns-txt-record-modify-fail.xml
 create mode 100644 tests/networkxml2xmlupdatein/dns-txt-record-modify-success.xml
 create mode 100644 tests/networkxml2xmlupdateout/nat-network-dns-txt-modify-ok.xml

diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index fc387f9566..dd362b6ab2 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -3351,12 +3351,6 @@ virNetworkDefUpdateDNSTxt(virNetworkDef *def,
     bool isAdd = (command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST ||
                   command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST);
 
-    if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY) {
-        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
-                       _("DNS TXT records cannot be modified, only added or deleted"));
-        goto cleanup;
-    }
-
     if (virNetworkDefUpdateCheckElementName(def, ctxt->node, "txt") < 0)
         goto cleanup;
 
@@ -3395,6 +3389,18 @@ virNetworkDefUpdateDNSTxt(virNetworkDef *def,
         virNetworkDNSTxtDefClear(&dns->txts[foundIdx]);
         VIR_DELETE_ELEMENT(dns->txts, foundIdx, dns->ntxts);
 
+    } else if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY) {
+
+        if (foundIdx == dns->ntxts) {
+            virReportError(VIR_ERR_OPERATION_INVALID,
+                           _("couldn't locate a matching DNS TXT record in network %1$s"),
+                           def->name);
+            goto cleanup;
+        }
+
+        VIR_FREE(dns->txts[foundIdx].value);
+        dns->txts[foundIdx].value = g_strdup(txt.value);
+
     } else {
         virNetworkDefUpdateUnknownCommand(command);
         goto cleanup;
diff --git a/tests/networkxml2xmlupdatein/dns-txt-record-modify-fail.xml b/tests/networkxml2xmlupdatein/dns-txt-record-modify-fail.xml
new file mode 100644
index 0000000000..75ed475fe1
--- /dev/null
+++ b/tests/networkxml2xmlupdatein/dns-txt-record-modify-fail.xml
@@ -0,0 +1 @@
+<txt name='notexisted' value='modified example'/>
diff --git a/tests/networkxml2xmlupdatein/dns-txt-record-modify-success.xml b/tests/networkxml2xmlupdatein/dns-txt-record-modify-success.xml
new file mode 100644
index 0000000000..e16c352253
--- /dev/null
+++ b/tests/networkxml2xmlupdatein/dns-txt-record-modify-success.xml
@@ -0,0 +1 @@
+<txt name='example' value='modified example'/>
diff --git a/tests/networkxml2xmlupdateout/nat-network-dns-txt-modify-ok.xml b/tests/networkxml2xmlupdateout/nat-network-dns-txt-modify-ok.xml
new file mode 100644
index 0000000000..4b4dda094a
--- /dev/null
+++ b/tests/networkxml2xmlupdateout/nat-network-dns-txt-modify-ok.xml
@@ -0,0 +1,26 @@
+<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>
+    <txt name='example' value='modified example'/>
+  </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 59e6ce98e5..875cede035 100644
--- a/tests/networkxml2xmlupdatetest.c
+++ b/tests/networkxml2xmlupdatetest.c
@@ -306,6 +306,15 @@ mymain(void)
                  "dns-txt-record-snowman",
                  "nat-network-dns-txt-record",
                  VIR_NETWORK_UPDATE_COMMAND_DELETE);
+    DO_TEST("modify-dns-txt-record",
+            "dns-txt-record-modify-success",
+            "nat-network-dns-txt-record",
+            "nat-network-dns-txt-modify-ok",
+            VIR_NETWORK_UPDATE_COMMAND_MODIFY);
+    DO_TEST_FAIL("modify-missing-dns-txt-record",
+            "dns-txt-record-modify-fail",
+            "nat-network-dns-txt-record",
+            VIR_NETWORK_UPDATE_COMMAND_MODIFY);
 
 
     section = VIR_NETWORK_SECTION_DNS_SRV;
-- 
2.45.2
Re: [PATCH] network: allow "modify" option for DNS-Txt records
Posted by Michal Prívozník 3 months, 1 week ago
On 7/9/24 17:23, Adam Julis wrote:
> The "modify" command allows to replace an existing record (its
> text value). The primary key is the name of the record. If
> duplicity or missing record detected, throw error.
> 
> Tests in networkxml2xmlupdatetest.c contain replacements of an
> existing DNS-text record and failure due to non-existing record.
> 
> Resolves: https://gitlab.com/libvirt/libvirt/-/issues/639
> Signed-off-by: Adam Julis <ajulis@redhat.com>
> ---
>  src/conf/network_conf.c                       | 18 ++++++++-----
>  .../dns-txt-record-modify-fail.xml            |  1 +
>  .../dns-txt-record-modify-success.xml         |  1 +
>  .../nat-network-dns-txt-modify-ok.xml         | 26 +++++++++++++++++++
>  tests/networkxml2xmlupdatetest.c              |  9 +++++++
>  5 files changed, 49 insertions(+), 6 deletions(-)
>  create mode 100644 tests/networkxml2xmlupdatein/dns-txt-record-modify-fail.xml
>  create mode 100644 tests/networkxml2xmlupdatein/dns-txt-record-modify-success.xml
>  create mode 100644 tests/networkxml2xmlupdateout/nat-network-dns-txt-modify-ok.xml
> 
> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
> index fc387f9566..dd362b6ab2 100644
> --- a/src/conf/network_conf.c
> +++ b/src/conf/network_conf.c
> @@ -3351,12 +3351,6 @@ virNetworkDefUpdateDNSTxt(virNetworkDef *def,
>      bool isAdd = (command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST ||
>                    command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST);
>  
> -    if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY) {
> -        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
> -                       _("DNS TXT records cannot be modified, only added or deleted"));
> -        goto cleanup;
> -    }
> -
>      if (virNetworkDefUpdateCheckElementName(def, ctxt->node, "txt") < 0)
>          goto cleanup;
>  
> @@ -3395,6 +3389,18 @@ virNetworkDefUpdateDNSTxt(virNetworkDef *def,
>          virNetworkDNSTxtDefClear(&dns->txts[foundIdx]);
>          VIR_DELETE_ELEMENT(dns->txts, foundIdx, dns->ntxts);
>  
> +    } else if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY) {
> +
> +        if (foundIdx == dns->ntxts) {
> +            virReportError(VIR_ERR_OPERATION_INVALID,
> +                           _("couldn't locate a matching DNS TXT record in network %1$s"),
> +                           def->name);
> +            goto cleanup;
> +        }
> +
> +        VIR_FREE(dns->txts[foundIdx].value);
> +        dns->txts[foundIdx].value = g_strdup(txt.value);

Instead of strdup()-ing the string, you may as well g_steal_pointer() it.

> +
>      } else {
>          virNetworkDefUpdateUnknownCommand(command);
>          goto cleanup;

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Michal