[PATCH V2 4/4] qemu: Check for valid save image formats when loading driver config

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

Splitting the change to virQEMUDriverConfig struct into a separate patch
felt a bit awkward compared to one patch, but I'm fine to do that if
folks prefer.

 src/qemu/qemu_conf.c      | 35 +++++++++++++++++++++++++++++------
 src/qemu/qemu_conf.h      |  6 +++---
 src/qemu/qemu_driver.c    | 35 +++++++----------------------------
 src/qemu/qemu_saveimage.h |  1 -
 src/qemu/qemu_snapshot.c  | 11 +++--------
 5 files changed, 42 insertions(+), 46 deletions(-)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index b73dda7e10..14cfc289b3 100644
--- 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_OPERATION_FAILED,
+                       _("Invalid save_image_format '%1$s' specified in configuration file"),
+                       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_OPERATION_FAILED,
+                       _("Invalid dump_image_format '%1$s' specified in configuration file"),
+                       dumpstr);
+        return -1;
+    }
+
+    if (virConfGetValueString(conf, "snapshot_image_format", &snapstr) < 0)
+        return -1;
+    if (snapstr && (cfg->snapshotImageFormat = qemuSaveFormatTypeFromString(snapstr)) < 0) {
+        virReportError(VIR_ERR_OPERATION_FAILED,
+                       _("Invalid snapshot_image_format '%1$s' specified in configuration file"),
+                       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 97214f72d0..3e1b41af73 100644
--- 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 4d94985635..76b808b98f 100644
--- 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 0d8ee542af..58f8252c8a 100644
--- 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 416a772b92..891e67e98b 100644
--- 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;
 
-- 
2.43.0



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux