diff mbox

[v15,8/9] Add set_cachesize command

Message ID 1341492709-13897-9-git-send-email-owasserm@redhat.com
State New
Headers show

Commit Message

Orit Wasserman July 5, 2012, 12:51 p.m. UTC
Change XBZRLE cache size in bytes (the size should be a power of 2).
If XBZRLE cache size is too small there will be many cache miss.

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>
Signed-off-by: Orit Wasserman <owasserm@redhat.com>
---
 arch_init.c      |   10 ++++++++++
 hmp-commands.hx  |   19 +++++++++++++++++++
 hmp.c            |   13 +++++++++++++
 hmp.h            |    1 +
 migration.c      |   17 ++++++++++++++++-
 migration.h      |    2 ++
 qapi-schema.json |   16 ++++++++++++++++
 qmp-commands.hx  |   23 +++++++++++++++++++++++
 8 files changed, 100 insertions(+), 1 deletions(-)

Comments

Eric Blake July 5, 2012, 2:14 p.m. UTC | #1
On 07/05/2012 06:51 AM, Orit Wasserman wrote:
> Change XBZRLE cache size in bytes (the size should be a power of 2).
> If XBZRLE cache size is too small there will be many cache miss.
> 
> 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>
> Signed-off-by: Orit Wasserman <owasserm@redhat.com>
> ---

