Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
---
 src/conf/domain_conf.c | 251 ++++++++++-------------------------------
 1 file changed, 59 insertions(+), 192 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9d98f487ea..822426bc4e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12905,49 +12905,30 @@ virDomainGraphicsDefParseXMLSpice(virDomainGraphicsDef *def,
                                   unsigned int flags)
 {
     xmlNodePtr cur;
-    int defaultModeVal;
-    g_autofree char *port = virXMLPropString(node, "port");
-    g_autofree char *tlsPort = virXMLPropString(node, "tlsPort");
     g_autofree char *autoport = virXMLPropString(node, "autoport");
-    g_autofree char *defaultMode = virXMLPropString(node, "defaultMode");
 
     if (virDomainGraphicsListensParseXML(def, node, ctxt, flags) < 0)
         return -1;
 
-    if (port) {
-        if (virStrToLong_i(port, NULL, 10, &def->data.spice.port) < 0) {
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("cannot parse spice port %s"), port);
-            return -1;
-        }
-    } else {
-        def->data.spice.port = 0;
-    }
+    def->data.spice.port = 0;
+    if (virXMLPropInt(node, "port", 10, VIR_XML_PROP_NONE,
+                      &def->data.spice.port) < 0)
+        return -1;
 
-    if (tlsPort) {
-        if (virStrToLong_i(tlsPort, NULL, 10, &def->data.spice.tlsPort) < 0) {
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("cannot parse spice tlsPort %s"), tlsPort);
-            return -1;
-        }
-    } else {
-        def->data.spice.tlsPort = 0;
-    }
+    def->data.spice.tlsPort = 0;
+    if (virXMLPropInt(node, "tlsPort", 10, VIR_XML_PROP_NONE,
+                      &def->data.spice.tlsPort) < 0)
+        return -1;
 
     if (STREQ_NULLABLE(autoport, "yes"))
         def->data.spice.autoport = true;
 
     def->data.spice.defaultMode = VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_ANY;
 
-    if (defaultMode) {
-        if ((defaultModeVal = virDomainGraphicsSpiceChannelModeTypeFromString(defaultMode)) < 0) {
-            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                           _("unknown default spice channel mode %s"),
-                           defaultMode);
-            return -1;
-        }
-        def->data.spice.defaultMode = defaultModeVal;
-    }
+    if (virXMLPropEnum(node, "defaultMode",
+                       virDomainGraphicsSpiceChannelModeTypeFromString,
+                       VIR_XML_PROP_NONE, &def->data.spice.defaultMode) < 0)
+        return -1;
 
     if (def->data.spice.port == -1 && def->data.spice.tlsPort == -1) {
         /* Legacy compat syntax, used -1 for auto-port */
@@ -12969,200 +12950,86 @@ virDomainGraphicsDefParseXMLSpice(virDomainGraphicsDef *def,
     while (cur != NULL) {
         if (cur->type == XML_ELEMENT_NODE) {
             if (virXMLNodeNameEqual(cur, "channel")) {
-                int nameval, modeval;
-                g_autofree char *name = NULL;
-                g_autofree char *mode = NULL;
-
-                name = virXMLPropString(cur, "name");
-                mode = virXMLPropString(cur, "mode");
+                virDomainGraphicsSpiceChannelName name;
+                virDomainGraphicsSpiceChannelMode mode;
 
-                if (!name || !mode) {
-                    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                                   _("spice channel missing name/mode"));
+                if (virXMLPropEnum(cur, "name",
+                                   virDomainGraphicsSpiceChannelNameTypeFromString,
+                                   VIR_XML_PROP_REQUIRED, &name) < 0)
                     return -1;
-                }
 
-                if ((nameval = virDomainGraphicsSpiceChannelNameTypeFromString(name)) < 0) {
-                    virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                                   _("unknown spice channel name %s"),
-                                   name);
-                    return -1;
-                }
-                if ((modeval = virDomainGraphicsSpiceChannelModeTypeFromString(mode)) < 0) {
-                    virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                                   _("unknown spice channel mode %s"),
-                                   mode);
+                if (virXMLPropEnum(cur, "mode",
+                                   virDomainGraphicsSpiceChannelModeTypeFromString,
+                                   VIR_XML_PROP_REQUIRED, &mode) < 0)
                     return -1;
-                }
 
-                def->data.spice.channels[nameval] = modeval;
+                def->data.spice.channels[name] = mode;
             } else if (virXMLNodeNameEqual(cur, "image")) {
-                int compressionVal;
-                g_autofree char *compression = virXMLPropString(cur, "compression");
+                virDomainGraphicsSpiceImageCompression compression;
 
-                if (!compression) {
-                    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                                   _("spice image missing compression"));
+                if (virXMLPropEnum(cur, "compression",
+                                   virDomainGraphicsSpiceImageCompressionTypeFromString,
+                                   VIR_XML_PROP_NONZERO, &compression) < 0)
                     return -1;
-                }
 
-                if ((compressionVal =
-                     virDomainGraphicsSpiceImageCompressionTypeFromString(compression)) <= 0) {
-                    virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                                   _("unknown spice image compression %s"),
-                                   compression);
-                    return -1;
-                }
-
-                def->data.spice.image = compressionVal;
+                def->data.spice.image = compression;
             } else if (virXMLNodeNameEqual(cur, "jpeg")) {
-                int compressionVal;
-                g_autofree char *compression = virXMLPropString(cur, "compression");
+                virDomainGraphicsSpiceJpegCompression compression;
 
-                if (!compression) {
-                    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                                   _("spice jpeg missing compression"));
+                if (virXMLPropEnum(cur, "compression",
+                                   virDomainGraphicsSpiceJpegCompressionTypeFromString,
+                                   VIR_XML_PROP_NONZERO, &compression) < 0)
                     return -1;
-                }
 
