qemu: Check for valid save image formats when loading driver config

Checking for valid 'foo_image_format' settings in qemu.conf is not done
until the settings are used. Move the checks to
virQEMUDriverConfigLoadSaveEntry, allowing to report incorrect format
settings at driver startup.

This change was made easier by also changing the corresponding fields
in the virQEMUDriverConfig to 'int', which is more in line with the
other fields that represent enumerated types.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index b73dda7e109c7f0d1d1a2165658d018a01f76160..b376841388b968e8fd2291116e9df5bff504fdae 100644 (file)
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -36,6 +36,7 @@
 #include "qemu_domain.h"
 #include "qemu_firmware.h"
 #include "qemu_namespace.h"
+#include "qemu_saveimage.h"
 #include "qemu_security.h"
 #include "viruuid.h"
 #include "virconf.h"
@@ -374,9 +375,6 @@ static void virQEMUDriverConfigDispose(void *obj)
     g_free(cfg->slirpHelperName);
     g_free(cfg->dbusDaemonName);
 
-    g_free(cfg->saveImageFormat);
-    g_free(cfg->dumpImageFormat);
-    g_free(cfg->snapshotImageFormat);
     g_free(cfg->autoDumpPath);
 
     g_strfreev(cfg->securityDriverNames);
@@ -626,12 +624,37 @@ static int
 virQEMUDriverConfigLoadSaveEntry(virQEMUDriverConfig *cfg,
                                  virConf *conf)
 {
-    if (virConfGetValueString(conf, "save_image_format", &cfg->saveImageFormat) < 0)
+    g_autofree char *savestr = NULL;
+    g_autofree char *dumpstr = NULL;
+    g_autofree char *snapstr = NULL;
+
+    if (virConfGetValueString(conf, "save_image_format", &savestr) < 0)
         return -1;
-    if (virConfGetValueString(conf, "dump_image_format", &cfg->dumpImageFormat) < 0)
+    if (savestr && (cfg->saveImageFormat = qemuSaveFormatTypeFromString(savestr)) < 0) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       _("Invalid save_image_format '%1$s'"),
+                       savestr);
         return -1;
-    if (virConfGetValueString(conf, "snapshot_image_format", &cfg->snapshotImageFormat) < 0)
+    }
+
+    if (virConfGetValueString(conf, "dump_image_format", &dumpstr) < 0)
         return -1;
+    if (dumpstr && (cfg->dumpImageFormat = qemuSaveFormatTypeFromString(dumpstr)) < 0) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       _("Invalid dump_image_format '%1$s'"),
+                       dumpstr);
+        return -1;
+    }
+
+    if (virConfGetValueString(conf, "snapshot_image_format", &snapstr) < 0)
+        return -1;
+    if (snapstr && (cfg->snapshotImageFormat = qemuSaveFormatTypeFromString(snapstr)) < 0) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       _("Invalid snapshot_image_format '%1$s'"),
+                       snapstr);
+        return -1;
+    }
+
     if (virConfGetValueString(conf, "auto_dump_path", &cfg->autoDumpPath) < 0)
         return -1;
     if (virConfGetValueBool(conf, "auto_dump_bypass_cache", &cfg->autoDumpBypassCache) < 0)

diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index 97214f72d0a47a2079e0afa296b525c840cc4726..3e1b41af7387b3031463689c5ac4724b4be5d2e6 100644 (file)
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -193,9 +193,9 @@ struct _virQEMUDriverConfig {
     bool securityDefaultConfined;
     bool securityRequireConfined;
 
-    char *saveImageFormat;
-    char *dumpImageFormat;
-    char *snapshotImageFormat;
+    int saveImageFormat;
+    int dumpImageFormat;
+    int snapshotImageFormat;
 
     char *autoDumpPath;
     bool autoDumpBypassCache;

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ceaef9817f5ef7421a46ab176fb8cbdd63dec6c9..ff3fb8bc00b7e4b1b6e1bf458eb1d3656d3f8d3c 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2731,7 +2731,6 @@ qemuDomainManagedSaveHelper(virQEMUDriver *driver,
     g_autoptr(virQEMUDriverConfig) cfg = NULL;
     g_autoptr(virCommand) compressor = NULL;
     g_autofree char *path = NULL;
-    int format = QEMU_SAVE_FORMAT_RAW;
 
     if (virDomainObjCheckActive(vm) < 0)
         return -1;
@@ -2743,18 +2742,14 @@ qemuDomainManagedSaveHelper(virQEMUDriver *driver,
     }
 
     cfg = virQEMUDriverGetConfig(driver);
-    if (cfg->saveImageFormat &&
-        (format = qemuSaveFormatTypeFromString(cfg->saveImageFormat)) < 0)
-        return -1;
-
-    if (qemuSaveImageGetCompressionProgram(format, &compressor, "save") < 0)
+    if (qemuSaveImageGetCompressionProgram(cfg->saveImageFormat, &compressor, "save") < 0)
         return -1;
 
     path = qemuDomainManagedSavePath(driver, vm);
 
     VIR_INFO("Saving state of domain '%s' to '%s'", vm->def->name, path);
 
-    if (qemuDomainSaveInternal(driver, vm, path, format,
+    if (qemuDomainSaveInternal(driver, vm, path, cfg->saveImageFormat,
                                compressor, dxml, flags) < 0)
         return -1;
 
@@ -2768,7 +2763,6 @@ qemuDomainSaveFlags(virDomainPtr dom, const char *path, const char *dxml,
                     unsigned int flags)
 {
     virQEMUDriver *driver = dom->conn->privateData;
-    int format = QEMU_SAVE_FORMAT_RAW;
     g_autoptr(virCommand) compressor = NULL;
     int ret = -1;
     virDomainObj *vm = NULL;
@@ -2779,11 +2773,7 @@ qemuDomainSaveFlags(virDomainPtr dom, const char *path, const char *dxml,
                   VIR_DOMAIN_SAVE_PAUSED, -1);
 
     cfg = virQEMUDriverGetConfig(driver);
-    if (cfg->saveImageFormat &&
-        (format = qemuSaveFormatTypeFromString(cfg->saveImageFormat)) < 0)
-        goto cleanup;
-
-    if (qemuSaveImageGetCompressionProgram(format, &compressor, "save") < 0)
+    if (qemuSaveImageGetCompressionProgram(cfg->saveImageFormat, &compressor, "save") < 0)
         goto cleanup;
 
     if (!(vm = qemuDomainObjFromDomain(dom)))
@@ -2795,7 +2785,7 @@ qemuDomainSaveFlags(virDomainPtr dom, const char *path, const char *dxml,
     if (virDomainObjCheckActive(vm) < 0)
         goto cleanup;
 
-    ret = qemuDomainSaveInternal(driver, vm, path, format,
+    ret = qemuDomainSaveInternal(driver, vm, path, cfg->saveImageFormat,
                                  compressor, dxml, flags);
 
  cleanup:
@@ -2821,7 +2811,6 @@ qemuDomainSaveParams(virDomainPtr dom,
     g_autoptr(virCommand) compressor = NULL;
     const char *to = NULL;
     const char *dxml = NULL;
-    int format = QEMU_SAVE_FORMAT_RAW;
     int ret = -1;
 
     virCheckFlags(VIR_DOMAIN_SAVE_BYPASS_CACHE |
@@ -2855,17 +2844,13 @@ qemuDomainSaveParams(virDomainPtr dom,
     }
 
     cfg = virQEMUDriverGetConfig(driver);
-    if (cfg->saveImageFormat &&
-        (format = qemuSaveFormatTypeFromString(cfg->saveImageFormat)) < 0)
-        goto cleanup;
-
-    if (qemuSaveImageGetCompressionProgram(format, &compressor, "save") < 0)
+    if (qemuSaveImageGetCompressionProgram(cfg->saveImageFormat, &compressor, "save") < 0)
         goto cleanup;
 
     if (virDomainObjCheckActive(vm) < 0)
         goto cleanup;
 
-    ret = qemuDomainSaveInternal(driver, vm, to, format,
+    ret = qemuDomainSaveInternal(driver, vm, to, cfg->saveImageFormat,
                                  compressor, dxml, flags);
 
  cleanup:
@@ -3069,14 +3054,8 @@ doCoreDump(virQEMUDriver *driver,
     const char *memory_dump_format = NULL;
     g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
     g_autoptr(virCommand) compressor = NULL;
-    int format = QEMU_SAVE_FORMAT_RAW;
-
-    if (cfg->dumpImageFormat) {
-        if ((format = qemuSaveFormatTypeFromString(cfg->dumpImageFormat)) < 0)
-            goto cleanup;
-    }
 
-    if (qemuSaveImageGetCompressionProgram(format, &compressor, "dump") < 0)
+    if (qemuSaveImageGetCompressionProgram(cfg->dumpImageFormat, &compressor, "dump") < 0)
         goto cleanup;
 
     /* Create an empty file with appropriate ownership.  */

diff --git a/src/qemu/qemu_saveimage.h b/src/qemu/qemu_saveimage.h
index 0d8ee542af3ff167623f9e36e3fe0f566c916483..58f8252c8a01d96de1f16a5e5aca21ab46833edd 100644 (file)
--- a/src/qemu/qemu_saveimage.h
+++ b/src/qemu/qemu_saveimage.h
@@ -20,7 +20,6 @@
 
 #include "virconftypes.h"
 
-#include "qemu_conf.h"
 #include "qemu_domain.h"
 
 /* It would be nice to replace 'Qemud' with 'Qemu' but

diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 416a772b9281e261ad94f4b91e7b45a20e858d1b..891e67e98b0f6c6b61ad23b8a0e550698931acd2 100644 (file)
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -1597,7 +1597,6 @@ qemuSnapshotCreateActiveExternal(virQEMUDriver *driver,
     bool memory_existing = false;
     bool thaw = false;
     bool pmsuspended = false;
-    int format = QEMU_SAVE_FORMAT_RAW;
     g_autoptr(virCommand) compressor = NULL;
     virQEMUSaveData *data = NULL;
     g_autoptr(GHashTable) blockNamedNodeData = NULL;
@@ -1674,12 +1673,8 @@ qemuSnapshotCreateActiveExternal(virQEMUDriver *driver,
                                           JOB_MASK(VIR_JOB_SUSPEND) |
                                           JOB_MASK(VIR_JOB_MIGRATION_OP)));
 
-        if (cfg->snapshotImageFormat &&
-            (format = qemuSaveFormatTypeFromString(cfg->snapshotImageFormat)) < 0)
-            goto cleanup;
-
-        if (qemuSaveImageGetCompressionProgram(format, &compressor,
-                                               "snapshot") < 0)
+        if (qemuSaveImageGetCompressionProgram(cfg->snapshotImageFormat,
+                                               &compressor, "snapshot") < 0)
             goto cleanup;
 
         if (!(xml = qemuDomainDefFormatLive(driver, priv->qemuCaps,
@@ -1690,7 +1685,7 @@ qemuSnapshotCreateActiveExternal(virQEMUDriver *driver,
 
         if (!(data = virQEMUSaveDataNew(xml,
                                         (qemuDomainSaveCookie *) snapdef->cookie,
-                                        resume, format, driver->xmlopt)))
+                                        resume, cfg->snapshotImageFormat, driver->xmlopt)))
             goto cleanup;
         xml = NULL;