Patchwork [v9,07/10] Add XBZRLE option to migrate command

login
register
mail settings
Submitter Orit Wasserman
Date April 11, 2012, 6:49 p.m.
Message ID <1334170153-9503-8-git-send-email-owasserm@redhat.com>
Download mbox | patch
Permalink /patch/151856/
State New
Headers show

Comments

Orit Wasserman - April 11, 2012, 6:49 p.m.
Signed-off-by: Orit Wasserman <owasserm@redhat.com>
Signed-off-by: Benoit Hudzia <benoit.hudzia@sap.com>
Signed-off-by: Petter Svard <petters@cs.umu.se>
Signed-off-by: Aidan Shribman <aidan.shribman@sap.com>
---
 hmp-commands.hx  |   20 ++++++++++++--------
 hmp.c            |    4 +++-
 migration.c      |    9 +++++++++
 qapi-schema.json |    2 +-
 qmp-commands.hx  |    4 +++-
 5 files changed, 28 insertions(+), 11 deletions(-)
Juan Quintela - April 18, 2012, 4:57 p.m.
Orit Wasserman <owasserm@redhat.com> wrote:
> Signed-off-by: Orit Wasserman <owasserm@redhat.com>
> Signed-off-by: Benoit Hudzia <benoit.hudzia@sap.com>
> Signed-off-by: Petter Svard <petters@cs.umu.se>
> Signed-off-by: Aidan Shribman <aidan.shribman@sap.com>
> ---
>  hmp-commands.hx  |   20 ++++++++++++--------
>  hmp.c            |    4 +++-
>  migration.c      |    9 +++++++++
>  qapi-schema.json |    2 +-
>  qmp-commands.hx  |    4 +++-
>  5 files changed, 28 insertions(+), 11 deletions(-)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index a6f5a84..a73b538 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -798,23 +798,27 @@ ETEXI
>  
>      {
>          .name       = "migrate",
> -        .args_type  = "detach:-d,blk:-b,inc:-i,uri:s",
> -        .params     = "[-d] [-b] [-i] uri",
> -        .help       = "migrate to URI (using -d to not wait for completion)"
> -		      "\n\t\t\t -b for migration without shared storage with"
> -		      " full copy of disk\n\t\t\t -i for migration without "
> -		      "shared storage with incremental copy of disk "
> -		      "(base image shared between src and destination)",
> +        .args_type  = "detach:-d,blk:-b,inc:-i,xbzrle:-x,uri:s",
> +        .params     = "[-d] [-b] [-i] [-x] uri",
> +        .help       = "migrate to URI"
> +                      "\n\t -d to not wait for completion"
> +		      "\n\t -b for migration without shared storage with"
> +		      " full copy of disk"

There are tabs on that two lines, or only spaces on the next ones, your
choice O:-)

Rest looks ok.