-                if ((compressionVal =
-                     virDomainGraphicsSpiceJpegCompressionTypeFromString(compression)) <= 0) {
-                    virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                                   _("unknown spice jpeg compression %s"),
-                                   compression);
-                    return -1;
-                }
-
-                def->data.spice.jpeg = compressionVal;
+                def->data.spice.jpeg = compression;
             } else if (virXMLNodeNameEqual(cur, "zlib")) {
-                int compressionVal;
-                g_autofree char *compression = virXMLPropString(cur, "compression");
-
-                if (!compression) {
-                    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                                   _("spice zlib missing compression"));
-                    return -1;
-                }
+                virDomainGraphicsSpiceZlibCompression compression;
 
-                if ((compressionVal =
-                     virDomainGraphicsSpiceZlibCompressionTypeFromString(compression)) <= 0) {
-                    virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                                   _("unknown spice zlib compression %s"),
-                                   compression);
+                if (virXMLPropEnum(cur, "compression",
+                                   virDomainGraphicsSpiceZlibCompressionTypeFromString,
+                                   VIR_XML_PROP_REQUIRED, &compression) < 0)
                     return -1;
-                }
 
-                def->data.spice.zlib = compressionVal;
+                def->data.spice.zlib = compression;
             } else if (virXMLNodeNameEqual(cur, "playback")) {
-                int compressionVal;
-                g_autofree char *compression = virXMLPropString(cur, "compression");
-
-                if (!compression) {
-                    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                                   _("spice playback missing compression"));
-                    return -1;
-                }
-
-                if ((compressionVal =
-                     virTristateSwitchTypeFromString(compression)) <= 0) {
-                    virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                                   _("unknown spice playback compression"));
+                if (virXMLPropTristateSwitch(cur, "compression",
+                                             VIR_XML_PROP_NONZERO,
+                                             &def->data.spice.playback) < 0)
                     return -1;
-                }
 
-                def->data.spice.playback = compressionVal;
             } else if (virXMLNodeNameEqual(cur, "streaming")) {
-                int modeVal;
-                g_autofree char *mode = virXMLPropString(cur, "mode");
+                virDomainGraphicsSpiceStreamingMode mode;
 
-                if (!mode) {
-                    virReportError(VIR_ERR_XML_ERROR, "%s",
-                                   _("spice streaming missing mode"));
-                    return -1;
-                }
-                if ((modeVal =
-                     virDomainGraphicsSpiceStreamingModeTypeFromString(mode)) <= 0) {
-                    virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                                   _("unknown spice streaming mode"));
+                if (virXMLPropEnum(cur, "mode",
+                                   virDomainGraphicsSpiceStreamingModeTypeFromString,
+                                   VIR_XML_PROP_REQUIRED | VIR_XML_PROP_NONZERO,
+                                   &mode) < 0)
                     return -1;
-                }
 
-                def->data.spice.streaming = modeVal;
+                def->data.spice.streaming = mode;
             } else if (virXMLNodeNameEqual(cur, "clipboard")) {
-                int copypasteVal;
-                g_autofree char *copypaste = virXMLPropString(cur, "copypaste");
-
-                if (!copypaste) {
-                    virReportError(VIR_ERR_XML_ERROR, "%s",
-                                   _("spice clipboard missing copypaste"));
+                if (virXMLPropTristateBool(cur, "copypaste",
+                                           VIR_XML_PROP_REQUIRED,
+                                           &def->data.spice.copypaste) < 0)
                     return -1;
-                }
-
-                if ((copypasteVal =
-                     virTristateBoolTypeFromString(copypaste)) <= 0) {
-                    virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                                   _("unknown copypaste value '%s'"), copypaste);
-                    return -1;
-                }
-
-                def->data.spice.copypaste = copypasteVal;
             } else if (virXMLNodeNameEqual(cur, "filetransfer")) {
-                int enableVal;
-                g_autofree char *enable = virXMLPropString(cur, "enable");
-
-                if (!enable) {
-                    virReportError(VIR_ERR_XML_ERROR, "%s",
-                                   _("spice filetransfer missing enable"));
+                if (virXMLPropTristateBool(cur, "enable",
+                                           VIR_XML_PROP_REQUIRED,
+                                           &def->data.spice.filetransfer) < 0)
                     return -1;
-                }
-
-                if ((enableVal =
-                     virTristateBoolTypeFromString(enable)) <= 0) {
-                    virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                                   _("unknown enable value '%s'"), enable);
-                    return -1;
-                }
-
-                def->data.spice.filetransfer = enableVal;
             } else if (virXMLNodeNameEqual(cur, "gl")) {
-                int enableVal;
-                g_autofree char *enable = virXMLPropString(cur, "enable");
-                g_autofree char *rendernode = virXMLPropString(cur, "rendernode");
+                def->data.spice.rendernode = virXMLPropString(cur, "rendernode");
 
-                if (!enable) {
-                    virReportError(VIR_ERR_XML_ERROR, "%s",
-                                   _("spice gl element missing enable"));
-                    return -1;
-                }
-
-                if ((enableVal =
-                     virTristateBoolTypeFromString(enable)) <= 0) {
-                    virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                                   _("unknown enable value '%s'"), enable);
+                if (virXMLPropTristateBool(cur, "enable",
+                                           VIR_XML_PROP_REQUIRED,
+                                           &def->data.spice.gl) < 0)
                     return -1;
-                }
-
-                def->data.spice.gl = enableVal;
-                def->data.spice.rendernode = g_steal_pointer(&rendernode);
-
             } else if (virXMLNodeNameEqual(cur, "mouse")) {
-                int modeVal;
-                g_autofree char *mode = virXMLPropString(cur, "mode");
-
-                if (!mode) {
-                    virReportError(VIR_ERR_XML_ERROR, "%s",
-                                   _("spice mouse missing mode"));
+                if (virXMLPropEnum(cur, "mode",
+                                   virDomainGraphicsSpiceMouseModeTypeFromString,
+                                   VIR_XML_PROP_REQUIRED | VIR_XML_PROP_NONZERO,
+                                   &def->data.spice.mousemode) < 0)
                     return -1;
-                }
-
-                if ((modeVal = virDomainGraphicsSpiceMouseModeTypeFromString(mode)) <= 0) {
-                    virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                                   _("unknown mouse mode value '%s'"),
-                                   mode);
-                    return -1;
-                }
-
-                def->data.spice.mousemode = modeVal;
             }
         }
         cur = cur->next;
-- 
2.26.3

