[libvirt] [PATCH] also allow use of XZ for Qemu image compression
While this patch stays minimal by simply adding XZ/xz to the list, I think it would be better to remove lzma, since it uses an inferior format (which lacks an integrity check), and has been effectively subsumed by xz. Let me know if you'd like that, and I'll prepare the slightly more invasive patch.
From afb7275f14e8062e6d5eebf704e1fd0f582cb407 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Tue, 8 Sep 2009 20:52:37 +0200 Subject: [PATCH] also allow use of XZ for Qemu image compression
* src/qemu_driver.c (enum qemud_save_formats) [QEMUD_SAVE_FORMAT_XZ]: New member. [QEMUD_SAVE_FORMAT_LZMA]: Mark as deprecated. (qemudDomainSave, qemudDomainRestore): Handle the new member. * src/qemu.conf: Mention xz, too. --- src/qemu.conf | 2 +- src/qemu_driver.c | 10 +++++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/qemu.conf b/src/qemu.conf index 06babc4..342bb8a 100644 --- a/src/qemu.conf +++ b/src/qemu.conf @@ -134,7 +134,7 @@ # memory from the domain is dumped out directly to a file. If you have # guests with a large amount of memory, however, this can take up quite # a bit of space. If you would like to compress the images while they -# are being saved to disk, you can also set "gzip", "bzip2", "lzma" +# are being saved to disk, you can also set "gzip", "bzip2", "lzma", "xz", # or "lzop" for save_image_format. Note that this means you slow down # the process of saving a domain in order to save disk space. # diff --git a/src/qemu_driver.c b/src/qemu_driver.c index f64d70b..7b64712 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -3622,7 +3622,8 @@ enum qemud_save_formats { QEMUD_SAVE_FORMAT_RAW, QEMUD_SAVE_FORMAT_GZIP, QEMUD_SAVE_FORMAT_BZIP2, - QEMUD_SAVE_FORMAT_LZMA, + QEMUD_SAVE_FORMAT_LZMA, /* deprecated, in favor of xz */ + QEMUD_SAVE_FORMAT_XZ, QEMUD_SAVE_FORMAT_LZOP, }; @@ -3664,6 +3665,8 @@ static int qemudDomainSave(virDomainPtr dom, header.compressed = QEMUD_SAVE_FORMAT_BZIP2; else if (STREQ(driver->saveImageFormat, "lzma")) header.compressed = QEMUD_SAVE_FORMAT_LZMA; + else if (STREQ(driver->saveImageFormat, "xz")) + header.compressed = QEMUD_SAVE_FORMAT_XZ; else if (STREQ(driver->saveImageFormat, "lzop")) header.compressed = QEMUD_SAVE_FORMAT_LZOP; else { @@ -3758,6 +3761,9 @@ static int qemudDomainSave(virDomainPtr dom, 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_XZ) + internalret = virAsprintf(&command, "migrate \"exec:" + "xz -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); @@ -4383,6 +4389,8 @@ static int qemudDomainRestore(virConnectPtr conn, intermediate_argv[0] = "bzip2"; else if (header.compressed == QEMUD_SAVE_FORMAT_LZMA) intermediate_argv[0] = "lzma"; + else if (header.compressed == QEMUD_SAVE_FORMAT_XZ) + intermediate_argv[0] = "xz"; else if (header.compressed == QEMUD_SAVE_FORMAT_LZOP) intermediate_argv[0] = "lzop"; else if (header.compressed != QEMUD_SAVE_FORMAT_RAW) { -- 1.6.5.rc0.164.g5f6b0
Jim Meyering wrote:
While this patch stays minimal by simply adding XZ/xz to the list, I think it would be better to remove lzma, since it uses an inferior format (which lacks an integrity check), and has been effectively subsumed by xz.
Let me know if you'd like that, and I'll prepare the slightly more invasive patch.
I'm on the fence about it. While I do understand the situation now (thanks for explaining), I think keeping lzma for compatibility with older distros might be a good idea. Either way, we have to keep the LZMA slot in the enum "free", since it's part of the on-disk ABI for the save format. And on that note...
diff --git a/src/qemu_driver.c b/src/qemu_driver.c index f64d70b..7b64712 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -3622,7 +3622,8 @@ enum qemud_save_formats { QEMUD_SAVE_FORMAT_RAW, QEMUD_SAVE_FORMAT_GZIP, QEMUD_SAVE_FORMAT_BZIP2, - QEMUD_SAVE_FORMAT_LZMA, + QEMUD_SAVE_FORMAT_LZMA, /* deprecated, in favor of xz */ + QEMUD_SAVE_FORMAT_XZ, QEMUD_SAVE_FORMAT_LZOP, };
You'll need to add QEMUD_SAVE_FORMAT_XZ to the end of the enum, to maintain on-disk compatibility. Otherwise it looks good. -- Chris Lalancette
Chris Lalancette wrote:
Jim Meyering wrote:
While this patch stays minimal by simply adding XZ/xz to the list, I think it would be better to remove lzma, since it uses an inferior format (which lacks an integrity check), and has been effectively subsumed by xz.
Let me know if you'd like that, and I'll prepare the slightly more invasive patch.
I'm on the fence about it. While I do understand the situation now (thanks for explaining), I think keeping lzma for compatibility with older distros might be a good idea. Either way, we have to keep the LZMA slot in the enum "free", since it's part of the on-disk ABI for the save format. And on that note...
diff --git a/src/qemu_driver.c b/src/qemu_driver.c index f64d70b..7b64712 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -3622,7 +3622,8 @@ enum qemud_save_formats { QEMUD_SAVE_FORMAT_RAW, QEMUD_SAVE_FORMAT_GZIP, QEMUD_SAVE_FORMAT_BZIP2, - QEMUD_SAVE_FORMAT_LZMA, + QEMUD_SAVE_FORMAT_LZMA, /* deprecated, in favor of xz */ + QEMUD_SAVE_FORMAT_XZ, QEMUD_SAVE_FORMAT_LZOP, };
You'll need to add QEMUD_SAVE_FORMAT_XZ to the end of the enum, to maintain on-disk compatibility. Otherwise it looks good.
Thanks. Good point. Here's an amended patch: Note, I've reordered the if/else placement to match enum member ordering, too.
From 24309516094b058cce9f5eb123877bd5eec2cfc2 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Tue, 8 Sep 2009 20:52:37 +0200 Subject: [PATCH] also allow use of XZ for Qemu image compression
* src/qemu_driver.c (enum qemud_save_formats) [QEMUD_SAVE_FORMAT_XZ]: New member. [QEMUD_SAVE_FORMAT_LZMA]: Mark as deprecated. (qemudDomainSave, qemudDomainRestore): Handle the new member. * src/qemu.conf: Mention xz, too. --- src/qemu.conf | 2 +- src/qemu_driver.c | 10 +++++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/qemu.conf b/src/qemu.conf index 06babc4..342bb8a 100644 --- a/src/qemu.conf +++ b/src/qemu.conf @@ -134,7 +134,7 @@ # memory from the domain is dumped out directly to a file. If you have # guests with a large amount of memory, however, this can take up quite # a bit of space. If you would like to compress the images while they -# are being saved to disk, you can also set "gzip", "bzip2", "lzma" +# are being saved to disk, you can also set "gzip", "bzip2", "lzma", "xz", # or "lzop" for save_image_format. Note that this means you slow down # the process of saving a domain in order to save disk space. # diff --git a/src/qemu_driver.c b/src/qemu_driver.c index f64d70b..99b3390 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -3622,8 +3622,9 @@ enum qemud_save_formats { QEMUD_SAVE_FORMAT_RAW, QEMUD_SAVE_FORMAT_GZIP, QEMUD_SAVE_FORMAT_BZIP2, - QEMUD_SAVE_FORMAT_LZMA, + QEMUD_SAVE_FORMAT_LZMA, /* deprecated, in favor of xz */ QEMUD_SAVE_FORMAT_LZOP, + QEMUD_SAVE_FORMAT_XZ, }; struct qemud_save_header { @@ -3666,6 +3667,8 @@ static int qemudDomainSave(virDomainPtr dom, 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")); @@ -3761,6 +3764,9 @@ static int qemudDomainSave(virDomainPtr dom, 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 { qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR, _("Invalid compress format %d"), @@ -4385,6 +4391,8 @@ static int qemudDomainRestore(virConnectPtr conn, intermediate_argv[0] = "lzma"; else if (header.compressed == QEMUD_SAVE_FORMAT_LZOP) intermediate_argv[0] = "lzop"; + else if (header.compressed == QEMUD_SAVE_FORMAT_XZ) + intermediate_argv[0] = "xz"; else if (header.compressed != QEMUD_SAVE_FORMAT_RAW) { qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, _("Unknown compressed save format %d"), -- 1.6.5.rc0.164.g5f6b0
Jim Meyering wrote:
Chris Lalancette wrote:
Jim Meyering wrote:
While this patch stays minimal by simply adding XZ/xz to the list, I think it would be better to remove lzma, since it uses an inferior format (which lacks an integrity check), and has been effectively subsumed by xz.
Let me know if you'd like that, and I'll prepare the slightly more invasive patch.
I'm on the fence about it. While I do understand the situation now (thanks for explaining), I think keeping lzma for compatibility with older distros might be a good idea. Either way, we have to keep the LZMA slot in the enum "free", since it's part of the on-disk ABI for the save format. And on that note...
diff --git a/src/qemu_driver.c b/src/qemu_driver.c index f64d70b..7b64712 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -3622,7 +3622,8 @@ enum qemud_save_formats { QEMUD_SAVE_FORMAT_RAW, QEMUD_SAVE_FORMAT_GZIP, QEMUD_SAVE_FORMAT_BZIP2, - QEMUD_SAVE_FORMAT_LZMA, + QEMUD_SAVE_FORMAT_LZMA, /* deprecated, in favor of xz */ + QEMUD_SAVE_FORMAT_XZ, QEMUD_SAVE_FORMAT_LZOP, };
You'll need to add QEMUD_SAVE_FORMAT_XZ to the end of the enum, to maintain on-disk compatibility. Otherwise it looks good.
Thanks. Good point. Here's an amended patch: Note, I've reordered the if/else placement to match enum member ordering, too. ...
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. For now, here's the added comment and enum initializers.
From ef653538806f981ca6ca5bb270e7a8fb1618f6d4 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Tue, 8 Sep 2009 20:52:37 +0200 Subject: [PATCH] also allow use of XZ for Qemu image compression
* src/qemu_driver.c (enum qemud_save_formats) [QEMUD_SAVE_FORMAT_XZ]: New member. [QEMUD_SAVE_FORMAT_LZMA]: Mark as deprecated. Use an explicit value for each member. (qemudDomainSave, qemudDomainRestore): Handle the new member. * src/qemu.conf: Mention xz, too. --- src/qemu.conf | 2 +- src/qemu_driver.c | 21 ++++++++++++++++----- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/src/qemu.conf b/src/qemu.conf index 06babc4..342bb8a 100644 --- a/src/qemu.conf +++ b/src/qemu.conf @@ -134,7 +134,7 @@ # memory from the domain is dumped out directly to a file. If you have # guests with a large amount of memory, however, this can take up quite # a bit of space. If you would like to compress the images while they -# are being saved to disk, you can also set "gzip", "bzip2", "lzma" +# are being saved to disk, you can also set "gzip", "bzip2", "lzma", "xz", # or "lzop" for save_image_format. Note that this means you slow down # the process of saving a domain in order to save disk space. # diff --git a/src/qemu_driver.c b/src/qemu_driver.c index f64d70b..0cdcd98 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -3619,11 +3619,15 @@ static char *qemudEscapeShellArg(const char *in) #define QEMUD_SAVE_VERSION 2 enum qemud_save_formats { - QEMUD_SAVE_FORMAT_RAW, - QEMUD_SAVE_FORMAT_GZIP, - QEMUD_SAVE_FORMAT_BZIP2, - QEMUD_SAVE_FORMAT_LZMA, - QEMUD_SAVE_FORMAT_LZOP, + QEMUD_SAVE_FORMAT_RAW = 0, + QEMUD_SAVE_FORMAT_GZIP = 1, + QEMUD_SAVE_FORMAT_BZIP2 = 2, + QEMUD_SAVE_FORMAT_LZMA = 3, /* deprecated, in favor of xz */ + QEMUD_SAVE_FORMAT_LZOP = 4, + QEMUD_SAVE_FORMAT_XZ = 5, + /* Note: add new members only at the end. + These values are used in the on-disk format. + Do not change or re-use numbers. */ }; struct qemud_save_header { @@ -3666,6 +3670,8 @@ static int qemudDomainSave(virDomainPtr dom, 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")); @@ -3761,6 +3767,9 @@ static int qemudDomainSave(virDomainPtr dom, 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 { qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR, _("Invalid compress format %d"), @@ -4385,6 +4394,8 @@ static int qemudDomainRestore(virConnectPtr conn, intermediate_argv[0] = "lzma"; else if (header.compressed == QEMUD_SAVE_FORMAT_LZOP) intermediate_argv[0] = "lzop"; + else if (header.compressed == QEMUD_SAVE_FORMAT_XZ) + intermediate_argv[0] = "xz"; else if (header.compressed != QEMUD_SAVE_FORMAT_RAW) { qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, _("Unknown compressed save format %d"), -- 1.6.5.rc0.164.g5f6b0
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. For now, here's the added comment and enum initializers.
ACK, this looks good, and your above cleanups also sound like they are good ideas. -- Chris Lalancette
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@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
On Wed, Sep 09, 2009 at 10:17:48AM +0200, Jim Meyering wrote:
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.
yup using VIR_ENUM_IMPL, and VIR_ENUM_DECL is a good cleanup, could you apply this to the current git, we will then reduce options later, ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
Daniel Veillard wrote:
On Wed, Sep 09, 2009 at 10:17:48AM +0200, Jim Meyering wrote:
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.
yup using VIR_ENUM_IMPL, and VIR_ENUM_DECL is a good cleanup, could you apply this to the current git, we will then reduce options later,
ACK,
Thanks. Pushed that, along with the other ack'd changes. Note that that includes the one to use strchrnul, which required updated .gnulib/. And a .gnulib/ update requires that anyone building re-run ./autogen.sh. But don't worry: if anyone forgets, "make" will fail with a diagnostic telling them to do just that.
On Wed, Sep 09, 2009 at 09:24:17AM +0200, Jim Meyering wrote:
Jim Meyering wrote:
Chris Lalancette wrote:
Jim Meyering wrote:
While this patch stays minimal by simply adding XZ/xz to the list, I think it would be better to remove lzma, since it uses an inferior format (which lacks an integrity check), and has been effectively subsumed by xz.
Let me know if you'd like that, and I'll prepare the slightly more invasive patch.
I'm on the fence about it. While I do understand the situation now (thanks for explaining), I think keeping lzma for compatibility with older distros might be a good idea. Either way, we have to keep the LZMA slot in the enum "free", since it's part of the on-disk ABI for the save format. And on that note...
diff --git a/src/qemu_driver.c b/src/qemu_driver.c index f64d70b..7b64712 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -3622,7 +3622,8 @@ enum qemud_save_formats { QEMUD_SAVE_FORMAT_RAW, QEMUD_SAVE_FORMAT_GZIP, QEMUD_SAVE_FORMAT_BZIP2, - QEMUD_SAVE_FORMAT_LZMA, + QEMUD_SAVE_FORMAT_LZMA, /* deprecated, in favor of xz */ + QEMUD_SAVE_FORMAT_XZ, QEMUD_SAVE_FORMAT_LZOP, };
You'll need to add QEMUD_SAVE_FORMAT_XZ to the end of the enum, to maintain on-disk compatibility. Otherwise it looks good.
Thanks. Good point. Here's an amended patch: Note, I've reordered the if/else placement to match enum member ordering, too. ...
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. For now, here's the added comment and enum initializers.
From ef653538806f981ca6ca5bb270e7a8fb1618f6d4 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Tue, 8 Sep 2009 20:52:37 +0200 Subject: [PATCH] also allow use of XZ for Qemu image compression
* src/qemu_driver.c (enum qemud_save_formats) [QEMUD_SAVE_FORMAT_XZ]: New member. [QEMUD_SAVE_FORMAT_LZMA]: Mark as deprecated. Use an explicit value for each member. (qemudDomainSave, qemudDomainRestore): Handle the new member. * src/qemu.conf: Mention xz, too. --- src/qemu.conf | 2 +- src/qemu_driver.c | 21 ++++++++++++++++----- 2 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/src/qemu.conf b/src/qemu.conf index 06babc4..342bb8a 100644 --- a/src/qemu.conf +++ b/src/qemu.conf @@ -134,7 +134,7 @@ # memory from the domain is dumped out directly to a file. If you have # guests with a large amount of memory, however, this can take up quite # a bit of space. If you would like to compress the images while they -# are being saved to disk, you can also set "gzip", "bzip2", "lzma" +# are being saved to disk, you can also set "gzip", "bzip2", "lzma", "xz", # or "lzop" for save_image_format. Note that this means you slow down # the process of saving a domain in order to save disk space. # diff --git a/src/qemu_driver.c b/src/qemu_driver.c index f64d70b..0cdcd98 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -3619,11 +3619,15 @@ static char *qemudEscapeShellArg(const char *in) #define QEMUD_SAVE_VERSION 2
enum qemud_save_formats { - QEMUD_SAVE_FORMAT_RAW, - QEMUD_SAVE_FORMAT_GZIP, - QEMUD_SAVE_FORMAT_BZIP2, - QEMUD_SAVE_FORMAT_LZMA, - QEMUD_SAVE_FORMAT_LZOP, + QEMUD_SAVE_FORMAT_RAW = 0, + QEMUD_SAVE_FORMAT_GZIP = 1, + QEMUD_SAVE_FORMAT_BZIP2 = 2, + QEMUD_SAVE_FORMAT_LZMA = 3, /* deprecated, in favor of xz */ + QEMUD_SAVE_FORMAT_LZOP = 4, + QEMUD_SAVE_FORMAT_XZ = 5, + /* Note: add new members only at the end. + These values are used in the on-disk format. + Do not change or re-use numbers. */ };
struct qemud_save_header { @@ -3666,6 +3670,8 @@ static int qemudDomainSave(virDomainPtr dom, 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")); @@ -3761,6 +3767,9 @@ static int qemudDomainSave(virDomainPtr dom, 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 { qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR, _("Invalid compress format %d"), @@ -4385,6 +4394,8 @@ static int qemudDomainRestore(virConnectPtr conn, intermediate_argv[0] = "lzma"; else if (header.compressed == QEMUD_SAVE_FORMAT_LZOP) intermediate_argv[0] = "lzop"; + else if (header.compressed == QEMUD_SAVE_FORMAT_XZ) + intermediate_argv[0] = "xz"; else if (header.compressed != QEMUD_SAVE_FORMAT_RAW) { qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED, _("Unknown compressed save format %d"),
Hum ... in practice I think it adds a new dependancy to libvirt. We didn't raise that for the lzop patch but if we start to exec binaries we should check for their presence at runtime somehow. I'm not against the patch, actually I have xz on my machine but not lzop, but the whole code right now looks quite fragile, we don't offer the capacity to detect what compressors are available on the Node and just leave the API user to a game of try/failure checking. That doesn't sounds too good to me. One way is to add the dependancy at the packaging level (at least libvirt.spec.in for now) but right now requesting both lzop and xz is just bad IMHO, so some kind of runtime detection should be done. For gzip and bzip2 this wasn't really a probem because they are really ubiquitous, but now we are suggesting 2 options which are very likely to not be both present. I dislike the fact that we are lowering the API reliability for an optimization in disk space usage. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On Wed, Sep 09, 2009 at 10:49:46AM +0200, Daniel Veillard wrote:
> On Wed, Sep 09, 2009 at 09:24:17AM +0200, Jim Meyering wrote:
> > >>> diff --git a/src/qemu_driver.c b/src/qemu_driver.c
> > >>> index f64d70b..7b64712 100644
> > >>> --- a/src/qemu_driver.c
> > >>> +++ b/src/qemu_driver.c
> > >>> @@ -3622,7 +3622,8 @@ enum qemud_save_formats {
> > >>> QEMUD_SAVE_FORMAT_RAW,
> > >>> QEMUD_SAVE_FORMAT_GZIP,
> > >>> QEMUD_SAVE_FORMAT_BZIP2,
> > >>> - QEMUD_SAVE_FORMAT_LZMA,
> > >>> + QEMUD_SAVE_FORMAT_LZMA, /* deprecated, in favor of xz */
> > >>> + QEMUD_SAVE_FORMAT_XZ,
> > >>> QEMUD_SAVE_FORMAT_LZOP,
> > >>> };
> > >>
> > >> You'll need to add QEMUD_SAVE_FORMAT_XZ to the end of the enum, to maintain
> > >> on-disk compatibility. Otherwise it looks good.
[...]
> > qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR,
> > _("Invalid compress format %d"),
> > @@ -4385,6 +4394,8 @@ static int qemudDomainRestore(virConnectPtr conn,
> > intermediate_argv[0] = "lzma";
> > else if (header.compressed == QEMUD_SAVE_FORMAT_LZOP)
> > intermediate_argv[0] = "lzop";
> > + else if (header.compressed == QEMUD_SAVE_FORMAT_XZ)
> > + intermediate_argv[0] = "xz";
> > else if (header.compressed != QEMUD_SAVE_FORMAT_RAW) {
> > qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
> > _("Unknown compressed save format %d"),
>
> Hum ... in practice I think it adds a new dependancy to libvirt. We
> didn't raise that for the lzop patch but if we start to exec binaries
> we should check for their presence at runtime somehow.
> I'm not against the patch, actually I have xz on my machine but not
> lzop, but the whole code right now looks quite fragile, we don't offer
> the capacity to detect what compressors are available on the Node and
> just leave the API user to a game of try/failure checking. That doesn't
> sounds too good to me. One way is to add the dependancy at the packaging
> level (at least libvirt.spec.in for now) but right now requesting both
> lzop and xz is just bad IMHO, so some kind of runtime detection should
> be done. For gzip and bzip2 this wasn't really a probem because they are
> really ubiquitous, but now we are suggesting 2 options which are very
> likely to not be both present.
>
> I dislike the fact that we are lowering the API reliability for an
> optimization in disk space usage.
Hum, I realize that support of LZOP was added after 0.7.0, so we never
made a release with it (well except for git snapshot which may have been
pushed).
I wonder if the best is not to just drop the lzop option altogether
and stick xz as a package dependancy until we have found a way to
provide at the API level which compression options are actually
available.
Opinions ?
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
Daniel Veillard wrote:
Hum, I realize that support of LZOP was added after 0.7.0, so we never made a release with it (well except for git snapshot which may have been pushed). I wonder if the best is not to just drop the lzop option altogether and stick xz as a package dependancy until we have found a way to provide at the API level which compression options are actually available.
Opinions ?
Dropping lzop sounds good. It seems lzop is not very popular. We don't need that many choices. Maybe even nuke lzma too before we're stuck with it forever. Technically, we can do that, since it was added only a month ago, also after 0.7.0: v0.7.0-35-g2d6a581
On Wed, Sep 09, 2009 at 11:57:40AM +0200, Jim Meyering wrote:
Daniel Veillard wrote:
Hum, I realize that support of LZOP was added after 0.7.0, so we never made a release with it (well except for git snapshot which may have been pushed). I wonder if the best is not to just drop the lzop option altogether and stick xz as a package dependancy until we have found a way to provide at the API level which compression options are actually available.
Opinions ?
Dropping lzop sounds good. It seems lzop is not very popular. We don't need that many choices.
Maybe even nuke lzma too before we're stuck with it forever. Technically, we can do that, since it was added only a month ago, also after 0.7.0:
v0.7.0-35-g2d6a581
Actually om my machine here lzma is provided as a backward compat option by xz, so yes I'm inclined to remove that option too: aphio:~ -> which lzma /usr/bin/lzma paphio:~ -> rpm -qf /usr/bin/lzma xz-lzma-compat-4.999.8-0.8.beta.20090817git.fc11.x86_64 I will post a patch later, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
Daniel Veillard wrote:
On Wed, Sep 09, 2009 at 11:57:40AM +0200, Jim Meyering wrote:
Daniel Veillard wrote:
Hum, I realize that support of LZOP was added after 0.7.0, so we never made a release with it (well except for git snapshot which may have been pushed). I wonder if the best is not to just drop the lzop option altogether and stick xz as a package dependancy until we have found a way to provide at the API level which compression options are actually available.
Opinions ?
Dropping lzop sounds good. It seems lzop is not very popular. We don't need that many choices.
Maybe even nuke lzma too before we're stuck with it forever. Technically, we can do that, since it was added only a month ago, also after 0.7.0:
v0.7.0-35-g2d6a581
Actually om my machine here lzma is provided as a backward compat option by xz, so yes I'm inclined to remove that option too:
aphio:~ -> which lzma /usr/bin/lzma paphio:~ -> rpm -qf /usr/bin/lzma xz-lzma-compat-4.999.8-0.8.beta.20090817git.fc11.x86_64
I will post a patch later,
FYI, xz can decompress lzma-compressed input: $ echo foooo |lzma -c|xz -dc foooo so libvirt won't need the "lzma" command, as long as xz is available.
On Wed, Sep 09, 2009 at 12:08:13PM +0200, Jim Meyering wrote:
Daniel Veillard wrote:
On Wed, Sep 09, 2009 at 11:57:40AM +0200, Jim Meyering wrote:
Daniel Veillard wrote:
Hum, I realize that support of LZOP was added after 0.7.0, so we never made a release with it (well except for git snapshot which may have been pushed). I wonder if the best is not to just drop the lzop option altogether and stick xz as a package dependancy until we have found a way to provide at the API level which compression options are actually available.
Opinions ?
Dropping lzop sounds good. It seems lzop is not very popular. We don't need that many choices.
Maybe even nuke lzma too before we're stuck with it forever. Technically, we can do that, since it was added only a month ago, also after 0.7.0:
v0.7.0-35-g2d6a581
Actually om my machine here lzma is provided as a backward compat option by xz, so yes I'm inclined to remove that option too:
aphio:~ -> which lzma /usr/bin/lzma paphio:~ -> rpm -qf /usr/bin/lzma xz-lzma-compat-4.999.8-0.8.beta.20090817git.fc11.x86_64
I will post a patch later,
FYI, xz can decompress lzma-compressed input:
$ echo foooo |lzma -c|xz -dc foooo
so libvirt won't need the "lzma" command, as long as xz is available.
Okay, I suggest the following patch removing the 2 extra compressors and making sure the package including the daemon, if compiled with qemu has the proper dependancies. xz package dependancy is IMHO a small price to pay: Size: 443012 to garantee reliability and bzip2 and gzip are so standard that it should not be a problem for anybody to list them explicitely. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
Daniel Veillard wrote:
On Wed, Sep 09, 2009 at 12:08:13PM +0200, Jim Meyering wrote:
Daniel Veillard wrote:
On Wed, Sep 09, 2009 at 11:57:40AM +0200, Jim Meyering wrote:
Daniel Veillard wrote:
Hum, I realize that support of LZOP was added after 0.7.0, so we never made a release with it (well except for git snapshot which may have been pushed). I wonder if the best is not to just drop the lzop option altogether and stick xz as a package dependancy until we have found a way to provide at the API level which compression options are actually available.
Opinions ?
Dropping lzop sounds good. It seems lzop is not very popular. We don't need that many choices.
Maybe even nuke lzma too before we're stuck with it forever. Technically, we can do that, since it was added only a month ago, also after 0.7.0:
v0.7.0-35-g2d6a581
Actually om my machine here lzma is provided as a backward compat option by xz, so yes I'm inclined to remove that option too:
aphio:~ -> which lzma /usr/bin/lzma paphio:~ -> rpm -qf /usr/bin/lzma xz-lzma-compat-4.999.8-0.8.beta.20090817git.fc11.x86_64
I will post a patch later,
FYI, xz can decompress lzma-compressed input:
$ echo foooo |lzma -c|xz -dc foooo
so libvirt won't need the "lzma" command, as long as xz is available.
Okay, I suggest the following patch removing the 2 extra compressors and making sure the package including the daemon, if compiled with qemu has the proper dependancies. xz package dependancy is IMHO a small price to pay: Size: 443012 to garantee reliability and bzip2 and gzip are so standard that it should not be a problem for anybody to list them explicitely.
The changes to the C code look fine to me. However seeing the added dependency on xz makes me wish for a "strongly-recommended" sort of tag, rather than a hard dependency on a tool that one may never opt to use. ACK, in spite of that.
On Wed, Sep 09, 2009 at 04:16:59PM +0200, Jim Meyering wrote:
Daniel Veillard wrote:
Okay, I suggest the following patch removing the 2 extra compressors and making sure the package including the daemon, if compiled with qemu has the proper dependancies. xz package dependancy is IMHO a small price to pay: Size: 443012 to garantee reliability and bzip2 and gzip are so standard that it should not be a problem for anybody to list them explicitely.
The changes to the C code look fine to me. However seeing the added dependency on xz makes me wish for a "strongly-recommended" sort of tag, rather than a hard dependency on a tool that one may never opt to use.
Hum ... let's not start to discuss core rpm decisions :) I would say that we drag an impressive list of dependancies on other libs or low level tools and I doubt any deployement will ever be able to use them all. Honnestly xz is really one of the less intrusive of the lot !
ACK, in spite of that.
Thanks, pushed ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
Jim Meyering wrote:
Daniel Veillard wrote:
Hum, I realize that support of LZOP was added after 0.7.0, so we never made a release with it (well except for git snapshot which may have been pushed). I wonder if the best is not to just drop the lzop option altogether and stick xz as a package dependancy until we have found a way to provide at the API level which compression options are actually available.
Opinions ?
Dropping lzop sounds good. It seems lzop is not very popular. We don't need that many choices.
lzop may not be popular, but it is distinct -- a minimum of 3x faster than every other compressor offered as an option. As the decision to use compression at all is offered as a disk space vs performance tradeoff, having an option with minimal performance impact is crucial inasmuch as it makes compression valuable to users for whom the tradeoff otherwise might not have made sense at all.
Charles Duffy wrote:
Jim Meyering wrote:
Daniel Veillard wrote:
Hum, I realize that support of LZOP was added after 0.7.0, so we never made a release with it (well except for git snapshot which may have been pushed). I wonder if the best is not to just drop the lzop option altogether and stick xz as a package dependancy until we have found a way to provide at the API level which compression options are actually available.
Opinions ?
Dropping lzop sounds good. It seems lzop is not very popular. We don't need that many choices.
lzop may not be popular, but it is distinct -- a minimum of 3x faster than every other compressor offered as an option.
As the decision to use compression at all is offered as a disk space vs performance tradeoff, having an option with minimal performance impact is crucial inasmuch as it makes compression valuable to users for whom the tradeoff otherwise might not have made sense at all.
Good point about it being one of the fastest. I shouldn't have mentioned the subjective "popular". Usefulness trumps that. I suppose Daniel, Cc'd, will decide.
Jim Meyering wrote:
Good point about it being one of the fastest. I shouldn't have mentioned the subjective "popular". Usefulness trumps that. I suppose Daniel, Cc'd, will decide.
Per off-list discussion with DV, I'm providing some numbers. Sort order is space used on disk, lowest to highest, with a 251MB file created by virDomainSave. comptime[*] decomptime[*] space xz[default] 3m45.890s 4.960s 42MB xz[0] 0m17.450s 6.080s 54MB bzip2 0m56.290s 11.080s 58MB gzip 0m13.790s 2.260s 64MB lzop 0m1.970s 0.800s 87MB I believe this makes the distinct niche of each of the compressors clear, with the exclusion of bzip2 (which is both slower and produces larger output compared to xz -0). [*] "time" is user-mode CPU time, as reported by the bash "time" builtin on an Intel Xeon E7330 @ 2.40GHz.
[Pardon the repost -- fixed the table formatting in this version] Jim Meyering wrote:
Good point about it being one of the fastest. I shouldn't have mentioned the subjective "popular". Usefulness trumps that. I suppose Daniel, Cc'd, will decide.
Per off-list discussion with DV, I'm providing some numbers. Sort order is space used on disk, lowest to highest, with a 251MB file created by virDomainSave. comptime[*] decomptime[*] space xz[default] 3m45.890s 4.960s 42MB xz[0] 0m17.450s 6.080s 54MB bzip2 0m56.290s 11.080s 58MB gzip 0m13.790s 2.260s 64MB lzop 0m1.970s 0.800s 87MB I believe this makes the distinct niche of each of the compressors clear, with the exclusion of bzip2 (which is both slower and produces larger output compared to xz -0). [*] "time" is user-mode CPU time, as reported by the bash "time" builtin on an Intel Xeon E7330 @ 2.40GHz.
participants (5)
-
Charles Duffy -
Charles Duffy -
Chris Lalancette -
Daniel Veillard -
Jim Meyering