10:43 a.m.
On 4/6/23 18:23, K Shiva wrote:
From: K Shiva Kiran <shiva_kr(a)riseup.net>
In an effort to separate the validation steps from the Parse stage,
a few validation checks of virDomainGraphicsListenDef have been moved from
virDomainGraphicsListenDefParseXML() in domain_conf.c to
virDomainGraphicsDefListensValidate() in domain_validate.c
Signed-off-by: K Shiva <shiva_kr(a)riseup.net>
---
This is a v4 of:
https://listman.redhat.com/archives/libvir-list/2023-April/239286.html
This commit has been squashed with v3.
diff to v3:
- Dropped virDomainGraphicsListenDefValidate() as per suggestion
- Moved the above's code to virDomainGraphicsDefListensValidate()
- Moved an existing if() condition in virDomainGraphicsDefListensValidate()
into the switch() statement of the freshly moved code
src/conf/domain_conf.c | 30 +-----------------------------
src/conf/domain_validate.c | 37 +++++++++++++++++++++++++++++++------
2 files changed, 32 insertions(+), 35 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..988f0854a5 100644
--- a/src/conf/domain_validate.c
+++ b/src/conf/domain_validate.c
@@ -2631,14 +2631,39 @@ static int
virDomainGraphicsDefListensValidate(const virDomainGraphicsDef *def)
{
size_t i;
+ const char *graphicsType = virDomainGraphicsTypeToString(def->type);
for (i = 0; i < def->nListens; i++) {
- if (def->listens[i].type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK
&&
- !def->listens[i].network) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("'network' attribute is required for "
- "listen type 'network'"));
- return -1;
+ switch (def->listens[i].type) {
+ case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK:
+ if (!def->listens[i].network) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("'network' attribute is required for
"
+ "listen type 'network'"));
We can take this opportunity and move this error message onto a single line.
Error messages are exempt from the 80 chars long line rule. The reason
is that git-grepping the message is as easy as:
git grep "any substring of error message"
but if error message is split like this onto multiple lines one can only
guess how it is split.
git grep "is required for listen"
Fixed and pushed. Congratulations on your first libvirt contribution!
Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com>
Michal