On 4/6/23 14:51, K Shiva wrote:

In an effort to separate the validation steps from the Parse stage, a part of the validation of virDomainGraphicsListenDef has been moved to domain_validate.h.

Signed-off-by: K Shiva <shiva_kr@riseup.net>

Sorry for not raising this earlier, but we tend to use legal names here.

--- This is a v3 of: https://listman.redhat.com/archives/libvir-list/2023-April/239265.html

diff to v2: - Cleaned patch as to be directly applied onto master branch

src/conf/domain_conf.c | 30 +----------------------------- src/conf/domain_validate.c | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 29 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 70e4d52ee6..8df751cc46 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10986,7 +10986,6 @@ virDomainGraphicsAuthDefParseXML(xmlNodePtr node, /** * virDomainGraphicsListenDefParseXML: * @def: listen def pointer to be filled - * @graphics: graphics def pointer * @node: xml node of <listen/> element * @parent: xml node of <graphics/> element * @flags: bit-wise or of VIR_DOMAIN_DEF_PARSE_* @@ -10998,13 +10997,11 @@ virDomainGraphicsAuthDefParseXML(xmlNodePtr node, */ static int virDomainGraphicsListenDefParseXML(virDomainGraphicsListenDef *def, - virDomainGraphicsDef *graphics, xmlNodePtr node, xmlNodePtr parent, unsigned int flags) { int ret = -1; - const char *graphicsType = virDomainGraphicsTypeToString(graphics->type); g_autofree char *address = virXMLPropString(node, "address"); g_autofree char *network = virXMLPropString(node, "network"); g_autofree char *socketPath = virXMLPropString(node, "socket"); @@ -11021,31 +11018,6 @@ virDomainGraphicsListenDefParseXML(virDomainGraphicsListenDef *def, VIR_XML_PROP_REQUIRED, &def->type) < 0) goto error;

- switch (def->type) { - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET: - if (graphics->type != VIR_DOMAIN_GRAPHICS_TYPE_VNC && - graphics->type != VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("listen type 'socket' is not available for graphics type '%1$s'"), - graphicsType); - goto error; - } - break; - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE: - if (graphics->type != VIR_DOMAIN_GRAPHICS_TYPE_SPICE && - graphics->type != VIR_DOMAIN_GRAPHICS_TYPE_VNC) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("listen type 'none' is not available for graphics type '%1$s'"), - graphicsType); - goto error; - } - break; - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: - case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_LAST: - break; - } - if (def->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS) { if (address && addressCompat && STRNEQ(address, addressCompat)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -11148,7 +11120,7 @@ virDomainGraphicsListensParseXML(virDomainGraphicsDef *def, def->listens = g_new0(virDomainGraphicsListenDef, nListens);

for (i = 0; i < nListens; i++) { - if (virDomainGraphicsListenDefParseXML(&def->listens[i], def, + if (virDomainGraphicsListenDefParseXML(&def->listens[i], listenNodes[i], i == 0 ? node : NULL, flags) < 0) diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index 9265fef4f9..9a1a8ee156 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -2627,6 +2627,39 @@ virDomainAudioDefValidate(const virDomainDef *def, return 0; }

+static int +virDomainGraphicsListenDefValidate(const virDomainGraphicsListenDef *def, + const virDomainGraphicsDef *graphics) +{ + const char *graphicsType = virDomainGraphicsTypeToString(graphics->type); + + switch (def->type) { + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET: + if (graphics->type != VIR_DOMAIN_GRAPHICS_TYPE_VNC && + graphics->type != VIR_DOMAIN_GRAPHICS_TYPE_SPICE) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("listen type 'socket' is not available for graphics type '%1$s'"), + graphicsType); + return -1; + } + break; + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE: + if (graphics->type != VIR_DOMAIN_GRAPHICS_TYPE_SPICE && + graphics->type != VIR_DOMAIN_GRAPHICS_TYPE_VNC) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("listen type 'none' is not available for graphics type '%1$s'"), + graphicsType); + return -1; + } + break; + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: + case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_LAST: + break; + } + return 0; +} + static int virDomainGraphicsDefListensValidate(const virDomainGraphicsDef *def) { @@ -2640,6 +2673,8 @@ virDomainGraphicsDefListensValidate(const virDomainGraphicsDef *def) "listen type 'network'")); return -1; } + if (virDomainGraphicsListenDefValidate(&def->listens[i], def) < 0) + return -1;

Now, earlier in this loop (not visible in this limited context though) there's a check for VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK type. I don't think it is special so we have two options: a) move it into virDomainGraphicsListenDefValidate(), or b) move those checks out of virDomainGraphicsListenDefValidate() right into this loop; rendering virDomainGraphicsListenDefValidate() to be an empty function which can then be dropped. I don't have any preference, do you? Michal