[PATCH] virDomainGraphicsListenDefParseXML: validate attribute 'network' for listen type 'network'

Amneesh Singh posted 1 patch 2 years ago
Test syntax-check passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20220412131024.100841-1-natto@weirdnatto.in
src/conf/domain_conf.c | 7 +++++++
1 file changed, 7 insertions(+)
[PATCH] virDomainGraphicsListenDefParseXML: validate attribute 'network' for listen type 'network'
Posted by Amneesh Singh 2 years ago
Related: https://gitlab.com/libvirt/libvirt/-/issues/93
Signed-off-by: Amneesh Singh <natto@weirdnatto.in>
---
 src/conf/domain_conf.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index bd28840..f1651e3 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12268,6 +12268,13 @@ virDomainGraphicsListenDefParseXML(virDomainGraphicsListenDef *def,
             goto error;
         }
         def->network = g_steal_pointer(&network);
+    } else {
+        if (def->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK) {
+            virReportError(VIR_ERR_XML_ERROR, "%s",
+                           _("'network' attribute is required for listen "
+                             "type 'network'"));
+            goto error;
+        }
     }
 
     if (socketPath && socketPath[0]) {
-- 
2.35.1
Re: [PATCH] virDomainGraphicsListenDefParseXML: validate attribute 'network' for listen type 'network'
Posted by Martin Kletzander 2 years ago
On Tue, Apr 12, 2022 at 06:40:24PM +0530, Amneesh Singh wrote:
>Related: https://gitlab.com/libvirt/libvirt/-/issues/93
>Signed-off-by: Amneesh Singh <natto@weirdnatto.in>
>---
> src/conf/domain_conf.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
>diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>index bd28840..f1651e3 100644
>--- a/src/conf/domain_conf.c
>+++ b/src/conf/domain_conf.c
>@@ -12268,6 +12268,13 @@ virDomainGraphicsListenDefParseXML(virDomainGraphicsListenDef *def,
>             goto error;
>         }
>         def->network = g_steal_pointer(&network);
>+    } else {
>+        if (def->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK) {
>+            virReportError(VIR_ERR_XML_ERROR, "%s",
>+                           _("'network' attribute is required for listen "
>+                             "type 'network'"));
>+            goto error;

Not bad, but since this function is called when loading existing domains
on daemons startup, then this error would cause that domain config not
to be loaded and it would disappear from the list of domains.

What's more, not all <graphics type='...'> support <listen
type='network'/>, so requiring that unconditionally would not be very
user friendly.

For validations like this we have virDomainDefValidate() which actually
does avoid exactly this scenario (see the comment of the function).  And
you even say "validate" in the subject ;)

If all hypervisors support <listen type='network'/>, then you can
validate it inside virDomainDefValidateInternal(), probably in a new
function.

>+        }
>     }
>
>     if (socketPath && socketPath[0]) {
>-- 
>2.35.1
>