src/conf/domain_conf.c | 30 +----------------------------- src/conf/domain_validate.c | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 29 deletions(-)
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>
---
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;
}
return 0;
--
2.40.0On 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
> 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 Greetings Sir Let's go with option b), as it reduces the number of functions, moreover the function virDomainGraphicsListenDefValidate() has a pretty similar name to it's caller virDomainGraphicsDefListensValidate() and could be kindof annoying. Do I apply your suggestion and resend the patch? Thank You Shiva
On 4/6/23 20:43, Shiva wrote: >> 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 > > Greetings Sir > > Let's go with option b), as it reduces the number of functions, > moreover the function virDomainGraphicsListenDefValidate() has a > pretty similar name to it's caller > virDomainGraphicsDefListensValidate() and could be kindof annoying. > > Do I apply your suggestion and resend the patch? > > Thank You > > Shiva > My apologies for cluttering the mailing list. I have applied your suggestions and have sent a v4 Patch, signed with my full name. I have also squashed v4 with v3. Thank you Shiva
© 2016 - 2026 Red Hat, Inc.