On Thu, Aug 08, 2024 at 05:38:01PM -0600, Jim Fehlig via Devel wrote:
> Add a 'save_image_version' setting to qemu.conf to control the image
> version when saving a VM with 'virsh save' or 'virsh managedsave'.
> Default to the new version 3.
>
> Signed-off-by: Jim Fehlig <jfehlig@xxxxxxxx>
> ---
> src/qemu/libvirtd_qemu.aug | 1 +
> src/qemu/qemu.conf.in | 6 ++++++
> src/qemu/qemu_conf.c | 16 ++++++++++++++++
> src/qemu/qemu_conf.h | 5 +++++
> src/qemu/qemu_driver.c | 3 +--
> src/qemu/qemu_saveimage.c | 11 ++++++-----
> src/qemu/qemu_saveimage.h | 8 ++++----
> src/qemu/qemu_snapshot.c | 4 ++--
> src/qemu/test_libvirtd_qemu.aug.in | 1 +
> 9 files changed, 42 insertions(+), 13 deletions(-)
>
> diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
> index 2b6526538f..acbfcd9fc3 100644
> --- a/src/qemu/libvirtd_qemu.aug
> +++ b/src/qemu/libvirtd_qemu.aug
> @@ -92,6 +92,7 @@ module Libvirtd_qemu =
> | str_array_entry "namespaces"
>
> let save_entry = str_entry "save_image_format"
> + | int_entry "save_image_version"
> | str_entry "dump_image_format"
> | str_entry "snapshot_image_format"
> | str_entry "auto_dump_path"
> diff --git a/src/qemu/qemu.conf.in b/src/qemu/qemu.conf.in
> index 6bc2140dcb..b5df8c1cc6 100644
> --- a/src/qemu/qemu.conf.in
> +++ b/src/qemu/qemu.conf.in
> @@ -590,6 +590,11 @@
> # at scheduled saving, and it is an error if the specified save_image_format
> # is not valid, or the requested compression program can't be found.
> #
> +# save_image_version is applicable when using 'virsh save' or 'virsh managed-save'.
> +# Currently only versions 2 and 3 are supported, with version 3 being the default.
> +# If saved images must be compatible with an older libvirt without this setting,
> +# then set save_image_format_version to 2.
> +#
> # dump_image_format is used when you use 'virsh dump' at emergency
> # crashdump, and if the specified dump_image_format is not valid, or
> # the requested compression program can't be found, this falls
> @@ -601,6 +606,7 @@
> # or the requested compression program can't be found.
> #
> #save_image_format = "raw"
> +#save_image_version = 3
> #dump_image_format = "raw"
> #snapshot_image_format = "raw"
>
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index b36bede6c3..ab4122708c 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -66,6 +66,10 @@ VIR_LOG_INIT("qemu.qemu_conf");
> #define QEMU_MIGRATION_PORT_MIN 49152
> #define QEMU_MIGRATION_PORT_MAX 49215
>
> +/* Need to reconsile definition here and in qemu_saveimage.h */
> +#define QEMU_SAVE_VERSION 3
You've not used this - instead you used QEMU_SAVE_IMAGE_VERSION
defined in qemu_conf.h
> +
> +
> VIR_ENUM_IMPL(virQEMUSchedCore,
> QEMU_SCHED_CORE_LAST,
> "none",
> @@ -246,6 +250,8 @@ virQEMUDriverConfig *virQEMUDriverConfigNew(bool privileged,
> cfg->migrationPortMin = QEMU_MIGRATION_PORT_MIN;
> cfg->migrationPortMax = QEMU_MIGRATION_PORT_MAX;
>
> + cfg->saveImageVersion = QEMU_SAVE_IMAGE_VERSION;
> +
> /* For privileged driver, try and find hugetlbfs mounts automatically.
> * Non-privileged driver requires admin to create a dir for the
> * user, chown it, and then let user configure it manually. */
> @@ -608,6 +614,16 @@ static int
> virQEMUDriverConfigLoadSaveEntry(virQEMUDriverConfig *cfg,
> virConf *conf)
> {
> + if (virConfGetValueUInt(conf, "save_image_version", &cfg->saveImageVersion) < 0)
> + return -1;
> + if (cfg->saveImageVersion < QEMU_SAVE_IMAGE_VERSION_MIN ||
> + cfg->saveImageVersion > QEMU_SAVE_IMAGE_VERSION) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Invalid save image version %1$u"),
> + cfg->saveImageVersion);
> + return -1;
> + }
> +
> if (virConfGetValueString(conf, "save_image_format", &cfg->saveImageFormat) < 0)
> return -1;
> if (virConfGetValueString(conf, "dump_image_format", &cfg->dumpImageFormat) < 0)
> diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
> index aa1e1a626c..55abede7e3 100644
> --- a/src/qemu/qemu_conf.h
> +++ b/src/qemu/qemu_conf.h
> @@ -45,6 +45,10 @@
>
> #define QEMU_DRIVER_NAME "QEMU"
>
> +#define QEMU_SAVE_IMAGE_VERSION_MIN 2
> +#define QEMU_SAVE_IMAGE_VERSION 3
Call the second one 'MAX' for parity, and also to make it more
obvious that we're not automatically using the 'MAX' by default.
> +
> +
> typedef enum {
> QEMU_SCHED_CORE_NONE = 0,
> QEMU_SCHED_CORE_VCPUS,
> @@ -193,6 +197,7 @@ struct _virQEMUDriverConfig {
> bool securityRequireConfined;
>
> char *saveImageFormat;
> + unsigned int saveImageVersion;
> char *dumpImageFormat;
> char *snapshotImageFormat;
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 736602333e..6c0d3e097c 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -2683,8 +2683,7 @@ qemuDomainSaveInternal(virQEMUDriver *driver,
> if (!(cookie = qemuDomainSaveCookieNew(vm)))
> goto endjob;
>
> - if (!(data = virQEMUSaveDataNew(xml, cookie, was_running, compressed,
> - driver->xmlopt)))
> + if (!(data = virQEMUSaveDataNew(driver, xml, cookie, was_running, compressed)))
> goto endjob;
> xml = NULL;
>
> diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c
> index 50fec33f54..bffa0a3397 100644
> --- a/src/qemu/qemu_saveimage.c
> +++ b/src/qemu/qemu_saveimage.c
> @@ -95,25 +95,26 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC(virQEMUSaveData, virQEMUSaveDataFree);
> * This function steals @domXML on success.
> */
> virQEMUSaveData *
> -virQEMUSaveDataNew(char *domXML,
> +virQEMUSaveDataNew(virQEMUDriver *driver,
> + char *domXML,
> qemuDomainSaveCookie *cookieObj,
> bool running,
> - int compressed,
> - virDomainXMLOption *xmlopt)
> + int compressed)
> {
> virQEMUSaveData *data = NULL;
> virQEMUSaveHeader *header;
> + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
>
> data = g_new0(virQEMUSaveData, 1);
>
> if (cookieObj &&
> !(data->cookie = virSaveCookieFormat((virObject *) cookieObj,
> - virDomainXMLOptionGetSaveCookie(xmlopt))))
> + virDomainXMLOptionGetSaveCookie(driver->xmlopt))))
> goto error;
>
> header = &data->header;
> memcpy(header->magic, QEMU_SAVE_PARTIAL, sizeof(header->magic));
> - header->version = QEMU_SAVE_VERSION;
> + header->version = cfg->saveImageVersion;
> header->was_running = running ? 1 : 0;
> header->compressed = compressed;
>
> diff --git a/src/qemu/qemu_saveimage.h b/src/qemu/qemu_saveimage.h
> index 9dd7de292d..3cee846f14 100644
> --- a/src/qemu/qemu_saveimage.h
> +++ b/src/qemu/qemu_saveimage.h
> @@ -28,7 +28,7 @@
> */
> #define QEMU_SAVE_MAGIC "LibvirtQemudSave"
> #define QEMU_SAVE_PARTIAL "LibvirtQemudPart"
> -#define QEMU_SAVE_VERSION 3
> +#define QEMU_SAVE_VERSION QEMU_SAVE_IMAGE_VERSION
Surely we shouldn't need this constant anymore since you changed
the qemu_saveimage.c code to use 'cfg->saveImageVersion' instead
of 'QEMU_SAVE_VERSION'
>
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 :|
