2:04 a.m.
The error will never be reported, remove the codes, and also
improve the docs in qemu.conf to tell user the truth.
---
src/qemu/qemu.conf | 4 ++++
src/qemu/qemu_driver.c | 16 +++++-----------
2 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index 145062c..75d945b 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -199,6 +199,10 @@
# save_image_format is used when you use 'virsh save' at scheduled saving.
# dump_image_format is used when you use 'virsh dump' at emergency crashdump.
#
+# If the specified format is not valid (the valid formats are "raw",
"lzop",
+# "gzip", "bzip2", and "xz"), or the compress program is
not available, "raw"
+# will be used as the format silently without error or warning.
+#
# save_image_format = "raw"
# dump_image_format = "raw"
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b928004..48fc46a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2712,19 +2712,13 @@ getCompressionType(struct qemud_driver *driver)
*/
if (driver->dumpImageFormat) {
compress = qemudSaveCompressionTypeFromString(driver->dumpImageFormat);
- if (compress < 0) {
- qemuReportError(VIR_ERR_OPERATION_FAILED, "%s",
- _("Invalid dump image format specified in "
- "configuration file, using raw"));
+ /* Use "raw" as the format if the specified format is not valid,
+ * or the compress program is not available,
+ */
+ if (compress < 0)
return QEMUD_SAVE_FORMAT_RAW;
- }
- if (!qemudCompressProgramAvailable(compress)) {
- qemuReportError(VIR_ERR_OPERATION_FAILED,
- "%s", _("Compression program for dump image
format "
- "in configuration file isn't available,
"
- "using raw"));
+ if (!qemudCompressProgramAvailable(compress))
return QEMUD_SAVE_FORMAT_RAW;
- }
}
return compress;
}
--
1.7.6
10:43 a.m.
New subject: [libvirt] [PATCH] qemu: Remove bogus codes in function getCompressionType
On 07/26/2011 01:04 AM, Osier Yang wrote:
The error will never be reported, remove the codes, and also
improve the docs in qemu.conf to tell user the truth.
---
src/qemu/qemu.conf | 4 ++++
src/qemu/qemu_driver.c | 16 +++++-----------
2 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index 145062c..75d945b 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -199,6 +199,10 @@
# save_image_format is used when you use 'virsh save' at scheduled saving.
# dump_image_format is used when you use 'virsh dump' at emergency crashdump.
#
+# If the specified format is not valid (the valid formats are "raw",
"lzop",
+# "gzip", "bzip2", and "xz"), or the compress program is
not available, "raw"
It's a maintenance nightmare to document the list of compression types
twice, especially when it appears only one paragraph earlier. The
current wording ties the earlier list to only save_image_format, so a
better approach is one that makes it clear that a single list is good
for both variables. Maybe a better wording would be:
# 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 "lzop", "gzip",
"bzip2", or
# "xz" for either image variable. Note that this means you slow down
# the process of saving a domain in order to save disk space; the list
# above is in descending order by performance and ascending order by
# compression ratio.
#
# save_image_format is used when you use 'virsh save' at scheduled
# saving, and it is an error if the requested compression is not found.
#
# dump_image_format is used when you use 'virsh dump' at emergency
# crashdump, and if the requested compression is not found, this falls
# back to "raw" compression.
#
+# will be used as the format silently without error or warning.
Can we at least get a log message? It could be nice when inspecting a
dump file to learn why it was not compressed, by then reading the log
file and seeing a message that:
+ /* Use "raw" as the format if the specified format
is not valid,
+ * or the compress program is not available,
+ */
+ if (compress< 0)
return QEMUD_SAVE_FORMAT_RAW;
the requested qemu.conf format was invalid, or
- }
- if (!qemudCompressProgramAvailable(compress)) {
- qemuReportError(VIR_ERR_OPERATION_FAILED,
- "%s", _("Compression program for dump image
format "
- "in configuration file isn't available,
"
- "using raw"));
+ if (!qemudCompressProgramAvailable(compress))
return QEMUD_SAVE_FORMAT_RAW;
the requested compression is valid, but not found on the machine.
I agree that qemuReportError() is not the right approach, since we are
proceeding with raw as the fallback, but silence instead of a log
message about why we used the fallback is not ideal.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
5:58 a.m.
New subject: [libvirt] [v2] qemu: Improve docs for virsh dump format
The error in getCompressionType will never be reported, change
the errors codes into warning (VIR_WARN("%s", _(foo)); doesn't break
syntax-check rule), and also improve the docs in qemu.conf to tell
user the truth.
---
src/qemu/qemu.conf | 10 ++++++++--
src/qemu/qemu_driver.c | 15 ++++++++-------
2 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index 145062c..7de6863 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -196,8 +196,14 @@
# saving a domain in order to save disk space; the list above is in descending
# order by performance and ascending order by compression ratio.
#
-# save_image_format is used when you use 'virsh save' at scheduled saving.
-# dump_image_format is used when you use 'virsh dump' at emergency crashdump.
+# save_image_format is used when you use 'virsh save' at scheduled
+# saving, and it is an error if the specified save_image_format is
+# not valid, or the according compression program can't be found.
+#
+# 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 according compression program can't be found, this falls
+# back to "raw" compression.
#
# save_image_format = "raw"
# dump_image_format = "raw"
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 0066c55..e9bd421 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2712,17 +2712,18 @@ getCompressionType(struct qemud_driver *driver)
*/
if (driver->dumpImageFormat) {
compress = qemudSaveCompressionTypeFromString(driver->dumpImageFormat);
+ /* Use "raw" as the format if the specified format is not valid,
+ * or the compress program is not available,
+ */
if (compress < 0) {
- qemuReportError(VIR_ERR_OPERATION_FAILED, "%s",
- _("Invalid dump image format specified in "
- "configuration file, using raw"));
+ VIR_WARN("%s", _("Invalid dump image format specified in
"
+ "configuration file, using raw"));
return QEMUD_SAVE_FORMAT_RAW;
}
if (!qemudCompressProgramAvailable(compress)) {
- qemuReportError(VIR_ERR_OPERATION_FAILED,
- "%s", _("Compression program for dump image
format "
- "in configuration file isn't available,
"
- "using raw"));
+ VIR_WARN("%s", _("Compression program for dump image format
"
+ "in configuration file isn't available, "
+ "using raw"));
return QEMUD_SAVE_FORMAT_RAW;
}
}
--
1.7.6
11:18 a.m.
New subject: [libvirt] [v2] qemu: Improve docs for virsh dump format
On 07/27/2011 04:58 AM, Osier Yang wrote:
The error in getCompressionType will never be reported, change
the errors codes into warning (VIR_WARN("%s", _(foo)); doesn't break
syntax-check rule), and also improve the docs in qemu.conf to tell
user the truth.
---
src/qemu/qemu.conf | 10 ++++++++--
src/qemu/qemu_driver.c | 15 ++++++++-------
2 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index 145062c..7de6863 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -196,8 +196,14 @@
# saving a domain in order to save disk space; the list above is in descending
# order by performance and ascending order by compression ratio.
#
-# save_image_format is used when you use 'virsh save' at scheduled saving.
-# dump_image_format is used when you use 'virsh dump' at emergency crashdump.
+# save_image_format is used when you use 'virsh save' at scheduled
+# saving, and it is an error if the specified save_image_format is
+# not valid, or the according compression program can't be found.
s/according/requested/
+#
+# 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 according compression program can't be found, this falls
s/according/requested/
+++ b/src/qemu/qemu_driver.c
@@ -2712,17 +2712,18 @@ getCompressionType(struct qemud_driver *driver)
*/
if (driver->dumpImageFormat) {
compress = qemudSaveCompressionTypeFromString(driver->dumpImageFormat);
+ /* Use "raw" as the format if the specified format is not valid,
+ * or the compress program is not available,
+ */
s/,/./ at the comment end.
ACK with those nits fixed.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
8:42 p.m.
New subject: [libvirt] [v2] qemu: Improve docs for virsh dump format
于 2011年07月28日 00:18, Eric Blake 写道:
On 07/27/2011 04:58 AM, Osier Yang wrote:
> The error in getCompressionType will never be reported, change
> the errors codes into warning (VIR_WARN("%s", _(foo)); doesn't break
> syntax-check rule), and also improve the docs in qemu.conf to tell
> user the truth.
> ---
> src/qemu/qemu.conf | 10 ++++++++--
> src/qemu/qemu_driver.c | 15 ++++++++-------
> 2 files changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
> index 145062c..7de6863 100644
> --- a/src/qemu/qemu.conf
> +++ b/src/qemu/qemu.conf
> @@ -196,8 +196,14 @@
> # saving a domain in order to save disk space; the list above is in
> descending
> # order by performance and ascending order by compression ratio.
> #
> -# save_image_format is used when you use 'virsh save' at scheduled
> saving.
> -# dump_image_format is used when you use 'virsh dump' at emergency
> crashdump.
> +# save_image_format is used when you use 'virsh save' at scheduled
> +# saving, and it is an error if the specified save_image_format is
> +# not valid, or the according compression program can't be found.
s/according/requested/
> +#
> +# 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 according compression program can't be found, this falls
s/according/requested/
> +++ b/src/qemu/qemu_driver.c
> @@ -2712,17 +2712,18 @@ getCompressionType(struct qemud_driver *driver)
> */
> if (driver->dumpImageFormat) {
> compress =
> qemudSaveCompressionTypeFromString(driver->dumpImageFormat);
> + /* Use "raw" as the format if the specified format is not
> valid,
> + * or the compress program is not available,
> + */
s/,/./ at the comment end.
ACK with those nits fixed.
Thanks, pushed with the fixing.
Osier
4866
days inactive
4868
days old
4 comments
2 participants
participants (2)
-
Eric Blake -
Osier Yang