On Thu, Aug 13, 2009 at 10:47:18AM +0200, Chris Lalancette wrote:
> Implement a compressed save image format for qemu. While ideally
> we would have the choice between compressed/non-compressed
> available to the libvirt API, unfortunately there is no "flags"
> parameter to the virDomainSave() API. Therefore, implement this
> as a qemu.conf option. Both gzip and bzip2 are implemented, and
> it should be very easy to implement additional compression
> methods.
>
> One open question is if/how we should detect the gzip and bzip2
> binaries. One way to do it is to do compile-time setting of the
> paths (via configure.in), but that doesn't seem like a great thing
> to do. Another solution (my preferred solution) is not to detect
> at all; when we go to run the commands that need them, if they
> aren't available, or aren't available in one of the standard paths,
> then we'll fail. Maybe somebody else has another option or
> opinion, though.
>
> In the future, we'll have a more robust (managed) save/restore API,
> at which time we can expose this functionality properly in the API.
>
> V2: get rid of redundant dd command and just use >> to append data.
> V3: Add back the missing pieces for the enum and bumping the save version.
> V4: Make the compressed field in the save_header an int.
> Implement LZMA compression.
>
> Signed-off-by: Chris Lalancette <clalance@xxxxxxxxxx>
ACK, this looks good now. Bonus points if you do a follow-up
patch to add the new param to libvirtd_qemu.aug and
test_libvirtd_qemu.aug.
> ---
> src/qemu.conf | 10 ++++++
> src/qemu_conf.c | 11 ++++++
> src/qemu_conf.h | 2 +
> src/qemu_driver.c | 93 +++++++++++++++++++++++++++++++++++++++++++++++++----
> 4 files changed, 109 insertions(+), 7 deletions(-)
>
> diff --git a/src/qemu.conf b/src/qemu.conf
> index 653f487..1f10b43 100644
> --- a/src/qemu.conf
> +++ b/src/qemu.conf
> @@ -129,3 +129,13 @@
> # "/dev/ptmx", "/dev/kvm", "/dev/kqemu",
> # "/dev/rtc", "/dev/hpet", "/dev/net/tun",
> #]
> +
> +# The default format for Qemu/KVM guest save images is raw; that is, the
> +# 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", or "lzma"
> +# for save_image_format. Note that this means you slow down the
> +# process of saving a domain in order to save disk space.
> +#
> +# save_image_format = "raw"
> diff --git a/src/qemu_conf.c b/src/qemu_conf.c
> index 7ca5a15..ed87e13 100644
> --- a/src/qemu_conf.c
> +++ b/src/qemu_conf.c
> @@ -280,6 +280,17 @@ int qemudLoadDriverConfig(struct qemud_driver *driver,
> driver->cgroupDeviceACL[i] = NULL;
> }
>
> + p = virConfGetValue (conf, "save_image_format");
> + CHECK_TYPE ("save_image_format", VIR_CONF_STRING);
> + if (p && p->str) {
> + VIR_FREE(driver->saveImageFormat);
> + if (!(driver->saveImageFormat = strdup(p->str))) {
> + virReportOOMError(NULL);
> + virConfFree(conf);
> + return -1;
> + }
> + }
> +
> virConfFree (conf);
> return 0;
> }
> diff --git a/src/qemu_conf.h b/src/qemu_conf.h
> index 8f4ef6a..e34baab 100644
> --- a/src/qemu_conf.h
> +++ b/src/qemu_conf.h
> @@ -111,6 +111,8 @@ struct qemud_driver {
>
> char *securityDriverName;
> virSecurityDriverPtr securityDriver;
> +
> + char *saveImageFormat;
> };
>
>
> diff --git a/src/qemu_driver.c b/src/qemu_driver.c
> index 20906ef..3fd153d 100644
> --- a/src/qemu_driver.c
> +++ b/src/qemu_driver.c
> @@ -3411,18 +3411,27 @@ static char *qemudEscapeShellArg(const char *in)
> }
>
> #define QEMUD_SAVE_MAGIC "LibvirtQemudSave"
> -#define QEMUD_SAVE_VERSION 1
> +#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,
> +};
>
> struct qemud_save_header {
> char magic[sizeof(QEMUD_SAVE_MAGIC)-1];
> int version;
> int xml_len;
> int was_running;
> - int unused[16];
> + int compressed;
> + int unused[15];
> };
>
> static int qemudDomainSave(virDomainPtr dom,
> - const char *path) {
> + const char *path)
> +{
> struct qemud_driver *driver = dom->conn->privateData;
> virDomainObjPtr vm;
> char *command = NULL;
> @@ -3433,11 +3442,28 @@ static int qemudDomainSave(virDomainPtr dom,
> struct qemud_save_header header;
> int ret = -1;
> virDomainEventPtr event = NULL;
> + int internalret;
>
> memset(&header, 0, sizeof(header));
> memcpy(header.magic, QEMUD_SAVE_MAGIC, sizeof(header.magic));
> header.version = QEMUD_SAVE_VERSION;
>
> + 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 {
> + qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
> + "%s", _("Invalid save image format specified in configuration file"));
> + return -1;
> + }
> +
> qemuDriverLock(driver);
> vm = virDomainFindByUUID(&driver->domains, dom->uuid);
>
> @@ -3510,11 +3536,28 @@ static int qemudDomainSave(virDomainPtr dom,
> virReportOOMError(dom->conn);
> goto cleanup;
> }
> - if (virAsprintf(&command, "migrate \"exec:"
> - "dd of='%s' oflag=append conv=notrunc 2>/dev/null"
> - "\"", safe_path) == -1) {
> +
> + 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 {
> + qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR,
> + _("Invalid compress format %d"),
> + header.compressed);
> + goto cleanup;
> + }
> + if (internalret < 0) {
> virReportOOMError(dom->conn);
> - command = NULL;
> goto cleanup;
> }
>
> @@ -4035,6 +4078,9 @@ static int qemudDomainRestore(virConnectPtr conn,
> char *xml = NULL;
> struct qemud_save_header header;
> virDomainEventPtr event = NULL;
> + int intermediatefd = -1;
> + pid_t intermediate_pid = -1;
> + int childstat;
>
> qemuDriverLock(driver);
> /* Verify the header and read the XML */
> @@ -4124,8 +4170,41 @@ static int qemudDomainRestore(virConnectPtr conn,
> }
> def = NULL;
>
> + if (header.version == 2) {
> + const char *intermediate_argv[3] = { NULL, "-dc", NULL };
> + if (header.compressed == QEMUD_SAVE_FORMAT_GZIP)
> + intermediate_argv[0] = "gzip";
> + else if (header.compressed == QEMUD_SAVE_FORMAT_BZIP2)
> + intermediate_argv[0] = "bzip2";
> + else if (header.compressed == QEMUD_SAVE_FORMAT_LZMA)
> + intermediate_argv[0] = "lzma";
> + else if (header.compressed != QEMUD_SAVE_FORMAT_RAW) {
> + qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
> + _("Unknown compressed save format %d"),
> + header.compressed);
> + goto cleanup;
> + }
> + if (intermediate_argv[0] != NULL) {
> + intermediatefd = fd;
> + fd = -1;
> + if (virExec(conn, intermediate_argv, NULL, NULL,
> + &intermediate_pid, intermediatefd, &fd, NULL, 0) < 0) {
> + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
> + _("Failed to start decompression binary %s"),
> + intermediate_argv[0]);
> + goto cleanup;
> + }
> + }
> + }
> /* Set the migration source and start it up. */
> ret = qemudStartVMDaemon(conn, driver, vm, "stdio", fd);
> + if (intermediate_pid != -1) {
> + /* Wait for intermediate process to exit */
> + while (waitpid(intermediate_pid, &childstat, 0) == -1 &&
> + errno == EINTR);
> + }
> + if (intermediatefd != -1)
> + close(intermediatefd);
> close(fd);
> fd = -1;
> if (ret < 0) {
Oh actually need a VIR_FREE(driver->save_image_format) in qemudShutdown()
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list