Re: [PATCH] virDomainGraphicsListenDefParseXML: validate attribute 'network' for listen type 'network'
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(a)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
Attachments:
- signature.asc (application/pgp-signature — 833 bytes)