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

[Martin Kletzander]

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 at 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
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20220413/f97c8562/attachment.sig>