| Message ID | 1334170153-9503-10-git-send-email-owasserm@redhat.com |
|---|---|
| State | New |
| Headers | show |
Orit Wasserman <owasserm@redhat.com> 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. > + .name = "migrate_set_cachesize", > + .args_type = "value:o", This mean that we can assign values like 256M, right? i.e. no way to pass negative values? > index 7578163..071a1b9 100644 > --- a/migration.c > +++ b/migration.c > @@ -167,7 +167,7 @@ MigrationCapList *qmp_query_migration_caps(Error **errp) > MigrationCapList *caps_list = g_malloc0(sizeof(*caps_list)); > > caps_list->value = g_malloc(sizeof(*caps_list->value)); > - caps_list->value->name = g_strdup("uleb"); > + caps_list->value->name = g_strdup("xbzrle"); > caps_list->next = NULL; > Shouldn't this go in the previous patche? > +void qmp_migrate_set_cachesize(int64_t value, Error **errp) > +{ > + MigrationState *s; > + > + /* Check for truncation */ > + if (value != (size_t)value) { > + error_set(errp, QERR_INVALID_PARAMETER_VALUE, "cache size", > + "exceeding address space"); > + return; > + } > + > + /* power of 2 */ > + if (value != 1 && (value & (value - 1))) { > + error_set(errp, QERR_INVALID_PARAMETER_VALUE, "cache size", > + "needs to be power of 2"); > + return; > + } Don't we have a function to calculate if a value is a power of 2 in qemu? I guess we need one _now_. > + > + value = MIN(UINT64_MAX, value); > + if (value == migrate_cache_size) { > + return; > + } > + > + migrate_cache_size = value; Can't we just remove the migrate_cache_size as a new field of migration_state. bandwidth_limit is the example on how to do it. Thanks, Juan.
On 04/11/2012 01:49 PM, 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: 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> > --- > arch_init.c | 5 +++++ > hmp-commands.hx | 15 +++++++++++++++ > hmp.c | 13 +++++++++++++ > hmp.h | 1 + > migration.c | 31 ++++++++++++++++++++++++++++++- > migration.h | 2 ++ > qapi-schema.json | 13 +++++++++++++ > qmp-commands.hx | 22 ++++++++++++++++++++++ > 8 files changed, 101 insertions(+), 1 deletions(-) > > diff --git a/arch_init.c b/arch_init.c > index 793f0be..af9fcc3 100644 > --- a/arch_init.c > +++ b/arch_init.c > @@ -175,6 +175,11 @@ typedef struct __attribute__((packed)) XBZRLEHeader { > > static uint8 *prev_cache_page; > > +void xbzrle_cache_resize(int64_t new_size) > +{ > + cache_resize(new_size); > +} If it needs to be a power of 2, then take an integer order instead of a byte. That makes the function impossible to misuse. Regards, Anthony Liguori
On 04/18/2012 08:07 PM, Juan Quintela wrote: > Orit Wasserman <owasserm@redhat.com> 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. > >> + .name = "migrate_set_cachesize", >> + .args_type = "value:o", > > This mean that we can assign values like 256M, right? i.e. no way to > pass negative values? >> index 7578163..071a1b9 100644 >> --- a/migration.c >> +++ b/migration.c >> @@ -167,7 +167,7 @@ MigrationCapList *qmp_query_migration_caps(Error **errp) >> MigrationCapList *caps_list = g_malloc0(sizeof(*caps_list)); >> >> caps_list->value = g_malloc(sizeof(*caps_list->value)); >> - caps_list->value->name = g_strdup("uleb"); >> + caps_list->value->name = g_strdup("xbzrle"); >> caps_list->next = NULL; >> > > Shouldn't this go in the previous patche?sure > >> +void qmp_migrate_set_cachesize(int64_t value, Error **errp) >> +{ >> + MigrationState *s; >> + >> + /* Check for truncation */ >> + if (value != (size_t)value) { >> + error_set(errp, QERR_INVALID_PARAMETER_VALUE, "cache size", >> + "exceeding address space"); >> + return; >> + } >> + >> + /* power of 2 */ >> + if (value != 1 && (value & (value - 1))) { >> + error_set(errp, QERR_INVALID_PARAMETER_VALUE, "cache size", >> + "needs to be power of 2"); >> + return; >> + } > > Don't we have a function to calculate if a value is a power of 2 in > qemu? I guess we need one _now_. I will look into it. maybe there is a function like that ... . > >> + >> + value = MIN(UINT64_MAX, value); >> + if (value == migrate_cache_size) { >> + return; >> + } >> + >> + migrate_cache_size = value; > > Can't we just remove the migrate_cache_size as a new field of > migration_state. bandwidth_limit is the example on how to do it. Ok Thanks, Orit > > Thanks, Juan.
On 04/18/2012 08:37 PM, Anthony Liguori wrote: > On 04/11/2012 01:49 PM, 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: 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> >> --- >> arch_init.c | 5 +++++ >> hmp-commands.hx | 15 +++++++++++++++ >> hmp.c | 13 +++++++++++++ >> hmp.h | 1 + >> migration.c | 31 ++++++++++++++++++++++++++++++- >> migration.h | 2 ++ >> qapi-schema.json | 13 +++++++++++++ >> qmp-commands.hx | 22 ++++++++++++++++++++++ >> 8 files changed, 101 insertions(+), 1 deletions(-) >> >> diff --git a/arch_init.c b/arch_init.c >> index 793f0be..af9fcc3 100644 >> --- a/arch_init.c >> +++ b/arch_init.c >> @@ -175,6 +175,11 @@ typedef struct __attribute__((packed)) XBZRLEHeader { >> >> static uint8 *prev_cache_page; >> >> +void xbzrle_cache_resize(int64_t new_size) >> +{ >> + cache_resize(new_size); >> +} > > If it needs to be a power of 2, then take an integer order instead of a byte. That makes the function impossible to misuse. That is a good idea. > > Regards, > > Anthony Liguori
diff --git a/arch_init.c b/arch_init.c index 793f0be..af9fcc3 100644 --- a/arch_init.c +++ b/arch_init.c @@ -175,6 +175,11 @@ typedef struct __attribute__((packed)) XBZRLEHeader { static uint8 *prev_cache_page; +void xbzrle_cache_resize(int64_t new_size) +{ + cache_resize(new_size); +} + /***********************************************************/ /* XBRLE page cache implementation */ static CacheItem *cache_item_get(unsigned long pos, int item) diff --git a/hmp-commands.hx b/hmp-commands.hx index a73b538..6ef2b6b 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -833,6 +833,21 @@ 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 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 d681bfc..b569887 100644 --- a/hmp.c +++ b/hmp.c @@ -740,6 +740,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 868bfb3..4cce377 100644 --- a/hmp.h +++ b/hmp.h @@ -52,6 +52,7 @@ void hmp_snapshot_blkdev(Monitor *mon, const QDict *qdict); 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_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 7578163..071a1b9 100644 --- a/migration.c +++ b/migration.c @@ -167,7 +167,7 @@ MigrationCapList *qmp_query_migration_caps(Error **errp) MigrationCapList *caps_list = g_malloc0(sizeof(*caps_list)); caps_list->value = g_malloc(sizeof(*caps_list->value)); - caps_list->value->name = g_strdup("uleb"); + caps_list->value->name = g_strdup("xbzrle"); caps_list->next = NULL; return caps_list; @@ -465,6 +465,35 @@ void qmp_migrate_cancel(Error **errp) migrate_fd_cancel(migrate_get_current()); } +void qmp_migrate_set_cachesize(int64_t value, Error **errp) +{ + MigrationState *s; + + /* Check for truncation */ + if (value != (size_t)value) { + error_set(errp, QERR_INVALID_PARAMETER_VALUE, "cache size", + "exceeding address space"); + return; + } + + /* power of 2 */ + if (value != 1 && (value & (value - 1))) { + error_set(errp, QERR_INVALID_PARAMETER_VALUE, "cache size", + "needs to be power of 2"); + return; + } + + value = MIN(UINT64_MAX, value); + if (value == migrate_cache_size) { + return; + } + + migrate_cache_size = value; + s = migrate_get_current(); + s->params.xbzrle_cache_size = value; + 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 06ce8ab..3426c94 100644 --- a/migration.h +++ b/migration.h @@ -109,4 +109,6 @@ int decode_page(uint8_t *src, int slen, uint8_t *dst, int dlen); int migrate_use_xbzrle(void); int64_t migrate_xbzrle_cache_size(void); +void xbzrle_cache_resize(int64_t new_size); + #endif diff --git a/qapi-schema.json b/qapi-schema.json index 775fe56..956caed 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -1310,6 +1310,19 @@ { 'command': 'migrate_set_speed', 'data': {'value': 'int'} } ## +# @migrate_set_cachesize +# +# Set XBZRLE cache size +# +# @value: cache size in bytes +# +# Returns: nothing on success +# +# Since: 1.1 +## +{ '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 b994951..3f2c9e2 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -522,6 +522,28 @@ 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 + +Arguments: + +- "value": cache size in bytes (json-int) + +Example: + +-> { "execute": "migrate_set_cachesize", "arguments": { "value": 512 } } +<- { "return": {} } + +EQMP { .name = "migrate_set_speed",