From ec50e0cf6389ff176ff6584929a9995c9a47829a Mon Sep 17 00:00:00 2001 From: Jim Fehlig Date: Thu, 20 Feb 2025 14:28:35 -0700 Subject: [PATCH] qemu: Check for valid save image formats when loading driver config MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Daniel P. Berrangé --- 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..b376841388 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_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 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 ceaef9817f..ff3fb8bc00 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.47.3