[PATCH V2 0/4] qemu: Small cleanups to SaveImageGetCompressionProgram
V2 of https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/message/SKVP... Changes in V2: * V1 moved the special-case handling of invalid dump image format to doCoreDump. V2 removes it, and fails on invalid configuration similar to the other save-related operations. * New patch4 that moves checking for a valid save image format from time of use to config file parsing at driver startup Jim Fehlig (4): qemu: Move declaration of virQEMUSaveFormat to header file qemu: Don't ignore dump image format errors qemu: Change return value of SaveImageGetCompressionProgram qemu: Check for valid save image formats when loading driver config src/qemu/qemu_conf.c | 35 ++++++++++++--- src/qemu/qemu_conf.h | 6 +-- src/qemu/qemu_driver.c | 30 ++++--------- src/qemu/qemu_saveimage.c | 92 +++++++-------------------------------- src/qemu/qemu_saveimage.h | 25 +++++++++-- src/qemu/qemu_snapshot.c | 8 ++-- 6 files changed, 80 insertions(+), 116 deletions(-) -- 2.43.0
Allow use of the enum outside of qemu_saveimage. Signed-off-by: Jim Fehlig <jfehlig@suse.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> --- src/qemu/qemu_saveimage.c | 19 ------------------- src/qemu/qemu_saveimage.h | 19 +++++++++++++++++++ 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c index 5c889fee11..403e4c9679 100644 --- a/src/qemu/qemu_saveimage.c +++ b/src/qemu/qemu_saveimage.c @@ -37,25 +37,6 @@ VIR_LOG_INIT("qemu.qemu_saveimage"); -typedef enum { - QEMU_SAVE_FORMAT_RAW = 0, - QEMU_SAVE_FORMAT_GZIP = 1, - QEMU_SAVE_FORMAT_BZIP2 = 2, - /* - * Deprecated by xz and never used as part of a release - * QEMU_SAVE_FORMAT_LZMA - */ - QEMU_SAVE_FORMAT_XZ = 3, - QEMU_SAVE_FORMAT_LZOP = 4, - QEMU_SAVE_FORMAT_ZSTD = 5, - /* Note: add new members only at the end. - These values are used in the on-disk format. - Do not change or re-use numbers. */ - - QEMU_SAVE_FORMAT_LAST -} virQEMUSaveFormat; - -VIR_ENUM_DECL(qemuSaveFormat); VIR_ENUM_IMPL(qemuSaveFormat, QEMU_SAVE_FORMAT_LAST, "raw", diff --git a/src/qemu/qemu_saveimage.h b/src/qemu/qemu_saveimage.h index 53ae222467..8e755e1eb5 100644 --- a/src/qemu/qemu_saveimage.h +++ b/src/qemu/qemu_saveimage.h @@ -32,6 +32,25 @@ G_STATIC_ASSERT(sizeof(QEMU_SAVE_MAGIC) == sizeof(QEMU_SAVE_PARTIAL)); +typedef enum { + QEMU_SAVE_FORMAT_RAW = 0, + QEMU_SAVE_FORMAT_GZIP = 1, + QEMU_SAVE_FORMAT_BZIP2 = 2, + /* + * Deprecated by xz and never used as part of a release + * QEMU_SAVE_FORMAT_LZMA + */ + QEMU_SAVE_FORMAT_XZ = 3, + QEMU_SAVE_FORMAT_LZOP = 4, + QEMU_SAVE_FORMAT_ZSTD = 5, + /* Note: add new members only at the end. + These values are used in the on-disk format. + Do not change or re-use numbers. */ + + QEMU_SAVE_FORMAT_LAST +} virQEMUSaveFormat; +VIR_ENUM_DECL(qemuSaveFormat); + typedef struct _virQEMUSaveHeader virQEMUSaveHeader; struct _virQEMUSaveHeader { char magic[sizeof(QEMU_SAVE_MAGIC)-1]; -- 2.43.0
Long ago, without justification, commit 48cb9f0542 changed qemuGetCompressionProgram (since renamed to qemuSaveImageGetCompressionProgram) to ignore configuration errors for dump operations. Like the other save-related operations, user provided configuration should be verified and an error reported if it cannot be honored. Remove the special handling of configuration errors in qemuSaveImageGetCompressionProgram and change the dump logic to fail when dump image format cannot be supported. Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/qemu/qemu_driver.c | 19 ++++-------- src/qemu/qemu_saveimage.c | 61 ++++++++++----------------------------- src/qemu/qemu_saveimage.h | 3 +- src/qemu/qemu_snapshot.c | 2 +- 4 files changed, 24 insertions(+), 61 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 80c918312b..ac7c67b3a1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2744,8 +2744,7 @@ qemuDomainManagedSaveHelper(virQEMUDriver *driver, cfg = virQEMUDriverGetConfig(driver); if ((format = qemuSaveImageGetCompressionProgram(cfg->saveImageFormat, - &compressor, - "save", false)) < 0) + &compressor, "save")) < 0) return -1; path = qemuDomainManagedSavePath(driver, vm); @@ -2778,8 +2777,7 @@ qemuDomainSaveFlags(virDomainPtr dom, const char *path, const char *dxml, cfg = virQEMUDriverGetConfig(driver); if ((format = qemuSaveImageGetCompressionProgram(cfg->saveImageFormat, - &compressor, - "save", false)) < 0) + &compressor, "save")) < 0) goto cleanup; if (!(vm = qemuDomainObjFromDomain(dom))) @@ -2852,8 +2850,7 @@ qemuDomainSaveParams(virDomainPtr dom, cfg = virQEMUDriverGetConfig(driver); if ((format = qemuSaveImageGetCompressionProgram(cfg->saveImageFormat, - &compressor, - "save", false)) < 0) + &compressor, "save")) < 0) goto cleanup; if (virDomainObjCheckActive(vm) < 0) @@ -3064,13 +3061,9 @@ doCoreDump(virQEMUDriver *driver, g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); g_autoptr(virCommand) compressor = NULL; - /* We reuse "save" flag for "dump" here. Then, we can support the same - * format in "save" and "dump". This path doesn't need the compression - * program to exist and can ignore the return value - it only cares to - * get the compressor */ - ignore_value(qemuSaveImageGetCompressionProgram(cfg->dumpImageFormat, - &compressor, - "dump", true)); + if (qemuSaveImageGetCompressionProgram(cfg->dumpImageFormat, + &compressor, "dump") < 0) + goto cleanup; /* Create an empty file with appropriate ownership. */ if (dump_flags & VIR_DUMP_BYPASS_CACHE) { diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c index 403e4c9679..9bb76c05f8 100644 --- a/src/qemu/qemu_saveimage.c +++ b/src/qemu/qemu_saveimage.c @@ -513,24 +513,15 @@ qemuSaveImageCreate(virQEMUDriver *driver, * @compresspath: Pointer to a character string to store the fully qualified * path from virFindFileInPath. * @styleFormat: String representing the style of format (dump, save, snapshot) - * @use_raw_on_fail: Boolean indicating how to handle the error path. For - * callers that are OK with invalid data or inability to - * find the compression program, just return a raw format - * and let the path remain as NULL. * - * Returns: - * virQEMUSaveFormat - Integer representation of the save image - * format to be used for particular style - * (e.g. dump, save, or snapshot). - * QEMU_SAVE_FORMAT_RAW - If there is no qemu.conf imageFormat value or - * no there was an error, then just return RAW - * indicating none. + * On success, returns an integer representation of the save image format to be + * used for a particular style (e.g. dump, save, or snapshot). Returns -1 on + * failure. */ int qemuSaveImageGetCompressionProgram(const char *imageFormat, virCommand **compressor, - const char *styleFormat, - bool use_raw_on_fail) + const char *styleFormat) { int ret; const char *prog; @@ -540,14 +531,22 @@ qemuSaveImageGetCompressionProgram(const char *imageFormat, if (!imageFormat) return QEMU_SAVE_FORMAT_RAW; - if ((ret = qemuSaveFormatTypeFromString(imageFormat)) < 0) - goto error; + if ((ret = qemuSaveFormatTypeFromString(imageFormat)) < 0) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("Invalid %1$s image format specified in configuration file"), + styleFormat); + return -1; + } if (ret == QEMU_SAVE_FORMAT_RAW) return QEMU_SAVE_FORMAT_RAW; - if (!(prog = virFindFileInPath(imageFormat))) - goto error; + if (!(prog = virFindFileInPath(imageFormat))) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("Compression program for %1$s image format in configuration file isn't available"), + styleFormat); + return -1; + } *compressor = virCommandNew(prog); virCommandAddArg(*compressor, "-c"); @@ -555,34 +554,6 @@ qemuSaveImageGetCompressionProgram(const char *imageFormat, virCommandAddArg(*compressor, "-3"); return ret; - - error: - if (ret < 0) { - if (use_raw_on_fail) - VIR_WARN("Invalid %s image format specified in " - "configuration file, using raw", - styleFormat); - else - virReportError(VIR_ERR_OPERATION_FAILED, - _("Invalid %1$s image format specified in configuration file"), - styleFormat); - } else { - if (use_raw_on_fail) - VIR_WARN("Compression program for %s image format in " - "configuration file isn't available, using raw", - styleFormat); - else - virReportError(VIR_ERR_OPERATION_FAILED, - _("Compression program for %1$s image format in configuration file isn't available"), - styleFormat); - } - - /* Use "raw" as the format if the specified format is not valid, - * or the compress program is not available. */ - if (use_raw_on_fail) - return QEMU_SAVE_FORMAT_RAW; - - return -1; } diff --git a/src/qemu/qemu_saveimage.h b/src/qemu/qemu_saveimage.h index 8e755e1eb5..aa905768de 100644 --- a/src/qemu/qemu_saveimage.h +++ b/src/qemu/qemu_saveimage.h @@ -112,8 +112,7 @@ qemuSaveImageOpen(virQEMUDriver *driver, int qemuSaveImageGetCompressionProgram(const char *imageFormat, virCommand **compressor, - const char *styleFormat, - bool use_raw_on_fail) + const char *styleFormat) ATTRIBUTE_NONNULL(2); int diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index f7d6272907..c2a98c1296 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -1676,7 +1676,7 @@ qemuSnapshotCreateActiveExternal(virQEMUDriver *driver, if ((format = qemuSaveImageGetCompressionProgram(cfg->snapshotImageFormat, &compressor, - "snapshot", false)) < 0) + "snapshot")) < 0) goto cleanup; if (!(xml = qemuDomainDefFormatLive(driver, priv->qemuCaps, -- 2.43.0
On Thu, Feb 20, 2025 at 05:11:36PM -0700, Jim Fehlig via Devel wrote:
Long ago, without justification, commit 48cb9f0542 changed qemuGetCompressionProgram (since renamed to qemuSaveImageGetCompressionProgram) to ignore configuration errors for dump operations. Like the other save-related operations, user provided configuration should be verified and an error reported if it cannot be honored.
Remove the special handling of configuration errors in qemuSaveImageGetCompressionProgram and change the dump logic to fail when dump image format cannot be supported.
Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/qemu/qemu_driver.c | 19 ++++-------- src/qemu/qemu_saveimage.c | 61 ++++++++++----------------------------- src/qemu/qemu_saveimage.h | 3 +- src/qemu/qemu_snapshot.c | 2 +- 4 files changed, 24 insertions(+), 61 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
qemuSaveImageGetCompressionProgram is a bit overloaded. Along with getting a compression program, it checks the validity of the image format and returns the integer representation of the format. Change the function to only handle retrieving the specified compression program, returning success or failure. Checking the validity of the image format can be left to the calling functions. Signed-off-by: Jim Fehlig <jfehlig@suse.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> --- src/qemu/qemu_driver.c | 36 +++++++++++++++++++++++++----------- src/qemu/qemu_saveimage.c | 30 +++++++++--------------------- src/qemu/qemu_saveimage.h | 2 +- src/qemu/qemu_snapshot.c | 11 +++++++---- 4 files changed, 42 insertions(+), 37 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ac7c67b3a1..4d94985635 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2731,7 +2731,7 @@ qemuDomainManagedSaveHelper(virQEMUDriver *driver, g_autoptr(virQEMUDriverConfig) cfg = NULL; g_autoptr(virCommand) compressor = NULL; g_autofree char *path = NULL; - int format; + int format = QEMU_SAVE_FORMAT_RAW; if (virDomainObjCheckActive(vm) < 0) return -1; @@ -2743,8 +2743,11 @@ qemuDomainManagedSaveHelper(virQEMUDriver *driver, } cfg = virQEMUDriverGetConfig(driver); - if ((format = qemuSaveImageGetCompressionProgram(cfg->saveImageFormat, - &compressor, "save")) < 0) + if (cfg->saveImageFormat && + (format = qemuSaveFormatTypeFromString(cfg->saveImageFormat)) < 0) + return -1; + + if (qemuSaveImageGetCompressionProgram(format, &compressor, "save") < 0) return -1; path = qemuDomainManagedSavePath(driver, vm); @@ -2765,7 +2768,7 @@ qemuDomainSaveFlags(virDomainPtr dom, const char *path, const char *dxml, unsigned int flags) { virQEMUDriver *driver = dom->conn->privateData; - int format; + int format = QEMU_SAVE_FORMAT_RAW; g_autoptr(virCommand) compressor = NULL; int ret = -1; virDomainObj *vm = NULL; @@ -2776,8 +2779,11 @@ qemuDomainSaveFlags(virDomainPtr dom, const char *path, const char *dxml, VIR_DOMAIN_SAVE_PAUSED, -1); cfg = virQEMUDriverGetConfig(driver); - if ((format = qemuSaveImageGetCompressionProgram(cfg->saveImageFormat, - &compressor, "save")) < 0) + if (cfg->saveImageFormat && + (format = qemuSaveFormatTypeFromString(cfg->saveImageFormat)) < 0) + goto cleanup; + + if (qemuSaveImageGetCompressionProgram(format, &compressor, "save") < 0) goto cleanup; if (!(vm = qemuDomainObjFromDomain(dom))) @@ -2815,7 +2821,7 @@ qemuDomainSaveParams(virDomainPtr dom, g_autoptr(virCommand) compressor = NULL; const char *to = NULL; const char *dxml = NULL; - int format; + int format = QEMU_SAVE_FORMAT_RAW; int ret = -1; virCheckFlags(VIR_DOMAIN_SAVE_BYPASS_CACHE | @@ -2849,8 +2855,11 @@ qemuDomainSaveParams(virDomainPtr dom, } cfg = virQEMUDriverGetConfig(driver); - if ((format = qemuSaveImageGetCompressionProgram(cfg->saveImageFormat, - &compressor, "save")) < 0) + if (cfg->saveImageFormat && + (format = qemuSaveFormatTypeFromString(cfg->saveImageFormat)) < 0) + goto cleanup; + + if (qemuSaveImageGetCompressionProgram(format, &compressor, "save") < 0) goto cleanup; if (virDomainObjCheckActive(vm) < 0) @@ -3060,9 +3069,14 @@ 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 (qemuSaveImageGetCompressionProgram(cfg->dumpImageFormat, - &compressor, "dump") < 0) + if (cfg->dumpImageFormat) { + if ((format = qemuSaveFormatTypeFromString(cfg->dumpImageFormat)) < 0) + goto cleanup; + } + + if (qemuSaveImageGetCompressionProgram(format, &compressor, "dump") < 0) goto cleanup; /* Create an empty file with appropriate ownership. */ diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c index 9bb76c05f8..d940bfb5c3 100644 --- a/src/qemu/qemu_saveimage.c +++ b/src/qemu/qemu_saveimage.c @@ -508,38 +508,26 @@ qemuSaveImageCreate(virQEMUDriver *driver, /* qemuSaveImageGetCompressionProgram: - * @imageFormat: String representation from qemu.conf of the image format - * being used (dump, save, or snapshot). + * @format: Integer representation of the image format being used + * (dump, save, or snapshot). * @compresspath: Pointer to a character string to store the fully qualified * path from virFindFileInPath. * @styleFormat: String representing the style of format (dump, save, snapshot) * - * On success, returns an integer representation of the save image format to be - * used for a particular style (e.g. dump, save, or snapshot). Returns -1 on - * failure. + * Returns -1 on failure, 0 on success. */ int -qemuSaveImageGetCompressionProgram(const char *imageFormat, +qemuSaveImageGetCompressionProgram(int format, virCommand **compressor, const char *styleFormat) { - int ret; + const char *imageFormat = qemuSaveFormatTypeToString(format); const char *prog; *compressor = NULL; - if (!imageFormat) - return QEMU_SAVE_FORMAT_RAW; - - if ((ret = qemuSaveFormatTypeFromString(imageFormat)) < 0) { - virReportError(VIR_ERR_OPERATION_FAILED, - _("Invalid %1$s image format specified in configuration file"), - styleFormat); - return -1; - } - - if (ret == QEMU_SAVE_FORMAT_RAW) - return QEMU_SAVE_FORMAT_RAW; + if (format == QEMU_SAVE_FORMAT_RAW) + return 0; if (!(prog = virFindFileInPath(imageFormat))) { virReportError(VIR_ERR_OPERATION_FAILED, @@ -550,10 +538,10 @@ qemuSaveImageGetCompressionProgram(const char *imageFormat, *compressor = virCommandNew(prog); virCommandAddArg(*compressor, "-c"); - if (ret == QEMU_SAVE_FORMAT_XZ) + if (format == QEMU_SAVE_FORMAT_XZ) virCommandAddArg(*compressor, "-3"); - return ret; + return 0; } diff --git a/src/qemu/qemu_saveimage.h b/src/qemu/qemu_saveimage.h index aa905768de..0d8ee542af 100644 --- a/src/qemu/qemu_saveimage.h +++ b/src/qemu/qemu_saveimage.h @@ -110,7 +110,7 @@ qemuSaveImageOpen(virQEMUDriver *driver, ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4); int -qemuSaveImageGetCompressionProgram(const char *imageFormat, +qemuSaveImageGetCompressionProgram(int format, virCommand **compressor, const char *styleFormat) ATTRIBUTE_NONNULL(2); diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index c2a98c1296..416a772b92 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -1597,7 +1597,7 @@ qemuSnapshotCreateActiveExternal(virQEMUDriver *driver, bool memory_existing = false; bool thaw = false; bool pmsuspended = false; - int format; + int format = QEMU_SAVE_FORMAT_RAW; g_autoptr(virCommand) compressor = NULL; virQEMUSaveData *data = NULL; g_autoptr(GHashTable) blockNamedNodeData = NULL; @@ -1674,9 +1674,12 @@ qemuSnapshotCreateActiveExternal(virQEMUDriver *driver, JOB_MASK(VIR_JOB_SUSPEND) | JOB_MASK(VIR_JOB_MIGRATION_OP))); - if ((format = qemuSaveImageGetCompressionProgram(cfg->snapshotImageFormat, - &compressor, - "snapshot")) < 0) + if (cfg->snapshotImageFormat && + (format = qemuSaveFormatTypeFromString(cfg->snapshotImageFormat)) < 0) + goto cleanup; + + if (qemuSaveImageGetCompressionProgram(format, &compressor, + "snapshot") < 0) goto cleanup; if (!(xml = qemuDomainDefFormatLive(driver, priv->qemuCaps, -- 2.43.0
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> --- 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
On 2/20/25 17:11, Jim Fehlig wrote:
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> ---
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);
I'm not sure about the best error code here, but VIR_ERR_OPERATION_FAILED isn't it. Probably VIR_ERR_CONFIG_UNSUPPORTED, over VIR_ERR_CONF_SYNTAX? Also, none of the existing error messages in this file are suffixed with "specified in configuration file". I'll remove that bit in my local branch. Regards, Jim
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;
On Thu, Feb 20, 2025 at 05:39:17PM -0700, Jim Fehlig via Devel wrote:
On 2/20/25 17:11, Jim Fehlig wrote:
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> ---
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);
I'm not sure about the best error code here, but VIR_ERR_OPERATION_FAILED isn't it. Probably VIR_ERR_CONFIG_UNSUPPORTED, over VIR_ERR_CONF_SYNTAX? Also, none of the existing error messages in this file are suffixed with "specified in configuration file". I'll remove that bit in my local branch.
Yep, CONFIG_UNSUPPORTED with those few words removed is good. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Thu, Feb 20, 2025 at 05:11:38PM -0700, Jim Fehlig via Devel wrote:
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> ---
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.
It is ok as is.
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(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> with one comment inline...
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,
Probably VIR_ERR_CONFIG_UNSUPPORTED is a better bet.
+ _("Invalid save_image_format '%1$s' specified in configuration file"), + savestr); return -1; - if (virConfGetValueString(conf, "snapshot_image_format", &cfg->snapshotImageFormat) < 0) + } +
With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
participants (2)
-
Daniel P. Berrangé -
Jim Fehlig