[libvirt] [PATCH 0/4] Add support for compression of managedsave and external snapshot memory images
plus a few refactors. Peter Krempa (4): qemu: Fix coding style in qemuDomainSaveFlags() qemu: refactor qemuCompressProgramAvailable() qemu: managedsave: Add support for compressing managed save images qemu: snapshot: Add support for compressing external snapshot memory src/qemu/qemu.conf | 12 ++++++++ src/qemu/qemu_conf.c | 3 ++ src/qemu/qemu_conf.h | 2 ++ src/qemu/qemu_driver.c | 78 ++++++++++++++++++++++++++++++++++++-------------- 4 files changed, 74 insertions(+), 21 deletions(-) -- 1.8.3.2
Avoid mixed brace style in an if statement and fix formatting of error messages. --- src/qemu/qemu_driver.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c71aecc..b9f340c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3158,7 +3158,7 @@ qemuDomainSaveFlags(virDomainPtr dom, const char *path, const char *dxml, unsigned int flags) { virQEMUDriverPtr driver = dom->conn->privateData; - int compressed; + int compressed = QEMU_SAVE_FORMAT_RAW; int ret = -1; virDomainObjPtr vm = NULL; virQEMUDriverConfigPtr cfg = NULL; @@ -3168,20 +3168,18 @@ qemuDomainSaveFlags(virDomainPtr dom, const char *path, const char *dxml, VIR_DOMAIN_SAVE_PAUSED, -1); cfg = virQEMUDriverGetConfig(driver); - if (cfg->saveImageFormat == NULL) - compressed = QEMU_SAVE_FORMAT_RAW; - else { + if (cfg->saveImageFormat) { compressed = qemuSaveCompressionTypeFromString(cfg->saveImageFormat); if (compressed < 0) { - virReportError(VIR_ERR_OPERATION_FAILED, - "%s", _("Invalid save image format specified " - "in configuration file")); + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("Invalid save image format specified " + "in configuration file")); goto cleanup; } if (!qemuCompressProgramAvailable(compressed)) { - virReportError(VIR_ERR_OPERATION_FAILED, - "%s", _("Compression program for image format " - "in configuration file isn't available")); + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("Compression program for image format " + "in configuration file isn't available")); goto cleanup; } } -- 1.8.3.2
On 10/09/2013 10:31 AM, Peter Krempa wrote:
Avoid mixed brace style in an if statement and fix formatting of error messages. --- src/qemu/qemu_driver.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 10/09/13 18:38, Eric Blake wrote:
On 10/09/2013 10:31 AM, Peter Krempa wrote:
Avoid mixed brace style in an if statement and fix formatting of error messages. --- src/qemu/qemu_driver.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-)
ACK.
Pushed; Thanks. Peter
--- src/qemu/qemu_driver.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b9f340c..cfdbb9a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3138,18 +3138,18 @@ cleanup: } /* Returns true if a compression program is available in PATH */ -static bool qemuCompressProgramAvailable(virQEMUSaveFormat compress) +static bool +qemuCompressProgramAvailable(virQEMUSaveFormat compress) { - const char *prog; - char *c; + char *path; if (compress == QEMU_SAVE_FORMAT_RAW) return true; - prog = qemuSaveCompressionTypeToString(compress); - c = virFindFileInPath(prog); - if (!c) + + if (!(path = virFindFileInPath(qemuSaveCompressionTypeToString(compress)))) return false; - VIR_FREE(c); + + VIR_FREE(path); return true; } -- 1.8.3.2
On 10/09/2013 10:31 AM, Peter Krempa wrote:
--- src/qemu/qemu_driver.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 10/09/13 22:37, Eric Blake wrote:
On 10/09/2013 10:31 AM, Peter Krempa wrote:
--- src/qemu/qemu_driver.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
ACK.
Pushed; Thanks. Peter
The regular save image code has the support to compress images using a specified algorithm. This was not implemented for managed save although it shares most of the backend code. --- src/qemu/qemu.conf | 6 ++++++ src/qemu/qemu_conf.c | 2 ++ src/qemu/qemu_conf.h | 1 + src/qemu/qemu_driver.c | 23 +++++++++++++++++++++-- 4 files changed, 30 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 5fd6263..7cf67df 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -278,8 +278,14 @@ # the requested compression program can't be found, this falls # back to "raw" compression. # +# managedsave_image_format is used when a domain is saved to a location managed +# by libvirt for example by using 'virsh managedsave'. It is an error if the +# specified format isn't valid, or the requested compression program can't be +# found. +# #save_image_format = "raw" #dump_image_format = "raw" +#managedsave_image_format = "raw" # When a domain is configured to be auto-dumped when libvirtd receives a # watchdog event from qemu guest, libvirtd will save dump files in directory diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 1a41caf..3e3cc28 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -521,6 +521,8 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, GET_VALUE_STR("save_image_format", cfg->saveImageFormat); GET_VALUE_STR("dump_image_format", cfg->dumpImageFormat); + GET_VALUE_STR("managedsave_image_format", cfg->managedsaveImageFormat); + GET_VALUE_STR("auto_dump_path", cfg->autoDumpPath); GET_VALUE_BOOL("auto_dump_bypass_cache", cfg->autoDumpBypassCache); GET_VALUE_BOOL("auto_start_bypass_cache", cfg->autoStartBypassCache); diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index da29a2a..3a68c4a 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -144,6 +144,7 @@ struct _virQEMUDriverConfig { char *saveImageFormat; char *dumpImageFormat; + char *managedsaveImageFormat; char *autoDumpPath; bool autoDumpBypassCache; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index cfdbb9a..bd69e5f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3232,6 +3232,8 @@ static int qemuDomainManagedSave(virDomainPtr dom, unsigned int flags) { virQEMUDriverPtr driver = dom->conn->privateData; + virQEMUDriverConfigPtr cfg = NULL; + int compressed = QEMU_SAVE_FORMAT_RAW; virDomainObjPtr vm; char *name = NULL; int ret = -1; @@ -3257,13 +3259,29 @@ qemuDomainManagedSave(virDomainPtr dom, unsigned int flags) goto cleanup; } + cfg = virQEMUDriverGetConfig(driver); + if (cfg->managedsaveImageFormat) { + compressed = qemuSaveCompressionTypeFromString(cfg->managedsaveImageFormat); + if (compressed < 0) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("Invalid managedsave image format specified " + "in configuration file")); + goto cleanup; + } + if (!qemuCompressProgramAvailable(compressed)) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("Compression program for image format " + "in configuration file isn't available")); + goto cleanup; + } + } + if (!(name = qemuDomainManagedSavePath(driver, vm))) goto cleanup; VIR_INFO("Saving state of domain '%s' to '%s'", vm->def->name, name); - if ((ret = qemuDomainSaveInternal(driver, dom, vm, name, - QEMU_SAVE_FORMAT_RAW, + if ((ret = qemuDomainSaveInternal(driver, dom, vm, name, compressed, NULL, flags)) == 0) vm->hasManagedSave = true; @@ -3273,6 +3291,7 @@ cleanup: if (vm) virObjectUnlock(vm); VIR_FREE(name); + virObjectUnref(cfg); return ret; } -- 1.8.3.2
On Wed, Oct 09, 2013 at 06:31:31PM +0200, Peter Krempa wrote:
The regular save image code has the support to compress images using a specified algorithm. This was not implemented for managed save although it shares most of the backend code. --- src/qemu/qemu.conf | 6 ++++++ src/qemu/qemu_conf.c | 2 ++ src/qemu/qemu_conf.h | 1 + src/qemu/qemu_driver.c | 23 +++++++++++++++++++++-- 4 files changed, 30 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 5fd6263..7cf67df 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -278,8 +278,14 @@ # the requested compression program can't be found, this falls # back to "raw" compression. # +# managedsave_image_format is used when a domain is saved to a location managed +# by libvirt for example by using 'virsh managedsave'. It is an error if the +# specified format isn't valid, or the requested compression program can't be +# found. +# #save_image_format = "raw" #dump_image_format = "raw" +#managedsave_image_format = "raw"
I'm wondering if we could justifiably just use the existing 'save_image_format' for managed save too. dump needed a separate option since that's a clearly semantically different API set, but save vs managed save is basically just API sugar, so I feel we could just use the same config option. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 10/09/2013 10:33 AM, Daniel P. Berrange wrote:
On Wed, Oct 09, 2013 at 06:31:31PM +0200, Peter Krempa wrote:
The regular save image code has the support to compress images using a specified algorithm. This was not implemented for managed save although it shares most of the backend code. --- src/qemu/qemu.conf | 6 ++++++ src/qemu/qemu_conf.c | 2 ++ src/qemu/qemu_conf.h | 1 + src/qemu/qemu_driver.c | 23 +++++++++++++++++++++-- 4 files changed, 30 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 5fd6263..7cf67df 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -278,8 +278,14 @@ # the requested compression program can't be found, this falls # back to "raw" compression. # +# managedsave_image_format is used when a domain is saved to a location managed +# by libvirt for example by using 'virsh managedsave'. It is an error if the +# specified format isn't valid, or the requested compression program can't be +# found. +# #save_image_format = "raw" #dump_image_format = "raw" +#managedsave_image_format = "raw"
I'm wondering if we could justifiably just use the existing 'save_image_format' for managed save too.
dump needed a separate option since that's a clearly semantically different API set, but save vs managed save is basically just API sugar, so I feel we could just use the same config option.
Agreed. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 10/09/13 18:33, Daniel P. Berrange wrote:
On Wed, Oct 09, 2013 at 06:31:31PM +0200, Peter Krempa wrote:
The regular save image code has the support to compress images using a specified algorithm. This was not implemented for managed save although it shares most of the backend code. --- src/qemu/qemu.conf | 6 ++++++ src/qemu/qemu_conf.c | 2 ++ src/qemu/qemu_conf.h | 1 + src/qemu/qemu_driver.c | 23 +++++++++++++++++++++-- 4 files changed, 30 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 5fd6263..7cf67df 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -278,8 +278,14 @@ # the requested compression program can't be found, this falls # back to "raw" compression. # +# managedsave_image_format is used when a domain is saved to a location managed +# by libvirt for example by using 'virsh managedsave'. It is an error if the +# specified format isn't valid, or the requested compression program can't be +# found. +# #save_image_format = "raw" #dump_image_format = "raw" +#managedsave_image_format = "raw"
I'm wondering if we could justifiably just use the existing 'save_image_format' for managed save too.
Technically we definitely could. The formats are automatically detected.
dump needed a separate option since that's a clearly semantically different API set, but save vs managed save is basically just API sugar, so I feel we could just use the same config option.
I chose to do it in a configurable manner as the use of a compression algorithm sometimes introduces significant slowing of the saving process which might be unfortunate when managed save is used for saving guests at host shutdown while use of the regular save mechanism can tolerate longer saving periods. Peter
The regular save image code has the support to compress images using a specified algorithm. This was not implemented for external checkpoints although it shares most of the backend code. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1017227 --- src/qemu/qemu.conf | 6 ++++++ src/qemu/qemu_conf.c | 1 + src/qemu/qemu_conf.h | 1 + src/qemu/qemu_driver.c | 23 +++++++++++++++++++++-- 4 files changed, 29 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 7cf67df..aff5e6d 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -283,9 +283,15 @@ # specified format isn't valid, or the requested compression program can't be # found. # +# snapshot_image_format specifies the compression algorithm of the memory save +# image when an external snapshot of a domain is taken. This does not apply +# on disk image format. It is an error if the specified format isn't valid, +# or the requested compression program can't be found. +# #save_image_format = "raw" #dump_image_format = "raw" #managedsave_image_format = "raw" +#snapshot_image_format = "raw" # When a domain is configured to be auto-dumped when libvirtd receives a # watchdog event from qemu guest, libvirtd will save dump files in directory diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 3e3cc28..ecc9a54 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -522,6 +522,7 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, GET_VALUE_STR("save_image_format", cfg->saveImageFormat); GET_VALUE_STR("dump_image_format", cfg->dumpImageFormat); GET_VALUE_STR("managedsave_image_format", cfg->managedsaveImageFormat); + GET_VALUE_STR("snapshot_image_format", cfg->snapshotImageFormat); GET_VALUE_STR("auto_dump_path", cfg->autoDumpPath); GET_VALUE_BOOL("auto_dump_bypass_cache", cfg->autoDumpBypassCache); diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 3a68c4a..947e1b4 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -145,6 +145,7 @@ struct _virQEMUDriverConfig { char *saveImageFormat; char *dumpImageFormat; char *managedsaveImageFormat; + char *snapshotImageFormat; char *autoDumpPath; bool autoDumpBypassCache; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index bd69e5f..64eec4d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12116,6 +12116,8 @@ qemuDomainSnapshotCreateActiveExternal(virConnectPtr conn, bool transaction = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_TRANSACTION); int thaw = 0; /* 1 if freeze succeeded, -1 if freeze failed */ bool pmsuspended = false; + virQEMUDriverConfigPtr cfg = NULL; + int compressed = QEMU_SAVE_FORMAT_RAW; if (qemuDomainObjBeginAsyncJob(driver, vm, QEMU_ASYNC_JOB_SNAPSHOT) < 0) goto cleanup; @@ -12177,12 +12179,28 @@ qemuDomainSnapshotCreateActiveExternal(virConnectPtr conn, JOB_MASK(QEMU_JOB_SUSPEND) | JOB_MASK(QEMU_JOB_MIGRATION_OP)); + cfg = virQEMUDriverGetConfig(driver); + if (cfg->snapshotImageFormat) { + compressed = qemuSaveCompressionTypeFromString(cfg->snapshotImageFormat); + if (compressed < 0) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("Invalid snapshot image format specified " + "in configuration file")); + goto cleanup; + } + if (!qemuCompressProgramAvailable(compressed)) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("Compression program for image format " + "in configuration file isn't available")); + goto cleanup; + } + } + if (!(xml = qemuDomainDefFormatLive(driver, vm->def, true, true))) goto endjob; if ((ret = qemuDomainSaveMemory(driver, vm, snap->def->file, - xml, QEMU_SAVE_FORMAT_RAW, - resume, 0, + xml, compressed, resume, 0, QEMU_ASYNC_JOB_SNAPSHOT)) < 0) goto endjob; @@ -12271,6 +12289,7 @@ endjob: cleanup: VIR_FREE(xml); + virObjectUnref(cfg); if (memory_unlink && ret < 0) unlink(snap->def->file); -- 1.8.3.2
On Wed, Oct 09, 2013 at 06:31:32PM +0200, Peter Krempa wrote:
The regular save image code has the support to compress images using a specified algorithm. This was not implemented for external checkpoints although it shares most of the backend code.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1017227 --- src/qemu/qemu.conf | 6 ++++++ src/qemu/qemu_conf.c | 1 + src/qemu/qemu_conf.h | 1 + src/qemu/qemu_driver.c | 23 +++++++++++++++++++++-- 4 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 7cf67df..aff5e6d 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -283,9 +283,15 @@ # specified format isn't valid, or the requested compression program can't be # found. # +# snapshot_image_format specifies the compression algorithm of the memory save +# image when an external snapshot of a domain is taken. This does not apply +# on disk image format. It is an error if the specified format isn't valid, +# or the requested compression program can't be found. +# #save_image_format = "raw" #dump_image_format = "raw" #managedsave_image_format = "raw" +#snapshot_image_format = "raw"
Same question here as previous patch. A snapshot is really just another form of saved image. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 10/09/13 18:38, Daniel P. Berrange wrote:
On Wed, Oct 09, 2013 at 06:31:32PM +0200, Peter Krempa wrote:
The regular save image code has the support to compress images using a specified algorithm. This was not implemented for external checkpoints although it shares most of the backend code.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1017227 --- src/qemu/qemu.conf | 6 ++++++ src/qemu/qemu_conf.c | 1 + src/qemu/qemu_conf.h | 1 + src/qemu/qemu_driver.c | 23 +++++++++++++++++++++-- 4 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 7cf67df..aff5e6d 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -283,9 +283,15 @@ # specified format isn't valid, or the requested compression program can't be # found. # +# snapshot_image_format specifies the compression algorithm of the memory save +# image when an external snapshot of a domain is taken. This does not apply +# on disk image format. It is an error if the specified format isn't valid, +# or the requested compression program can't be found. +# #save_image_format = "raw" #dump_image_format = "raw" #managedsave_image_format = "raw" +#snapshot_image_format = "raw"
Same question here as previous patch. A snapshot is really just another form of saved image.
Here I have a bit stronger opinion compared to the managedsave case. I really think that users might prefer different settings for snapshots especially that snapshots can be created with a live guest, where a compression algorithm may inhibit successful finish of creating a snapshot as the migration procedure will be too slow to accomodate changing a lot of pages in the guest memory. Thus it might be desired to store snapshots uncompressed or just with a quick algorithm. The second option would be to disable compression for live snapshots, but that might be unfortunate too.
Daniel
Peter
participants (3)
-
Daniel P. Berrange -
Eric Blake -
Peter Krempa