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

[Ján Tomko]

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 at redhat.com>
>---
> 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20160927/7c4473e1/attachment-0001.sig>