[libvirt] [PATCH 5/7] qemu: Use qemuGetCompressionProgram for error paths

[Chen Hanxiao]

At 2016-09-22 05:30:53, "John Ferlan" <jferlan at redhat.com> 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 | 99 ++++++++++++++++++++------------------------------
> 1 file changed, 40 insertions(+), 59 deletions(-)
>
>diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>index 4cd0a07..505dd29 100644
>--- a/src/qemu/qemu_driver.c
>+++ b/src/qemu/qemu_driver.c
>@@ -3270,6 +3270,9 @@ qemuCompressProgramAvailable(virQEMUSaveFormat compress)
> /* qemuGetCompressionProgram:
>  * @imageFormat: String representation from qemu.conf for the compression
>  *               image format being used (dump, save, or 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
>@@ -3280,7 +3283,8 @@ qemuCompressProgramAvailable(virQEMUSaveFormat compress)
>  *                           indicating none.
>  */
> static virQEMUSaveFormat
>-qemuGetCompressionProgram(const char *imageFormat)
>+qemuGetCompressionProgram(const char *imageFormat,
>+                          bool use_raw_on_fail)
> {
>     virQEMUSaveFormat ret;
> 
>@@ -3296,17 +3300,31 @@ qemuGetCompressionProgram(const char *imageFormat)
>     return ret;
> 
>  error:
>-    if (ret < 0)
>-        VIR_WARN("%s", _("Invalid dump image format specified in "
>-                         "configuration file, using raw"));
>-    else
>-        VIR_WARN("%s", _("Compression program for dump image format "
>-                         "in configuration file isn't available, "
>-                         "using raw"));
>+    if (ret < 0) {
>+        if (use_raw_on_fail)
>+            VIR_WARN("%s", _("Invalid dump image format specified in "
>+                             "configuration file, using raw"));
>+        else
>+            virReportError(VIR_ERR_OPERATION_FAILED, "%s",
>+                           _("Invalid save image format specified "
>+                             "in configuration file"));
>+    } else {
>+        if (use_raw_on_fail)
>+            VIR_WARN("%s", _("Compression program for dump image format "
>+                             "in configuration file isn't available, "
>+                             "using raw"));
>+        else
>+            virReportError(VIR_ERR_OPERATION_FAILED, "%s",
>+                           _("Compression program for image format "
>+                             "in configuration file isn't available"));
>+    }
> 

Actually we had to deal with:
    saveImageFormat
    snapshotImageFormat
    dumpImageFormat
I think a switch case with a macro should be fitted.

Patch 6, 7 looks good to me.

Regards,
- Chen