https://bugzilla.redhat.com/show_bug.cgi?id=2171384

Signed-off-by: Ján Tomko <jtomko@redhat.com>
---
 src/conf/domain_validate.c | 28 ++++++++++++++++++++--------
 src/conf/domain_validate.h |  2 ++
 src/libvirt_private.syms   |  1 +
 3 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
index 80d6a2ffd9..4c76eb7f46 100644
--- a/src/conf/domain_validate.c
+++ b/src/conf/domain_validate.c
@@ -1735,8 +1735,9 @@ virDomainDefIOMMUValidate(const virDomainDef *def)
 }
 
 
-static int
-virDomainDefFSValidate(const virDomainDef *def)
+int
+virDomainDefFSValidate(const virDomainDef *def,
+                       const virDomainFSDef *newfs)
 {
     size_t i;
     g_autoptr(GHashTable) dsts = virHashNew(NULL);
@@ -1754,8 +1755,18 @@ virDomainDefFSValidate(const virDomainDef *def)
             return -1;
         }
 
-        if (virHashAddEntry(dsts, fs->dst, (void *) 0x1) < 0)
+        if (virHashAddEntry(dsts, fs->dst, (void *) fs) < 0)
+            return -1;
+    }
+
+    if (newfs) {
+        const virDomainFSDef *fs = g_hash_table_lookup(dsts, newfs->dst);
+        if (fs && fs != newfs) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("filesystem target '%1$s' specified twice"),
+                           newfs->dst);
             return -1;
+        }
     }
 
     return 0;
@@ -1856,9 +1867,6 @@ virDomainDefValidateInternal(const virDomainDef *def,
     if (virDomainNumaDefValidate(def->numa) < 0)
         return -1;
 
-    if (virDomainDefFSValidate(def) < 0)
-        return -1;
-
     if (virDomainDefValidateIOThreads(def) < 0)
         return -1;
 
@@ -2573,7 +2581,8 @@ virDomainShmemDefValidate(const virDomainShmemDef *shmem)
 }
 
 static int
-virDomainFSDefValidate(const virDomainFSDef *fs)
+virDomainFSDefValidate(const virDomainDef *def,
+                       const virDomainFSDef *fs)
 {
     if (fs->dst == NULL) {
         const char *source = fs->src->path;
@@ -2592,6 +2601,9 @@ virDomainFSDefValidate(const virDomainFSDef *fs)
         return -1;
     }
 
+    if (virDomainDefFSValidate(def, fs) < 0)
+        return -1;
+
     return 0;
 }
 
@@ -2885,7 +2897,7 @@ virDomainDeviceDefValidateInternal(const virDomainDeviceDef *dev,
         return virDomainShmemDefValidate(dev->data.shmem);
 
     case VIR_DOMAIN_DEVICE_FS:
-        return virDomainFSDefValidate(dev->data.fs);
+        return virDomainFSDefValidate(def, dev->data.fs);
 
     case VIR_DOMAIN_DEVICE_AUDIO:
         return virDomainAudioDefValidate(def, dev->data.audio);
diff --git a/src/conf/domain_validate.h b/src/conf/domain_validate.h
index fc441cef5b..437cbe4d2e 100644
--- a/src/conf/domain_validate.h
+++ b/src/conf/domain_validate.h
@@ -47,3 +47,5 @@ int virDomainDiskDefSourceLUNValidate(const virStorageSource *src);
 
 int virDomainDefOSValidate(const virDomainDef *def,
                            virDomainXMLOption *xmlopt);
+int virDomainDefFSValidate(const virDomainDef *def,
+                           const virDomainFSDef *newfs);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 436d5a0770..65089c1aaa 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -793,6 +793,7 @@ virDomainDefPostParse;
 
 # conf/domain_validate.h
 virDomainActualNetDefValidate;
+virDomainDefFSValidate;
 virDomainDefOSValidate;
 virDomainDefValidate;
 virDomainDeviceValidateAliasForHotplug;
-- 
2.40.1

On Wed, Jun 07, 2023 at 05:43:37PM +0200, Ján Tomko wrote:
>https://bugzilla.redhat.com/show_bug.cgi?id=2171384
>
>Signed-off-by: Ján Tomko <jtomko@redhat.com>
>---
> src/conf/domain_validate.c | 28 ++++++++++++++++++++--------
> src/conf/domain_validate.h |  2 ++
> src/libvirt_private.syms   |  1 +
> 3 files changed, 23 insertions(+), 8 deletions(-)
>
>diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
>index 80d6a2ffd9..4c76eb7f46 100644
>--- a/src/conf/domain_validate.c
>+++ b/src/conf/domain_validate.c
>@@ -1735,8 +1735,9 @@ virDomainDefIOMMUValidate(const virDomainDef *def)
> }
>
>
>-static int
>-virDomainDefFSValidate(const virDomainDef *def)
>+int
>+virDomainDefFSValidate(const virDomainDef *def,
>+                       const virDomainFSDef *newfs)
> {
>     size_t i;
>     g_autoptr(GHashTable) dsts = virHashNew(NULL);
>@@ -1754,8 +1755,18 @@ virDomainDefFSValidate(const virDomainDef *def)
>             return -1;
>         }
>
>-        if (virHashAddEntry(dsts, fs->dst, (void *) 0x1) < 0)
>+        if (virHashAddEntry(dsts, fs->dst, (void *) fs) < 0)
>+            return -1;
>+    }
>+
>+    if (newfs) {
>+        const virDomainFSDef *fs = g_hash_table_lookup(dsts, newfs->dst);
>+        if (fs && fs != newfs) {
>+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>+                           _("filesystem target '%1$s' specified twice"),
>+                           newfs->dst);
>             return -1;
>+        }
>     }
>
>     return 0;
>@@ -1856,9 +1867,6 @@ virDomainDefValidateInternal(const virDomainDef *def,
>     if (virDomainNumaDefValidate(def->numa) < 0)
>         return -1;
>
>-    if (virDomainDefFSValidate(def) < 0)
>-        return -1;
>-
>     if (virDomainDefValidateIOThreads(def) < 0)
>         return -1;
>
>@@ -2573,7 +2581,8 @@ virDomainShmemDefValidate(const virDomainShmemDef *shmem)
> }
>
> static int
>-virDomainFSDefValidate(const virDomainFSDef *fs)
>+virDomainFSDefValidate(const virDomainDef *def,
>+                       const virDomainFSDef *fs)
> {
>     if (fs->dst == NULL) {
>         const char *source = fs->src->path;
>@@ -2592,6 +2601,9 @@ virDomainFSDefValidate(const virDomainFSDef *fs)
>         return -1;
>     }
>
>+    if (virDomainDefFSValidate(def, fs) < 0)
>+        return -1;
>+

Why not just open-code it here?  The function is not called from
anywhere else, has the same parameters and it's really confusing to see
what's the difference between virDomainFSDefValidate and
virDomainDefFSValidate to be honest.

With that fixed up:

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