[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] Compressed save image format for Qemu.



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 redhat com>

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 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]