Thanks, Juan.
Anthony Liguori - April 18, 2012, 5:33 p.m.
On 04/11/2012 01:49 PM, Orit Wasserman wrote:
> Signed-off-by: Orit Wasserman<owasserm@redhat.com>
> Signed-off-by: Benoit Hudzia<benoit.hudzia@sap.com>
> Signed-off-by: Petter Svard<petters@cs.umu.se>
> Signed-off-by: Aidan Shribman<aidan.shribman@sap.com>
> ---
>   hmp-commands.hx  |   20 ++++++++++++--------
>   hmp.c            |    4 +++-
>   migration.c      |    9 +++++++++
>   qapi-schema.json |    2 +-
>   qmp-commands.hx  |    4 +++-
>   5 files changed, 28 insertions(+), 11 deletions(-)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index a6f5a84..a73b538 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -798,23 +798,27 @@ ETEXI
>
>       {
>           .name       = "migrate",
> -        .args_type  = "detach:-d,blk:-b,inc:-i,uri:s",
> -        .params     = "[-d] [-b] [-i] uri",
> -        .help       = "migrate to URI (using -d to not wait for completion)"
> -		      "\n\t\t\t -b for migration without shared storage with"
> -		      " full copy of disk\n\t\t\t -i for migration without "
> -		      "shared storage with incremental copy of disk "
> -		      "(base image shared between src and destination)",
> +        .args_type  = "detach:-d,blk:-b,inc:-i,xbzrle:-x,uri:s",
> +        .params     = "[-d] [-b] [-i] [-x] uri",
> +        .help       = "migrate to URI"
> +                      "\n\t -d to not wait for completion"
> +		      "\n\t -b for migration without shared storage with"
> +		      " full copy of disk"
> +                      "\n\t -i for migration without"
> +                      " shared storage with incremental copy of disk"
> +                      " (base image shared between source and destination)"
> +                      "\n\t -x to use XBZRLE page delta compression",
>           .mhandler.cmd = hmp_migrate,
>       },
>
>
>   STEXI
> -@item migrate [-d] [-b] [-i] @var{uri}
> +@item migrate [-d] [-b] [-i] [-x] @var{uri}
>   @findex migrate
>   Migrate to @var{uri} (using -d to not wait for completion).
>   	-b for migration with full copy of disk
>   	-i for migration with incremental copy of disk (base image is shared)
> +	-x to use XBZRLE page delta compression
>   ETEXI
>
>       {
> diff --git a/hmp.c b/hmp.c
> index f3e5163..891cac6 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -907,10 +907,12 @@ void hmp_migrate(Monitor *mon, const QDict *qdict)
>       int detach = qdict_get_try_bool(qdict, "detach", 0);
>       int blk = qdict_get_try_bool(qdict, "blk", 0);
>       int inc = qdict_get_try_bool(qdict, "inc", 0);
> +    int xbzrle = qdict_get_try_bool(qdict, "xbzrle", 0);
>       const char *uri = qdict_get_str(qdict, "uri");
>       Error *err = NULL;
>
> -    qmp_migrate(uri, !!blk, blk, !!inc, inc, false, false,&err);
> +    qmp_migrate(uri, !!blk, blk, !!inc, inc, false, false, !!xbzrle, xbzrle,
> +&err);
>       if (err) {
>           monitor_printf(mon, "migrate: %s\n", error_get_pretty(err));
>           error_free(err);
> diff --git a/migration.c b/migration.c
> index a524d5a..cea75aa 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -43,6 +43,11 @@ enum {
>
>   #define MAX_THROTTLE  (32<<  20)      /* Migration speed throttling */
>
> +/* Migration XBZRLE cache size */
> +#define DEFAULT_MIGRATE_CACHE_SIZE (64 * 1024 * 1024)
> +
> +static int64_t migrate_cache_size = DEFAULT_MIGRATE_CACHE_SIZE;
> +
>   static NotifierList migration_state_notifiers =
>       NOTIFIER_LIST_INITIALIZER(migration_state_notifiers);
>
> @@ -389,6 +394,7 @@ void migrate_del_blocker(Error *reason)
>
>   void qmp_migrate(const char *uri, bool has_blk, bool blk,
>                    bool has_inc, bool inc, bool has_detach, bool detach,
> +                 bool has_xbzrle, bool xbzrle,
>                    Error **errp)
>   {
>       MigrationState *s = migrate_get_current();
> @@ -399,6 +405,9 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
>       params.blk = blk;
>       params.shared = inc;
>
> +    params.use_xbzrle = xbzrle;
> +    params.xbzrle_cache_size =  migrate_cache_size;
> +
>       if (s->state == MIG_STATE_ACTIVE) {
>           error_set(errp, QERR_MIGRATION_ACTIVE);
>           return;
> diff --git a/qapi-schema.json b/qapi-schema.json
> index ace55f3..200e4fc 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1683,7 +1683,7 @@
>   # Since: 0.14.0
>   ##
>   { 'command': 'migrate',
> -  'data': {'uri': 'str', '*blk': 'bool', '*inc': 'bool', '*detach': 'bool' } }
> +  'data': {'uri': 'str', '*blk': 'bool', '*inc': 'bool', '*detach': 'bool', '*xbzrle': 'bool' } }

You need to update the schema documentation to describe this parameter 
(including when it was introduced).

But I think having the parameter in the first place is a bad idea.

Regards,

Anthony Liguori

Patch

diff --git a/hmp-commands.hx b/hmp-commands.hx
index a6f5a84..a73b538 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -798,23 +798,27 @@  ETEXI
 
     {
         .name       = "migrate",
-        .args_type  = "detach:-d,blk:-b,inc:-i,uri:s",
-        .params     = "[-d] [-b] [-i] uri",
-        .help       = "migrate to URI (using -d to not wait for completion)"
-		      "\n\t\t\t -b for migration without shared storage with"
-		      " full copy of disk\n\t\t\t -i for migration without "
-		      "shared storage with incremental copy of disk "
-		      "(base image shared between src and destination)",
+        .args_type  = "detach:-d,blk:-b,inc:-i,xbzrle:-x,uri:s",
+        .params     = "[-d] [-b] [-i] [-x] uri",
+        .help       = "migrate to URI"
+                      "\n\t -d to not wait for completion"
+		      "\n\t -b for migration without shared storage with"
+		      " full copy of disk"
+                      "\n\t -i for migration without"
+                      " shared storage with incremental copy of disk"
+                      " (base image shared between source and destination)"
+                      "\n\t -x to use XBZRLE page delta compression",
         .mhandler.cmd = hmp_migrate,
     },
 
 
 STEXI
-@item migrate [-d] [-b] [-i] @var{uri}
+@item migrate [-d] [-b] [-i] [-x] @var{uri}
 @findex migrate
 Migrate to @var{uri} (using -d to not wait for completion).
 	-b for migration with full copy of disk
 	-i for migration with incremental copy of disk (base image is shared)
+	-x to use XBZRLE page delta compression
 ETEXI
 
     {
diff --git a/hmp.c b/hmp.c
index f3e5163..891cac6 100644
--- a/hmp.c
+++ b/hmp.c
@@ -907,10 +907,12 @@  void hmp_migrate(Monitor *mon, const QDict *qdict)
     int detach = qdict_get_try_bool(qdict, "detach", 0);
     int blk = qdict_get_try_bool(qdict, "blk", 0);
     int inc = qdict_get_try_bool(qdict, "inc", 0);
+    int xbzrle = qdict_get_try_bool(qdict, "xbzrle", 0);
     const char *uri = qdict_get_str(qdict, "uri");
     Error *err = NULL;
 
-    qmp_migrate(uri, !!blk, blk, !!inc, inc, false, false, &err);
+    qmp_migrate(uri, !!blk, blk, !!inc, inc, false, false, !!xbzrle, xbzrle,
+                &err);
     if (err) {
         monitor_printf(mon, "migrate: %s\n", error_get_pretty(err));
         error_free(err);
diff --git a/migration.c b/migration.c
index a524d5a..cea75aa 100644
--- a/migration.c
+++ b/migration.c
@@ -43,6 +43,11 @@  enum {
 
 #define MAX_THROTTLE  (32 << 20)      /* Migration speed throttling */
 
+/* Migration XBZRLE cache size */
+#define DEFAULT_MIGRATE_CACHE_SIZE (64 * 1024 * 1024)
+
+static int64_t migrate_cache_size = DEFAULT_MIGRATE_CACHE_SIZE;
+
 static NotifierList migration_state_notifiers =
     NOTIFIER_LIST_INITIALIZER(migration_state_notifiers);
 
@@ -389,6 +394,7 @@  void migrate_del_blocker(Error *reason)
 
 void qmp_migrate(const char *uri, bool has_blk, bool blk,
                  bool has_inc, bool inc, bool has_detach, bool detach,
+                 bool has_xbzrle, bool xbzrle,
                  Error **errp)
 {
     MigrationState *s = migrate_get_current();
@@ -399,6 +405,9 @@  void qmp_migrate(const char *uri, bool has_blk, bool blk,
     params.blk = blk;
     params.shared = inc;
 
+    params.use_xbzrle = xbzrle;
+    params.xbzrle_cache_size =  migrate_cache_size;
+
     if (s->state == MIG_STATE_ACTIVE) {
         error_set(errp, QERR_MIGRATION_ACTIVE);
         return;
diff --git a/qapi-schema.json b/qapi-schema.json
index ace55f3..200e4fc 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1683,7 +1683,7 @@ 
 # Since: 0.14.0
 ##
 { 'command': 'migrate',
-  'data': {'uri': 'str', '*blk': 'bool', '*inc': 'bool', '*detach': 'bool' } }
+  'data': {'uri': 'str', '*blk': 'bool', '*inc': 'bool', '*detach': 'bool', '*xbzrle': 'bool' } }
 
 # @xen-save-devices-state:
 #
diff --git a/qmp-commands.hx b/qmp-commands.hx
index c09ee85..f7a48de 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -469,7 +469,8 @@  EQMP
 
     {
         .name       = "migrate",
-        .args_type  = "detach:-d,blk:-b,inc:-i,uri:s",
+        .args_type  = "detach:-d,blk:-b,inc:-i,xbzrle:-x,uri:s",
+        .params     = "[-d] [-b] [-i] [-x] uri",
         .mhandler.cmd_new = qmp_marshal_input_migrate,
     },
 
@@ -484,6 +485,7 @@  Arguments:
 - "blk": block migration, full disk copy (json-bool, optional)
 - "inc": incremental disk copy (json-bool, optional)
 - "uri": Destination URI (json-string)
+- "xbzrle": to use XBZRLE page delta compression
 
 Example: