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@xxxxxxxxxx> 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