> +    {
> +        .name       = "migrate_set_cachesize",
> +        .args_type  = "value:o",
> +        .params     = "value",
> +        .help       = "set cache size (in bytes) for XBZRLE migrations,"
> +		      "the cache size will be rounded down to the nearest "
> +                      "power of 2.\n"
> +		      "The cache size effects the number of cache misses."
> +		      "In case of a high cache miss ratio you need to increase the cache size",

Inconsistent indentation.  Kill those TABs.

> +++ b/migration.c
> @@ -22,6 +22,7 @@
>  #include "qemu_socket.h"
>  #include "block-migration.h"
>  #include "qmp-commands.h"
> +#include <math.h>

Urgh.  You STILL haven't killed this bogus include, even though I've
been mentioning it since my v12 review.  I'm really getting frustrated
with how long this series is being dragged out.

>  
>  //#define DEBUG_MIGRATION
>  
> @@ -43,7 +44,7 @@ enum {
>  
>  #define MAX_THROTTLE  (32 << 20)      /* Migration speed throttling */
>  
> -/* Migration XBZRLE cache size */
> +/* Migration XBZRLE default cache size */

Again from my v12 review, this hunk should be squashed into the patch
that first introduces the comment.

> +migrate_set_cachesize
> +---------------------
> +
> +Set cache size to be used by XBZRLE migration, the cache size will be round down
> +to the nearset power of 2

Another v12 review leftover:

s/nearset/nearest/
Orit Wasserman July 6, 2012, 5:24 a.m. UTC | #2
On 07/05/2012 05:14 PM, Eric Blake wrote:
> On 07/05/2012 06:51 AM, Orit Wasserman wrote:
>> Change XBZRLE cache size in bytes (the size should be a power of 2).
>> If XBZRLE cache size is too small there will be many cache miss.
>>
>> 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>
>> Signed-off-by: Orit Wasserman <owasserm@redhat.com>
>> ---
> 
>> +    {
>> +        .name       = "migrate_set_cachesize",
>> +        .args_type  = "value:o",
>> +        .params     = "value",
>> +        .help       = "set cache size (in bytes) for XBZRLE migrations,"
>> +		      "the cache size will be rounded down to the nearest "
>> +                      "power of 2.\n"
>> +		      "The cache size effects the number of cache misses."
>> +		      "In case of a high cache miss ratio you need to increase the cache size",
> 
> Inconsistent indentation.  Kill those TABs.
> 
>> +++ b/migration.c
>> @@ -22,6 +22,7 @@
>>  #include "qemu_socket.h"
>>  #include "block-migration.h"
>>  #include "qmp-commands.h"
>> +#include <math.h>
> 
> Urgh.  You STILL haven't killed this bogus include, even though I've
> been mentioning it since my v12 review.  I'm really getting frustrated
> with how long this series is being dragged out.
Again I'm really sorry I will be more careful next time

Thanks,
Orit
> 
>>  
>>  //#define DEBUG_MIGRATION
>>  
>> @@ -43,7 +44,7 @@ enum {
>>  
>>  #define MAX_THROTTLE  (32 << 20)      /* Migration speed throttling */
>>  
>> -/* Migration XBZRLE cache size */
>> +/* Migration XBZRLE default cache size */
> 
> Again from my v12 review, this hunk should be squashed into the patch
> that first introduces the comment.
> 
>> +migrate_set_cachesize
>> +---------------------
>> +
>> +Set cache size to be used by XBZRLE migration, the cache size will be round down
>> +to the nearset power of 2
> 
> Another v12 review leftover:
> 
> s/nearset/nearest/
>
diff mbox

Patch

diff --git a/arch_init.c b/arch_init.c
index 7b65551..da7a630 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -192,6 +192,16 @@  static struct {
     .cache = NULL,
 };
 
+
+int64_t xbzrle_cache_resize(int64_t new_size)
+{
+    if (XBZRLE.cache != NULL) {
+        return cache_resize(XBZRLE.cache, new_size / TARGET_PAGE_SIZE) *
+            TARGET_PAGE_SIZE;
+    }
+    return pow2floor(new_size);
+}
+
 static void save_block_hdr(QEMUFile *f, RAMBlock *block, ram_addr_t offset,
         int cont, int flag)
 {
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 9245bef..8864a15 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -829,6 +829,25 @@  STEXI
 @item migrate_cancel
 @findex migrate_cancel
 Cancel the current VM migration.
+
+ETEXI
+
+    {
+        .name       = "migrate_set_cachesize",
+        .args_type  = "value:o",
+        .params     = "value",
+        .help       = "set cache size (in bytes) for XBZRLE migrations,"
+		      "the cache size will be rounded down to the nearest "
+                      "power of 2.\n"
+		      "The cache size effects the number of cache misses."
+		      "In case of a high cache miss ratio you need to increase the cache size",
+        .mhandler.cmd = hmp_migrate_set_cachesize,
+    },
+
+STEXI
+@item migrate_set_cachesize @var{value}
+@findex migrate_set_cache
+Set cache size to @var{value} (in bytes) for xbzrle migrations.
 ETEXI
 
     {
diff --git a/hmp.c b/hmp.c
index b0440e6..5833e1c 100644
--- a/hmp.c
+++ b/hmp.c
@@ -758,6 +758,19 @@  void hmp_migrate_set_downtime(Monitor *mon, const QDict *qdict)
     qmp_migrate_set_downtime(value, NULL);
 }
 
+void hmp_migrate_set_cachesize(Monitor *mon, const QDict *qdict)
+{
+    int64_t value = qdict_get_int(qdict, "value");
+    Error *err = NULL;
+
+    qmp_migrate_set_cachesize(value, &err);
+    if (err) {
+        monitor_printf(mon, "%s\n", error_get_pretty(err));
+        error_free(err);
+        return;
+    }
+}
+
 void hmp_migrate_set_speed(Monitor *mon, const QDict *qdict)
 {
     int64_t value = qdict_get_int(qdict, "value");
diff --git a/hmp.h b/hmp.h
index 09ba198..7c5117d 100644
--- a/hmp.h
+++ b/hmp.h
@@ -53,6 +53,7 @@  void hmp_migrate_cancel(Monitor *mon, const QDict *qdict);
 void hmp_migrate_set_downtime(Monitor *mon, const QDict *qdict);
 void hmp_migrate_set_speed(Monitor *mon, const QDict *qdict);
 void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict);
+void hmp_migrate_set_cachesize(Monitor *mon, const QDict *qdict);
 void hmp_set_password(Monitor *mon, const QDict *qdict);
 void hmp_expire_password(Monitor *mon, const QDict *qdict);
 void hmp_eject(Monitor *mon, const QDict *qdict);
diff --git a/migration.c b/migration.c
index 2525b18..9df6fc5 100644
--- a/migration.c
+++ b/migration.c
@@ -22,6 +22,7 @@ 
 #include "qemu_socket.h"
 #include "block-migration.h"
 #include "qmp-commands.h"
+#include <math.h>
 
 //#define DEBUG_MIGRATION
 
@@ -43,7 +44,7 @@  enum {
 
 #define MAX_THROTTLE  (32 << 20)      /* Migration speed throttling */
 
-/* Migration XBZRLE cache size */
+/* Migration XBZRLE default cache size */
 #define DEFAULT_MIGRATE_CACHE_SIZE (64 * 1024 * 1024)
 
 static NotifierList migration_state_notifiers =
@@ -533,6 +534,20 @@  void qmp_migrate_cancel(Error **errp)
     migrate_fd_cancel(migrate_get_current());
 }
 
+void qmp_migrate_set_cachesize(int64_t value, Error **errp)
+{
+    MigrationState *s = migrate_get_current();
+
+    /* Check for truncation */
+    if (value != (size_t)value) {
+        error_set(errp, QERR_INVALID_PARAMETER_VALUE, "cache size",
+                  "exceeding address space");
+        return;
+    }
+
+    s->xbzrle_cache_size = xbzrle_cache_resize(value);
+}
+
 void qmp_migrate_set_speed(int64_t value, Error **errp)
 {
     MigrationState *s;
diff --git a/migration.h b/migration.h
index 9b61e70..a73a34a 100644
--- a/migration.h
+++ b/migration.h
@@ -108,4 +108,6 @@  int xbzrle_decode_buffer(uint8_t *src, int slen, uint8_t *dst, int dlen);
 int migrate_use_xbzrle(void);
 int64_t migrate_xbzrle_cache_size(void);
 
+int64_t xbzrle_cache_resize(int64_t new_size);
+
 #endif
diff --git a/qapi-schema.json b/qapi-schema.json
index a8408fd..c13577d 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1390,6 +1390,22 @@ 
 { 'command': 'migrate_set_speed', 'data': {'value': 'int'} }
 
 ##
+# @migrate_set_cachesize
+#
+# Set XBZRLE cache size
+#
+# @value: cache size in bytes
+#
+# The size will be rounded down to the nearest power of 2.
+# The cache size can be modified before and during ongoing migration
+#
+# Returns: nothing on success
+#
+# Since: 1.2
+##
+{ 'command': 'migrate_set_cachesize', 'data': {'value': 'int'} }
+
+##
 # @ObjectPropertyInfo:
 #
 # @name: the name of the property
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 3ee6e00..3ae46dc 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -520,6 +520,29 @@  Example:
 <- { "return": {} }
 
 EQMP
+{
+        .name       = "migrate_set_cachesize",
+        .args_type  = "value:o",
+        .mhandler.cmd_new = qmp_marshal_input_migrate_set_cachesize,
+    },
+
+SQMP
+migrate_set_cachesize
+---------------------
+
+Set cache size to be used by XBZRLE migration, the cache size will be round down
+to the nearset power of 2
+
+Arguments:
+
+- "value": cache size in bytes (json-int)
+
+Example:
+
+-> { "execute": "migrate_set_cachesize", "arguments": { "value": 536870912 } }
+<- { "return": {} }
+
+EQMP
 
     {
         .name       = "migrate_set_speed",