On Wed, Apr 27, 2022 at 11:13:39PM +0200, Claudio Fontana wrote: > use zstd which is the only really interesting one. > > Signed-off-by: Claudio Fontana <cfontana@xxxxxxx> > --- > src/qemu/qemu_capabilities.c | 2 + > src/qemu/qemu_capabilities.h | 1 + > src/qemu/qemu_migration.c | 6 +++ > src/qemu/qemu_migration_params.c | 49 +++++++++---------- > src/qemu/qemu_migration_params.h | 6 +++ > src/qemu/qemu_saveimage.c | 6 +++ > .../caps_5.0.0.aarch64.xml | 1 + > .../qemucapabilitiesdata/caps_5.0.0.ppc64.xml | 1 + > .../caps_5.0.0.riscv64.xml | 1 + > .../caps_5.0.0.x86_64.xml | 1 + > .../qemucapabilitiesdata/caps_5.1.0.sparc.xml | 1 + > .../caps_5.1.0.x86_64.xml | 1 + > .../caps_5.2.0.aarch64.xml | 1 + > .../qemucapabilitiesdata/caps_5.2.0.ppc64.xml | 1 + > .../caps_5.2.0.riscv64.xml | 1 + > .../qemucapabilitiesdata/caps_5.2.0.s390x.xml | 1 + > .../caps_5.2.0.x86_64.xml | 1 + > .../caps_6.0.0.aarch64.xml | 1 + > .../qemucapabilitiesdata/caps_6.0.0.s390x.xml | 1 + > .../caps_6.0.0.x86_64.xml | 1 + > .../caps_6.1.0.x86_64.xml | 1 + > .../caps_6.2.0.aarch64.xml | 1 + > .../qemucapabilitiesdata/caps_6.2.0.ppc64.xml | 1 + > .../caps_6.2.0.x86_64.xml | 1 + > .../caps_7.0.0.aarch64.xml | 1 + > .../qemucapabilitiesdata/caps_7.0.0.ppc64.xml | 1 + > .../caps_7.0.0.x86_64.xml | 1 + > 27 files changed, 66 insertions(+), 25 deletions(-) > diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c > index 7bab913fe5..f9c86de67e 100644 > --- a/src/qemu/qemu_migration.c > +++ b/src/qemu/qemu_migration.c > @@ -5950,6 +5950,12 @@ qemuMigrationSrcToFileAux(virQEMUDriver *driver, virDomainObj *vm, > QEMU_MIGRATION_PARAM_MULTIFD_CHANNELS, > nchannels) < 0) > return -1; > + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_PARAM_MULTIFD_COMPRESSION)) { > + if (qemuMigrationParamsSetString(migParams, > + QEMU_MIGRATION_PARAM_MULTIFD_COMPRESSION, > + "zstd") < 0) > + return -1; > + } snip > diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c > index 2bc81035ae..a1e9e1c3e8 100644 > --- a/src/qemu/qemu_saveimage.c > +++ b/src/qemu/qemu_saveimage.c > @@ -607,6 +607,12 @@ int qemuSaveImageLoadMultiFd(virConnectPtr conn, virDomainObj *vm, int oflags, > QEMU_MIGRATION_PARAM_MULTIFD_CHANNELS, > nchannels) < 0) > goto cleanup; > + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_PARAM_MULTIFD_COMPRESSION)) { > + if (qemuMigrationParamsSetString(migParams, > + QEMU_MIGRATION_PARAM_MULTIFD_COMPRESSION, > + "zstd") < 0) > + goto cleanup; > + } > if (qemuMigrationParamsApply(driver, vm, asyncJob, migParams) < 0) > goto cleanup; > This is going to break if an image is savd with a QEMU that does not support zstd, and is later restored with a QEMU that has enabled zstd. The current save/restore code supports compression, settable globally via /etc/libvirt/qemu.conf - raw, gzip, bzip2, xz, lzop. With your new APIs for save/restore with TypedParameters, we could make the compression an API driven choice, which would be nice. Also, we should add zstd to the choices with existing non-parallel migrate. When we add parallel migrate, we should wire up whichever options make sense, but we need to record the use of the compression in the save image header. IOW, when loading we only want to use zstd if the original image header shows use of zstd *and* QEMU supports it. So I'd suggest we want 3 patches. One for adding zstd as a choice, one of making compression tunable fvia the APIs, and finally one for parallel migrate. We don';t have to support all algorithms with parallel migrate - we can do just a subset that make sense or are possible. 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 :|