Re: [PATCH v2 6/8] qemu: Use qemuGetCompressionProgram for error paths

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

 



On Fri, Sep 23, 2016 at 07:30:54AM -0400, John Ferlan wrote:
Let's do some more code reuse - there are 3 other callers that care to
check/get the compress program. Each of those though cares whether the
requested cfg image is valid and exists. So, add a parameter to handle
those cases.

NB: We won't need to initialize the returned value in the case where
the cfg image doesn't exist since the called program will handle that.

Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
---
src/qemu/qemu_driver.c | 103 +++++++++++++++++++++----------------------------
1 file changed, 43 insertions(+), 60 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7dfba50..8a47262 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3271,6 +3271,9 @@ qemuCompressProgramAvailable(virQEMUSaveFormat compress)
 * @imageFormat: String representation from qemu.conf for the compression
 *               image format being used (dump, save, or snapshot).
 * @styleFormat: String representing the style of format (dump, save, snapshot)
+ * @use_raw_on_fail: Boolean indicating how to handle the error path. For
+ *                   callers that are OK with invalid data or inability to
+ *                   find the compression program, just return a raw format
 *
 * Returns:
 *    virQEMUSaveFormat    - Integer representation of the compression
@@ -3282,7 +3285,8 @@ qemuCompressProgramAvailable(virQEMUSaveFormat compress)
 */
static virQEMUSaveFormat
qemuGetCompressionProgram(const char *imageFormat,
-                          const char *styleFormat)
+                          const char *styleFormat,
+                          bool use_raw_on_fail)
{
    virQEMUSaveFormat ret;

@@ -3298,18 +3302,34 @@ qemuGetCompressionProgram(const char *imageFormat,
    return ret;

 error:
-    if (ret < 0)
-        VIR_WARN("Invalid %s image format specified in "
-                 "configuration file, using raw",
-                 styleFormat);
-    else
-        VIR_WARN("Compression program for %s image format in "
-                 "configuration file isn't available, using raw",
-                 styleFormat);
+    if (ret < 0) {
+        if (use_raw_on_fail)
+            VIR_WARN("Invalid %s image format specified in "
+                     "configuration file, using raw",
+                     styleFormat);
+        else
+            virReportError(VIR_ERR_OPERATION_FAILED,
+                           _("Invalid %s image format specified "
+                             "in configuration file"),
+                           styleFormat);
+    } else {
+        if (use_raw_on_fail)
+            VIR_WARN("Compression program for %s image format in "
+                     "configuration file isn't available, using raw",
+                     styleFormat);
+        else
+            virReportError(VIR_ERR_OPERATION_FAILED,
+                           _("Compression program for %s image format "
+                             "in configuration file isn't available"),
+                           styleFormat);
+    }

This might work for the English language, but constructing a translatable
message in this matter is unfriendly to translators and not worth
shaving off a few lines of code.

Also, logging the "styleFormat" (sic) only makes sense for the dump,
which might not have been a result of an API call, otherwise we're
better off putting the image format in here.

If you call the snapshot API and get an error saying "xz" was not found,
you know it's not for the core dump format.

Jan

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]