* [Qemu-devel] [PATCH 00/11] Migration next v6
2012-07-25 14:50 [Qemu-devel] [PATCH 00/11] Migration next v6 Orit Wasserman
@ 2012-07-25 14:50 ` Orit Wasserman
2012-07-25 14:50 ` [Qemu-devel] [PATCH 01/11] Add migration capabilities Orit Wasserman
` (10 subsequent siblings)
11 siblings, 0 replies; 33+ messages in thread
From: Orit Wasserman @ 2012-07-25 14:50 UTC (permalink / raw)
To: qemu-devel
Cc: peter.maydell, aliguori, quintela, stefanha, mdroth, lcapitulino,
blauwirbel, Orit Wasserman, chegu_vinod, avi, pbonzini, eblake
Those are the latest XBZRLE patches (part of the migration next branch).
changes from v5:
Add query-migrate-parameters that display the migration parameters
(currently only capabilities I will add the other parameters separately).
Remove capabilities from query-migrate.
Please review
Juan Quintela (1):
Restart optimization on stage3 update version
Orit Wasserman (10):
Add migration capabilities
Add migrate_set_parameter and query-migrate-parameters
Add XBZRLE documentation
Add cache handling functions
Add uleb encoding/decoding functions
Add xbzrle_encode_buffer and xbzrle_decode_buffer functions
Add XBZRLE to ram_save_block and ram_save_live
Add migrate_set_cachesize command
Add migration accounting for normal and duplicate pages
Add XBZRLE statistics
Makefile.objs | 1 +
arch_init.c | 263 +++++++++++++++++++++++++++++++++++++++++++-
cutils.c | 42 +++++++
docs/xbzrle.txt | 136 +++++++++++++++++++++++
hmp-commands.hx | 38 +++++++
hmp.c | 124 +++++++++++++++++++++
hmp.h | 4 +
include/qemu/page_cache.h | 79 ++++++++++++++
migration.c | 123 +++++++++++++++++++++-
migration.h | 21 ++++
monitor.c | 14 +++
page_cache.c | 216 +++++++++++++++++++++++++++++++++++++
qapi-schema.json | 121 ++++++++++++++++++++-
qemu-common.h | 21 ++++
qmp-commands.hx | 141 ++++++++++++++++++++++++-
savevm.c | 159 +++++++++++++++++++++++++++
16 files changed, 1490 insertions(+), 13 deletions(-)
create mode 100644 docs/xbzrle.txt
create mode 100644 include/qemu/page_cache.h
create mode 100644 page_cache.c
--
1.7.7.6
^ permalink raw reply [flat|nested] 33+ messages in thread* [Qemu-devel] [PATCH 01/11] Add migration capabilities
2012-07-25 14:50 [Qemu-devel] [PATCH 00/11] Migration next v6 Orit Wasserman
2012-07-25 14:50 ` Orit Wasserman
@ 2012-07-25 14:50 ` Orit Wasserman
2012-07-25 14:50 ` [Qemu-devel] [PATCH 02/11] Add migrate_set_parameter and query-migrate-parameters Orit Wasserman
` (9 subsequent siblings)
11 siblings, 0 replies; 33+ messages in thread
From: Orit Wasserman @ 2012-07-25 14:50 UTC (permalink / raw)
To: qemu-devel
Cc: peter.maydell, aliguori, quintela, stefanha, mdroth, lcapitulino,
blauwirbel, Orit Wasserman, chegu_vinod, avi, pbonzini, eblake
Add migration capabilities that can be queried by the management.
The management can query the source QEMU and the destination QEMU in order to
verify both support some migration capability (currently only XBZRLE).
Signed-off-by: Orit Wasserman <owasserm@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
hmp-commands.hx | 2 ++
hmp.c | 19 +++++++++++++++++++
hmp.h | 1 +
migration.c | 11 +++++++++++
monitor.c | 7 +++++++
qapi-schema.json | 39 +++++++++++++++++++++++++++++++++++++++
qmp-commands.hx | 24 ++++++++++++++++++++++++
7 files changed, 103 insertions(+), 0 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index eea8b32..8786148 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1417,6 +1417,8 @@ show CPU statistics
show user network stack connection states
@item info migrate
show migration status
+@item info migration_capabilities
+show migration capabilities
@item info balloon
show balloon information
@item info qtree
diff --git a/hmp.c b/hmp.c
index 6b72a64..5c7d0be 100644
--- a/hmp.c
+++ b/hmp.c
@@ -161,6 +161,25 @@ void hmp_info_migrate(Monitor *mon)
qapi_free_MigrationInfo(info);
}
+void hmp_info_migration_capabilities(Monitor *mon)
+{
+ MigrationCapabilityStatusList *caps_list, *cap;
+
+ caps_list = qmp_query_migration_capabilities(NULL);
+ if (!caps_list) {
+ monitor_printf(mon, "No migration capabilities found\n");
+ return;
+ }
+
+ for (cap = caps_list; cap; cap = cap->next) {
+ monitor_printf(mon, "%s: %s ",
+ MigrationCapability_lookup[cap->value->capability],
+ cap->value->state ? "on" : "off");
+ }
+
+ qapi_free_MigrationCapabilityStatusList(caps_list);
+}
+
void hmp_info_cpus(Monitor *mon)
{
CpuInfoList *cpu_list, *cpu;
diff --git a/hmp.h b/hmp.h
index 8d2b0d7..2fb44ca 100644
--- a/hmp.h
+++ b/hmp.h
@@ -25,6 +25,7 @@ void hmp_info_uuid(Monitor *mon);
void hmp_info_chardev(Monitor *mon);
void hmp_info_mice(Monitor *mon);
void hmp_info_migrate(Monitor *mon);
+void hmp_info_migration_capabilities(Monitor *mon);
void hmp_info_cpus(Monitor *mon);
void hmp_info_block(Monitor *mon);
void hmp_info_blockstats(Monitor *mon);
diff --git a/migration.c b/migration.c
index 8db1b43..8c27347 100644
--- a/migration.c
+++ b/migration.c
@@ -166,6 +166,17 @@ MigrationInfo *qmp_query_migrate(Error **errp)
return info;
}
+MigrationCapabilityStatusList *qmp_query_migration_capabilities(Error **errp)
+{
+ MigrationCapabilityStatusList *caps_list = g_malloc0(sizeof(*caps_list));
+
+ caps_list->value = g_malloc(sizeof(*caps_list->value));
+ caps_list->value->capability = MIGRATION_CAPABILITY_XBZRLE;
+ caps_list->next = NULL;
+
+ return caps_list;
+}
+
/* shared migration helpers */
static int migrate_fd_cleanup(MigrationState *s)
diff --git a/monitor.c b/monitor.c
index 09aa3cd..fd57c5e 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2662,6 +2662,13 @@ static mon_cmd_t info_cmds[] = {
.mhandler.info = hmp_info_migrate,
},
{
+ .name = "migration-capabilities",
+ .args_type = "",
+ .params = "",
+ .help = "show migration capabilities",
+ .mhandler.info = hmp_info_migration_capabilities,
+ },
+ {
.name = "balloon",
.args_type = "",
.params = "",
diff --git a/qapi-schema.json b/qapi-schema.json
index a92adb1..b4d4dd6 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -306,6 +306,45 @@
{ 'command': 'query-migrate', 'returns': 'MigrationInfo' }
##
+# @MigrationCapability
+#
+# Migration capabilities enumeration
+#
+# @xbzrle: Migration supports xbzrle (Xor Based Zero Length Encoding).
+# This feature allows us to minimize migration traffic for certain work
+# loads, by sending compressed difference of the pages
+#
+# Since: 1.2
+##
+{ 'enum': 'MigrationCapability',
+ 'data': ['xbzrle'] }
+
+##
+# @MigrationCapabilityStatus
+#
+# Migration capability information
+#
+# @capability: capability enum
+#
+# @state: capability state bool
+#
+# Since: 1.2
+##
+{ 'type': 'MigrationCapabilityStatus',
+ 'data': { 'capability' : 'MigrationCapability', 'state' : 'bool' } }
+
+##
+# @query-migration-capabilities
+#
+# Returns information about current migration process capabilities.
+#
+# Returns: @MigrationCapabilityStatus list
+#
+# Since: 1.2
+##
+{ 'command': 'query-migration-capabilities', 'returns': ['MigrationCapabilityStatus'] }
+
+##
# @MouseInfo:
#
# Information about a mouse device.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index e3cf3c5..c0ed14c 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2141,6 +2141,30 @@ EQMP
},
SQMP
+query-migration-capabilities
+-------
+
+Query migration capabilities
+
+- "xbzrle": xbzrle support
+
+Arguments:
+
+Example:
+
+-> { "execute": "query-migration-capabilities"}
+<- { "return": [ { "capability": "xbzrle", "state": true },
+ { "capability": "foobar", "state": false } ] }
+
+EQMP
+
+ {
+ .name = "query-migration-capabilities",
+ .args_type = "",
+ .mhandler.cmd_new = qmp_marshal_input_query_migration_capabilities,
+ },
+
+SQMP
query-balloon
-------------
--
1.7.7.6
^ permalink raw reply related [flat|nested] 33+ messages in thread* [Qemu-devel] [PATCH 02/11] Add migrate_set_parameter and query-migrate-parameters
2012-07-25 14:50 [Qemu-devel] [PATCH 00/11] Migration next v6 Orit Wasserman
2012-07-25 14:50 ` Orit Wasserman
2012-07-25 14:50 ` [Qemu-devel] [PATCH 01/11] Add migration capabilities Orit Wasserman
@ 2012-07-25 14:50 ` Orit Wasserman
2012-07-25 14:50 ` [Qemu-devel] [PATCH 03/11] Add XBZRLE documentation Orit Wasserman
` (8 subsequent siblings)
11 siblings, 0 replies; 33+ messages in thread
From: Orit Wasserman @ 2012-07-25 14:50 UTC (permalink / raw)
To: qemu-devel
Cc: peter.maydell, aliguori, quintela, stefanha, mdroth, lcapitulino,
blauwirbel, Orit Wasserman, chegu_vinod, avi, pbonzini, eblake
The management can enable/disable a capability for the next migration by using
migrate_set_parameter command.
The management can query the current migration capabilities using
query-migrate-parameters
Signed-off-by: Orit Wasserman <owasserm@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
hmp-commands.hx | 16 ++++++++++++
hmp.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
hmp.h | 2 +
migration.c | 42 +++++++++++++++++++++++++++++++-
migration.h | 2 +
monitor.c | 7 +++++
qapi-schema.json | 32 ++++++++++++++++++++++++
qmp-commands.hx | 60 ++++++++++++++++++++++++++++++++++++++++++++-
8 files changed, 229 insertions(+), 3 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 8786148..3e15338 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -861,6 +861,20 @@ Set maximum tolerated downtime (in seconds) for migration.
ETEXI
{
+ .name = "migrate_set_parameter",
+ .args_type = "capability:s,state:b",
+ .params = "capability state",
+ .help = "Enable/Disable the usage of a capability for migration",
+ .mhandler.cmd = hmp_migrate_set_parameter,
+ },
+
+STEXI
+@item migrate_set_parameter @var{capability} @var{state}
+@findex migrate_set_parameter
+Enable/Disable the usage of a capability @var{capability} for migration.
+ETEXI
+
+ {
.name = "client_migrate_info",
.args_type = "protocol:s,hostname:s,port:i?,tls-port:i?,cert-subject:s?",
.params = "protocol hostname port tls-port cert-subject",
@@ -1419,6 +1433,8 @@ show user network stack connection states
show migration status
@item info migration_capabilities
show migration capabilities
+@item info migrate_parameters
+show current migration parameters
@item info balloon
show balloon information
@item info qtree
diff --git a/hmp.c b/hmp.c
index 5c7d0be..f2f63fd 100644
--- a/hmp.c
+++ b/hmp.c
@@ -131,8 +131,22 @@ void hmp_info_mice(Monitor *mon)
void hmp_info_migrate(Monitor *mon)
{
MigrationInfo *info;
+ MigrationCapabilityStatusList *cap;
+ MigrationParameters *params;
info = qmp_query_migrate(NULL);
+ params = qmp_query_migrate_parameters(NULL);
+
+ /* do not display parameters during setup */
+ if (info->has_status && params->capabilities) {
+ monitor_printf(mon, "capabilities: ");
+ for (cap = params->capabilities; cap; cap = cap->next) {
+ monitor_printf(mon, "%s: %s ",
+ MigrationCapability_lookup[cap->value->capability],
+ cap->value->state ? "on" : "off");
+ }
+ monitor_printf(mon, "\n");
+ }
if (info->has_status) {
monitor_printf(mon, "Migration status: %s\n", info->status);
@@ -159,6 +173,7 @@ void hmp_info_migrate(Monitor *mon)
}
qapi_free_MigrationInfo(info);
+ qapi_free_MigrationParameters(params);
}
void hmp_info_migration_capabilities(Monitor *mon)
@@ -180,6 +195,27 @@ void hmp_info_migration_capabilities(Monitor *mon)
qapi_free_MigrationCapabilityStatusList(caps_list);
}
+void hmp_info_migrate_parameters(Monitor *mon)
+{
+
+ MigrationCapabilityStatusList *cap;
+ MigrationParameters *params;
+
+ params = qmp_query_migrate_parameters(NULL);
+
+ if (params->capabilities) {
+ monitor_printf(mon, "capabilities: ");
+ for (cap = params->capabilities; cap; cap = cap->next) {
+ monitor_printf(mon, "%s: %s ",
+ MigrationCapability_lookup[cap->value->capability],
+ cap->value->state ? "on" : "off");
+ }
+ monitor_printf(mon, "\n");
+ }
+
+ qapi_free_MigrationParameters(params);
+}
+
void hmp_info_cpus(Monitor *mon)
{
CpuInfoList *cpu_list, *cpu;
@@ -754,6 +790,41 @@ void hmp_migrate_set_speed(Monitor *mon, const QDict *qdict)
qmp_migrate_set_speed(value, NULL);
}
+void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
+{
+ const char *cap = qdict_get_str(qdict, "capability");
+ bool state = qdict_get_bool(qdict, "state");
+ Error *err = NULL;
+ MigrationCapabilityStatusList *params = NULL;
+ int i;
+
+ for (i = 0; i < MIGRATION_CAPABILITY_MAX; i++) {
+ if (strcmp(cap, MigrationCapability_lookup[i]) == 0) {
+ if (!params) {
+ params = g_malloc0(sizeof(*params));
+ }
+ params->value = g_malloc0(sizeof(*params->value));
+ params->value->capability = i;
+ params->value->state = state;
+ params->next = NULL;
+ qmp_migrate_set_parameters(params, &err);
+ break;
+ }
+ }
+
+ if (i == MIGRATION_CAPABILITY_MAX) {
+ error_set(&err, QERR_INVALID_PARAMETER, cap);
+ }
+
+ qapi_free_MigrationCapabilityStatusList(params);
+
+ if (err) {
+ monitor_printf(mon, "migrate_set_parameter: %s\n",
+ error_get_pretty(err));
+ error_free(err);
+ }
+}
+
void hmp_set_password(Monitor *mon, const QDict *qdict)
{
const char *protocol = qdict_get_str(qdict, "protocol");
diff --git a/hmp.h b/hmp.h
index 2fb44ca..ceb2f06 100644
--- a/hmp.h
+++ b/hmp.h
@@ -26,6 +26,7 @@ void hmp_info_chardev(Monitor *mon);
void hmp_info_mice(Monitor *mon);
void hmp_info_migrate(Monitor *mon);
void hmp_info_migration_capabilities(Monitor *mon);
+void hmp_info_migrate_parameters(Monitor *mon);
void hmp_info_cpus(Monitor *mon);
void hmp_info_block(Monitor *mon);
void hmp_info_blockstats(Monitor *mon);
@@ -52,6 +53,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_parameter(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 8c27347..e844290 100644
--- a/migration.c
+++ b/migration.c
@@ -113,6 +113,25 @@ uint64_t migrate_max_downtime(void)
return max_downtime;
}
+MigrationParameters *qmp_query_migrate_parameters(Error **errp)
+{
+ MigrationParameters *params = g_malloc0(sizeof(*params));
+ MigrationState *s = migrate_get_current();
+ int i;
+
+ params->capabilities = g_malloc0(sizeof(*params->capabilities));
+
+ for (i = 0; i < MIGRATION_CAPABILITY_MAX; i++) {
+ params->capabilities->value =
+ g_malloc(sizeof(*params->capabilities->value));
+ params->capabilities->value->capability = i;
+ params->capabilities->value->state = s->enabled_capabilities[i];
+ params->capabilities->next = NULL;
+ }
+
+ return params;
+}
+
MigrationInfo *qmp_query_migrate(Error **errp)
{
MigrationInfo *info = g_malloc0(sizeof(*info));
@@ -177,6 +196,22 @@ MigrationCapabilityStatusList *qmp_query_migration_capabilities(Error **errp)
return caps_list;
}
+void qmp_migrate_set_parameters(MigrationCapabilityStatusList *params,
+ Error **errp)
+{
+ MigrationState *s = migrate_get_current();
+ MigrationCapabilityStatusList *cap;
+
+ if (s->state == MIG_STATE_ACTIVE) {
+ error_set(errp, QERR_MIGRATION_ACTIVE);
+ return;
+ }
+
+ for (cap = params; cap; cap = cap->next) {
+ s->enabled_capabilities[cap->value->capability] = cap->value->state;
+ }
+}
+
/* shared migration helpers */
static int migrate_fd_cleanup(MigrationState *s)
@@ -386,12 +421,17 @@ static MigrationState *migrate_init(const MigrationParams *params)
{
MigrationState *s = migrate_get_current();
int64_t bandwidth_limit = s->bandwidth_limit;
+ bool enabled_capabilities[MIGRATION_CAPABILITY_MAX];
+
+ memcpy(enabled_capabilities, s->enabled_capabilities,
+ sizeof(enabled_capabilities));
memset(s, 0, sizeof(*s));
s->bandwidth_limit = bandwidth_limit;
s->params = *params;
+ memcpy(s->enabled_capabilities, enabled_capabilities,
+ sizeof(enabled_capabilities));
- s->bandwidth_limit = bandwidth_limit;
s->state = MIG_STATE_SETUP;
s->total_time = qemu_get_clock_ms(rt_clock);
diff --git a/migration.h b/migration.h
index 57572a6..713aae0 100644
--- a/migration.h
+++ b/migration.h
@@ -19,6 +19,7 @@
#include "notify.h"
#include "error.h"
#include "vmstate.h"
+#include "qapi-types.h"
struct MigrationParams {
bool blk;
@@ -39,6 +40,7 @@ struct MigrationState
void *opaque;
MigrationParams params;
int64_t total_time;
+ bool enabled_capabilities[MIGRATION_CAPABILITY_MAX];
};
void process_incoming_migration(QEMUFile *f);
diff --git a/monitor.c b/monitor.c
index fd57c5e..e07af97 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2669,6 +2669,13 @@ static mon_cmd_t info_cmds[] = {
.mhandler.info = hmp_info_migration_capabilities,
},
{
+ .name = "migrate-parameters",
+ .args_type = "",
+ .params = "",
+ .help = "show current migration parameters",
+ .mhandler.info = hmp_info_migrate_parameters,
+ },
+ {
.name = "balloon",
.args_type = "",
.params = "",
diff --git a/qapi-schema.json b/qapi-schema.json
index b4d4dd6..9d6759d 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -345,6 +345,38 @@
{ 'command': 'query-migration-capabilities', 'returns': ['MigrationCapabilityStatus'] }
##
+# @migrate-set-parameters
+#
+# Enable/Disable the following migration capabilities (like xbzrle)
+#
+# Since: 1.2
+##
+{ 'command': 'migrate-set-parameters',
+ 'data': { 'capabilities': ['MigrationCapabilityStatus'] } }
+
+##
+# @MigrationParameters
+#
+# @capabilities: @MigrationCapabilityStatus list contain current migration
+# capabilities status
+# Since: 1.2
+##
+{ 'type': 'MigrationParameters',
+ 'data': {'capabilities': ['MigrationCapabilityStatus']} }
+
+
+##
+# @query-migrate-parameters
+#
+# Returns information about the current migration capabilities status
+#
+# Returns: @MigrationParameters
+#
+# Since: 1.2
+##
+{ 'command': 'query-migrate-parameters', 'returns': 'MigrationParameters' }
+
+##
# @MouseInfo:
#
# Information about a mouse device.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index c0ed14c..7f40e2a 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2081,7 +2081,6 @@ The main json-object contains the following:
- "transferred": amount transferred (json-int)
- "remaining": amount remaining (json-int)
- "total": total (json-int)
-
Examples:
1. Before the first migration
@@ -2092,7 +2091,15 @@ Examples:
2. Migration is done and has succeeded
-> { "execute": "query-migrate" }
-<- { "return": { "status": "completed" } }
+<- { "return": {
+ "status": "completed",
+ "ram":{
+ "transferred":123,
+ "remaining":123,
+ "total":246
+ }
+ }
+ }
3. Migration is done and has failed
@@ -2165,6 +2172,55 @@ EQMP
},
SQMP
+migrate-set-parameters
+-------
+
+Enable/Disable migration capabilities
+
+- "xbzrle": xbzrle support
+
+Arguments:
+
+Example:
+
+-> { "execute": "migrate-set-parameters" , "arguments":
+ { "parameters": [ { "capability": "xbzrle", "state": true } ] } }
+
+EQMP
+
+ {
+ .name = "migrate_set_parameters",
+ .args_type = "parameters:O",
+ .params = "capability:s,state:b",
+ .mhandler.cmd_new = qmp_marshal_input_migrate_set_parameters,
+ },
+SQMP
+query-migrate-parameters
+-------
+
+Query current migration parameters
+
+- "capabilities": migration capabilities state
+ - "xbzrle" : XBZRLE state (json-bool)
+
+Arguments:
+
+Example:
+
+-> { "execute": "query-migrate-parameters" }
+<- { "return": {
+ "capabilities" : [ { "capability" : "xbzrle", "state" : false } ]
+ }
+ }
+EQMP
+
+ {
+ .name = "query-migrate-parameters",
+ .args_type = "",
+ .mhandler.cmd_new = qmp_marshal_input_query_migrate_parameters,
+ },
+
+SQMP
query-balloon
-------------
--
1.7.7.6
^ permalink raw reply related [flat|nested] 33+ messages in thread* [Qemu-devel] [PATCH 03/11] Add XBZRLE documentation
2012-07-25 14:50 [Qemu-devel] [PATCH 00/11] Migration next v6 Orit Wasserman
` (2 preceding siblings ...)
2012-07-25 14:50 ` [Qemu-devel] [PATCH 02/11] Add migrate_set_parameter and query-migrate-parameters Orit Wasserman
@ 2012-07-25 14:50 ` Orit Wasserman
2012-07-25 14:50 ` [Qemu-devel] [PATCH 04/11] Add cache handling functions Orit Wasserman
` (7 subsequent siblings)
11 siblings, 0 replies; 33+ messages in thread
From: Orit Wasserman @ 2012-07-25 14:50 UTC (permalink / raw)
To: qemu-devel
Cc: peter.maydell, aliguori, quintela, stefanha, mdroth, lcapitulino,
blauwirbel, Orit Wasserman, chegu_vinod, avi, pbonzini, eblake
Signed-off-by: Orit Wasserman <owasserm@redhat.com>
---
docs/xbzrle.txt | 136 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 136 insertions(+), 0 deletions(-)
create mode 100644 docs/xbzrle.txt
diff --git a/docs/xbzrle.txt b/docs/xbzrle.txt
new file mode 100644
index 0000000..f70e851
--- /dev/null
+++ b/docs/xbzrle.txt
@@ -0,0 +1,136 @@
+XBZRLE (Xor Based Zero Run Length Encoding)
+===========================================
+
+Using XBZRLE (Xor Based Zero Run Length Encoding) allows for the reduction
+of VM downtime and the total live-migration time of Virtual machines.
+It is particularly useful for virtual machines running memory write intensive
+workloads that are typical of large enterprise applications such as SAP ERP
+Systems, and generally speaking for any application that uses a sparse memory
+update pattern.
+
+Instead of sending the changed guest memory page this solution will send a
+compressed version of the updates, thus reducing the amount of data sent during
+live migration.
+In order to be able to calculate the update, the previous memory pages need to
+be stored on the source. Those pages are stored in a dedicated cache
+(hash table) and are
+accessed by their address.
+The larger the cache size the better the chances are that the page has already
+been stored in the cache.
+A small cache size will result in high cache miss rate.
+Cache size can be changed before and during migration.
+
+Format
+=======
+
+The compression format performs a XOR between the previous and current content
+of the page, where zero represents an unchanged value.
+The page data delta is represented by zero and non zero runs.
+A zero run is represented by its length (in bytes).
+A non zero run is represented by its length (in bytes) and the new data.
+The run length is encoded using ULEB128 (http://en.wikipedia.org/wiki/LEB128)
+
+There can be more than one valid encoding, the sender may send a longer encoding
+for the benefit of reducing computation cost.
+
+page = zrun nzrun
+ | zrun nzrun page
+
+zrun = length
+
+nzrun = length byte...
+
+length = uleb128 encoded integer
+
+On the sender side XBZRLE is used as a compact delta encoding of page updates,
+retrieving the old page content from the cache (default size of 512 MB). The
+receiving side uses the existing page's content and XBZRLE to decode the new
+page's content.
+
+This work was originally based on research results published
+VEE 2011: Evaluation of Delta Compression Techniques for Efficient Live
+Migration of Large Virtual Machines by Benoit, Svard, Tordsson and Elmroth.
+Additionally the delta encoder XBRLE was improved further using the XBZRLE
+instead.
+
+XBZRLE has a sustained bandwidth of 2-2.5 GB/s for typical workloads making it
+ideal for in-line, real-time encoding such as is needed for live-migration.
+
+Example
+old buffer:
+1001 zeros
+05 06 07 08 09 0a 0b 0c 0d 0e 0f 10 11 12 13 68 00 00 6b 00 6d
+3074 zeros
+
+new buffer:
+1001 zeros
+01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f 68 00 00 67 00 69
+3074 zeros
+
+encoded buffer:
+
+encoded length 24
+e9 07 0f 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f 03 01 67 01 01 69
+
+Migration Capabilities
+======================
+In order to use XBZRLE the destination QEMU version should be able to
+decode the new format.
+Adding a new migration capabilities command that will allow external management
+to query for it support.
+A typical use for the destination
+ {qemu} info migrate_capabilities
+ {qemu} xbzrle, ...
+
+In order to enable capabilities for future live migration,
+a new command migrate_set_parameter is introduced:
+ {qemu} migrate_set_parameter xbzrle
+
+Usage
+======
+
+1. Activate xbzrle
+2. Set the XBZRLE cache size - the cache size is in MBytes and should be a
+power of 2. The cache default value is 64MBytes.
+3. start outgoing migration
+
+A typical usage scenario:
+On the incoming QEMU:
+ {qemu} migrate_set_parameter xbzrle on
+On the outgoing QEMU:
+ {qemu} migrate_set_parameter xbzrle on
+ {qemu} migrate_set_cachesize 256m
+ {qemu} migrate -d tcp:destination.host:4444
+ {qemu} info migrate
+ ...
+ cache size: 67108864 bytes
+ transferred ram-duplicate: A kbytes
+ transferred ram-normal: B kbytes
+ transferred ram-xbrle: C kbytes
+ overflow ram-xbrle: D pages
+ cache-miss ram-xbrle: E pages
+
+cache-miss: the number of cache misses to date - high cache-miss rate
+indicates that the cache size is set too low.
+overflow: the number of overflows in the decoding which where the delta could
+not be compressed. This can happen if the changes in the pages are too large
+or there are many short changes; for example, changing every second byte (half a
+page).
+
+Testing: Testing indicated that live migration with XBZRLE was completed in 110
+seconds, whereas without it would not be able to complete.
+
+A simple synthetic memory r/w load generator:
+.. include <stdlib.h>
+.. include <stdio.h>
+.. int main()
+.. {
+.. char *buf = (char *) calloc(4096, 4096);
+.. while (1) {
+.. int i;
+.. for (i = 0; i < 4096 * 4; i++) {
+.. buf[i * 4096 / 4]++;
+.. }
+.. printf(".");
+.. }
+.. }
--
1.7.7.6
^ permalink raw reply related [flat|nested] 33+ messages in thread* [Qemu-devel] [PATCH 04/11] Add cache handling functions
2012-07-25 14:50 [Qemu-devel] [PATCH 00/11] Migration next v6 Orit Wasserman
` (3 preceding siblings ...)
2012-07-25 14:50 ` [Qemu-devel] [PATCH 03/11] Add XBZRLE documentation Orit Wasserman
@ 2012-07-25 14:50 ` Orit Wasserman
2012-07-26 21:51 ` Eric Blake
2012-07-25 14:50 ` [Qemu-devel] [PATCH 05/11] Add uleb encoding/decoding functions Orit Wasserman
` (6 subsequent siblings)
11 siblings, 1 reply; 33+ messages in thread
From: Orit Wasserman @ 2012-07-25 14:50 UTC (permalink / raw)
To: qemu-devel
Cc: peter.maydell, aliguori, quintela, Petter Svard, stefanha,
mdroth, Benoit Hudzia, lcapitulino, blauwirbel, Orit Wasserman,
chegu_vinod, avi, Aidan Shribman, pbonzini, eblake
Add LRU page cache mechanism.
The page are accessed by their address.
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>
---
Makefile.objs | 1 +
cutils.c | 9 ++
include/qemu/page_cache.h | 79 ++++++++++++++++
page_cache.c | 216 +++++++++++++++++++++++++++++++++++++++++++++
qemu-common.h | 13 +++
5 files changed, 318 insertions(+), 0 deletions(-)
create mode 100644 include/qemu/page_cache.h
create mode 100644 page_cache.c
diff --git a/Makefile.objs b/Makefile.objs
index 5ebbcfa..e0fb69b 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -77,6 +77,7 @@ common-obj-y += qemu-char.o #aio.o
common-obj-y += block-migration.o iohandler.o
common-obj-y += pflib.o
common-obj-y += bitmap.o bitops.o
+common-obj-y += page_cache.o
common-obj-$(CONFIG_POSIX) += migration-exec.o migration-unix.o migration-fd.o
common-obj-$(CONFIG_WIN32) += version.o
diff --git a/cutils.c b/cutils.c
index e2bc1b8..b0bdd4b 100644
--- a/cutils.c
+++ b/cutils.c
@@ -375,3 +375,12 @@ int qemu_parse_fd(const char *param)
}
return fd;
}
+
+/* round down to the nearest power of 2*/
+int64_t pow2floor(int64_t value)
+{
+ if (!is_power_of_2(value)) {
+ value = 0x8000000000000000ULL >> clz64(value);
+ }
+ return value;
+}
diff --git a/include/qemu/page_cache.h b/include/qemu/page_cache.h
new file mode 100644
index 0000000..3839ac7
--- /dev/null
+++ b/include/qemu/page_cache.h
@@ -0,0 +1,79 @@
+/*
+ * Page cache for QEMU
+ * The cache is base on a hash of the page address
+ *
+ * Copyright 2012 Red Hat, Inc. and/or its affiliates
+ *
+ * Authors:
+ * Orit Wasserman <owasserm@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef PAGE_CACHE_H
+#define PAGE_CACHE_H
+
+/* Page cache for storing guest pages */
+typedef struct PageCache PageCache;
+
+/**
+ * cache_init: Initialize the page cache
+ *
+ *
+ * Returns new allocated cache or NULL on error
+ *
+ * @cache pointer to the PageCache struct
+ * @num_pages: cache maximal number of cached pages
+ * @page_size: cache page size
+ */
+PageCache *cache_init(int64_t num_pages, unsigned int page_size);
+
+/**
+ * cache_fini: free all cache resources
+ * @cache pointer to the PageCache struct
+ */
+void cache_fini(PageCache *cache);
+
+/**
+ * cache_is_cached: Checks to see if the page is cached
+ *
+ * Returns %true if page is cached
+ *
+ * @cache pointer to the PageCache struct
+ * @addr: page addr
+ */
+bool cache_is_cached(const PageCache *cache, uint64_t addr);
+
+/**
+ * get_cached_data: Get the data cached for an addr
+ *
+ * Returns pointer to the data cached or NULL if not cached
+ *
+ * @cache pointer to the PageCache struct
+ * @addr: page addr
+ */
+uint8_t *get_cached_data(const PageCache *cache, uint64_t addr);
+
+/**
+ * cache_insert: insert the page into the cache. the previous value will be overwritten
+ *
+ * @cache pointer to the PageCache struct
+ * @addr: page address
+ * @pdata: pointer to the page
+ */
+void cache_insert(PageCache *cache, uint64_t addr, uint8_t *pdata);
+
+/**
+ * cache_resize: resize the page cache. In case of size reduction the extra
+ * pages will be freed
+ *
+ * Returns -1 on error new cache size on success
+ *
+ * @cache pointer to the PageCache struct
+ * @num_pages: new page cache size (in pages)
+ */
+int64_t cache_resize(PageCache *cache, int64_t num_pages);
+
+#endif
diff --git a/page_cache.c b/page_cache.c
new file mode 100644
index 0000000..8110273
--- /dev/null
+++ b/page_cache.c
@@ -0,0 +1,216 @@
+/*
+ * Page cache for QEMU
+ * The cache is base on a hash of the page address
+ *
+ * Copyright 2012 Red Hat, Inc. and/or its affiliates
+ *
+ * Authors:
+ * Orit Wasserman <owasserm@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <strings.h>
+#include <string.h>
+#include <sys/time.h>
+#include <sys/types.h>
+#include <stdbool.h>
+#include <glib.h>
+#include <strings.h>
+
+#include "qemu-common.h"
+#include "qemu/page_cache.h"
+
+#ifdef DEBUG_CACHE
+#define DPRINTF(fmt, ...) \
+ do { fprintf(stdout, "cache: " fmt, ## __VA_ARGS__); } while (0)
+#else
+#define DPRINTF(fmt, ...) \
+ do { } while (0)
+#endif
+
+typedef struct CacheItem CacheItem;
+
+struct CacheItem {
+ uint64_t it_addr;
+ uint64_t it_age;
+ uint8_t *it_data;
+};
+
+struct PageCache {
+ CacheItem *page_cache;
+ unsigned int page_size;
+ int64_t max_num_items;
+ uint64_t max_item_age;
+ int64_t num_items;
+};
+
+PageCache *cache_init(int64_t num_pages, unsigned int page_size)
+{
+ int64_t i;
+
+ PageCache *cache = g_malloc(sizeof(*cache));
+
+ if (num_pages <= 0) {
+ DPRINTF("invalid number of pages\n");
+ return NULL;
+ }
+
+ /* round down to the nearest power of 2 */
+ if (!is_power_of_2(num_pages)) {
+ num_pages = pow2floor(num_pages);
+ DPRINTF("rounding down to %" PRId64 "\n", num_pages);
+ }
+ cache->page_size = page_size;
+ cache->num_items = 0;
+ cache->max_item_age = 0;
+ cache->max_num_items = num_pages;
+
+ DPRINTF("Setting cache buckets to %" PRId64 "\n", cache->max_num_items);
+
+ cache->page_cache = g_malloc((cache->max_num_items) *
+ sizeof(*cache->page_cache));
+
+ for (i = 0; i < cache->max_num_items; i++) {
+ cache->page_cache[i].it_data = NULL;
+ cache->page_cache[i].it_age = 0;
+ cache->page_cache[i].it_addr = -1;
+ }
+
+ return cache;
+}
+
+void cache_fini(PageCache *cache)
+{
+ int64_t i;
+
+ g_assert(cache);
+ g_assert(cache->page_cache);
+
+ for (i = 0; i < cache->max_num_items; i++) {
+ g_free(cache->page_cache[i].it_data);
+ }
+
+ g_free(cache->page_cache);
+ cache->page_cache = NULL;
+}
+
+static unsigned long cache_get_cache_pos(const PageCache *cache,
+ uint64_t address)
+{
+ unsigned long pos;
+
+ g_assert(cache->max_num_items);
+ pos = (address / cache->page_size) & (cache->max_num_items - 1);
+ return pos;
+}
+
+bool cache_is_cached(const PageCache *cache, uint64_t addr)
+{
+ unsigned long pos;
+
+ g_assert(cache);
+ g_assert(cache->page_cache);
+
+ pos = cache_get_cache_pos(cache, addr);
+
+ return (cache->page_cache[pos].it_addr == addr);
+}
+
+static CacheItem *cache_get_by_addr(const PageCache *cache, uint64_t addr)
+{
+ unsigned long pos;
+
+ g_assert(cache);
+ g_assert(cache->page_cache);
+
+ pos = cache_get_cache_pos(cache, addr);
+
+ return &cache->page_cache[pos];
+}
+
+uint8_t *get_cached_data(const PageCache *cache, uint64_t addr)
+{
+ return cache_get_by_addr(cache, addr)->it_data;
+}
+
+void cache_insert(PageCache *cache, uint64_t addr, uint8_t *pdata)
+{
+
+ CacheItem *it = NULL;
+
+ g_assert(cache);
+ g_assert(cache->page_cache);
+
+ /* actual update of entry */
+ it = cache_get_by_addr(cache, addr);
+
+ if (!it->it_data) {
+ cache->num_items++;
+ }
+
+ it->it_data = pdata;
+ it->it_age = ++cache->max_item_age;
+ it->it_addr = addr;
+}
+
+int64_t cache_resize(PageCache *cache, int64_t new_num_pages)
+{
+ PageCache *new_cache;
+ int64_t i;
+
+ CacheItem *old_it, *new_it;
+
+ g_assert(cache);
+
+ /* cache was not inited */
+ if (cache->page_cache == NULL) {
+ return -1;
+ }
+
+ /* same size */
+ if (pow2floor(new_num_pages) == cache->max_num_items) {
+ return cache->max_num_items;
+ }
+
+ new_cache = cache_init(new_num_pages, cache->page_size);
+ if (!(new_cache)) {
+ DPRINTF("Error creating new cache\n");
+ return -1;
+ }
+
+ /* move all data from old cache */
+ for (i = 0; i < cache->max_num_items; i++) {
+ old_it = &cache->page_cache[i];
+ if (old_it->it_addr != -1) {
+ /* check for collision , if there is, keep the first value */
+ new_it = cache_get_by_addr(new_cache, old_it->it_addr);
+ if (new_it->it_data) {
+ /* keep the oldest page */
+ if (new_it->it_age >= old_it->it_age) {
+ g_free(old_it->it_data);
+ } else {
+ g_free(new_it->it_data);
+ new_it->it_data = old_it->it_data;
+ new_it->it_age = old_it->it_age;
+ new_it->it_addr = old_it->it_addr;
+ }
+ } else {
+ cache_insert(new_cache, old_it->it_addr, old_it->it_data);
+ }
+ }
+ }
+
+ cache->page_cache = new_cache->page_cache;
+ cache->max_num_items = new_cache->max_num_items;
+ cache->num_items = new_cache->num_items;
+
+ g_free(new_cache);
+
+ return cache->max_num_items;
+}
diff --git a/qemu-common.h b/qemu-common.h
index 09676f5..195bab5 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -1,3 +1,4 @@
+
/* Common header file that is included by all of qemu. */
#ifndef QEMU_COMMON_H
#define QEMU_COMMON_H
@@ -411,6 +412,18 @@ static inline uint64_t muldiv64(uint64_t a, uint32_t b, uint32_t c)
/* Round number up to multiple */
#define QEMU_ALIGN_UP(n, m) QEMU_ALIGN_DOWN((n) + (m) - 1, (m))
+static inline bool is_power_of_2(int64_t value)
+{
+ if (!value) {
+ return 0;
+ }
+
+ return !(value & (value - 1));
+}
+
+/* round down to the nearest power of 2*/
+int64_t pow2floor(int64_t value);
+
#include "module.h"
#endif
--
1.7.7.6
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH 04/11] Add cache handling functions
2012-07-25 14:50 ` [Qemu-devel] [PATCH 04/11] Add cache handling functions Orit Wasserman
@ 2012-07-26 21:51 ` Eric Blake
2012-07-29 6:16 ` Orit Wasserman
0 siblings, 1 reply; 33+ messages in thread
From: Eric Blake @ 2012-07-26 21:51 UTC (permalink / raw)
To: Orit Wasserman
Cc: peter.maydell, aliguori, quintela, stefanha, qemu-devel, mdroth,
blauwirbel, Petter Svard, Benoit Hudzia, avi, Aidan Shribman,
pbonzini, lcapitulino, chegu_vinod
[-- Attachment #1: Type: text/plain, Size: 3131 bytes --]
On 07/25/2012 08:50 AM, Orit Wasserman wrote:
> Add LRU page cache mechanism.
> The page are accessed by their address.
>
> 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>
> +
> +PageCache *cache_init(int64_t num_pages, unsigned int page_size)
> +{
> + int64_t i;
> +
> + PageCache *cache = g_malloc(sizeof(*cache));
> +
> + if (num_pages <= 0) {
> + DPRINTF("invalid number of pages\n");
> + return NULL;
Unless memory returned by g_malloc() is automatically garbage collected,
then this is a memory leak.
> +static unsigned long cache_get_cache_pos(const PageCache *cache,
> + uint64_t address)
> +{
> + unsigned long pos;
On a 32-bit platform, this could be 32 bits...
> +
> + g_assert(cache->max_num_items);
> + pos = (address / cache->page_size) & (cache->max_num_items - 1);
while cache->max_num_items is int64_t and could thus overflow. Then
again, a 32-bit platform can't access more than 4G memory, so I think
this limitation is theoretical; still, I can't help but wonder if you
should be consistently using size_t instead of a mix of 'unsigned int',
'int32_t', and 'unsigned long' in referring to sizing within your cache
table.
> +int64_t cache_resize(PageCache *cache, int64_t new_num_pages)
> +{
> +
> + /* move all data from old cache */
> + for (i = 0; i < cache->max_num_items; i++) {
> + old_it = &cache->page_cache[i];
> + if (old_it->it_addr != -1) {
> + /* check for collision , if there is, keep the first value */
No space before ',' in English sentences.
The comment about 'keep the first value' is wrong, you are keeping the
'MRU page'...
> + new_it = cache_get_by_addr(new_cache, old_it->it_addr);
> + if (new_it->it_data) {
> + /* keep the oldest page */
...also wrong, you are keeping the MRU page, not the oldest page...
> + if (new_it->it_age >= old_it->it_age) {
> + g_free(old_it->it_data);
since a larger it_age implies more recently used.
> +++ b/qemu-common.h
> @@ -1,3 +1,4 @@
> +
> /* Common header file that is included by all of qemu. */
> #ifndef QEMU_COMMON_H
> #define QEMU_COMMON_H
> @@ -411,6 +412,18 @@ static inline uint64_t muldiv64(uint64_t a, uint32_t b, uint32_t c)
> /* Round number up to multiple */
> #define QEMU_ALIGN_UP(n, m) QEMU_ALIGN_DOWN((n) + (m) - 1, (m))
>
> +static inline bool is_power_of_2(int64_t value)
> +{
> + if (!value) {
> + return 0;
> + }
> +
> + return !(value & (value - 1));
Technically undefined by C99 if value is INT64_MIN, since 'value - 1'
then overflows. Do you want this function to take uint64_t instead, to
guarantee defined results even for 0x8000000000000000?
--
Eric Blake eblake@redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH 04/11] Add cache handling functions
2012-07-26 21:51 ` Eric Blake
@ 2012-07-29 6:16 ` Orit Wasserman
0 siblings, 0 replies; 33+ messages in thread
From: Orit Wasserman @ 2012-07-29 6:16 UTC (permalink / raw)
To: Eric Blake
Cc: peter.maydell, aliguori, quintela, stefanha, qemu-devel, mdroth,
blauwirbel, Petter Svard, Benoit Hudzia, avi, Aidan Shribman,
pbonzini, lcapitulino, chegu_vinod
On 07/27/2012 12:51 AM, Eric Blake wrote:
> On 07/25/2012 08:50 AM, Orit Wasserman wrote:
>> Add LRU page cache mechanism.
>> The page are accessed by their address.
>>
>> 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>
>
>> +
>> +PageCache *cache_init(int64_t num_pages, unsigned int page_size)
>> +{
>> + int64_t i;
>> +
>> + PageCache *cache = g_malloc(sizeof(*cache));
>> +
>> + if (num_pages <= 0) {
>> + DPRINTF("invalid number of pages\n");
>> + return NULL;
>
> Unless memory returned by g_malloc() is automatically garbage collected,
> then this is a memory leak.
>
good catch I will move the check up.
>> +static unsigned long cache_get_cache_pos(const PageCache *cache,
>> + uint64_t address)
>> +{
>> + unsigned long pos;
>
> On a 32-bit platform, this could be 32 bits...
I will switch it to uint64_t.
>
>> +
>> + g_assert(cache->max_num_items);
>> + pos = (address / cache->page_size) & (cache->max_num_items - 1);
>
> while cache->max_num_items is int64_t and could thus overflow. Then
> again, a 32-bit platform can't access more than 4G memory, so I think
> this limitation is theoretical; still, I can't help but wonder if you
> should be consistently using size_t instead of a mix of 'unsigned int',
> 'int32_t', and 'unsigned long' in referring to sizing within your cache
> table.
>
same here
>> +int64_t cache_resize(PageCache *cache, int64_t new_num_pages)
>> +{
>
>> +
>> + /* move all data from old cache */
>> + for (i = 0; i < cache->max_num_items; i++) {
>> + old_it = &cache->page_cache[i];
>> + if (old_it->it_addr != -1) {
>> + /* check for collision , if there is, keep the first value */
>
> No space before ',' in English sentences.
>
> The comment about 'keep the first value' is wrong, you are keeping the
> 'MRU page'...
>
I will fix it
>> + new_it = cache_get_by_addr(new_cache, old_it->it_addr);
>> + if (new_it->it_data) {
>> + /* keep the oldest page */
>
> ...also wrong, you are keeping the MRU page, not the oldest page...
here too
>
>> + if (new_it->it_age >= old_it->it_age) {
>> + g_free(old_it->it_data);
>
> since a larger it_age implies more recently used.
>
>> +++ b/qemu-common.h
>> @@ -1,3 +1,4 @@
>> +
>> /* Common header file that is included by all of qemu. */
>> #ifndef QEMU_COMMON_H
>> #define QEMU_COMMON_H
>> @@ -411,6 +412,18 @@ static inline uint64_t muldiv64(uint64_t a, uint32_t b, uint32_t c)
>> /* Round number up to multiple */
>> #define QEMU_ALIGN_UP(n, m) QEMU_ALIGN_DOWN((n) + (m) - 1, (m))
>>
>> +static inline bool is_power_of_2(int64_t value)
>> +{
>> + if (!value) {
>> + return 0;
>> + }
>> +
>> + return !(value & (value - 1));
>
> Technically undefined by C99 if value is INT64_MIN, since 'value - 1'
> then overflows. Do you want this function to take uint64_t instead, to
> guarantee defined results even for 0x8000000000000000?
>
good idea.
Thanks,
Orit
^ permalink raw reply [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH 05/11] Add uleb encoding/decoding functions
2012-07-25 14:50 [Qemu-devel] [PATCH 00/11] Migration next v6 Orit Wasserman
` (4 preceding siblings ...)
2012-07-25 14:50 ` [Qemu-devel] [PATCH 04/11] Add cache handling functions Orit Wasserman
@ 2012-07-25 14:50 ` Orit Wasserman
2012-07-25 14:50 ` [Qemu-devel] [PATCH 06/11] Add xbzrle_encode_buffer and xbzrle_decode_buffer functions Orit Wasserman
` (5 subsequent siblings)
11 siblings, 0 replies; 33+ messages in thread
From: Orit Wasserman @ 2012-07-25 14:50 UTC (permalink / raw)
To: qemu-devel
Cc: peter.maydell, aliguori, quintela, stefanha, mdroth, lcapitulino,
blauwirbel, Orit Wasserman, chegu_vinod, avi, pbonzini, eblake
Implement Unsigned Little Endian Base 128.
Signed-off-by: Orit Wasserman <owasserm@redhat.com>
---
cutils.c | 33 +++++++++++++++++++++++++++++++++
qemu-common.h | 8 ++++++++
2 files changed, 41 insertions(+), 0 deletions(-)
diff --git a/cutils.c b/cutils.c
index b0bdd4b..700f943 100644
--- a/cutils.c
+++ b/cutils.c
@@ -384,3 +384,36 @@ int64_t pow2floor(int64_t value)
}
return value;
}
+
+/*
+ * Implementation of ULEB128 (http://en.wikipedia.org/wiki/LEB128)
+ * Input is limited to 14-bit numbers
+ */
+int uleb128_encode_small(uint8_t *out, uint32_t n)
+{
+ g_assert(n <= 0x3fff);
+ if (n < 0x80) {
+ *out++ = n;
+ return 1;
+ } else {
+ *out++ = (n & 0x7f) | 0x80;
+ *out++ = n >> 7;
+ return 2;
+ }
+}
+
+int uleb128_decode_small(const uint8_t *in, uint32_t *n)
+{
+ if (!(*in & 0x80)) {
+ *n = *in++;
+ return 1;
+ } else {
+ *n = *in++ & 0x7f;
+ /* we exceed 14 bit number */
+ if (*in & 0x80) {
+ return -1;
+ }
+ *n |= *in++ << 7;
+ return 2;
+ }
+}
diff --git a/qemu-common.h b/qemu-common.h
index 195bab5..3188bdd 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -426,4 +426,12 @@ int64_t pow2floor(int64_t value);
#include "module.h"
+/*
+ * Implementation of ULEB128 (http://en.wikipedia.org/wiki/LEB128)
+ * Input is limited to 14-bit numbers
+ */
+
+int uleb128_encode_small(uint8_t *out, uint32_t n);
+int uleb128_decode_small(const uint8_t *in, uint32_t *n);
+
#endif
--
1.7.7.6
^ permalink raw reply related [flat|nested] 33+ messages in thread* [Qemu-devel] [PATCH 06/11] Add xbzrle_encode_buffer and xbzrle_decode_buffer functions
2012-07-25 14:50 [Qemu-devel] [PATCH 00/11] Migration next v6 Orit Wasserman
` (5 preceding siblings ...)
2012-07-25 14:50 ` [Qemu-devel] [PATCH 05/11] Add uleb encoding/decoding functions Orit Wasserman
@ 2012-07-25 14:50 ` Orit Wasserman
2012-07-26 21:58 ` Eric Blake
2012-08-15 16:22 ` Igor Mitsyanko
2012-07-25 14:50 ` [Qemu-devel] [PATCH 07/11] Add XBZRLE to ram_save_block and ram_save_live Orit Wasserman
` (4 subsequent siblings)
11 siblings, 2 replies; 33+ messages in thread
From: Orit Wasserman @ 2012-07-25 14:50 UTC (permalink / raw)
To: qemu-devel
Cc: peter.maydell, aliguori, quintela, Petter Svard, stefanha,
mdroth, Benoit Hudzia, lcapitulino, blauwirbel, Orit Wasserman,
chegu_vinod, avi, Aidan Shribman, pbonzini, eblake
For performance we are encoding long word at a time.
For nzrun we use long-word-at-a-time NULL-detection tricks from strcmp():
using ((lword - 0x0101010101010101) & (~lword) & 0x8080808080808080) test
to find out if any byte in the long word is zero.
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>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
migration.h | 4 ++
savevm.c | 159 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 163 insertions(+), 0 deletions(-)
diff --git a/migration.h b/migration.h
index 713aae0..743c366 100644
--- a/migration.h
+++ b/migration.h
@@ -100,4 +100,8 @@ void migrate_add_blocker(Error *reason);
*/
void migrate_del_blocker(Error *reason);
+int xbzrle_encode_buffer(uint8_t *old_buf, uint8_t *new_buf, int slen,
+ uint8_t *dst, int dlen);
+int xbzrle_decode_buffer(uint8_t *src, int slen, uint8_t *dst, int dlen);
+
#endif
diff --git a/savevm.c b/savevm.c
index 6e82b2d..c5fd13f 100644
--- a/savevm.c
+++ b/savevm.c
@@ -2392,3 +2392,162 @@ void vmstate_register_ram_global(MemoryRegion *mr)
{
vmstate_register_ram(mr, NULL);
}
+
+/*
+ page = zrun nzrun
+ | zrun nzrun page
+
+ zrun = length
+
+ nzrun = length byte...
+
+ length = uleb128 encoded integer
+ */
+int xbzrle_encode_buffer(uint8_t *old_buf, uint8_t *new_buf, int slen,
+ uint8_t *dst, int dlen)
+{
+ uint32_t zrun_len = 0, nzrun_len = 0;
+ int d = 0, i = 0;
+ long res, xor;
+ uint8_t *nzrun_start = NULL;
+
+ g_assert(!(((uintptr_t)old_buf | (uintptr_t)new_buf | slen) %
+ sizeof(long)));
+
+ while (i < slen) {
+ /* overflow */
+ if (d + 2 > dlen) {
+ return -1;
+ }
+
+ /* not aligned to sizeof(long) */
+ res = (slen - i) % sizeof(long);
+ while (res && old_buf[i] == new_buf[i]) {
+ zrun_len++;
+ i++;
+ res--;
+ }
+
+ /* word at a time for speed */
+ if (!res) {
+ while (i < slen &&
+ (*(long *)(old_buf + i)) == (*(long *)(new_buf + i))) {
+ i += sizeof(long);
+ zrun_len += sizeof(long);
+ }
+
+ /* go over the rest */
+ while (i < slen && old_buf[i] == new_buf[i]) {
+ zrun_len++;
+ i++;
+ }
+ }
+
+ /* buffer unchanged */
+ if (zrun_len == slen) {
+ return 0;
+ }
+
+ /* skip last zero run */
+ if (i == slen) {
+ return d;
+ }
+
+ d += uleb128_encode_small(dst + d, zrun_len);
+
+ zrun_len = 0;
+ nzrun_start = new_buf + i;
+
+ /* overflow */
+ if (d + 2 > dlen) {
+ return -1;
+ }
+ /* not aligned to sizeof(long) */
+ res = (slen - i) % sizeof(long);
+ while (res && old_buf[i] != new_buf[i]) {
+ i++;
+ nzrun_len++;
+ res--;
+ }
+
+ /* word at a time for speed, use of 32-bit long okay */
+ if (!res) {
+ /* truncation to 32-bit long okay */
+ long mask = 0x0101010101010101ULL;
+ while (i < slen) {
+ xor = *(long *)(old_buf + i) ^ *(long *)(new_buf + i);
+ if ((xor - mask) & ~xor & (mask << 7)) {
+ /* found the end of an nzrun within the current long */
+ while (old_buf[i] != new_buf[i]) {
+ nzrun_len++;
+ i++;
+ }
+ break;
+ } else {
+ i += sizeof(long);
+ nzrun_len += sizeof(long);
+ }
+ }
+ }
+
+ d += uleb128_encode_small(dst + d, nzrun_len);
+ /* overflow */
+ if (d + nzrun_len > dlen) {
+ return -1;
+ }
+ memcpy(dst + d, nzrun_start, nzrun_len);
+ d += nzrun_len;
+ nzrun_len = 0;
+ }
+
+ return d;
+}
+
+int xbzrle_decode_buffer(uint8_t *src, int slen, uint8_t *dst, int dlen)
+{
+ int i = 0, d = 0;
+ int ret;
+ uint32_t count = 0;
+
+ while (i < slen) {
+
+ /* zrun */
+ if ((slen - i) < 2) {
+ return -1;
+ }
+
+ ret = uleb128_decode_small(src + i, &count);
+ if (ret < 0 || (i && !count)) {
+ return -1;
+ }
+ i += ret;
+ d += count;
+
+ /* overflow */
+ if (d > dlen) {
+ return -1;
+ }
+
+ /* nzrun */
+ if ((slen - i) < 2) {
+ return -1;
+ }
+
+ ret = uleb128_decode_small(src + i, &count);
+ if (ret < 0 || !count) {
+ return -1;
+ }
+ i += ret;
+
+ /* overflow */
+ if (d + count > dlen || i + count > slen) {
+ return -1;
+ }
+
+ memcpy(dst + d , src + i, count);
+ d += count;
+ i += count;
+ }
+
+ return d;
+}
--
1.7.7.6
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH 06/11] Add xbzrle_encode_buffer and xbzrle_decode_buffer functions
2012-07-25 14:50 ` [Qemu-devel] [PATCH 06/11] Add xbzrle_encode_buffer and xbzrle_decode_buffer functions Orit Wasserman
@ 2012-07-26 21:58 ` Eric Blake
2012-08-15 16:22 ` Igor Mitsyanko
1 sibling, 0 replies; 33+ messages in thread
From: Eric Blake @ 2012-07-26 21:58 UTC (permalink / raw)
To: Orit Wasserman
Cc: peter.maydell, aliguori, quintela, stefanha, qemu-devel, mdroth,
blauwirbel, Petter Svard, Benoit Hudzia, avi, Aidan Shribman,
pbonzini, lcapitulino, chegu_vinod
[-- Attachment #1: Type: text/plain, Size: 1180 bytes --]
On 07/25/2012 08:50 AM, Orit Wasserman wrote:
> For performance we are encoding long word at a time.
> For nzrun we use long-word-at-a-time NULL-detection tricks from strcmp():
[Technically, 'NUL' is the byte with value 0x00, 'NULL' is the pointer
with typical value 0x00000000 or 0x0000000000000000, depending on
whether you are 32-bit or 64-bit. But it's not worth rewriting this
commit message just to delete an 'L']
> using ((lword - 0x0101010101010101) & (~lword) & 0x8080808080808080) test
> to find out if any byte in the long word is zero.
>
> 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>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
> +int xbzrle_decode_buffer(uint8_t *src, int slen, uint8_t *dst, int dlen)
> +
> + memcpy(dst + d , src + i, count);
I think coding style prefers no space before ','. But if the patch
checker didn't warn...
--
Eric Blake eblake@redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 06/11] Add xbzrle_encode_buffer and xbzrle_decode_buffer functions
2012-07-25 14:50 ` [Qemu-devel] [PATCH 06/11] Add xbzrle_encode_buffer and xbzrle_decode_buffer functions Orit Wasserman
2012-07-26 21:58 ` Eric Blake
@ 2012-08-15 16:22 ` Igor Mitsyanko
2012-08-15 16:36 ` Eric Blake
1 sibling, 1 reply; 33+ messages in thread
From: Igor Mitsyanko @ 2012-08-15 16:22 UTC (permalink / raw)
To: Orit Wasserman
Cc: peter.maydell, aliguori, quintela, stefanha, qemu-devel, eblake,
mdroth, blauwirbel, Petter Svard, Benoit Hudzia, avi,
Aidan Shribman, pbonzini, lcapitulino, chegu_vinod
This patch broke master build, it causes compilation error with gcc 4.6.1:
/home/mackross/eclipse_linux_cdt_space/qemu_exynos4/savevm.c: In
function ‘xbzrle_encode_buffer’:
/home/mackross/eclipse_linux_cdt_space/qemu_exynos4/savevm.c:2476:13:
error: overflow in implicit constant conversion [-Werror=overflow]
cc1: all warnings being treated as errors
make[1]: *** [savevm.o] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [subdir-arm-softmmu] Error 2
On 07/25/2012 06:50 PM, Orit Wasserman wrote:
> For performance we are encoding long word at a time.
> For nzrun we use long-word-at-a-time NULL-detection tricks from strcmp():
> using ((lword - 0x0101010101010101) & (~lword) & 0x8080808080808080) test
> to find out if any byte in the long word is zero.
>
> 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>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
> migration.h | 4 ++
> savevm.c | 159 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 163 insertions(+), 0 deletions(-)
>
> diff --git a/migration.h b/migration.h
> index 713aae0..743c366 100644
> --- a/migration.h
> +++ b/migration.h
> @@ -100,4 +100,8 @@ void migrate_add_blocker(Error *reason);
> */
> void migrate_del_blocker(Error *reason);
>
> +int xbzrle_encode_buffer(uint8_t *old_buf, uint8_t *new_buf, int slen,
> + uint8_t *dst, int dlen);
> +int xbzrle_decode_buffer(uint8_t *src, int slen, uint8_t *dst, int dlen);
> +
> #endif
> diff --git a/savevm.c b/savevm.c
> index 6e82b2d..c5fd13f 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -2392,3 +2392,162 @@ void vmstate_register_ram_global(MemoryRegion *mr)
> {
> vmstate_register_ram(mr, NULL);
> }
> +
> +/*
> + page = zrun nzrun
> + | zrun nzrun page
> +
> + zrun = length
> +
> + nzrun = length byte...
> +
> + length = uleb128 encoded integer
> + */
> +int xbzrle_encode_buffer(uint8_t *old_buf, uint8_t *new_buf, int slen,
> + uint8_t *dst, int dlen)
> +{
> + uint32_t zrun_len = 0, nzrun_len = 0;
> + int d = 0, i = 0;
> + long res, xor;
> + uint8_t *nzrun_start = NULL;
> +
> + g_assert(!(((uintptr_t)old_buf | (uintptr_t)new_buf | slen) %
> + sizeof(long)));
> +
> + while (i < slen) {
> + /* overflow */
> + if (d + 2 > dlen) {
> + return -1;
> + }
> +
> + /* not aligned to sizeof(long) */
> + res = (slen - i) % sizeof(long);
> + while (res && old_buf[i] == new_buf[i]) {
> + zrun_len++;
> + i++;
> + res--;
> + }
> +
> + /* word at a time for speed */
> + if (!res) {
> + while (i < slen &&
> + (*(long *)(old_buf + i)) == (*(long *)(new_buf + i))) {
> + i += sizeof(long);
> + zrun_len += sizeof(long);
> + }
> +
> + /* go over the rest */
> + while (i < slen && old_buf[i] == new_buf[i]) {
> + zrun_len++;
> + i++;
> + }
> + }
> +
> + /* buffer unchanged */
> + if (zrun_len == slen) {
> + return 0;
> + }
> +
> + /* skip last zero run */
> + if (i == slen) {
> + return d;
> + }
> +
> + d += uleb128_encode_small(dst + d, zrun_len);
> +
> + zrun_len = 0;
> + nzrun_start = new_buf + i;
> +
> + /* overflow */
> + if (d + 2 > dlen) {
> + return -1;
> + }
> + /* not aligned to sizeof(long) */
> + res = (slen - i) % sizeof(long);
> + while (res && old_buf[i] != new_buf[i]) {
> + i++;
> + nzrun_len++;
> + res--;
> + }
> +
> + /* word at a time for speed, use of 32-bit long okay */
> + if (!res) {
> + /* truncation to 32-bit long okay */
> + long mask = 0x0101010101010101ULL;
> + while (i < slen) {
> + xor = *(long *)(old_buf + i) ^ *(long *)(new_buf + i);
> + if ((xor - mask) & ~xor & (mask << 7)) {
> + /* found the end of an nzrun within the current long */
> + while (old_buf[i] != new_buf[i]) {
> + nzrun_len++;
> + i++;
> + }
> + break;
> + } else {
> + i += sizeof(long);
> + nzrun_len += sizeof(long);
> + }
> + }
> + }
> +
> + d += uleb128_encode_small(dst + d, nzrun_len);
> + /* overflow */
> + if (d + nzrun_len > dlen) {
> + return -1;
> + }
> + memcpy(dst + d, nzrun_start, nzrun_len);
> + d += nzrun_len;
> + nzrun_len = 0;
> + }
> +
> + return d;
> +}
> +
> +int xbzrle_decode_buffer(uint8_t *src, int slen, uint8_t *dst, int dlen)
> +{
> + int i = 0, d = 0;
> + int ret;
> + uint32_t count = 0;
> +
> + while (i < slen) {
> +
> + /* zrun */
> + if ((slen - i) < 2) {
> + return -1;
> + }
> +
> + ret = uleb128_decode_small(src + i, &count);
> + if (ret < 0 || (i && !count)) {
> + return -1;
> + }
> + i += ret;
> + d += count;
> +
> + /* overflow */
> + if (d > dlen) {
> + return -1;
> + }
> +
> + /* nzrun */
> + if ((slen - i) < 2) {
> + return -1;
> + }
> +
> + ret = uleb128_decode_small(src + i, &count);
> + if (ret < 0 || !count) {
> + return -1;
> + }
> + i += ret;
> +
> + /* overflow */
> + if (d + count > dlen || i + count > slen) {
> + return -1;
> + }
> +
> + memcpy(dst + d , src + i, count);
> + d += count;
> + i += count;
> + }
> +
> + return d;
> +}
>
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH 06/11] Add xbzrle_encode_buffer and xbzrle_decode_buffer functions
2012-08-15 16:22 ` Igor Mitsyanko
@ 2012-08-15 16:36 ` Eric Blake
0 siblings, 0 replies; 33+ messages in thread
From: Eric Blake @ 2012-08-15 16:36 UTC (permalink / raw)
To: Igor Mitsyanko
Cc: peter.maydell, aliguori, quintela, Petter Svard, stefanha,
qemu-devel, mdroth, blauwirbel, Orit Wasserman, chegu_vinod,
Benoit Hudzia, avi, pbonzini, lcapitulino, Aidan Shribman
[-- Attachment #1: Type: text/plain, Size: 815 bytes --]
On 08/15/2012 10:22 AM, Igor Mitsyanko wrote:
> This patch broke master build, it causes compilation error with gcc 4.6.1:
>
> /home/mackross/eclipse_linux_cdt_space/qemu_exynos4/savevm.c: In
> function ‘xbzrle_encode_buffer’:
> /home/mackross/eclipse_linux_cdt_space/qemu_exynos4/savevm.c:2476:13:
> error: overflow in implicit constant conversion [-Werror=overflow]
> cc1: all warnings being treated as errors
You're the third to notice:
https://lists.gnu.org/archive/html/qemu-devel/2012-08/msg02446.html
https://lists.gnu.org/archive/html/qemu-devel/2012-08/msg02736.html
and it's already in the ppc PULL request:
https://lists.gnu.org/archive/html/qemu-devel/2012-08/msg02755.html
--
Eric Blake eblake@redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]
^ permalink raw reply [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH 07/11] Add XBZRLE to ram_save_block and ram_save_live
2012-07-25 14:50 [Qemu-devel] [PATCH 00/11] Migration next v6 Orit Wasserman
` (6 preceding siblings ...)
2012-07-25 14:50 ` [Qemu-devel] [PATCH 06/11] Add xbzrle_encode_buffer and xbzrle_decode_buffer functions Orit Wasserman
@ 2012-07-25 14:50 ` Orit Wasserman
2012-07-26 22:20 ` Eric Blake
2012-07-25 14:50 ` [Qemu-devel] [PATCH 08/11] Add migrate_set_cachesize command Orit Wasserman
` (3 subsequent siblings)
11 siblings, 1 reply; 33+ messages in thread
From: Orit Wasserman @ 2012-07-25 14:50 UTC (permalink / raw)
To: qemu-devel
Cc: peter.maydell, aliguori, quintela, Petter Svard, stefanha,
mdroth, Benoit Hudzia, lcapitulino, blauwirbel, Orit Wasserman,
chegu_vinod, avi, Aidan Shribman, pbonzini, eblake
In the outgoing migration check to see if the page is cached and
changed than send compressed page by using save_xbrle_page function.
In the incoming migration check to see if RAM_SAVE_FLAG_XBZRLE is set
and decompress the page (by using load_xbrle function).
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 | 175 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
migration.c | 24 ++++++++
migration.h | 4 ++
3 files changed, 200 insertions(+), 3 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index 78cdf50..3998edc 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -43,6 +43,7 @@
#include "hw/smbios.h"
#include "exec-memory.h"
#include "hw/pcspk.h"
+#include "qemu/page_cache.h"
#ifdef DEBUG_ARCH_INIT
#define DPRINTF(fmt, ...) \
@@ -102,6 +103,7 @@ const uint32_t arch_type = QEMU_ARCH;
#define RAM_SAVE_FLAG_PAGE 0x08
#define RAM_SAVE_FLAG_EOS 0x10
#define RAM_SAVE_FLAG_CONTINUE 0x20
+#define RAM_SAVE_FLAG_XBZRLE 0x40
#ifdef __ALTIVEC__
#include <altivec.h>
@@ -169,6 +171,30 @@ static int is_dup_page(uint8_t *page)
return 1;
}
+/* XBZRLE (Xor Based Zero Length Encoding */
+typedef struct XBZRLEHeader {
+ uint16_t xh_len;
+ uint8_t xh_flags;
+} XBZRLEHeader;
+
+/* struct contains XBZRLE cache and a static page
+ used by the compression */
+static struct {
+ /* buffer used for XBZRLE encoding */
+ uint8_t *encoded_buf;
+ /* buffer for storing page content */
+ uint8_t *current_buf;
+ /* buffer used for XBZRLE decoding */
+ uint8_t *decoded_buf;
+ /* Cache for XBZRLE */
+ PageCache *cache;
+} XBZRLE = {
+ .encoded_buf = NULL,
+ .current_buf = NULL,
+ .decoded_buf = NULL,
+ .cache = NULL,
+};
+
static void save_block_hdr(QEMUFile *f, RAMBlock *block, ram_addr_t offset,
int cont, int flag)
{
@@ -181,6 +207,61 @@ static void save_block_hdr(QEMUFile *f, RAMBlock *block, ram_addr_t offset,
}
+#define ENCODING_FLAG_XBZRLE 0x1
+
+static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data,
+ ram_addr_t current_addr, RAMBlock *block,
+ ram_addr_t offset, int cont)
+{
+ int encoded_len = 0, bytes_sent = -1;
+ XBZRLEHeader hdr = {
+ .xh_len = 0,
+ .xh_flags = 0,
+ };
+ uint8_t *prev_cached_page;
+
+ if (!cache_is_cached(XBZRLE.cache, current_addr)) {
+ cache_insert(XBZRLE.cache, current_addr,
+ g_memdup(current_data, TARGET_PAGE_SIZE));
+ return -1;
+ }
+
+ prev_cached_page = get_cached_data(XBZRLE.cache, current_addr);
+
+ /* save current buffer into memory */
+ memcpy(XBZRLE.current_buf, current_data, TARGET_PAGE_SIZE);
+
+ /* XBZRLE encoding (if there is no overflow) */
+ encoded_len = xbzrle_encode_buffer(prev_cached_page, XBZRLE.current_buf,
+ TARGET_PAGE_SIZE, XBZRLE.encoded_buf,
+ TARGET_PAGE_SIZE);
+ if (encoded_len == 0) {
+ DPRINTF("Skipping unmodified page\n");
+ return 0;
+ } else if (encoded_len == -1) {
+ DPRINTF("Overflow\n");
+ /* update data in the cache */
+ memcpy(prev_cached_page, current_data, TARGET_PAGE_SIZE);
+ return -1;
+ }
+
+ /* we need to update the data in the cache, in order to get the same data
+ we cached we decode the encoded page on the cached data */
+ memcpy(prev_cached_page, XBZRLE.current_buf, TARGET_PAGE_SIZE);
+
+ hdr.xh_len = encoded_len;
+ hdr.xh_flags |= ENCODING_FLAG_XBZRLE;
+
+ /* Send XBZRLE based compressed page */
+ save_block_hdr(f, block, offset, cont, RAM_SAVE_FLAG_XBZRLE);
+ qemu_put_byte(f, hdr.xh_flags);
+ qemu_put_be16(f, hdr.xh_len);
+ qemu_put_buffer(f, XBZRLE.encoded_buf, encoded_len);
+ bytes_sent = encoded_len + sizeof(hdr);
+
+ return bytes_sent;
+}
+
static RAMBlock *last_block;
static ram_addr_t last_offset;
@@ -198,6 +279,7 @@ static int ram_save_block(QEMUFile *f)
ram_addr_t offset = last_offset;
int bytes_sent = -1;
MemoryRegion *mr;
+ ram_addr_t current_addr;
if (!block)
block = QLIST_FIRST(&ram_list.blocks);
@@ -218,13 +300,24 @@ static int ram_save_block(QEMUFile *f)
save_block_hdr(f, block, offset, cont, RAM_SAVE_FLAG_COMPRESS);
qemu_put_byte(f, *p);
bytes_sent = 1;
- } else {
+ } else if (migrate_use_xbzrle()) {
+ current_addr = block->offset + offset;
+ bytes_sent = save_xbzrle_page(f, p, current_addr, block,
+ offset, cont);
+ p = get_cached_data(XBZRLE.cache, current_addr);
+ }
+
+ /* either we didn't send yet (we may have had XBZRLE overflow) */
+ if (bytes_sent == -1) {
save_block_hdr(f, block, offset, cont, RAM_SAVE_FLAG_PAGE);
qemu_put_buffer(f, p, TARGET_PAGE_SIZE);
bytes_sent = TARGET_PAGE_SIZE;
}
- break;
+ /* if page is unmodified, continue to the next */
+ if (bytes_sent != 0) {
+ break;
+ }
}
offset += TARGET_PAGE_SIZE;
@@ -302,6 +395,15 @@ static void sort_ram_list(void)
static void migration_end(void)
{
memory_global_dirty_log_stop();
+
+ if (migrate_use_xbzrle()) {
+ cache_fini(XBZRLE.cache);
+ g_free(XBZRLE.cache);
+ g_free(XBZRLE.encoded_buf);
+ g_free(XBZRLE.current_buf);
+ g_free(XBZRLE.decoded_buf);
+ XBZRLE.cache = NULL;
+ }
}
static void ram_migration_cancel(void *opaque)
@@ -321,7 +423,18 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
last_offset = 0;
sort_ram_list();
- /* Make sure all dirty bits are set */
+ if (migrate_use_xbzrle()) {
+ XBZRLE.cache = cache_init(migrate_xbzrle_cache_size() /
+ TARGET_PAGE_SIZE,
+ TARGET_PAGE_SIZE);
+ if (!XBZRLE.cache) {
+ DPRINTF("Error creating cache\n");
+ return -1;
+ }
+ XBZRLE.encoded_buf = g_malloc0(TARGET_PAGE_SIZE);
+ XBZRLE.current_buf = g_malloc(TARGET_PAGE_SIZE);
+ }
+
QLIST_FOREACH(block, &ram_list.blocks, next) {
for (addr = 0; addr < block->length; addr += TARGET_PAGE_SIZE) {
if (!memory_region_get_dirty(block->mr, addr, TARGET_PAGE_SIZE,
@@ -436,6 +549,49 @@ static int ram_save_complete(QEMUFile *f, void *opaque)
return 0;
}
+static int load_xbzrle(QEMUFile *f, ram_addr_t addr, void *host)
+{
+ int ret, rc = 0;
+ XBZRLEHeader hdr = {
+ .xh_len = 0,
+ .xh_flags = 0,
+ };
+
+ if (!XBZRLE.decoded_buf) {
+ XBZRLE.decoded_buf = g_malloc(TARGET_PAGE_SIZE);
+ }
+
+ /* extract RLE header */
+ hdr.xh_flags = qemu_get_byte(f);
+ hdr.xh_len = qemu_get_be16(f);
+
+ if (!(hdr.xh_flags & ENCODING_FLAG_XBZRLE)) {
+ fprintf(stderr, "Failed to load XBZRLE page - wrong compression!\n");
+ return -1;
+ }
+
+ if (hdr.xh_len > TARGET_PAGE_SIZE) {
+ fprintf(stderr, "Failed to load XBZRLE page - len overflow!\n");
+ return -1;
+ }
+ /* load data and decode */
+ qemu_get_buffer(f, XBZRLE.decoded_buf, hdr.xh_len);
+
+ /* decode RLE */
+ ret = xbzrle_decode_buffer(XBZRLE.decoded_buf, hdr.xh_len, host,
+ TARGET_PAGE_SIZE);
+ if (ret == -1) {
+ fprintf(stderr, "Failed to load XBZRLE page - decode error!\n");
+ rc = -1;
+ } else if (ret > TARGET_PAGE_SIZE) {
+ fprintf(stderr, "Failed to load XBZRLE page - size %d exceeds %d!\n",
+ ret, TARGET_PAGE_SIZE);
+ rc = -1;
+ }
+
+ return rc;
+}
+
static inline void *host_from_stream_offset(QEMUFile *f,
ram_addr_t offset,
int flags)
@@ -549,6 +705,19 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
}
qemu_get_buffer(f, host, TARGET_PAGE_SIZE);
+ } else if (flags & RAM_SAVE_FLAG_XBZRLE) {
+ if (!migrate_use_xbzrle()) {
+ return -EINVAL;
+ }
+ void *host = host_from_stream_offset(f, addr, flags);
+ if (!host) {
+ return -EINVAL;
+ }
+
+ if (load_xbzrle(f, addr, host) < 0) {
+ ret = -EINVAL;
+ goto done;
+ }
}
error = qemu_file_get_error(f);
if (error) {
diff --git a/migration.c b/migration.c
index e844290..167398a 100644
--- a/migration.c
+++ b/migration.c
@@ -43,6 +43,9 @@ enum {
#define MAX_THROTTLE (32 << 20) /* Migration speed throttling */
+/* Migration XBZRLE default cache size */
+#define DEFAULT_MIGRATE_CACHE_SIZE (64 * 1024 * 1024)
+
static NotifierList migration_state_notifiers =
NOTIFIER_LIST_INITIALIZER(migration_state_notifiers);
@@ -55,6 +58,7 @@ static MigrationState *migrate_get_current(void)
static MigrationState current_migration = {
.state = MIG_STATE_SETUP,
.bandwidth_limit = MAX_THROTTLE,
+ .xbzrle_cache_size = DEFAULT_MIGRATE_CACHE_SIZE,
};
return ¤t_migration;
@@ -422,6 +426,7 @@ static MigrationState *migrate_init(const MigrationParams *params)
MigrationState *s = migrate_get_current();
int64_t bandwidth_limit = s->bandwidth_limit;
bool enabled_capabilities[MIGRATION_CAPABILITY_MAX];
+ int64_t xbzrle_cache_size = s->xbzrle_cache_size;
memcpy(enabled_capabilities, s->enabled_capabilities,
sizeof(enabled_capabilities));
@@ -431,6 +436,7 @@ static MigrationState *migrate_init(const MigrationParams *params)
s->params = *params;
memcpy(s->enabled_capabilities, enabled_capabilities,
sizeof(enabled_capabilities));
+ s->xbzrle_cache_size = xbzrle_cache_size;
s->state = MIG_STATE_SETUP;
s->total_time = qemu_get_clock_ms(rt_clock);
@@ -529,3 +535,21 @@ void qmp_migrate_set_downtime(double value, Error **errp)
value = MAX(0, MIN(UINT64_MAX, value));
max_downtime = (uint64_t)value;
}
+
+int migrate_use_xbzrle(void)
+{
+ MigrationState *s;
+
+ s = migrate_get_current();
+
+ return s->enabled_capabilities[MIGRATION_CAPABILITY_XBZRLE];
+}
+
+int64_t migrate_xbzrle_cache_size(void)
+{
+ MigrationState *s;
+
+ s = migrate_get_current();
+
+ return s->xbzrle_cache_size;
+}
diff --git a/migration.h b/migration.h
index 743c366..cdf6787 100644
--- a/migration.h
+++ b/migration.h
@@ -41,6 +41,7 @@ struct MigrationState
MigrationParams params;
int64_t total_time;
bool enabled_capabilities[MIGRATION_CAPABILITY_MAX];
+ int64_t xbzrle_cache_size;
};
void process_incoming_migration(QEMUFile *f);
@@ -104,4 +105,7 @@ int xbzrle_encode_buffer(uint8_t *old_buf, uint8_t *new_buf, int slen,
uint8_t *dst, int dlen);
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);
+
#endif
--
1.7.7.6
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH 07/11] Add XBZRLE to ram_save_block and ram_save_live
2012-07-25 14:50 ` [Qemu-devel] [PATCH 07/11] Add XBZRLE to ram_save_block and ram_save_live Orit Wasserman
@ 2012-07-26 22:20 ` Eric Blake
2012-07-29 6:34 ` Orit Wasserman
0 siblings, 1 reply; 33+ messages in thread
From: Eric Blake @ 2012-07-26 22:20 UTC (permalink / raw)
To: Orit Wasserman
Cc: peter.maydell, aliguori, quintela, stefanha, qemu-devel, mdroth,
blauwirbel, Petter Svard, Benoit Hudzia, avi, Aidan Shribman,
pbonzini, lcapitulino, chegu_vinod
[-- Attachment #1: Type: text/plain, Size: 5786 bytes --]
On 07/25/2012 08:50 AM, Orit Wasserman wrote:
> In the outgoing migration check to see if the page is cached and
> changed than send compressed page by using save_xbrle_page function.
s/changed than/changed, then/
> In the incoming migration check to see if RAM_SAVE_FLAG_XBZRLE is set
> and decompress the page (by using load_xbrle function).
>
> 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>
> +/* struct contains XBZRLE cache and a static page
> + used by the compression */
> +static struct {
> + /* buffer used for XBZRLE encoding */
> + uint8_t *encoded_buf;
> + /* buffer for storing page content */
> + uint8_t *current_buf;
> + /* buffer used for XBZRLE decoding */
> + uint8_t *decoded_buf;
> + /* Cache for XBZRLE */
> + PageCache *cache;
> +} XBZRLE = {
> + .encoded_buf = NULL,
> + .current_buf = NULL,
> + .decoded_buf = NULL,
> + .cache = NULL,
> +};
Explicit zero-initialization of a static object is pointless; C already
guarantees that this will be the case without an initializer, due to the
static lifetime.
> +#define ENCODING_FLAG_XBZRLE 0x1
> +
> +static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data,
> + ram_addr_t current_addr, RAMBlock *block,
> + ram_addr_t offset, int cont)
> +{
> + int encoded_len = 0, bytes_sent = -1;
> + XBZRLEHeader hdr = {
> + .xh_len = 0,
> + .xh_flags = 0,
> + };
[In contrast, this explicit zero-initialization of an automatic variable
is essential.]
> +
> + /* we need to update the data in the cache, in order to get the same data
> + we cached we decode the encoded page on the cached data */
> + memcpy(prev_cached_page, XBZRLE.current_buf, TARGET_PAGE_SIZE);
Comment is out of date - a memcpy is not decoding onto the cache, but
overwriting the entire cached page.
> +
> + hdr.xh_len = encoded_len;
> + hdr.xh_flags |= ENCODING_FLAG_XBZRLE;
> +
> + /* Send XBZRLE based compressed page */
> + save_block_hdr(f, block, offset, cont, RAM_SAVE_FLAG_XBZRLE);
> + qemu_put_byte(f, hdr.xh_flags);
> + qemu_put_be16(f, hdr.xh_len);
> + qemu_put_buffer(f, XBZRLE.encoded_buf, encoded_len);
> + bytes_sent = encoded_len + sizeof(hdr);
This looks like an off by one. sizeof(hdr) is 4 (2 bytes for xh_len, 1
byte for xh_flags, then padded out to 2-byte multiple due to uint16_t
member); but you really only sent 3 bytes (1 for xh_flags, 2 for
xh_len), not 4.
> @@ -321,7 +423,18 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
> last_offset = 0;
> sort_ram_list();
>
> - /* Make sure all dirty bits are set */
Why are you losing this comment? It is still appropriate for the code
in context after your insertion.
> @@ -436,6 +549,49 @@ static int ram_save_complete(QEMUFile *f, void *opaque)
> return 0;
> }
>
> +static int load_xbzrle(QEMUFile *f, ram_addr_t addr, void *host)
> +{
> + int ret, rc = 0;
> + XBZRLEHeader hdr = {
> + .xh_len = 0,
> + .xh_flags = 0,
> + };
> +
> + if (!XBZRLE.decoded_buf) {
> + XBZRLE.decoded_buf = g_malloc(TARGET_PAGE_SIZE);
> + }
> +
> + /* extract RLE header */
> + hdr.xh_flags = qemu_get_byte(f);
> + hdr.xh_len = qemu_get_be16(f);
> +
> + if (!(hdr.xh_flags & ENCODING_FLAG_XBZRLE)) {
> + fprintf(stderr, "Failed to load XBZRLE page - wrong compression!\n");
Shouldn't you also validate that no other bits in xh_flags are set, so
as to allow future versions of the protocol to define additional bits if
we come up with further compression methods and gracefully be detected
when the receiving end does not understand those bits? That is, this
should be:
if (hdr.xh_flags != ENCODING_FLAG_XBZRLE) {
> + return -1;
> + }
> +
> + if (hdr.xh_len > TARGET_PAGE_SIZE) {
> + fprintf(stderr, "Failed to load XBZRLE page - len overflow!\n");
> + return -1;
> + }
> + /* load data and decode */
> + qemu_get_buffer(f, XBZRLE.decoded_buf, hdr.xh_len);
> +
> + /* decode RLE */
> + ret = xbzrle_decode_buffer(XBZRLE.decoded_buf, hdr.xh_len, host,
> + TARGET_PAGE_SIZE);
> + if (ret == -1) {
> + fprintf(stderr, "Failed to load XBZRLE page - decode error!\n");
> + rc = -1;
> + } else if (ret > TARGET_PAGE_SIZE) {
> + fprintf(stderr, "Failed to load XBZRLE page - size %d exceeds %d!\n",
> + ret, TARGET_PAGE_SIZE);
Technically, this fprintf is unreachable; xbzrle_decode_buffer should
return -1 instead of a value larger than its incoming dlen. You could
abort() here to indicate a programming error.
> @@ -549,6 +705,19 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
> }
>
> qemu_get_buffer(f, host, TARGET_PAGE_SIZE);
> + } else if (flags & RAM_SAVE_FLAG_XBZRLE) {
> + if (!migrate_use_xbzrle()) {
> + return -EINVAL;
> + }
> + void *host = host_from_stream_offset(f, addr, flags);
> + if (!host) {
> + return -EINVAL;
> + }
> +
> + if (load_xbzrle(f, addr, host) < 0) {
> + ret = -EINVAL;
> + goto done;
Is there any issue by returning early instead of using 'goto done' in
all three error locations?
--
Eric Blake eblake@redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH 07/11] Add XBZRLE to ram_save_block and ram_save_live
2012-07-26 22:20 ` Eric Blake
@ 2012-07-29 6:34 ` Orit Wasserman
0 siblings, 0 replies; 33+ messages in thread
From: Orit Wasserman @ 2012-07-29 6:34 UTC (permalink / raw)
To: Eric Blake
Cc: peter.maydell, aliguori, quintela, stefanha, qemu-devel, mdroth,
blauwirbel, Petter Svard, Benoit Hudzia, avi, Aidan Shribman,
pbonzini, lcapitulino, chegu_vinod
On 07/27/2012 01:20 AM, Eric Blake wrote:
> On 07/25/2012 08:50 AM, Orit Wasserman wrote:
>> In the outgoing migration check to see if the page is cached and
>> changed than send compressed page by using save_xbrle_page function.
>
> s/changed than/changed, then/
>
>> In the incoming migration check to see if RAM_SAVE_FLAG_XBZRLE is set
>> and decompress the page (by using load_xbrle function).
>>
>> 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>
>
>> +/* struct contains XBZRLE cache and a static page
>> + used by the compression */
>> +static struct {
>> + /* buffer used for XBZRLE encoding */
>> + uint8_t *encoded_buf;
>> + /* buffer for storing page content */
>> + uint8_t *current_buf;
>> + /* buffer used for XBZRLE decoding */
>> + uint8_t *decoded_buf;
>> + /* Cache for XBZRLE */
>> + PageCache *cache;
>> +} XBZRLE = {
>> + .encoded_buf = NULL,
>> + .current_buf = NULL,
>> + .decoded_buf = NULL,
>> + .cache = NULL,
>> +};
>
> Explicit zero-initialization of a static object is pointless; C already
> guarantees that this will be the case without an initializer, due to the
> static lifetime.
You are right but as it harmless I will keep it.
>
>> +#define ENCODING_FLAG_XBZRLE 0x1
>> +
>> +static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data,
>> + ram_addr_t current_addr, RAMBlock *block,
>> + ram_addr_t offset, int cont)
>> +{
>> + int encoded_len = 0, bytes_sent = -1;
>> + XBZRLEHeader hdr = {
>> + .xh_len = 0,
>> + .xh_flags = 0,
>> + };
>
> [In contrast, this explicit zero-initialization of an automatic variable
> is essential.]
>
>> +
>> + /* we need to update the data in the cache, in order to get the same data
>> + we cached we decode the encoded page on the cached data */
>> + memcpy(prev_cached_page, XBZRLE.current_buf, TARGET_PAGE_SIZE);
>
> Comment is out of date - a memcpy is not decoding onto the cache, but
> overwriting the entire cached page.
>
will be updated.
>> +
>> + hdr.xh_len = encoded_len;
>> + hdr.xh_flags |= ENCODING_FLAG_XBZRLE;
>> +
>> + /* Send XBZRLE based compressed page */
>> + save_block_hdr(f, block, offset, cont, RAM_SAVE_FLAG_XBZRLE);
>> + qemu_put_byte(f, hdr.xh_flags);
>> + qemu_put_be16(f, hdr.xh_len);
>> + qemu_put_buffer(f, XBZRLE.encoded_buf, encoded_len);
>> + bytes_sent = encoded_len + sizeof(hdr);
>
> This looks like an off by one. sizeof(hdr) is 4 (2 bytes for xh_len, 1
> byte for xh_flags, then padded out to 2-byte multiple due to uint16_t
> member); but you really only sent 3 bytes (1 for xh_flags, 2 for
> xh_len), not 4.
correct, I was think of removing the header completely anyway.
>
>> @@ -321,7 +423,18 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
>> last_offset = 0;
>> sort_ram_list();
>>
>> - /* Make sure all dirty bits are set */
>
> Why are you losing this comment? It is still appropriate for the code
> in context after your insertion.
looks like bad merge I will fix it.
>
>> @@ -436,6 +549,49 @@ static int ram_save_complete(QEMUFile *f, void *opaque)
>> return 0;
>> }
>>
>> +static int load_xbzrle(QEMUFile *f, ram_addr_t addr, void *host)
>> +{
>> + int ret, rc = 0;
>> + XBZRLEHeader hdr = {
>> + .xh_len = 0,
>> + .xh_flags = 0,
>> + };
>> +
>> + if (!XBZRLE.decoded_buf) {
>> + XBZRLE.decoded_buf = g_malloc(TARGET_PAGE_SIZE);
>> + }
>> +
>> + /* extract RLE header */
>> + hdr.xh_flags = qemu_get_byte(f);
>> + hdr.xh_len = qemu_get_be16(f);
>> +
>> + if (!(hdr.xh_flags & ENCODING_FLAG_XBZRLE)) {
>> + fprintf(stderr, "Failed to load XBZRLE page - wrong compression!\n");
>
> Shouldn't you also validate that no other bits in xh_flags are set, so
> as to allow future versions of the protocol to define additional bits if
> we come up with further compression methods and gracefully be detected
> when the receiving end does not understand those bits? That is, this
> should be:
>
> if (hdr.xh_flags != ENCODING_FLAG_XBZRLE) {
>
ok
>> + return -1;
>> + }
>> +
>> + if (hdr.xh_len > TARGET_PAGE_SIZE) {
>> + fprintf(stderr, "Failed to load XBZRLE page - len overflow!\n");
>> + return -1;
>> + }
>> + /* load data and decode */
>> + qemu_get_buffer(f, XBZRLE.decoded_buf, hdr.xh_len);
>> +
>> + /* decode RLE */
>> + ret = xbzrle_decode_buffer(XBZRLE.decoded_buf, hdr.xh_len, host,
>> + TARGET_PAGE_SIZE);
>> + if (ret == -1) {
>> + fprintf(stderr, "Failed to load XBZRLE page - decode error!\n");
>> + rc = -1;
>> + } else if (ret > TARGET_PAGE_SIZE) {
>> + fprintf(stderr, "Failed to load XBZRLE page - size %d exceeds %d!\n",
>> + ret, TARGET_PAGE_SIZE);
>
> Technically, this fprintf is unreachable; xbzrle_decode_buffer should
> return -1 instead of a value larger than its incoming dlen. You could
> abort() here to indicate a programming error.
ok
>
>> @@ -549,6 +705,19 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
>> }
>>
>> qemu_get_buffer(f, host, TARGET_PAGE_SIZE);
>> + } else if (flags & RAM_SAVE_FLAG_XBZRLE) {
>> + if (!migrate_use_xbzrle()) {
>> + return -EINVAL;
>> + }
>> + void *host = host_from_stream_offset(f, addr, flags);
>> + if (!host) {
>> + return -EINVAL;
>> + }
>> +
>> + if (load_xbzrle(f, addr, host) < 0) {
>> + ret = -EINVAL;
>> + goto done;
>
> Is there any issue by returning early instead of using 'goto done' in
> all three error locations?
>
I think that it is written in this way for tracing ...
Thanks,
Orit
^ permalink raw reply [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH 08/11] Add migrate_set_cachesize command
2012-07-25 14:50 [Qemu-devel] [PATCH 00/11] Migration next v6 Orit Wasserman
` (7 preceding siblings ...)
2012-07-25 14:50 ` [Qemu-devel] [PATCH 07/11] Add XBZRLE to ram_save_block and ram_save_live Orit Wasserman
@ 2012-07-25 14:50 ` Orit Wasserman
2012-07-26 22:22 ` Eric Blake
2012-07-25 14:50 ` [Qemu-devel] [PATCH 09/11] Add migration accounting for normal and duplicate pages Orit Wasserman
` (2 subsequent siblings)
11 siblings, 1 reply; 33+ messages in thread
From: Orit Wasserman @ 2012-07-25 14:50 UTC (permalink / raw)
To: qemu-devel
Cc: peter.maydell, aliguori, quintela, Petter Svard, stefanha,
mdroth, Benoit Hudzia, lcapitulino, blauwirbel, Orit Wasserman,
chegu_vinod, avi, Aidan Shribman, pbonzini, eblake
Change XBZRLE cache size in bytes (the size should be a power of 2, it will be
rounded down to the nearest 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 | 20 ++++++++++++++++++++
hmp.c | 13 +++++++++++++
hmp.h | 1 +
migration.c | 14 ++++++++++++++
migration.h | 2 ++
qapi-schema.json | 16 ++++++++++++++++
qmp-commands.hx | 23 +++++++++++++++++++++++
8 files changed, 99 insertions(+), 0 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index 3998edc..cdf13ef 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -195,6 +195,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 3e15338..07a0b52 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -829,6 +829,26 @@ 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_cachesize
+Set cache size to @var{value} (in bytes) for xbzrle migrations.
ETEXI
{
diff --git a/hmp.c b/hmp.c
index f2f63fd..9770d7b 100644
--- a/hmp.c
+++ b/hmp.c
@@ -784,6 +784,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_cache_size(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 ceb2f06..a5c2e01 100644
--- a/hmp.h
+++ b/hmp.h
@@ -54,6 +54,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 167398a..bc2231d 100644
--- a/migration.c
+++ b/migration.c
@@ -516,6 +516,20 @@ void qmp_migrate_cancel(Error **errp)
migrate_fd_cancel(migrate_get_current());
}
+void qmp_migrate_set_cache_size(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 cdf6787..337e225 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 9d6759d..04adcee 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1412,6 +1412,22 @@
{ 'command': 'migrate_set_speed', 'data': {'value': 'int'} }
##
+# @migrate-set-cache-size
+#
+# 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-cache-size', 'data': {'value': 'int'} }
+
+##
# @ObjectPropertyInfo:
#
# @name: the name of the property
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 7f40e2a..cfc950e 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -520,6 +520,29 @@ Example:
<- { "return": {} }
EQMP
+{
+ .name = "migrate-set-cache-size",
+ .args_type = "value:o",
+ .mhandler.cmd_new = qmp_marshal_input_migrate_set_cache_size,
+ },
+
+SQMP
+migrate-set-cache-size
+---------------------
+
+Set cache size to be used by XBZRLE migration, the cache size will be rounded
+down to the nearest power of 2
+
+Arguments:
+
+- "value": cache size in bytes (json-int)
+
+Example:
+
+-> { "execute": "migrate-set-cache-size", "arguments": { "value": 536870912 } }
+<- { "return": {} }
+
+EQMP
{
.name = "migrate_set_speed",
--
1.7.7.6
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH 08/11] Add migrate_set_cachesize command
2012-07-25 14:50 ` [Qemu-devel] [PATCH 08/11] Add migrate_set_cachesize command Orit Wasserman
@ 2012-07-26 22:22 ` Eric Blake
0 siblings, 0 replies; 33+ messages in thread
From: Eric Blake @ 2012-07-26 22:22 UTC (permalink / raw)
To: Orit Wasserman
Cc: peter.maydell, aliguori, quintela, stefanha, qemu-devel, mdroth,
blauwirbel, Petter Svard, Benoit Hudzia, avi, Aidan Shribman,
pbonzini, lcapitulino, chegu_vinod
[-- Attachment #1: Type: text/plain, Size: 1026 bytes --]
On 07/25/2012 08:50 AM, Orit Wasserman wrote:
> Change XBZRLE cache size in bytes (the size should be a power of 2, it will be
> rounded down to the nearest 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>
> +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."
s/effects/affects/
--
Eric Blake eblake@redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]
^ permalink raw reply [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH 09/11] Add migration accounting for normal and duplicate pages
2012-07-25 14:50 [Qemu-devel] [PATCH 00/11] Migration next v6 Orit Wasserman
` (8 preceding siblings ...)
2012-07-25 14:50 ` [Qemu-devel] [PATCH 08/11] Add migrate_set_cachesize command Orit Wasserman
@ 2012-07-25 14:50 ` Orit Wasserman
2012-07-26 22:41 ` Eric Blake
2012-07-25 14:50 ` [Qemu-devel] [PATCH 10/11] Add XBZRLE statistics Orit Wasserman
2012-07-25 14:50 ` [Qemu-devel] [PATCH 11/11] Restart optimization on stage3 update version Orit Wasserman
11 siblings, 1 reply; 33+ messages in thread
From: Orit Wasserman @ 2012-07-25 14:50 UTC (permalink / raw)
To: qemu-devel
Cc: peter.maydell, aliguori, quintela, Petter Svard, stefanha,
mdroth, Benoit Hudzia, lcapitulino, blauwirbel, Orit Wasserman,
chegu_vinod, avi, Aidan Shribman, pbonzini, eblake
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>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
arch_init.c | 38 ++++++++++++++++++++++++++++++++++++++
migration.c | 4 ++++
migration.h | 5 +++++
qapi-schema.json | 8 ++++++--
qmp-commands.hx | 2 ++
5 files changed, 55 insertions(+), 2 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index cdf13ef..e50cdf2 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -205,6 +205,40 @@ int64_t xbzrle_cache_resize(int64_t new_size)
return pow2floor(new_size);
}
+/* accounting for migration statistics */
+typedef struct AccountingInfo {
+ uint64_t dup_pages;
+ uint64_t norm_pages;
+ uint64_t iterations;
+} AccountingInfo;
+
+static AccountingInfo acct_info;
+
+static void acct_clear(void)
+{
+ memset(&acct_info, 0, sizeof(acct_info));
+}
+
+uint64_t dup_mig_bytes_transferred(void)
+{
+ return acct_info.dup_pages * TARGET_PAGE_SIZE;
+}
+
+uint64_t dup_mig_pages_transferred(void)
+{
+ return acct_info.dup_pages;
+}
+
+uint64_t norm_mig_bytes_transferred(void)
+{
+ return acct_info.norm_pages * TARGET_PAGE_SIZE;
+}
+
+uint64_t norm_mig_pages_transferred(void)
+{
+ return acct_info.norm_pages;
+}
+
static void save_block_hdr(QEMUFile *f, RAMBlock *block, ram_addr_t offset,
int cont, int flag)
{
@@ -307,6 +341,7 @@ static int ram_save_block(QEMUFile *f)
p = memory_region_get_ram_ptr(mr) + offset;
if (is_dup_page(p)) {
+ acct_info.dup_pages++;
save_block_hdr(f, block, offset, cont, RAM_SAVE_FLAG_COMPRESS);
qemu_put_byte(f, *p);
bytes_sent = 1;
@@ -322,6 +357,7 @@ static int ram_save_block(QEMUFile *f)
save_block_hdr(f, block, offset, cont, RAM_SAVE_FLAG_PAGE);
qemu_put_buffer(f, p, TARGET_PAGE_SIZE);
bytes_sent = TARGET_PAGE_SIZE;
+ acct_info.norm_pages++;
}
/* if page is unmodified, continue to the next */
@@ -443,6 +479,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
}
XBZRLE.encoded_buf = g_malloc0(TARGET_PAGE_SIZE);
XBZRLE.current_buf = g_malloc(TARGET_PAGE_SIZE);
+ acct_clear();
}
QLIST_FOREACH(block, &ram_list.blocks, next) {
@@ -490,6 +527,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
break;
}
bytes_transferred += bytes_sent;
+ acct_info.iterations++;
/* we want to check in the 1st loop, just in case it was the 1st time
and we had to sync the dirty bitmap.
qemu_get_clock_ns() is a bit expensive, so we only check each some
diff --git a/migration.c b/migration.c
index bc2231d..4dc99ba 100644
--- a/migration.c
+++ b/migration.c
@@ -156,6 +156,8 @@ MigrationInfo *qmp_query_migrate(Error **errp)
info->ram->total = ram_bytes_total();
info->ram->total_time = qemu_get_clock_ms(rt_clock)
- s->total_time;
+ info->ram->duplicate = dup_mig_pages_transferred();
+ info->ram->normal = norm_mig_pages_transferred();
if (blk_mig_active()) {
info->has_disk = true;
@@ -175,6 +177,8 @@ MigrationInfo *qmp_query_migrate(Error **errp)
info->ram->remaining = 0;
info->ram->total = ram_bytes_total();
info->ram->total_time = s->total_time;
+ info->ram->duplicate = dup_mig_pages_transferred();
+ info->ram->normal = norm_mig_pages_transferred();
break;
case MIG_STATE_ERROR:
info->has_status = true;
diff --git a/migration.h b/migration.h
index 337e225..e4a7cd7 100644
--- a/migration.h
+++ b/migration.h
@@ -87,6 +87,11 @@ uint64_t ram_bytes_total(void);
extern SaveVMHandlers savevm_ram_handlers;
+uint64_t dup_mig_bytes_transferred(void);
+uint64_t dup_mig_pages_transferred(void);
+uint64_t norm_mig_bytes_transferred(void);
+uint64_t norm_mig_pages_transferred(void);
+
/**
* @migrate_add_blocker - prevent migration from proceeding
*
diff --git a/qapi-schema.json b/qapi-schema.json
index 04adcee..88b7b56 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -264,11 +264,15 @@
# migration has ended, it returns the total migration
# time. (since 1.2)
#
-# Since: 0.14.0.
+# @duplicate: #optional, number of duplicate pages (since 1.2)
+#
+# @normal : #optional, number of normal pages (since 1.2)
+#
+# Since: 0.14.0
##
{ 'type': 'MigrationStats',
'data': {'transferred': 'int', 'remaining': 'int', 'total': 'int' ,
- 'total_time': 'int' } }
+ 'total_time': 'int', '*duplicate': 'int', '*normal': 'int' } }
##
# @MigrationInfo
diff --git a/qmp-commands.hx b/qmp-commands.hx
index cfc950e..d0e8878 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2099,6 +2099,8 @@ The main json-object contains the following:
- "transferred": amount transferred (json-int)
- "remaining": amount remaining (json-int)
- "total": total (json-int)
+ - "duplicate": number of duplicated pages (json-int)
+ - "normal" : number of normal pages transferred (json-int)
- "disk": only present if "status" is "active" and it is a block migration,
it is a json-object with the following disk information (in bytes):
- "transferred": amount transferred (json-int)
--
1.7.7.6
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH 09/11] Add migration accounting for normal and duplicate pages
2012-07-25 14:50 ` [Qemu-devel] [PATCH 09/11] Add migration accounting for normal and duplicate pages Orit Wasserman
@ 2012-07-26 22:41 ` Eric Blake
2012-07-29 6:57 ` Orit Wasserman
0 siblings, 1 reply; 33+ messages in thread
From: Eric Blake @ 2012-07-26 22:41 UTC (permalink / raw)
To: Orit Wasserman
Cc: peter.maydell, aliguori, quintela, stefanha, qemu-devel, mdroth,
blauwirbel, Petter Svard, Benoit Hudzia, avi, Aidan Shribman,
pbonzini, lcapitulino, chegu_vinod
[-- Attachment #1: Type: text/plain, Size: 2274 bytes --]
On 07/25/2012 08:50 AM, Orit Wasserman wrote:
> 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>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
> +++ b/qapi-schema.json
> @@ -264,11 +264,15 @@
> # migration has ended, it returns the total migration
> # time. (since 1.2)
> #
> -# Since: 0.14.0.
> +# @duplicate: #optional, number of duplicate pages (since 1.2)
I think I was the one that originally asked whether #optional was
appropriate for back-compat reasons when adding to a struct, but Luis
has since corrected me - #optional only makes sense for a return member
that will not appear in all uses of the struct in the current version.
But the number of duplicates is always available (even if it is 0), so
it should not be optional. That is, this line should be:
# @duplicate: number of duplicate pages (since 1.2)
> +#
> +# @normal : #optional, number of normal pages (since 1.2)
Likewise.
> +#
> +# Since: 0.14.0
> ##
> { 'type': 'MigrationStats',
> 'data': {'transferred': 'int', 'remaining': 'int', 'total': 'int' ,
> - 'total_time': 'int' } }
> + 'total_time': 'int', '*duplicate': 'int', '*normal': 'int' } }
and this should be 'duplicate' and 'normal'.
> +++ b/qmp-commands.hx
> @@ -2099,6 +2099,8 @@ The main json-object contains the following:
> - "transferred": amount transferred (json-int)
> - "remaining": amount remaining (json-int)
> - "total": total (json-int)
> + - "duplicate": number of duplicated pages (json-int)
> + - "normal" : number of normal pages transferred (json-int)
> - "disk": only present if "status" is "active" and it is a block migration,
> it is a json-object with the following disk information (in bytes):
> - "transferred": amount transferred (json-int)
>
Incomplete if we decide that 'duplicate' and 'normal' are not optional;
we should be updating example 4 and 5 to list the new fields.
--
Eric Blake eblake@redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH 09/11] Add migration accounting for normal and duplicate pages
2012-07-26 22:41 ` Eric Blake
@ 2012-07-29 6:57 ` Orit Wasserman
0 siblings, 0 replies; 33+ messages in thread
From: Orit Wasserman @ 2012-07-29 6:57 UTC (permalink / raw)
To: Eric Blake
Cc: peter.maydell, aliguori, quintela, stefanha, qemu-devel, mdroth,
blauwirbel, Petter Svard, Benoit Hudzia, avi, Aidan Shribman,
pbonzini, lcapitulino, chegu_vinod
On 07/27/2012 01:41 AM, Eric Blake wrote:
> On 07/25/2012 08:50 AM, Orit Wasserman wrote:
>> 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>
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>> +++ b/qapi-schema.json
>> @@ -264,11 +264,15 @@
>> # migration has ended, it returns the total migration
>> # time. (since 1.2)
>> #
>> -# Since: 0.14.0.
>> +# @duplicate: #optional, number of duplicate pages (since 1.2)
>
> I think I was the one that originally asked whether #optional was
> appropriate for back-compat reasons when adding to a struct, but Luis
> has since corrected me - #optional only makes sense for a return member
> that will not appear in all uses of the struct in the current version.
> But the number of duplicates is always available (even if it is 0), so
> it should not be optional. That is, this line should be:
>
> # @duplicate: number of duplicate pages (since 1.2)
>
>> +#
>> +# @normal : #optional, number of normal pages (since 1.2)
>
> Likewise.
>
>> +#
>> +# Since: 0.14.0
>> ##
>> { 'type': 'MigrationStats',
>> 'data': {'transferred': 'int', 'remaining': 'int', 'total': 'int' ,
>> - 'total_time': 'int' } }
>> + 'total_time': 'int', '*duplicate': 'int', '*normal': 'int' } }
>
> and this should be 'duplicate' and 'normal'.
>
>> +++ b/qmp-commands.hx
>> @@ -2099,6 +2099,8 @@ The main json-object contains the following:
>> - "transferred": amount transferred (json-int)
>> - "remaining": amount remaining (json-int)
>> - "total": total (json-int)
>> + - "duplicate": number of duplicated pages (json-int)
>> + - "normal" : number of normal pages transferred (json-int)
>> - "disk": only present if "status" is "active" and it is a block migration,
>> it is a json-object with the following disk information (in bytes):
>> - "transferred": amount transferred (json-int)
>>
>
> Incomplete if we decide that 'duplicate' and 'normal' are not optional;
> we should be updating example 4 and 5 to list the new fields.
>
I will fix it,
Orit
^ permalink raw reply [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH 10/11] Add XBZRLE statistics
2012-07-25 14:50 [Qemu-devel] [PATCH 00/11] Migration next v6 Orit Wasserman
` (9 preceding siblings ...)
2012-07-25 14:50 ` [Qemu-devel] [PATCH 09/11] Add migration accounting for normal and duplicate pages Orit Wasserman
@ 2012-07-25 14:50 ` Orit Wasserman
2012-07-26 22:48 ` Eric Blake
2012-07-25 14:50 ` [Qemu-devel] [PATCH 11/11] Restart optimization on stage3 update version Orit Wasserman
11 siblings, 1 reply; 33+ messages in thread
From: Orit Wasserman @ 2012-07-25 14:50 UTC (permalink / raw)
To: qemu-devel
Cc: peter.maydell, aliguori, quintela, Petter Svard, stefanha,
mdroth, Benoit Hudzia, lcapitulino, blauwirbel, Orit Wasserman,
chegu_vinod, avi, Aidan Shribman, pbonzini, eblake
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>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
arch_init.c | 28 ++++++++++++++++++++++++++++
hmp.c | 21 +++++++++++++++++++++
migration.c | 28 ++++++++++++++++++++++++++++
migration.h | 4 ++++
qapi-schema.json | 26 +++++++++++++++++++++++++-
qmp-commands.hx | 32 +++++++++++++++++++++++++++++++-
6 files changed, 137 insertions(+), 2 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index e50cdf2..f484bd5 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -209,7 +209,11 @@ int64_t xbzrle_cache_resize(int64_t new_size)
typedef struct AccountingInfo {
uint64_t dup_pages;
uint64_t norm_pages;
+ uint64_t xbzrle_bytes;
+ uint64_t xbzrle_pages;
+ uint64_t xbzrle_cache_miss;
uint64_t iterations;
+ uint64_t xbzrle_overflows;
} AccountingInfo;
static AccountingInfo acct_info;
@@ -239,6 +243,26 @@ uint64_t norm_mig_pages_transferred(void)
return acct_info.norm_pages;
}
+uint64_t xbzrle_mig_bytes_transferred(void)
+{
+ return acct_info.xbzrle_bytes;
+}
+
+uint64_t xbzrle_mig_pages_transferred(void)
+{
+ return acct_info.xbzrle_pages;
+}
+
+uint64_t xbzrle_mig_pages_cache_miss(void)
+{
+ return acct_info.xbzrle_cache_miss;
+}
+
+uint64_t xbzrle_mig_pages_overflow(void)
+{
+ return acct_info.xbzrle_overflows;
+}
+
static void save_block_hdr(QEMUFile *f, RAMBlock *block, ram_addr_t offset,
int cont, int flag)
{
@@ -267,6 +291,7 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data,
if (!cache_is_cached(XBZRLE.cache, current_addr)) {
cache_insert(XBZRLE.cache, current_addr,
g_memdup(current_data, TARGET_PAGE_SIZE));
+ acct_info.xbzrle_cache_miss++;
return -1;
}
@@ -284,6 +309,7 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data,
return 0;
} else if (encoded_len == -1) {
DPRINTF("Overflow\n");
+ acct_info.xbzrle_overflows++;
/* update data in the cache */
memcpy(prev_cached_page, current_data, TARGET_PAGE_SIZE);
return -1;
@@ -301,7 +327,9 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data,
qemu_put_byte(f, hdr.xh_flags);
qemu_put_be16(f, hdr.xh_len);
qemu_put_buffer(f, XBZRLE.encoded_buf, encoded_len);
+ acct_info.xbzrle_pages++;
bytes_sent = encoded_len + sizeof(hdr);
+ acct_info.xbzrle_bytes += bytes_sent;
return bytes_sent;
}
diff --git a/hmp.c b/hmp.c
index 9770d7b..383d5b1 100644
--- a/hmp.c
+++ b/hmp.c
@@ -172,6 +172,27 @@ void hmp_info_migrate(Monitor *mon)
info->disk->total >> 10);
}
+ if (info->has_xbzrle_cache) {
+ monitor_printf(mon, "cache size: %" PRIu64 " bytes\n",
+ info->xbzrle_cache->cache_size);
+ if (info->xbzrle_cache->has_xbzrle_bytes) {
+ monitor_printf(mon, "xbzrle transferred: %" PRIu64 " kbytes\n",
+ info->xbzrle_cache->xbzrle_bytes >> 10);
+ }
+ if (info->xbzrle_cache->has_xbzrle_pages) {
+ monitor_printf(mon, "xbzrle pages: %" PRIu64 " pages\n",
+ info->xbzrle_cache->xbzrle_pages);
+ }
+ if (info->xbzrle_cache->has_xbzrle_cache_miss) {
+ monitor_printf(mon, "xbzrle cache miss: %" PRIu64 "\n",
+ info->xbzrle_cache->xbzrle_cache_miss);
+ }
+ if (info->xbzrle_cache->has_xbzrle_overflow) {
+ monitor_printf(mon, "xbzrle overflow : %" PRIu64 "\n",
+ info->xbzrle_cache->xbzrle_overflow);
+ }
+ }
+
qapi_free_MigrationInfo(info);
qapi_free_MigrationParameters(params);
}
diff --git a/migration.c b/migration.c
index 4dc99ba..fb802bc 100644
--- a/migration.c
+++ b/migration.c
@@ -136,6 +136,23 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
return params;
}
+static void get_xbzrle_cache_stats(MigrationInfo *info)
+{
+ if (migrate_use_xbzrle()) {
+ info->has_xbzrle_cache = true;
+ info->xbzrle_cache = g_malloc0(sizeof(*info->xbzrle_cache));
+ info->xbzrle_cache->cache_size = migrate_xbzrle_cache_size();
+ info->xbzrle_cache->has_xbzrle_bytes = true;
+ info->xbzrle_cache->xbzrle_bytes = xbzrle_mig_bytes_transferred();
+ info->xbzrle_cache->has_xbzrle_pages = true;
+ info->xbzrle_cache->xbzrle_pages = xbzrle_mig_pages_transferred();
+ info->xbzrle_cache->has_xbzrle_cache_miss = true;
+ info->xbzrle_cache->xbzrle_cache_miss = xbzrle_mig_pages_cache_miss();
+ info->xbzrle_cache->has_xbzrle_overflow = true;
+ info->xbzrle_cache->xbzrle_overflow = xbzrle_mig_pages_overflow();
+ }
+}
+
MigrationInfo *qmp_query_migrate(Error **errp)
{
MigrationInfo *info = g_malloc0(sizeof(*info));
@@ -144,6 +161,13 @@ MigrationInfo *qmp_query_migrate(Error **errp)
switch (s->state) {
case MIG_STATE_SETUP:
/* no migration has happened ever */
+
+ /* display xbzrle cache size */
+ if (migrate_use_xbzrle()) {
+ info->has_xbzrle_cache = true;
+ info->xbzrle_cache = g_malloc0(sizeof(*info->xbzrle_cache));
+ info->xbzrle_cache->cache_size = migrate_xbzrle_cache_size();
+ }
break;
case MIG_STATE_ACTIVE:
info->has_status = true;
@@ -166,8 +190,12 @@ MigrationInfo *qmp_query_migrate(Error **errp)
info->disk->remaining = blk_mig_bytes_remaining();
info->disk->total = blk_mig_bytes_total();
}
+
+ get_xbzrle_cache_stats(info);
break;
case MIG_STATE_COMPLETED:
+ get_xbzrle_cache_stats(info);
+
info->has_status = true;
info->status = g_strdup("completed");
diff --git a/migration.h b/migration.h
index e4a7cd7..a9852fc 100644
--- a/migration.h
+++ b/migration.h
@@ -91,6 +91,10 @@ uint64_t dup_mig_bytes_transferred(void);
uint64_t dup_mig_pages_transferred(void);
uint64_t norm_mig_bytes_transferred(void);
uint64_t norm_mig_pages_transferred(void);
+uint64_t xbzrle_mig_bytes_transferred(void);
+uint64_t xbzrle_mig_pages_transferred(void);
+uint64_t xbzrle_mig_pages_overflow(void);
+uint64_t xbzrle_mig_pages_cache_miss(void);
/**
* @migrate_add_blocker - prevent migration from proceeding
diff --git a/qapi-schema.json b/qapi-schema.json
index 88b7b56..9418f23 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -273,6 +273,26 @@
{ 'type': 'MigrationStats',
'data': {'transferred': 'int', 'remaining': 'int', 'total': 'int' ,
'total_time': 'int', '*duplicate': 'int', '*normal': 'int' } }
+##
+# @XBZRLECacheStats
+#
+# Detailed XBZRLE migration cache statistics
+#
+# @cache_size: XBZRLE cache size
+#
+# @xbzrle_bytes: amount of bytes already transferred to the target VM
+#
+# @xbzrle_pages: amount of pages transferred to the target VM
+#
+# @xbzrle_cache_miss: number of cache miss
+#
+# @xbzrle_overflow: number of overflows
+#
+# Since: 1.2
+##
+{ 'type': 'XBZRLECacheStats',
+ 'data': {'cache-size': 'int', '*xbzrle-bytes': 'int', '*xbzrle-pages': 'int',
+ '*xbzrle-cache-miss': 'int', '*xbzrle-overflow': 'int' } }
##
# @MigrationInfo
@@ -292,11 +312,15 @@
# status, only returned if status is 'active' and it is a block
# migration
#
+# @xbzrle-cache: #optional @XBZRLECacheStats containing detailed XBZRLE
+# migration statistics (since 1.2)
+#
# Since: 0.14.0
##
{ 'type': 'MigrationInfo',
'data': {'*status': 'str', '*ram': 'MigrationStats',
- '*disk': 'MigrationStats'} }
+ '*disk': 'MigrationStats',
+ '*xbzrle-cache': 'XBZRLECacheStats'} }
##
# @query-migrate
diff --git a/qmp-commands.hx b/qmp-commands.hx
index d0e8878..946d921 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2106,10 +2106,16 @@ The main json-object contains the following:
- "transferred": amount transferred (json-int)
- "remaining": amount remaining (json-int)
- "total": total (json-int)
+- "cache": only present if "status" and XBZRLE is active.
+ It is a json-object with the following XBZRLE information:
+ - "cache-size": XBZRLE cache size
+ - "xbzrle-bytes": total XBZRLE bytes transferred
+ - "xbzrle-pages": number of XBZRLE compressed pages
+ - "cache-miss": number of cache misses
+ - "overflow": number of XBZRLE overflows
Examples:
1. Before the first migration
-
-> { "execute": "query-migrate" }
<- { "return": {} }
@@ -2164,6 +2170,30 @@ Examples:
}
}
+6. Migration is being performed and XBZRLE is active:
+
+-> { "execute": "query-migrate" }
+<- {
+ "return":{
+ "status":"active",
+ "capabilities" : [ { "capability": "xbzrle", "state" : true } ],
+ "ram":{
+ "total":1057024,
+ "remaining":1053304,
+ "transferred":3720,
+ "duplicate": 10,
+ "normal" : 3333
+ },
+ "cache":{
+ "cache-size": 1024
+ "xbzrle-transferred":20971520,
+ "xbzrle-pages":2444343,
+ "xbzrle-cache-miss":2244,
+ "xbzrle-overflow":34434
+ }
+ }
+ }
+
EQMP
{
--
1.7.7.6
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH 10/11] Add XBZRLE statistics
2012-07-25 14:50 ` [Qemu-devel] [PATCH 10/11] Add XBZRLE statistics Orit Wasserman
@ 2012-07-26 22:48 ` Eric Blake
2012-07-29 7:10 ` Orit Wasserman
0 siblings, 1 reply; 33+ messages in thread
From: Eric Blake @ 2012-07-26 22:48 UTC (permalink / raw)
To: Orit Wasserman
Cc: peter.maydell, aliguori, quintela, stefanha, qemu-devel, mdroth,
blauwirbel, Petter Svard, Benoit Hudzia, avi, Aidan Shribman,
pbonzini, lcapitulino, chegu_vinod
[-- Attachment #1: Type: text/plain, Size: 3414 bytes --]
On 07/25/2012 08:50 AM, Orit Wasserman wrote:
> 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>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
> +++ b/qapi-schema.json
> @@ -273,6 +273,26 @@
> { 'type': 'MigrationStats',
> 'data': {'transferred': 'int', 'remaining': 'int', 'total': 'int' ,
> 'total_time': 'int', '*duplicate': 'int', '*normal': 'int' } }
> +##
> +# @XBZRLECacheStats
> +#
> +# Detailed XBZRLE migration cache statistics
> +#
> +# @cache_size: XBZRLE cache size
s/cache_size/cache-size/, and so on throughout this struct (it is new,
so it should use '-' instead of '_'); especially since you already made
that change in the JSON itself.
> +#
> +# @xbzrle_bytes: amount of bytes already transferred to the target VM
> +#
> +# @xbzrle_pages: amount of pages transferred to the target VM
> +#
> +# @xbzrle_cache_miss: number of cache miss
> +#
> +# @xbzrle_overflow: number of overflows
> +#
> +# Since: 1.2
> +##
> +{ 'type': 'XBZRLECacheStats',
> + 'data': {'cache-size': 'int', '*xbzrle-bytes': 'int', '*xbzrle-pages': 'int',
> + '*xbzrle-cache-miss': 'int', '*xbzrle-overflow': 'int' } }
Why are you marking four of the five fields optional here, but not in
the text above? I don't think any of them should be optional.
>
> ##
> # @MigrationInfo
> @@ -292,11 +312,15 @@
> # status, only returned if status is 'active' and it is a block
> # migration
> #
> +# @xbzrle-cache: #optional @XBZRLECacheStats containing detailed XBZRLE
> +# migration statistics (since 1.2)
Now _this_ field is indeed optional - it should appear in the output
only when xbzrle is enabled. You may want to mention that in the
description (just like the previous field mentions that it was only
available for a block migration).
> +++ b/qmp-commands.hx
> @@ -2106,10 +2106,16 @@ The main json-object contains the following:
> - "transferred": amount transferred (json-int)
> - "remaining": amount remaining (json-int)
> - "total": total (json-int)
> +- "cache": only present if "status" and XBZRLE is active.
Naming mismatch; you named it 'xbzrle-cache' above.
> + It is a json-object with the following XBZRLE information:
> + - "cache-size": XBZRLE cache size
> + - "xbzrle-bytes": total XBZRLE bytes transferred
Just so I'm clear, is xbzrle-bytes is the number of compressed bytes
sent over the wire, or the number of uncompressed bytes? Likewise, at
the top level, is 'total' the number of bytes sent over the wire, or the
number of bytes after decompression?
That is, if I compare this field against 'total', which field will
always be bigger? Knowing this will let me compute my percentage of
savings due to compressed pages.
> + - "xbzrle-pages": number of XBZRLE compressed pages
> + - "cache-miss": number of cache misses
> + - "overflow": number of XBZRLE overflows
> Examples:
>
> 1. Before the first migration
> -
> -> { "execute": "query-migrate" }
Spurious whitespace change.
--
Eric Blake eblake@redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH 10/11] Add XBZRLE statistics
2012-07-26 22:48 ` Eric Blake
@ 2012-07-29 7:10 ` Orit Wasserman
0 siblings, 0 replies; 33+ messages in thread
From: Orit Wasserman @ 2012-07-29 7:10 UTC (permalink / raw)
To: Eric Blake
Cc: peter.maydell, aliguori, quintela, stefanha, qemu-devel, mdroth,
blauwirbel, Petter Svard, Benoit Hudzia, avi, Aidan Shribman,
pbonzini, lcapitulino, chegu_vinod
On 07/27/2012 01:48 AM, Eric Blake wrote:
> On 07/25/2012 08:50 AM, Orit Wasserman wrote:
>> 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>
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>> +++ b/qapi-schema.json
>> @@ -273,6 +273,26 @@
>> { 'type': 'MigrationStats',
>> 'data': {'transferred': 'int', 'remaining': 'int', 'total': 'int' ,
>> 'total_time': 'int', '*duplicate': 'int', '*normal': 'int' } }
>> +##
>> +# @XBZRLECacheStats
>> +#
>> +# Detailed XBZRLE migration cache statistics
>> +#
>> +# @cache_size: XBZRLE cache size
>
> s/cache_size/cache-size/, and so on throughout this struct (it is new,
> so it should use '-' instead of '_'); especially since you already made
> that change in the JSON itself.
>
ok
>> +#
>> +# @xbzrle_bytes: amount of bytes already transferred to the target VM
>> +#
>> +# @xbzrle_pages: amount of pages transferred to the target VM
>> +#
>> +# @xbzrle_cache_miss: number of cache miss
>> +#
>> +# @xbzrle_overflow: number of overflows
>> +#
>> +# Since: 1.2
>> +##
>> +{ 'type': 'XBZRLECacheStats',
>> + 'data': {'cache-size': 'int', '*xbzrle-bytes': 'int', '*xbzrle-pages': 'int',
>> + '*xbzrle-cache-miss': 'int', '*xbzrle-overflow': 'int' } }
>
> Why are you marking four of the five fields optional here, but not in
> the text above? I don't think any of them should be optional.
If the migration is not active only cache size will be available (requested by Luiz).
I will add it to the comment.
>
>>
>> ##
>> # @MigrationInfo
>> @@ -292,11 +312,15 @@
>> # status, only returned if status is 'active' and it is a block
>> # migration
>> #
>> +# @xbzrle-cache: #optional @XBZRLECacheStats containing detailed XBZRLE
>> +# migration statistics (since 1.2)
>
> Now _this_ field is indeed optional - it should appear in the output
> only when xbzrle is enabled. You may want to mention that in the
> description (just like the previous field mentions that it was only
> available for a block migration).
ok
>
>> +++ b/qmp-commands.hx
>> @@ -2106,10 +2106,16 @@ The main json-object contains the following:
>> - "transferred": amount transferred (json-int)
>> - "remaining": amount remaining (json-int)
>> - "total": total (json-int)
>> +- "cache": only present if "status" and XBZRLE is active.
>
> Naming mismatch; you named it 'xbzrle-cache' above.
ok
>
>> + It is a json-object with the following XBZRLE information:
>> + - "cache-size": XBZRLE cache size
>> + - "xbzrle-bytes": total XBZRLE bytes transferred
>
> Just so I'm clear, is xbzrle-bytes is the number of compressed bytes
> sent over the wire, or the number of uncompressed bytes? Likewise, at
> the top level, is 'total' the number of bytes sent over the wire, or the
> number of bytes after decompression?
xbzrle-bytes are the compressed bytes sent on the wire.
total is the total bytes sent on the wire (including xbzrle_bytes and duplicates ...).
>
> That is, if I compare this field against 'total', which field will
> always be bigger? Knowing this will let me compute my percentage of
> savings due to compressed pages.
total will be always bigger.
>
>> + - "xbzrle-pages": number of XBZRLE compressed pages
>> + - "cache-miss": number of cache misses
>> + - "overflow": number of XBZRLE overflows
>> Examples:
>>
>> 1. Before the first migration
>> -
>> -> { "execute": "query-migrate" }
>
> Spurious whitespace change.
>
Thanks,
Orit
^ permalink raw reply [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH 11/11] Restart optimization on stage3 update version
2012-07-25 14:50 [Qemu-devel] [PATCH 00/11] Migration next v6 Orit Wasserman
` (10 preceding siblings ...)
2012-07-25 14:50 ` [Qemu-devel] [PATCH 10/11] Add XBZRLE statistics Orit Wasserman
@ 2012-07-25 14:50 ` Orit Wasserman
2012-07-25 15:41 ` Vinod, Chegu
11 siblings, 1 reply; 33+ messages in thread
From: Orit Wasserman @ 2012-07-25 14:50 UTC (permalink / raw)
To: qemu-devel
Cc: peter.maydell, aliguori, quintela, stefanha, mdroth, lcapitulino,
blauwirbel, chegu_vinod, avi, pbonzini, eblake
From: Juan Quintela <quintela@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
arch_init.c | 24 +++++++++++++++---------
1 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index f484bd5..f555c27 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -279,7 +279,7 @@ static void save_block_hdr(QEMUFile *f, RAMBlock *block, ram_addr_t offset,
static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data,
ram_addr_t current_addr, RAMBlock *block,
- ram_addr_t offset, int cont)
+ ram_addr_t offset, int cont, bool last_stage)
{
int encoded_len = 0, bytes_sent = -1;
XBZRLEHeader hdr = {
@@ -289,8 +289,10 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data,
uint8_t *prev_cached_page;
if (!cache_is_cached(XBZRLE.cache, current_addr)) {
- cache_insert(XBZRLE.cache, current_addr,
- g_memdup(current_data, TARGET_PAGE_SIZE));
+ if (!last_stage) {
+ cache_insert(XBZRLE.cache, current_addr,
+ g_memdup(current_data, TARGET_PAGE_SIZE));
+ }
acct_info.xbzrle_cache_miss++;
return -1;
}
@@ -317,7 +319,9 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data,
/* we need to update the data in the cache, in order to get the same data
we cached we decode the encoded page on the cached data */
- memcpy(prev_cached_page, XBZRLE.current_buf, TARGET_PAGE_SIZE);
+ if (!last_stage) {
+ memcpy(prev_cached_page, XBZRLE.current_buf, TARGET_PAGE_SIZE);
+ }
hdr.xh_len = encoded_len;
hdr.xh_flags |= ENCODING_FLAG_XBZRLE;
@@ -345,7 +349,7 @@ static ram_addr_t last_offset;
* n: the amount of bytes written in other case
*/
-static int ram_save_block(QEMUFile *f)
+static int ram_save_block(QEMUFile *f, bool last_stage)
{
RAMBlock *block = last_block;
ram_addr_t offset = last_offset;
@@ -376,8 +380,10 @@ static int ram_save_block(QEMUFile *f)
} else if (migrate_use_xbzrle()) {
current_addr = block->offset + offset;
bytes_sent = save_xbzrle_page(f, p, current_addr, block,
- offset, cont);
- p = get_cached_data(XBZRLE.cache, current_addr);
+ offset, cont, last_stage);
+ if (!last_stage) {
+ p = get_cached_data(XBZRLE.cache, current_addr);
+ }
}
/* either we didn't send yet (we may have had XBZRLE overflow) */
@@ -549,7 +555,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
while ((ret = qemu_file_rate_limit(f)) == 0) {
int bytes_sent;
- bytes_sent = ram_save_block(f);
+ bytes_sent = ram_save_block(f, false);
/* no more blocks to sent */
if (bytes_sent < 0) {
break;
@@ -611,7 +617,7 @@ static int ram_save_complete(QEMUFile *f, void *opaque)
while (true) {
int bytes_sent;
- bytes_sent = ram_save_block(f);
+ bytes_sent = ram_save_block(f, true);
/* no more blocks to sent */
if (bytes_sent < 0) {
break;
--
1.7.7.6
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH 11/11] Restart optimization on stage3 update version
2012-07-25 14:50 ` [Qemu-devel] [PATCH 11/11] Restart optimization on stage3 update version Orit Wasserman
@ 2012-07-25 15:41 ` Vinod, Chegu
2012-07-26 5:03 ` Orit Wasserman
0 siblings, 1 reply; 33+ messages in thread
From: Vinod, Chegu @ 2012-07-25 15:41 UTC (permalink / raw)
To: Orit Wasserman, qemu-devel
Cc: peter.maydell, aliguori, quintela, stefanha, mdroth, lcapitulino,
blauwirbel, avi, pbonzini, eblake
[-- Attachment #1: Type: text/plain, Size: 3804 bytes --]
-----Original Message-----
From: Orit Wasserman [mailto:owasserm@redhat.com]
From: Juan Quintela <quintela@redhat.com<mailto:quintela@redhat.com>>
Signed-off-by: Juan Quintela <quintela@redhat.com<mailto:quintela@redhat.com>>
---
arch_init.c | 24 +++++++++++++++---------
1 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index f484bd5..f555c27 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -279,7 +279,7 @@ static void save_block_hdr(QEMUFile *f, RAMBlock *block, ram_addr_t offset,
static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data,
ram_addr_t current_addr, RAMBlock *block,
- ram_addr_t offset, int cont)
+ ram_addr_t offset, int cont, bool
+ last_stage)
{
int encoded_len = 0, bytes_sent = -1;
XBZRLEHeader hdr = {
@@ -289,8 +289,10 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data,
uint8_t *prev_cached_page;
if (!cache_is_cached(XBZRLE.cache, current_addr)) {
- cache_insert(XBZRLE.cache, current_addr,
- g_memdup(current_data, TARGET_PAGE_SIZE));
+ if (!last_stage) {
+ cache_insert(XBZRLE.cache, current_addr,
+ g_memdup(current_data, TARGET_PAGE_SIZE));
+ }
acct_info.xbzrle_cache_miss++;
return -1;
}
@@ -317,7 +319,9 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data,
/* we need to update the data in the cache, in order to get the same data
we cached we decode the encoded page on the cached data */
Nit : Are the above comments still valid...given that there is no decoding happening here ?
- memcpy(prev_cached_page, XBZRLE.current_buf, TARGET_PAGE_SIZE);
+ if (!last_stage) {
+ memcpy(prev_cached_page, XBZRLE.current_buf, TARGET_PAGE_SIZE);
+ }
hdr.xh_len = encoded_len;
hdr.xh_flags |= ENCODING_FLAG_XBZRLE; @@ -345,7 +349,7 @@ static ram_addr_t last_offset;
* n: the amount of bytes written in other case
*/
-static int ram_save_block(QEMUFile *f)
+static int ram_save_block(QEMUFile *f, bool last_stage)
{
RAMBlock *block = last_block;
ram_addr_t offset = last_offset;
@@ -376,8 +380,10 @@ static int ram_save_block(QEMUFile *f)
} else if (migrate_use_xbzrle()) {
current_addr = block->offset + offset;
bytes_sent = save_xbzrle_page(f, p, current_addr, block,
- offset, cont);
- p = get_cached_data(XBZRLE.cache, current_addr);
+ offset, cont, last_stage);
+ if (!last_stage) {
+ p = get_cached_data(XBZRLE.cache, current_addr);
+ }
}
/* either we didn't send yet (we may have had XBZRLE overflow) */ @@ -549,7 +555,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
while ((ret = qemu_file_rate_limit(f)) == 0) {
int bytes_sent;
- bytes_sent = ram_save_block(f);
+ bytes_sent = ram_save_block(f, false);
/* no more blocks to sent */
if (bytes_sent < 0) {
break;
@@ -611,7 +617,7 @@ static int ram_save_complete(QEMUFile *f, void *opaque)
while (true) {
int bytes_sent;
- bytes_sent = ram_save_block(f);
+ bytes_sent = ram_save_block(f, true);
/* no more blocks to sent */
if (bytes_sent < 0) {
break;
--
1.7.7.6
[-- Attachment #2: Type: text/html, Size: 12936 bytes --]
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH 11/11] Restart optimization on stage3 update version
2012-07-25 15:41 ` Vinod, Chegu
@ 2012-07-26 5:03 ` Orit Wasserman
0 siblings, 0 replies; 33+ messages in thread
From: Orit Wasserman @ 2012-07-26 5:03 UTC (permalink / raw)
To: Vinod, Chegu
Cc: peter.maydell, aliguori, quintela, stefanha, qemu-devel, mdroth,
blauwirbel, avi, pbonzini, lcapitulino, eblake
On 07/25/2012 06:41 PM, Vinod, Chegu wrote:
>
>
>
>
> -----Original Message-----
> From: Orit Wasserman [mailto:owasserm@redhat.com]
>
>
>
> From: Juan Quintela <quintela@redhat.com <mailto:quintela@redhat.com>>
>
>
>
> Signed-off-by: Juan Quintela <quintela@redhat.com <mailto:quintela@redhat.com>>
>
> ---
>
> arch_init.c | 24 +++++++++++++++---------
>
> 1 files changed, 15 insertions(+), 9 deletions(-)
>
>
>
> diff --git a/arch_init.c b/arch_init.c
>
> index f484bd5..f555c27 100644
>
> --- a/arch_init.c
>
> +++ b/arch_init.c
>
> @@ -279,7 +279,7 @@ static void save_block_hdr(QEMUFile *f, RAMBlock *block, ram_addr_t offset,
>
> static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data,
>
> ram_addr_t current_addr, RAMBlock *block,
>
> - ram_addr_t offset, int cont)
>
> + ram_addr_t offset, int cont, bool
>
> + last_stage)
>
> {
>
> int encoded_len = 0, bytes_sent = -1;
>
> XBZRLEHeader hdr = {
>
> @@ -289,8 +289,10 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data,
>
> uint8_t *prev_cached_page;
>
> if (!cache_is_cached(XBZRLE.cache, current_addr)) {
>
> - cache_insert(XBZRLE.cache, current_addr,
>
> - g_memdup(current_data, TARGET_PAGE_SIZE));
>
> + if (!last_stage) {
>
> + cache_insert(XBZRLE.cache, current_addr,
>
> + g_memdup(current_data, TARGET_PAGE_SIZE));
>
> + }
>
> acct_info.xbzrle_cache_miss++;
>
> return -1;
>
> }
>
> @@ -317,7 +319,9 @@ static int save_xbzrle_page(QEMUFile *f, uint8_t *current_data,
>
> /* we need to update the data in the cache, in order to get the same data
>
> we cached we decode the encoded page on the cached data */
>
>
>
> Nit : Are the above comments still valid…given that there is no decoding happening here ?
I will fix them.
Orit
>
>
>
> - memcpy(prev_cached_page, XBZRLE.current_buf, TARGET_PAGE_SIZE);
>
> + if (!last_stage) {
>
> + memcpy(prev_cached_page, XBZRLE.current_buf, TARGET_PAGE_SIZE);
>
> + }
>
> hdr.xh_len = encoded_len;
>
> hdr.xh_flags |= ENCODING_FLAG_XBZRLE; @@ -345,7 +349,7 @@ static ram_addr_t last_offset;
>
> * n: the amount of bytes written in other case
>
> */
>
> -static int ram_save_block(QEMUFile *f)
>
> +static int ram_save_block(QEMUFile *f, bool last_stage)
>
> {
>
> RAMBlock *block = last_block;
>
> ram_addr_t offset = last_offset;
>
> @@ -376,8 +380,10 @@ static int ram_save_block(QEMUFile *f)
>
> } else if (migrate_use_xbzrle()) {
>
> current_addr = block->offset + offset;
>
> bytes_sent = save_xbzrle_page(f, p, current_addr, block,
>
> - offset, cont);
>
> - p = get_cached_data(XBZRLE.cache, current_addr);
>
> + offset, cont, last_stage);
>
> + if (!last_stage) {
>
> + p = get_cached_data(XBZRLE.cache, current_addr);
>
> + }
>
> }
>
> /* either we didn't send yet (we may have had XBZRLE overflow) */ @@ -549,7 +555,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
>
> while ((ret = qemu_file_rate_limit(f)) == 0) {
>
> int bytes_sent;
>
> - bytes_sent = ram_save_block(f);
>
> + bytes_sent = ram_save_block(f, false);
>
> /* no more blocks to sent */
>
> if (bytes_sent < 0) {
>
> break;
>
> @@ -611,7 +617,7 @@ static int ram_save_complete(QEMUFile *f, void *opaque)
>
> while (true) {
>
> int bytes_sent;
>
> - bytes_sent = ram_save_block(f);
>
> + bytes_sent = ram_save_block(f, true);
>
> /* no more blocks to sent */
>
> if (bytes_sent < 0) {
>
> break;
>
> --
>
> 1.7.7.6
>
>
>
^ permalink raw reply [flat|nested] 33+ messages in thread