[libvirt] Re: [PATCH] also allow use of XZ for Qemu image compression

[Jim Meyering]

Jim Meyering wrote:
...
> At Chris' suggestion, I've added a comment warning not to do what I did ;-)
>
> To make it even more obvious that these numbers matter,
> I've assigned explicit constants in the enum:
>
>   +    QEMUD_SAVE_FORMAT_RAW = 0,
>   +    QEMUD_SAVE_FORMAT_GZIP = 1,
>   +    QEMUD_SAVE_FORMAT_BZIP2 = 2,
>
> Currently, if someone accidentally adds this (mistakenly reused number):
>
>   +    QEMUD_SAVE_FORMAT_OOPS = 2,
>
> the compiler won't catch it.
> However, with a small additional change to use VIR_ENUM_DECL
> and VIR_ENUM_IMPL, thus replacing the if/elsif/ chains,
> and the compiler *would* detect that mistake.
> It'd also help to factor out the QEMUD_SAVE_FORMAT_<> to "<>"
> association, rather than having literals like "gzip" and "bzip2"
> in two places.
>
> Now that I've described that, I'll prepare a separate patch.

This depends on the preceding patch.

>From 35ae054dc223a906a07ba65460b0e9cecca46f2c Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at redhat.com>
Date: Wed, 9 Sep 2009 10:10:38 +0200
Subject: [PATCH] qemu_driver.c: factor out duplication in compression-type handling

* src/qemu_driver.c (QEMUD_SAVE_FORMAT_LAST): Define.
(qemudSaveCompressionTypeFromString): Declare.
(qemudSaveCompressionTypeToString): Declare.
(qemudDomainSave): Use those functions rather than open-coding them.
Use "cat >> '%s' ..." in place of equivalent
"dd of='%s' oflag=append conv=notrunc ...".
---
 src/qemu_driver.c |   65 ++++++++++++++++++++++------------------------------
 1 files changed, 28 insertions(+), 37 deletions(-)

diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index 0cdcd98..9a9ae73 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -3628,8 +3628,19 @@ enum qemud_save_formats {
     /* Note: add new members only at the end.
        These values are used in the on-disk format.
        Do not change or re-use numbers. */
+
+    QEMUD_SAVE_FORMAT_LAST
 };

+VIR_ENUM_DECL(qemudSaveCompression)
+VIR_ENUM_IMPL(qemudSaveCompression, QEMUD_SAVE_FORMAT_LAST,
+              "raw",
+              "gzip",
+              "bzip2",
+              "lzma",
+              "lzop",
+              "xz")
+
 struct qemud_save_header {
     char magic[sizeof(QEMUD_SAVE_MAGIC)-1];
     int version;
@@ -3660,22 +3671,15 @@ static int qemudDomainSave(virDomainPtr dom,

     if (driver->saveImageFormat == NULL)
         header.compressed = QEMUD_SAVE_FORMAT_RAW;
-    else if (STREQ(driver->saveImageFormat, "raw"))
-        header.compressed = QEMUD_SAVE_FORMAT_RAW;
-    else if (STREQ(driver->saveImageFormat, "gzip"))
-        header.compressed = QEMUD_SAVE_FORMAT_GZIP;
-    else if (STREQ(driver->saveImageFormat, "bzip2"))
-        header.compressed = QEMUD_SAVE_FORMAT_BZIP2;
-    else if (STREQ(driver->saveImageFormat, "lzma"))
-        header.compressed = QEMUD_SAVE_FORMAT_LZMA;
-    else if (STREQ(driver->saveImageFormat, "lzop"))
-        header.compressed = QEMUD_SAVE_FORMAT_LZOP;
-    else if (STREQ(driver->saveImageFormat, "xz"))
-        header.compressed = QEMUD_SAVE_FORMAT_XZ;
     else {
-        qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
-                         "%s", _("Invalid save image format specified in configuration file"));
-        return -1;
+        header.compressed =
+            qemudSaveCompressionTypeFromString(driver->saveImageFormat);
+        if (header.compressed < 0) {
+            qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+                             "%s", _("Invalid save image format specified "
+                                     "in configuration file"));
+          return -1;
+        }
     }

     qemuDriverLock(driver);
@@ -3751,31 +3755,18 @@ static int qemudDomainSave(virDomainPtr dom,
         goto cleanup;
     }

-    if (header.compressed == QEMUD_SAVE_FORMAT_RAW)
-        internalret = virAsprintf(&command, "migrate \"exec:"
-                                  "dd of='%s' oflag=append conv=notrunc 2>/dev/null"
-                                  "\"", safe_path);
-    else if (header.compressed == QEMUD_SAVE_FORMAT_GZIP)
-        internalret = virAsprintf(&command, "migrate \"exec:"
-                                  "gzip -c >> '%s' 2>/dev/null\"", safe_path);
-    else if (header.compressed == QEMUD_SAVE_FORMAT_BZIP2)
-        internalret = virAsprintf(&command, "migrate \"exec:"
-                                  "bzip2 -c >> '%s' 2>/dev/null\"", safe_path);
-    else if (header.compressed == QEMUD_SAVE_FORMAT_LZMA)
-        internalret = virAsprintf(&command, "migrate \"exec:"
-                                  "lzma -c >> '%s' 2>/dev/null\"", safe_path);
-    else if (header.compressed == QEMUD_SAVE_FORMAT_LZOP)
-        internalret = virAsprintf(&command, "migrate \"exec:"
-                                  "lzop -c >> '%s' 2>/dev/null\"", safe_path);
-    else if (header.compressed == QEMUD_SAVE_FORMAT_XZ)
-        internalret = virAsprintf(&command, "migrate \"exec:"
-                                  "xz -c >> '%s' 2>/dev/null\"", safe_path);
-    else {
+    const char *prog = qemudSaveCompressionTypeToString(header.compressed);
+    if (prog == NULL) {
         qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR,
-                         _("Invalid compress format %d"),
-                         header.compressed);
+                         _("Invalid compress format %d"), header.compressed);
         goto cleanup;
     }
+
+    if (STREQ (prog, "raw"))
+        prog = "cat";
+    internalret = virAsprintf(&command, "migrate \"exec:"
+                              "%s -c >> '%s' 2>/dev/null\"", prog, safe_path);
+
     if (internalret < 0) {
         virReportOOMError(dom->conn);
         goto cleanup;
--
1.6.5.rc0.164.g5f6b0