5:25 p.m.
The current documentation of the various foo_image_format settings in
qemu.conf subtly implies they are only used for specifying compression.
Patch1 of this small series attempts to clarify and improve the description
of the settings. It defines image format as a way to specify the desired
layout of guest memory blocks on disk.
Patch2 changes the use of 'compressed' with 'format' throughout the
code, removing implication that format == compressed.
V2:
Replace more uses of 'compressed' with 'format' in patch2
Jim Fehlig (2):
qemu: conf: Improve the foo_image_format setting descriptions
qemu: Use consistent naming for save image format
src/qemu/qemu.conf.in | 40 +++++++++++++++++++++++----------------
src/qemu/qemu_driver.c | 30 ++++++++++++++---------------
src/qemu/qemu_saveimage.c | 32 +++++++++++++++----------------
src/qemu/qemu_saveimage.h | 4 ++--
src/qemu/qemu_snapshot.c | 6 +++---
5 files changed, 60 insertions(+), 52 deletions(-)
--
2.35.3
5:25 p.m.
New subject: [PATCH V2 1/2] qemu: conf: Improve the foo_image_format setting descriptions
The current description of the various foo_image_format settings can
be construded to imply the setting is only used to control compression
of the image. Improve the documentation to clarify that format describes
the representation of guest memory blocks on disk, which includes
compression among other possible layouts.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/qemu/qemu.conf.in | 40 ++++++++++++++++++++++++----------------
1 file changed, 24 insertions(+), 16 deletions(-)
diff --git a/src/qemu/qemu.conf.in b/src/qemu/qemu.conf.in
index 6bc2140dcb..d4fdd717ba 100644
--- a/src/qemu/qemu.conf.in
+++ b/src/qemu/qemu.conf.in
@@ -578,27 +578,35 @@
# "/dev/infiniband/uverbs0"
-# The default format for QEMU/KVM guest save images is raw; that is, the
-# memory from the domain is dumped out directly to a file. If you have
-# guests with a large amount of memory, however, this can take up quite
-# a bit of space. If you would like to compress the images while they
-# are being saved to disk, you can also set "zstd", "lzop",
"gzip", "bzip2",
-# or "xz" for save_image_format. Note that this means you slow down the
process
-# of saving a domain in order to save disk space.
+# The libvirt QEMU driver supports serveral different save image formats.
+# The term "format" is used loosely to describe how the save image data is
+# represented on disk. It could be a continguous stream of guest memory blocks,
+# or a stream of compressed memory blocks.
#
-# save_image_format is used when you use 'virsh save' or 'virsh
managedsave'
-# 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.
+# A continguous stream of guest memory blocks is the default format for QEMU/KVM
+# guest save images and is termed "raw". The raw format can consume
considerable
+# disk space when saving large memory guests. Various compression formats are
+# available for specifying a save image compressed by the named algorithm.
+# Supported compression formats are "zstd", "lzop", "gzip",
"bzip2", and "xz".
+
+# save_image_format can be used to select the desired save format. "raw" is
+# the traditional format used by libvirt and is also the default. The
+# compression formats can be used to save disk space, although this typically
+# results in longer save and restore times.
#
-# 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
-# back to "raw" compression.
+# save_image_format is used with 'virsh save' or 'virsh managedsave'. It
is
+# an error if the specified save_image_format is not valid, or cannot be
+# supported by the system.
#
-# snapshot_image_format specifies the compression algorithm of the memory save
+# dump_image_format is analogous to save_image_format and is used with
+# 'virsh dump' at emergency crashdump. If the specified dump_image_format is
+# not valid or cannot be supported by the system, this falls back to the
+# "raw" format.
+#
+# Likewise, snapshot_image_format specifies the format 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.
+# or the system cannot support the requested format.
#
#save_image_format = "raw"
#dump_image_format = "raw"
--
2.35.3
5:25 p.m.
New subject: [PATCH V2 2/2] qemu: Use consistent naming for save image format
The image format setting in qemu.conf is named 'save_image_format'. The
enum of supported format types is declared with name 'virQEMUSaveFormat'.
Let's be consistent and use 'format' instead of 'compressed' when
referring
to the save image format.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/qemu/qemu_driver.c | 30 +++++++++++++++---------------
src/qemu/qemu_saveimage.c | 32 ++++++++++++++++----------------
src/qemu/qemu_saveimage.h | 4 ++--
src/qemu/qemu_snapshot.c | 6 +++---
4 files changed, 36 insertions(+), 36 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 3801ad623a..b114f4b9c2 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2749,7 +2749,7 @@ qemuDomainManagedSaveHelper(virQEMUDriver *driver,
g_autoptr(virQEMUDriverConfig) cfg = NULL;
g_autoptr(virCommand) compressor = NULL;
g_autofree char *path = NULL;
- int compressed;
+ int format;
if (virDomainObjCheckActive(vm) < 0)
return -1;
@@ -2761,16 +2761,16 @@ qemuDomainManagedSaveHelper(virQEMUDriver *driver,
}
cfg = virQEMUDriverGetConfig(driver);
- if ((compressed = qemuSaveImageGetCompressionProgram(cfg->saveImageFormat,
- &compressor,
- "save", false)) <
0)
+ if ((format = qemuSaveImageGetCompressionProgram(cfg->saveImageFormat,
+ &compressor,
+ "save", false)) < 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, compressed,
+ if (qemuDomainSaveInternal(driver, vm, path, format,
compressor, dxml, flags) < 0)
return -1;
@@ -2784,7 +2784,7 @@ qemuDomainSaveFlags(virDomainPtr dom, const char *path, const char
*dxml,
unsigned int flags)
{
virQEMUDriver *driver = dom->conn->privateData;
- int compressed;
+ int format;
g_autoptr(virCommand) compressor = NULL;
int ret = -1;
virDomainObj *vm = NULL;
@@ -2795,9 +2795,9 @@ qemuDomainSaveFlags(virDomainPtr dom, const char *path, const char
*dxml,
VIR_DOMAIN_SAVE_PAUSED, -1);
cfg = virQEMUDriverGetConfig(driver);
- if ((compressed = qemuSaveImageGetCompressionProgram(cfg->saveImageFormat,
- &compressor,
- "save", false)) <
0)
+ if ((format = qemuSaveImageGetCompressionProgram(cfg->saveImageFormat,
+ &compressor,
+ "save", false)) < 0)
goto cleanup;
if (!(vm = qemuDomainObjFromDomain(dom)))
@@ -2809,7 +2809,7 @@ qemuDomainSaveFlags(virDomainPtr dom, const char *path, const char
*dxml,
if (virDomainObjCheckActive(vm) < 0)
goto cleanup;
- ret = qemuDomainSaveInternal(driver, vm, path, compressed,
+ ret = qemuDomainSaveInternal(driver, vm, path, format,
compressor, dxml, flags);
cleanup:
@@ -2835,7 +2835,7 @@ qemuDomainSaveParams(virDomainPtr dom,
g_autoptr(virCommand) compressor = NULL;
const char *to = NULL;
const char *dxml = NULL;
- int compressed;
+ int format;
int ret = -1;
virCheckFlags(VIR_DOMAIN_SAVE_BYPASS_CACHE |
@@ -2869,15 +2869,15 @@ qemuDomainSaveParams(virDomainPtr dom,
}
cfg = virQEMUDriverGetConfig(driver);
- if ((compressed = qemuSaveImageGetCompressionProgram(cfg->saveImageFormat,
- &compressor,
- "save", false)) <
0)
+ if ((format = qemuSaveImageGetCompressionProgram(cfg->saveImageFormat,
+ &compressor,
+ "save", false)) < 0)
goto cleanup;
if (virDomainObjCheckActive(vm) < 0)
goto cleanup;
- ret = qemuDomainSaveInternal(driver, vm, to, compressed,
+ ret = qemuDomainSaveInternal(driver, vm, to, format,
compressor, dxml, flags);
cleanup:
diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c
index 018ab5a222..69617e07eb 100644
--- a/src/qemu/qemu_saveimage.c
+++ b/src/qemu/qemu_saveimage.c
@@ -55,8 +55,8 @@ typedef enum {
QEMU_SAVE_FORMAT_LAST
} virQEMUSaveFormat;
-VIR_ENUM_DECL(qemuSaveCompression);
-VIR_ENUM_IMPL(qemuSaveCompression,
+VIR_ENUM_DECL(qemuSaveFormat);
+VIR_ENUM_IMPL(qemuSaveFormat,
QEMU_SAVE_FORMAT_LAST,
"raw",
"gzip",
@@ -72,7 +72,7 @@ qemuSaveImageBswapHeader(virQEMUSaveHeader *hdr)
hdr->version = GUINT32_SWAP_LE_BE(hdr->version);
hdr->data_len = GUINT32_SWAP_LE_BE(hdr->data_len);
hdr->was_running = GUINT32_SWAP_LE_BE(hdr->was_running);
- hdr->compressed = GUINT32_SWAP_LE_BE(hdr->compressed);
+ hdr->format = GUINT32_SWAP_LE_BE(hdr->format);
hdr->cookieOffset = GUINT32_SWAP_LE_BE(hdr->cookieOffset);
}
@@ -97,7 +97,7 @@ virQEMUSaveData *
virQEMUSaveDataNew(char *domXML,
qemuDomainSaveCookie *cookieObj,
bool running,
- int compressed,
+ int format,
virDomainXMLOption *xmlopt)
{
virQEMUSaveData *data = NULL;
@@ -114,7 +114,7 @@ virQEMUSaveDataNew(char *domXML,
memcpy(header->magic, QEMU_SAVE_PARTIAL, sizeof(header->magic));
header->version = QEMU_SAVE_VERSION;
header->was_running = running ? 1 : 0;
- header->compressed = compressed;
+ header->format = format;
data->xml = domXML;
return data;
@@ -227,22 +227,22 @@ virQEMUSaveDataFinish(virQEMUSaveData *data,
static virCommand *
-qemuSaveImageGetCompressionCommand(virQEMUSaveFormat compression)
+qemuSaveImageGetCompressionCommand(virQEMUSaveFormat format)
{
virCommand *ret = NULL;
- const char *prog = qemuSaveCompressionTypeToString(compression);
+ const char *prog = qemuSaveFormatTypeToString(format);
if (!prog) {
virReportError(VIR_ERR_OPERATION_FAILED,
_("Invalid compressed save format %1$d"),
- compression);
+ format);
return NULL;
}
ret = virCommandNew(prog);
virCommandAddArg(ret, "-dc");
- if (compression == QEMU_SAVE_FORMAT_LZOP)
+ if (format == QEMU_SAVE_FORMAT_LZOP)
virCommandAddArg(ret, "--ignore-warn");
return ret;
@@ -282,10 +282,10 @@ qemuSaveImageDecompressionStart(virQEMUSaveData *data,
if (header->version != 2)
return 0;
- if (header->compressed == QEMU_SAVE_FORMAT_RAW)
+ if (header->format == QEMU_SAVE_FORMAT_RAW)
return 0;
- if (!(cmd = qemuSaveImageGetCompressionCommand(header->compressed)))
+ if (!(cmd = qemuSaveImageGetCompressionCommand(header->format)))
return -1;
*intermediatefd = *fd;
@@ -443,8 +443,8 @@ qemuSaveImageCreate(virQEMUDriver *driver,
/* qemuSaveImageGetCompressionProgram:
- * @imageFormat: String representation from qemu.conf for the compression
- * image format being used (dump, save, or snapshot).
+ * @imageFormat: String representation from qemu.conf 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)
@@ -454,8 +454,8 @@ qemuSaveImageCreate(virQEMUDriver *driver,
* and let the path remain as NULL.
*
* Returns:
- * virQEMUSaveFormat - Integer representation of the compression
- * program to be used for particular style
+ * 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
@@ -475,7 +475,7 @@ qemuSaveImageGetCompressionProgram(const char *imageFormat,
if (!imageFormat)
return QEMU_SAVE_FORMAT_RAW;
- if ((ret = qemuSaveCompressionTypeFromString(imageFormat)) < 0)
+ if ((ret = qemuSaveFormatTypeFromString(imageFormat)) < 0)
goto error;
if (ret == QEMU_SAVE_FORMAT_RAW)
diff --git a/src/qemu/qemu_saveimage.h b/src/qemu/qemu_saveimage.h
index e541792153..0e58dd14b6 100644
--- a/src/qemu/qemu_saveimage.h
+++ b/src/qemu/qemu_saveimage.h
@@ -38,7 +38,7 @@ struct _virQEMUSaveHeader {
uint32_t version;
uint32_t data_len;
uint32_t was_running;
- uint32_t compressed;
+ uint32_t format;
uint32_t cookieOffset;
uint32_t unused[14];
};
@@ -121,7 +121,7 @@ virQEMUSaveData *
virQEMUSaveDataNew(char *domXML,
qemuDomainSaveCookie *cookieObj,
bool running,
- int compressed,
+ int format,
virDomainXMLOption *xmlopt);
void
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index f5260c4a22..0683bc5939 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -1302,7 +1302,7 @@ qemuSnapshotCreateActiveExternal(virQEMUDriver *driver,
bool memory_existing = false;
bool thaw = false;
bool pmsuspended = false;
- int compressed;
+ int format;
g_autoptr(virCommand) compressor = NULL;
virQEMUSaveData *data = NULL;
g_autoptr(GHashTable) blockNamedNodeData = NULL;
@@ -1379,7 +1379,7 @@ qemuSnapshotCreateActiveExternal(virQEMUDriver *driver,
JOB_MASK(VIR_JOB_SUSPEND) |
JOB_MASK(VIR_JOB_MIGRATION_OP)));
- if ((compressed =
qemuSaveImageGetCompressionProgram(cfg->snapshotImageFormat,
+ if ((format = qemuSaveImageGetCompressionProgram(cfg->snapshotImageFormat,
&compressor,
"snapshot",
false)) < 0)
goto cleanup;
@@ -1392,7 +1392,7 @@ qemuSnapshotCreateActiveExternal(virQEMUDriver *driver,
if (!(data = virQEMUSaveDataNew(xml,
(qemuDomainSaveCookie *) snapdef->cookie,
- resume, compressed, driver->xmlopt)))
+ resume, format, driver->xmlopt)))
goto cleanup;
xml = NULL;
--
2.35.3
5:14 p.m.
Hi All,
Any comments on this idea? Recall the motivation for this change is to
subsequently use the image format settings to request mapped-ram.
Regards,
Jim
On 8/16/24 16:25, Jim Fehlig wrote:
The current documentation of the various foo_image_format settings
in
qemu.conf subtly implies they are only used for specifying compression.
Patch1 of this small series attempts to clarify and improve the description
of the settings. It defines image format as a way to specify the desired
layout of guest memory blocks on disk.
Patch2 changes the use of 'compressed' with 'format' throughout the
code, removing implication that format == compressed.
V2:
Replace more uses of 'compressed' with 'format' in patch2
Jim Fehlig (2):
qemu: conf: Improve the foo_image_format setting descriptions
qemu: Use consistent naming for save image format
src/qemu/qemu.conf.in | 40 +++++++++++++++++++++++----------------
src/qemu/qemu_driver.c | 30 ++++++++++++++---------------
src/qemu/qemu_saveimage.c | 32 +++++++++++++++----------------
src/qemu/qemu_saveimage.h | 4 ++--
src/qemu/qemu_snapshot.c | 6 +++---
5 files changed, 60 insertions(+), 52 deletions(-)
3:37 a.m.
New subject: [PATCH V2 2/2] qemu: Use consistent naming for save image format
On Fri, Aug 16, 2024 at 04:25:26PM -0600, Jim Fehlig via Devel wrote:
The image format setting in qemu.conf is named
'save_image_format'. The
enum of supported format types is declared with name 'virQEMUSaveFormat'.
Let's be consistent and use 'format' instead of 'compressed' when
referring
to the save image format.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/qemu/qemu_driver.c | 30 +++++++++++++++---------------
src/qemu/qemu_saveimage.c | 32 ++++++++++++++++----------------
src/qemu/qemu_saveimage.h | 4 ++--
src/qemu/qemu_snapshot.c | 6 +++---
4 files changed, 36 insertions(+), 36 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 3801ad623a..b114f4b9c2 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2761,16 +2761,16 @@ qemuDomainManagedSaveHelper(virQEMUDriver *driver,
}
cfg = virQEMUDriverGetConfig(driver);
- if ((compressed = qemuSaveImageGetCompressionProgram(cfg->saveImageFormat,
- &compressor,
- "save", false)) <
0)
+ if ((format = qemuSaveImageGetCompressionProgram(cfg->saveImageFormat,
+ &compressor,
+ "save", false)) < 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, compressed,
+ if (qemuDomainSaveInternal(driver, vm, path, format,
compressor, dxml, flags) < 0)
return -1;
It'd be nice to also squash this in:
diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c
index aa5fb9a873b0..f27c21ca8cc9 100644
--- i/src/qemu/qemu_driver.c
+++ w/src/qemu/qemu_driver.c
@@ -2605,7 +2605,7 @@ static int
qemuDomainSaveInternal(virQEMUDriver *driver,
virDomainObj *vm,
const char *path,
- int compressed,
+ int format,
virCommand *compressor,
const char *xmlin,
unsigned int flags)
@@ -2683,7 +2683,7 @@ qemuDomainSaveInternal(virQEMUDriver *driver,
if (!(cookie = qemuDomainSaveCookieNew(vm)))
goto endjob;
- if (!(data = virQEMUSaveDataNew(xml, cookie, was_running, compressed,
+ if (!(data = virQEMUSaveDataNew(xml, cookie, was_running, format,
driver->xmlopt)))
goto endjob;
xml = NULL;
--
[...]
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index f5260c4a22..0683bc5939 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -1379,7 +1379,7 @@ qemuSnapshotCreateActiveExternal(virQEMUDriver *driver,
JOB_MASK(VIR_JOB_SUSPEND) |
JOB_MASK(VIR_JOB_MIGRATION_OP)));
- if ((compressed =
qemuSaveImageGetCompressionProgram(cfg->snapshotImageFormat,
+ if ((format = qemuSaveImageGetCompressionProgram(cfg->snapshotImageFormat,
&compressor,
"snapshot",
false)) < 0)
Indentation.
goto cleanup;
With the above changes
Reviewed-by: Martin Kletzander <mkletzan(a)redhat.com>
3:37 a.m.
New subject: [PATCH V2 1/2] qemu: conf: Improve the foo_image_format setting
descriptions
On Fri, Aug 16, 2024 at 04:25:25PM -0600, Jim Fehlig via Devel wrote:
The current description of the various foo_image_format settings can
be construded to imply the setting is only used to control compression
of the image. Improve the documentation to clarify that format describes
the representation of guest memory blocks on disk, which includes
compression among other possible layouts.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
Reviewed-by: Martin Kletzander <mkletzan(a)redhat.com>
12:25 p.m.
New subject: [PATCH V2 1/2] qemu: conf: Improve the foo_image_format setting
descriptions
On 10/2/24 02:37, Martin Kletzander wrote:
On Fri, Aug 16, 2024 at 04:25:25PM -0600, Jim Fehlig via Devel
wrote:
> The current description of the various foo_image_format settings can
> be construded to imply the setting is only used to control compression
> of the image. Improve the documentation to clarify that format describes
> the representation of guest memory blocks on disk, which includes
> compression among other possible layouts.
>
> Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
Reviewed-by: Martin Kletzander <mkletzan(a)redhat.com>
Thanks for reviewing this series Martin! Does it imply we can use the
image_format settings to support mapped-ram? It was previously discussed, but no
consensus AFAIK
https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/message/R...
Regards,
Jim
4:44 a.m.
New subject: [PATCH V2 1/2] qemu: conf: Improve the foo_image_format setting
descriptions
On Tue, Oct 08, 2024 at 11:25:29AM -0600, Jim Fehlig wrote:
On 10/2/24 02:37, Martin Kletzander wrote:
> On Fri, Aug 16, 2024 at 04:25:25PM -0600, Jim Fehlig via Devel wrote:
>> The current description of the various foo_image_format settings can
>> be construded to imply the setting is only used to control compression
>> of the image. Improve the documentation to clarify that format describes
>> the representation of guest memory blocks on disk, which includes
>> compression among other possible layouts.
>>
>> Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
>
> Reviewed-by: Martin Kletzander <mkletzan(a)redhat.com>
Thanks for reviewing this series Martin! Does it imply we can use the
image_format settings to support mapped-ram? It was previously discussed, but no
consensus AFAIK
https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/message/R...
I'm fine with that, I have your series selected for review, but to be
honest I'm getting a bit lost in the linked mail at the moment.
Regards,
Jim
60
days inactive
115
days old
7 comments
2 participants
participants (2)
-
Jim Fehlig -
Martin Kletzander