Re: [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