On a Tuesday in 2021, Tim Wiederhake wrote:
>Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
>---
> src/conf/domain_conf.c | 251 ++++++++++-------------------------------
> 1 file changed, 59 insertions(+), 192 deletions(-)
>
>diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>index 9d98f487ea..822426bc4e 100644
>--- a/src/conf/domain_conf.c
>+++ b/src/conf/domain_conf.c
>@@ -12969,200 +12950,86 @@ virDomainGraphicsDefParseXMLSpice(virDomainGraphicsDef *def,

[...]

>             } else if (virXMLNodeNameEqual(cur, "image")) {
>-                int compressionVal;
>-                g_autofree char *compression = virXMLPropString(cur, "compression");
>+                virDomainGraphicsSpiceImageCompression compression;
>
>-                if (!compression) {
>-                    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>-                                   _("spice image missing compression"));

Originally we errored out if the element was missing.

>+                if (virXMLPropEnum(cur, "compression",
>+                                   virDomainGraphicsSpiceImageCompressionTypeFromString,
>+                                   VIR_XML_PROP_NONZERO, &compression) < 0)

This needs VIR_XML_PROP_REQUIRED

>                     return -1;
>-                }
>
>-                if ((compressionVal =
>-                     virDomainGraphicsSpiceImageCompressionTypeFromString(compression)) <= 0) {
>-                    virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>-                                   _("unknown spice image compression %s"),
>-                                   compression);
>-                    return -1;
>-                }
>-
>-                def->data.spice.image = compressionVal;
>+                def->data.spice.image = compression;
>             } else if (virXMLNodeNameEqual(cur, "jpeg")) {
>-                int compressionVal;
>-                g_autofree char *compression = virXMLPropString(cur, "compression");
>+                virDomainGraphicsSpiceJpegCompression compression;
>
>-                if (!compression) {
>-                    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>-                                   _("spice jpeg missing compression"));
>+                if (virXMLPropEnum(cur, "compression",
>+                                   virDomainGraphicsSpiceJpegCompressionTypeFromString,
>+                                   VIR_XML_PROP_NONZERO, &compression) < 0)

Same here.

>                     return -1;
>-                }
>
>-                if ((compressionVal =
>-                     virDomainGraphicsSpiceJpegCompressionTypeFromString(compression)) <= 0) {
>-                    virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>-                                   _("unknown spice jpeg compression %s"),
>-                                   compression);
>-                    return -1;
>-                }
>-
>-                def->data.spice.jpeg = compressionVal;
>+                def->data.spice.jpeg = compression;
>             } else if (virXMLNodeNameEqual(cur, "zlib")) {
>-                int compressionVal;
>-                g_autofree char *compression = virXMLPropString(cur, "compression");
>-
>-                if (!compression) {
>-                    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>-                                   _("spice zlib missing compression"));
>-                    return -1;
>-                }
>+                virDomainGraphicsSpiceZlibCompression compression;
>
>-                if ((compressionVal =
>-                     virDomainGraphicsSpiceZlibCompressionTypeFromString(compression)) <= 0) {
>-                    virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>-                                   _("unknown spice zlib compression %s"),
>-                                   compression);
>+                if (virXMLPropEnum(cur, "compression",
>+                                   virDomainGraphicsSpiceZlibCompressionTypeFromString,
>+                                   VIR_XML_PROP_REQUIRED, &compression) < 0)

VIR_XML_PROP_NONZERO - zero was not allowed originally.

>                     return -1;
>-                }
>
>-                def->data.spice.zlib = compressionVal;
>+                def->data.spice.zlib = compression;
>             } else if (virXMLNodeNameEqual(cur, "playback")) {
>-                int compressionVal;
>-                g_autofree char *compression = virXMLPropString(cur, "compression");
>-
>-                if (!compression) {
>-                    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>-                                   _("spice playback missing compression"));
>-                    return -1;
>-                }
>-
>-                if ((compressionVal =
>-                     virTristateSwitchTypeFromString(compression)) <= 0) {
>-                    virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>-                                   _("unknown spice playback compression"));
>+                if (virXMLPropTristateSwitch(cur, "compression",
>+                                             VIR_XML_PROP_NONZERO,
>+                                             &def->data.spice.playback) < 0)

NONZERO is implied by virXMLPropTristateSwitch.

Assuming it is OK in this case where we checked for <= 0 but other
places might have accepted the word "default". Since we never generate
the default value and we never documented it, it might be okay to
remove. But that's out of scope of this patch and this hunk is currently
correct with:

s/VIR_XML_PROP_NONZERO/VIR_XML_PROP_REQUIRED/

>                     return -1;
>-                }
>
>-                def->data.spice.playback = compressionVal;

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano