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@xxxxxxxxxx>
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
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list