[libvirt] [PATCH v3 2/7] migration: add compress method option

[Jiri Denemark]

I still think that to enable migration in a nice backward compatible
way, a user should use VIR_MIGRATE_COMPRESSED flag *and* optionally
select the compression algorithm(s) via VIR_MIGRATE_PARAM_COMPRESSION
parameter. If VIR_MIGRATE_PARAM_COMPRESSION is not used, the hypervisor
(driver) will choose a default method.

On Thu, Jan 28, 2016 at 10:04:28 +0300, Nikolay Shirokovskiy wrote:
> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy at virtuozzo.com>
> ---
>  include/libvirt/libvirt-domain.h | 13 +++++-
>  src/qemu/qemu_driver.c           | 75 +++++++++++++++++++++++++++++---
>  src/qemu/qemu_migration.c        | 94 ++++++++++++++++++++++++++++------------
>  src/qemu/qemu_migration.h        | 16 +++++++
>  src/qemu/qemu_monitor.c          |  2 +-
>  src/qemu/qemu_monitor.h          |  1 +
>  6 files changed, 166 insertions(+), 35 deletions(-)
> 
> diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
> index 65f1618..e868515 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -655,7 +655,7 @@ typedef enum {
>                                                 * when supported */
>      VIR_MIGRATE_UNSAFE            = (1 << 9), /* force migration even if it is considered unsafe */
>      VIR_MIGRATE_OFFLINE           = (1 << 10), /* offline migrate */
> -    VIR_MIGRATE_COMPRESSED        = (1 << 11), /* compress data during migration */
> +    VIR_MIGRATE_COMPRESSED        = (1 << 11), /* use default compression method */

I'd imagine something like "compress data during migration using a
method selected by the hypervisor or via VIR_MIGRATE_PARAM_COMPRESSION
parameter.

>      VIR_MIGRATE_ABORT_ON_ERROR    = (1 << 12), /* abort migration on I/O errors happened during migration */
>      VIR_MIGRATE_AUTO_CONVERGE     = (1 << 13), /* force convergence */
>      VIR_MIGRATE_RDMA_PIN_ALL      = (1 << 14), /* RDMA memory pinning */
> @@ -757,6 +757,17 @@ typedef enum {
>   */
>  # define VIR_MIGRATE_PARAM_MIGRATE_DISKS    "migrate_disks"
>  
> +/**
> + * VIR_MIGRATE_PARAM_COMPRESSION:
> + *
> + * virDomainMigrate* params multiple field: string list of compression methods
> + * that are used to compress migration traffic. Note that this option cannot
> + * be used together with VIR_MIGRATE_COMPRESSED flag, use only one of them.

I'd drop the last sentence.

> + */
> +

This empty line should be dropped.

> +# define VIR_MIGRATE_PARAM_COMPRESSION    "compression"
> +
> +
>  /* Domain migration. */
>  virDomainPtr virDomainMigrate (virDomainPtr domain, virConnectPtr dconn,
>                                 unsigned long flags, const char *dname,
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 351e529..129da6d 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -12163,7 +12163,7 @@ qemuDomainMigratePrepare2(virConnectPtr dconn,
>      ret = qemuMigrationPrepareDirect(driver, dconn,
>                                       NULL, 0, NULL, NULL, /* No cookies */
>                                       uri_in, uri_out,
> -                                     &def, origname, NULL, 0, NULL, flags);
> +                                     &def, origname, NULL, 0, NULL, NULL, flags);
>  
>   cleanup:
>      VIR_FREE(origname);
> @@ -12216,7 +12216,7 @@ qemuDomainMigratePerform(virDomainPtr dom,
>       * Consume any cookie we were able to decode though
>       */
>      ret = qemuMigrationPerform(driver, dom->conn, vm,
> -                               NULL, dconnuri, uri, NULL, NULL, 0, NULL,
> +                               NULL, dconnuri, uri, NULL, NULL, 0, NULL, NULL,
>                                 cookie, cookielen,
>                                 NULL, NULL, /* No output cookies in v2 */
>                                 flags, dname, resource, false);
> @@ -12389,7 +12389,7 @@ qemuDomainMigratePrepare3(virConnectPtr dconn,
>                                       cookiein, cookieinlen,
>                                       cookieout, cookieoutlen,
>                                       uri_in, uri_out,
> -                                     &def, origname, NULL, 0, NULL, flags);
> +                                     &def, origname, NULL, 0, NULL, NULL, flags);
>  
>   cleanup:
>      VIR_FREE(origname);
> @@ -12398,6 +12398,60 @@ qemuDomainMigratePrepare3(virConnectPtr dconn,
>  }
>  
>  static int
> +qemuAddCompressionMethod(qemuMigrationCompressMethod value,
> +                           qemuMigrationCompressMethod *method)
> +{
> +    if (*method & value) {
> +        virReportError(VIR_ERR_INVALID_ARG, "%s",
> +                       _("Compression method is specified twice."));
> +        return -1;
> +    }
> +
> +    *method |= value;
> +    return 0;
> +}
> +
> +static int
> +qemuGetCompression(virTypedParameterPtr params, int nparams,
> +                   unsigned int flags,
> +                   qemuMigrationCompressionPtr compression)
> +{
> +    size_t i;
> +
> +    memset(compression, 0, sizeof(*compression));

Hmm, I think qemuMigrationCompression structure should be allocated on
the heap (rather than on the stack) and the allocator would also take
care of setting appropriate "unspecified" values. And please, move these
migration related functions in qemu_migrate.c.

Jirka