[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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 redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]