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 :|