* [PULL 00/15] Pull migration patches
@ 2020-02-28 9:24 Juan Quintela
2020-02-28 9:24 ` [PULL 01/15] multifd: Add multifd-compression parameter Juan Quintela
` (15 more replies)
0 siblings, 16 replies; 27+ messages in thread
From: Juan Quintela @ 2020-02-28 9:24 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Fam Zheng, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, qemu-block, Juan Quintela,
Dr. David Alan Gilbert, Markus Armbruster, Alex Bennée,
Stefan Hajnoczi, Paolo Bonzini, Philippe Mathieu-Daudé,
Hailiang Zhang
The following changes since commit 8b6b68e05b43f976714ca1d2afe01a64e1d82cba:
Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging (2020-02-27 19:15:15 +0000)
are available in the Git repository at:
https://github.com/juanquintela/qemu.git tags/pull-migration-pull-request
for you to fetch changes up to f51d0b4178738bba87d796eba7444f6cdb3aa0fd:
savevm: Don't call colo_init_ram_cache twice (2020-02-28 10:13:54 +0100)
----------------------------------------------------------------
Migration pull request
- cleanup redundant initilazation (chen)
- fix memleak in test_load_qlist (chen)
- several colo fixes (hailiang)
- multifd compression (juan)
- release list after use (pan)
- rename BLOCK_SIZE (stefan)
Please apply.
----------------------------------------------------------------
Chen Qun (2):
migration/vmstate: Remove redundant statement in
vmstate_save_state_v()
test-vmstate: Fix memleaks in test_load_qlist
Hailiang Zhang (3):
migration: fix COLO broken caused by a previous commit
migration/colo: wrap incoming checkpoint process into new helper
savevm: Don't call colo_init_ram_cache twice
Juan Quintela (8):
multifd: Add multifd-compression parameter
migration: Add support for modules
multifd: Make no compression operations into its own structure
multifd: Add multifd-zlib-level parameter
multifd: Add zlib compression multifd support
configure: Enable test and libs for zstd
multifd: Add multifd-zstd-level parameter
multifd: Add zstd compression multifd support
Pan Nengyuan (1):
migration/savevm: release gslist after dump_vmstate_json
Stefan Hajnoczi (1):
migration/block: rename BLOCK_SIZE macro
.gitlab-ci.yml | 1 +
.travis.yml | 1 +
configure | 30 ++
hw/core/qdev-properties.c | 13 +
include/hw/qdev-properties.h | 4 +
include/qemu/module.h | 2 +
migration/Makefile.objs | 2 +
migration/block.c | 39 +-
migration/colo.c | 260 +++++++-------
migration/migration.c | 76 +++-
migration/migration.h | 3 +
migration/multifd-zlib.c | 325 +++++++++++++++++
migration/multifd-zstd.c | 339 ++++++++++++++++++
migration/multifd.c | 191 +++++++++-
migration/multifd.h | 31 ++
migration/ram.c | 2 +-
migration/savevm.c | 1 +
migration/vmstate.c | 1 -
monitor/hmp-cmds.c | 21 ++
qapi/migration.json | 80 ++++-
softmmu/vl.c | 1 +
tests/docker/dockerfiles/centos7.docker | 3 +-
.../dockerfiles/fedora-i386-cross.docker | 3 +-
tests/docker/dockerfiles/fedora.docker | 3 +-
tests/docker/dockerfiles/ubuntu.docker | 1 +
tests/docker/dockerfiles/ubuntu1804.docker | 1 +
tests/qtest/migration-test.c | 30 +-
tests/test-vmstate.c | 6 +-
tests/vm/fedora | 5 +-
tests/vm/freebsd | 3 +
tests/vm/netbsd | 3 +
tests/vm/openbsd | 3 +
32 files changed, 1307 insertions(+), 177 deletions(-)
create mode 100644 migration/multifd-zlib.c
create mode 100644 migration/multifd-zstd.c
--
2.24.1
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PULL 01/15] multifd: Add multifd-compression parameter
2020-02-28 9:24 [PULL 00/15] Pull migration patches Juan Quintela
@ 2020-02-28 9:24 ` Juan Quintela
2020-02-28 9:24 ` [PULL 02/15] migration: Add support for modules Juan Quintela
` (14 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: Juan Quintela @ 2020-02-28 9:24 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Fam Zheng, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, qemu-block, Juan Quintela,
Dr. David Alan Gilbert, Markus Armbruster, Alex Bennée,
Stefan Hajnoczi, Paolo Bonzini, Philippe Mathieu-Daudé,
Hailiang Zhang
This will store the compression method to use. We start with none.
Signed-off-by: Juan Quintela <quintela@redhat.com>
Acked-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
Rename multifd-method to multifd-compression
---
hw/core/qdev-properties.c | 13 +++++++++++++
include/hw/qdev-properties.h | 4 ++++
migration/migration.c | 13 +++++++++++++
monitor/hmp-cmds.c | 13 +++++++++++++
qapi/migration.json | 30 +++++++++++++++++++++++++++---
tests/qtest/migration-test.c | 14 ++++++++++----
6 files changed, 80 insertions(+), 7 deletions(-)
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 7f93bfeb88..fa7edac020 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -8,6 +8,7 @@
#include "qapi/qmp/qerror.h"
#include "qemu/ctype.h"
#include "qemu/error-report.h"
+#include "qapi/qapi-types-migration.h"
#include "hw/block/block.h"
#include "net/hub.h"
#include "qapi/visitor.h"
@@ -639,6 +640,18 @@ const PropertyInfo qdev_prop_fdc_drive_type = {
.set_default_value = set_default_value_enum,
};
+/* --- MultiFDCompression --- */
+
+const PropertyInfo qdev_prop_multifd_compression = {
+ .name = "MultiFDCompression",
+ .description = "multifd_compression values, "
+ "none",
+ .enum_table = &MultiFDCompression_lookup,
+ .get = get_enum,
+ .set = set_enum,
+ .set_default_value = set_default_value_enum,
+};
+
/* --- pci address --- */
/*
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 906e697c58..f161604fb6 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -20,6 +20,7 @@ extern const PropertyInfo qdev_prop_chr;
extern const PropertyInfo qdev_prop_tpm;
extern const PropertyInfo qdev_prop_macaddr;
extern const PropertyInfo qdev_prop_on_off_auto;
+extern const PropertyInfo qdev_prop_multifd_compression;
extern const PropertyInfo qdev_prop_losttickpolicy;
extern const PropertyInfo qdev_prop_blockdev_on_error;
extern const PropertyInfo qdev_prop_bios_chs_trans;
@@ -184,6 +185,9 @@ extern const PropertyInfo qdev_prop_pcie_link_width;
DEFINE_PROP(_n, _s, _f, qdev_prop_macaddr, MACAddr)
#define DEFINE_PROP_ON_OFF_AUTO(_n, _s, _f, _d) \
DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_on_off_auto, OnOffAuto)
+#define DEFINE_PROP_MULTIFD_COMPRESSION(_n, _s, _f, _d) \
+ DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_multifd_compression, \
+ MultiFDCompression)
#define DEFINE_PROP_LOSTTICKPOLICY(_n, _s, _f, _d) \
DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_losttickpolicy, \
LostTickPolicy)
diff --git a/migration/migration.c b/migration/migration.c
index 8fb68795dc..bc744d1734 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -88,6 +88,7 @@
/* The delay time (in ms) between two COLO checkpoints */
#define DEFAULT_MIGRATE_X_CHECKPOINT_DELAY (200 * 100)
#define DEFAULT_MIGRATE_MULTIFD_CHANNELS 2
+#define DEFAULT_MIGRATE_MULTIFD_COMPRESSION MULTIFD_COMPRESSION_NONE
/* Background transfer rate for postcopy, 0 means unlimited, note
* that page requests can still exceed this limit.
@@ -798,6 +799,8 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
params->block_incremental = s->parameters.block_incremental;
params->has_multifd_channels = true;
params->multifd_channels = s->parameters.multifd_channels;
+ params->has_multifd_compression = true;
+ params->multifd_compression = s->parameters.multifd_compression;
params->has_xbzrle_cache_size = true;
params->xbzrle_cache_size = s->parameters.xbzrle_cache_size;
params->has_max_postcopy_bandwidth = true;
@@ -1315,6 +1318,9 @@ static void migrate_params_test_apply(MigrateSetParameters *params,
if (params->has_multifd_channels) {
dest->multifd_channels = params->multifd_channels;
}
+ if (params->has_multifd_compression) {
+ dest->multifd_compression = params->multifd_compression;
+ }
if (params->has_xbzrle_cache_size) {
dest->xbzrle_cache_size = params->xbzrle_cache_size;
}
@@ -1411,6 +1417,9 @@ static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
if (params->has_multifd_channels) {
s->parameters.multifd_channels = params->multifd_channels;
}
+ if (params->has_multifd_compression) {
+ s->parameters.multifd_compression = params->multifd_compression;
+ }
if (params->has_xbzrle_cache_size) {
s->parameters.xbzrle_cache_size = params->xbzrle_cache_size;
xbzrle_cache_resize(params->xbzrle_cache_size, errp);
@@ -3523,6 +3532,9 @@ static Property migration_properties[] = {
DEFINE_PROP_UINT8("multifd-channels", MigrationState,
parameters.multifd_channels,
DEFAULT_MIGRATE_MULTIFD_CHANNELS),
+ DEFINE_PROP_MULTIFD_COMPRESSION("multifd-compression", MigrationState,
+ parameters.multifd_compression,
+ DEFAULT_MIGRATE_MULTIFD_COMPRESSION),
DEFINE_PROP_SIZE("xbzrle-cache-size", MigrationState,
parameters.xbzrle_cache_size,
DEFAULT_MIGRATE_XBZRLE_CACHE_SIZE),
@@ -3613,6 +3625,7 @@ static void migration_instance_init(Object *obj)
params->has_x_checkpoint_delay = true;
params->has_block_incremental = true;
params->has_multifd_channels = true;
+ params->has_multifd_compression = true;
params->has_xbzrle_cache_size = true;
params->has_max_postcopy_bandwidth = true;
params->has_max_cpu_throttle = true;
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 53bc3f76c4..d9b4861f94 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -40,6 +40,7 @@
#include "qapi/qapi-commands-tpm.h"
#include "qapi/qapi-commands-ui.h"
#include "qapi/qapi-visit-net.h"
+#include "qapi/qapi-visit-migration.h"
#include "qapi/qmp/qdict.h"
#include "qapi/qmp/qerror.h"
#include "qapi/string-input-visitor.h"
@@ -448,6 +449,9 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict)
monitor_printf(mon, "%s: %u\n",
MigrationParameter_str(MIGRATION_PARAMETER_MULTIFD_CHANNELS),
params->multifd_channels);
+ monitor_printf(mon, "%s: %s\n",
+ MigrationParameter_str(MIGRATION_PARAMETER_MULTIFD_COMPRESSION),
+ MultiFDCompression_str(params->multifd_compression));
monitor_printf(mon, "%s: %" PRIu64 "\n",
MigrationParameter_str(MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE),
params->xbzrle_cache_size);
@@ -1739,6 +1743,7 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
MigrateSetParameters *p = g_new0(MigrateSetParameters, 1);
uint64_t valuebw = 0;
uint64_t cache_size;
+ MultiFDCompression compress_type;
Error *err = NULL;
int val, ret;
@@ -1824,6 +1829,14 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
p->has_multifd_channels = true;
visit_type_int(v, param, &p->multifd_channels, &err);
break;
+ case MIGRATION_PARAMETER_MULTIFD_COMPRESSION:
+ p->has_multifd_compression = true;
+ visit_type_MultiFDCompression(v, param, &compress_type, &err);
+ if (err) {
+ break;
+ }
+ p->multifd_compression = compress_type;
+ break;
case MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE:
p->has_xbzrle_cache_size = true;
visit_type_size(v, param, &cache_size, &err);
diff --git a/qapi/migration.json b/qapi/migration.json
index 52f3429969..40950ef47e 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -488,6 +488,19 @@
##
{ 'command': 'query-migrate-capabilities', 'returns': ['MigrationCapabilityStatus']}
+##
+# @MultiFDCompression:
+#
+# An enumeration of multifd compression methods.
+#
+# @none: no compression.
+#
+# Since: 5.0
+#
+##
+{ 'enum': 'MultiFDCompression',
+ 'data': [ 'none' ] }
+
##
# @MigrationParameter:
#
@@ -586,6 +599,9 @@
# @max-cpu-throttle: maximum cpu throttle percentage.
# Defaults to 99. (Since 3.1)
#
+# @multifd-compression: Which compression method to use.
+# Defaults to none. (Since 5.0)
+#
# Since: 2.4
##
{ 'enum': 'MigrationParameter',
@@ -598,7 +614,7 @@
'downtime-limit', 'x-checkpoint-delay', 'block-incremental',
'multifd-channels',
'xbzrle-cache-size', 'max-postcopy-bandwidth',
- 'max-cpu-throttle' ] }
+ 'max-cpu-throttle', 'multifd-compression' ] }
##
# @MigrateSetParameters:
@@ -688,6 +704,9 @@
# @max-cpu-throttle: maximum cpu throttle percentage.
# The default value is 99. (Since 3.1)
#
+# @multifd-compression: Which compression method to use.
+# Defaults to none. (Since 5.0)
+#
# Since: 2.4
##
# TODO either fuse back into MigrationParameters, or make
@@ -713,7 +732,8 @@
'*multifd-channels': 'int',
'*xbzrle-cache-size': 'size',
'*max-postcopy-bandwidth': 'size',
- '*max-cpu-throttle': 'int' } }
+ '*max-cpu-throttle': 'int',
+ '*multifd-compression': 'MultiFDCompression' } }
##
# @migrate-set-parameters:
@@ -823,6 +843,9 @@
# Defaults to 99.
# (Since 3.1)
#
+# @multifd-compression: Which compression method to use.
+# Defaults to none. (Since 5.0)
+#
# Since: 2.4
##
{ 'struct': 'MigrationParameters',
@@ -846,7 +869,8 @@
'*multifd-channels': 'uint8',
'*xbzrle-cache-size': 'size',
'*max-postcopy-bandwidth': 'size',
- '*max-cpu-throttle':'uint8'} }
+ '*max-cpu-throttle': 'uint8',
+ '*multifd-compression': 'MultiFDCompression' } }
##
# @query-migrate-parameters:
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index ccf313f288..f84c157285 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -378,7 +378,6 @@ static void migrate_check_parameter_str(QTestState *who, const char *parameter,
g_free(result);
}
-__attribute__((unused))
static void migrate_set_parameter_str(QTestState *who, const char *parameter,
const char *value)
{
@@ -1261,7 +1260,7 @@ static void test_migrate_auto_converge(void)
test_migrate_end(from, to, true);
}
-static void test_multifd_tcp(void)
+static void test_multifd_tcp(const char *method)
{
MigrateStart *args = migrate_start_new();
QTestState *from, *to;
@@ -1285,6 +1284,9 @@ static void test_multifd_tcp(void)
migrate_set_parameter_int(from, "multifd-channels", 16);
migrate_set_parameter_int(to, "multifd-channels", 16);
+ migrate_set_parameter_str(from, "multifd-compression", method);
+ migrate_set_parameter_str(to, "multifd-compression", method);
+
migrate_set_capability(from, "multifd", "true");
migrate_set_capability(to, "multifd", "true");
@@ -1316,6 +1318,11 @@ static void test_multifd_tcp(void)
g_free(uri);
}
+static void test_multifd_tcp_none(void)
+{
+ test_multifd_tcp("none");
+}
+
/*
* This test does:
* source target
@@ -1327,7 +1334,6 @@ static void test_multifd_tcp(void)
*
* And see that it works
*/
-
static void test_multifd_tcp_cancel(void)
{
MigrateStart *args = migrate_start_new();
@@ -1478,7 +1484,7 @@ int main(int argc, char **argv)
test_validate_uuid_dst_not_set);
qtest_add_func("/migration/auto_converge", test_migrate_auto_converge);
- qtest_add_func("/migration/multifd/tcp", test_multifd_tcp);
+ qtest_add_func("/migration/multifd/tcp/none", test_multifd_tcp_none);
qtest_add_func("/migration/multifd/tcp/cancel", test_multifd_tcp_cancel);
ret = g_test_run();
--
2.24.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PULL 02/15] migration: Add support for modules
2020-02-28 9:24 [PULL 00/15] Pull migration patches Juan Quintela
2020-02-28 9:24 ` [PULL 01/15] multifd: Add multifd-compression parameter Juan Quintela
@ 2020-02-28 9:24 ` Juan Quintela
2020-02-28 9:24 ` [PULL 03/15] multifd: Make no compression operations into its own structure Juan Quintela
` (13 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: Juan Quintela @ 2020-02-28 9:24 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Fam Zheng, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, qemu-block, Juan Quintela,
Dr. David Alan Gilbert, Markus Armbruster, Alex Bennée,
Stefan Hajnoczi, Paolo Bonzini, Philippe Mathieu-Daudé,
Hailiang Zhang
So we don't have to compile everything in, or have ifdefs
Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
include/qemu/module.h | 2 ++
softmmu/vl.c | 1 +
2 files changed, 3 insertions(+)
diff --git a/include/qemu/module.h b/include/qemu/module.h
index 684753d808..011ae1ae76 100644
--- a/include/qemu/module.h
+++ b/include/qemu/module.h
@@ -40,6 +40,7 @@ static void __attribute__((constructor)) do_qemu_init_ ## function(void) \
#endif
typedef enum {
+ MODULE_INIT_MIGRATION,
MODULE_INIT_BLOCK,
MODULE_INIT_OPTS,
MODULE_INIT_QOM,
@@ -59,6 +60,7 @@ typedef enum {
#define libqos_init(function) module_init(function, MODULE_INIT_LIBQOS)
#define fuzz_target_init(function) module_init(function, \
MODULE_INIT_FUZZ_TARGET)
+#define migration_init(function) module_init(function, MODULE_INIT_MIGRATION)
#define block_module_load_one(lib) module_load_one("block-", lib)
#define ui_module_load_one(lib) module_load_one("ui-", lib)
#define audio_module_load_one(lib) module_load_one("audio-", lib)
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 16ff5a16a3..705ee6f841 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2858,6 +2858,7 @@ void qemu_init(int argc, char **argv, char **envp)
qemu_init_exec_dir(argv[0]);
module_call_init(MODULE_INIT_QOM);
+ module_call_init(MODULE_INIT_MIGRATION);
qemu_add_opts(&qemu_drive_opts);
qemu_add_drive_opts(&qemu_legacy_drive_opts);
--
2.24.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PULL 03/15] multifd: Make no compression operations into its own structure
2020-02-28 9:24 [PULL 00/15] Pull migration patches Juan Quintela
2020-02-28 9:24 ` [PULL 01/15] multifd: Add multifd-compression parameter Juan Quintela
2020-02-28 9:24 ` [PULL 02/15] migration: Add support for modules Juan Quintela
@ 2020-02-28 9:24 ` Juan Quintela
2022-04-12 19:04 ` Peter Maydell
2020-02-28 9:24 ` [PULL 04/15] multifd: Add multifd-zlib-level parameter Juan Quintela
` (12 subsequent siblings)
15 siblings, 1 reply; 27+ messages in thread
From: Juan Quintela @ 2020-02-28 9:24 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Fam Zheng, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, qemu-block, Juan Quintela,
Dr. David Alan Gilbert, Markus Armbruster, Alex Bennée,
Stefan Hajnoczi, Paolo Bonzini, Philippe Mathieu-Daudé,
Hailiang Zhang
It will be used later.
Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
No comp value needs to be zero.
---
migration/migration.c | 9 ++
migration/migration.h | 1 +
migration/multifd.c | 185 ++++++++++++++++++++++++++++++++++++++++--
migration/multifd.h | 26 ++++++
migration/ram.c | 1 +
5 files changed, 214 insertions(+), 8 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index bc744d1734..eb7b0a2df0 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2245,6 +2245,15 @@ int migrate_multifd_channels(void)
return s->parameters.multifd_channels;
}
+MultiFDCompression migrate_multifd_compression(void)
+{
+ MigrationState *s;
+
+ s = migrate_get_current();
+
+ return s->parameters.multifd_compression;
+}
+
int migrate_use_xbzrle(void)
{
MigrationState *s;
diff --git a/migration/migration.h b/migration/migration.h
index 8473ddfc88..59fea02482 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -300,6 +300,7 @@ bool migrate_auto_converge(void);
bool migrate_use_multifd(void);
bool migrate_pause_before_switchover(void);
int migrate_multifd_channels(void);
+MultiFDCompression migrate_multifd_compression(void);
int migrate_use_xbzrle(void);
int64_t migrate_xbzrle_cache_size(void);
diff --git a/migration/multifd.c b/migration/multifd.c
index b3e8ae9bcc..97433e5135 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -38,6 +38,134 @@ typedef struct {
uint64_t unused2[4]; /* Reserved for future use */
} __attribute__((packed)) MultiFDInit_t;
+/* Multifd without compression */
+
+/**
+ * nocomp_send_setup: setup send side
+ *
+ * For no compression this function does nothing.
+ *
+ * Returns 0 for success or -1 for error
+ *
+ * @p: Params for the channel that we are using
+ * @errp: pointer to an error
+ */
+static int nocomp_send_setup(MultiFDSendParams *p, Error **errp)
+{
+ return 0;
+}
+
+/**
+ * nocomp_send_cleanup: cleanup send side
+ *
+ * For no compression this function does nothing.
+ *
+ * @p: Params for the channel that we are using
+ */
+static void nocomp_send_cleanup(MultiFDSendParams *p, Error **errp)
+{
+ return;
+}
+
+/**
+ * nocomp_send_prepare: prepare date to be able to send
+ *
+ * For no compression we just have to calculate the size of the
+ * packet.
+ *
+ * Returns 0 for success or -1 for error
+ *
+ * @p: Params for the channel that we are using
+ * @used: number of pages used
+ * @errp: pointer to an error
+ */
+static int nocomp_send_prepare(MultiFDSendParams *p, uint32_t used,
+ Error **errp)
+{
+ p->next_packet_size = used * qemu_target_page_size();
+ p->flags |= MULTIFD_FLAG_NOCOMP;
+ return 0;
+}
+
+/**
+ * nocomp_send_write: do the actual write of the data
+ *
+ * For no compression we just have to write the data.
+ *
+ * Returns 0 for success or -1 for error
+ *
+ * @p: Params for the channel that we are using
+ * @used: number of pages used
+ * @errp: pointer to an error
+ */
+static int nocomp_send_write(MultiFDSendParams *p, uint32_t used, Error **errp)
+{
+ return qio_channel_writev_all(p->c, p->pages->iov, used, errp);
+}
+
+/**
+ * nocomp_recv_setup: setup receive side
+ *
+ * For no compression this function does nothing.
+ *
+ * Returns 0 for success or -1 for error
+ *
+ * @p: Params for the channel that we are using
+ * @errp: pointer to an error
+ */
+static int nocomp_recv_setup(MultiFDRecvParams *p, Error **errp)
+{
+ return 0;
+}
+
+/**
+ * nocomp_recv_cleanup: setup receive side
+ *
+ * For no compression this function does nothing.
+ *
+ * @p: Params for the channel that we are using
+ */
+static void nocomp_recv_cleanup(MultiFDRecvParams *p)
+{
+}
+
+/**
+ * nocomp_recv_pages: read the data from the channel into actual pages
+ *
+ * For no compression we just need to read things into the correct place.
+ *
+ * Returns 0 for success or -1 for error
+ *
+ * @p: Params for the channel that we are using
+ * @used: number of pages used
+ * @errp: pointer to an error
+ */
+static int nocomp_recv_pages(MultiFDRecvParams *p, uint32_t used, Error **errp)
+{
+ uint32_t flags = p->flags & MULTIFD_FLAG_COMPRESSION_MASK;
+
+ if (flags != MULTIFD_FLAG_NOCOMP) {
+ error_setg(errp, "multifd %d: flags received %x flags expected %x",
+ p->id, flags, MULTIFD_FLAG_NOCOMP);
+ return -1;
+ }
+ return qio_channel_readv_all(p->c, p->pages->iov, used, errp);
+}
+
+static MultiFDMethods multifd_nocomp_ops = {
+ .send_setup = nocomp_send_setup,
+ .send_cleanup = nocomp_send_cleanup,
+ .send_prepare = nocomp_send_prepare,
+ .send_write = nocomp_send_write,
+ .recv_setup = nocomp_recv_setup,
+ .recv_cleanup = nocomp_recv_cleanup,
+ .recv_pages = nocomp_recv_pages
+};
+
+static MultiFDMethods *multifd_ops[MULTIFD_COMPRESSION__MAX] = {
+ [MULTIFD_COMPRESSION_NONE] = &multifd_nocomp_ops,
+};
+
static int multifd_send_initial_packet(MultiFDSendParams *p, Error **errp)
{
MultiFDInit_t msg = {};
@@ -246,6 +374,8 @@ struct {
* We will use atomic operations. Only valid values are 0 and 1.
*/
int exiting;
+ /* multifd ops */
+ MultiFDMethods *ops;
} *multifd_send_state;
/*
@@ -397,6 +527,7 @@ void multifd_save_cleanup(void)
}
for (i = 0; i < migrate_multifd_channels(); i++) {
MultiFDSendParams *p = &multifd_send_state->params[i];
+ Error *local_err = NULL;
socket_send_channel_destroy(p->c);
p->c = NULL;
@@ -410,6 +541,10 @@ void multifd_save_cleanup(void)
p->packet_len = 0;
g_free(p->packet);
p->packet = NULL;
+ multifd_send_state->ops->send_cleanup(p, &local_err);
+ if (local_err) {
+ migrate_set_error(migrate_get_current(), local_err);
+ }
}
qemu_sem_destroy(&multifd_send_state->channels_ready);
g_free(multifd_send_state->params);
@@ -494,7 +629,14 @@ static void *multifd_send_thread(void *opaque)
uint64_t packet_num = p->packet_num;
flags = p->flags;
- p->next_packet_size = used * qemu_target_page_size();
+ if (used) {
+ ret = multifd_send_state->ops->send_prepare(p, used,
+ &local_err);
+ if (ret != 0) {
+ qemu_mutex_unlock(&p->mutex);
+ break;
+ }
+ }
multifd_send_fill_packet(p);
p->flags = 0;
p->num_packets++;
@@ -513,8 +655,7 @@ static void *multifd_send_thread(void *opaque)
}
if (used) {
- ret = qio_channel_writev_all(p->c, p->pages->iov,
- used, &local_err);
+ ret = multifd_send_state->ops->send_write(p, used, &local_err);
if (ret != 0) {
break;
}
@@ -604,6 +745,7 @@ int multifd_save_setup(Error **errp)
multifd_send_state->pages = multifd_pages_init(page_count);
qemu_sem_init(&multifd_send_state->channels_ready, 0);
atomic_set(&multifd_send_state->exiting, 0);
+ multifd_send_state->ops = multifd_ops[migrate_multifd_compression()];
for (i = 0; i < thread_count; i++) {
MultiFDSendParams *p = &multifd_send_state->params[i];
@@ -623,6 +765,18 @@ int multifd_save_setup(Error **errp)
p->name = g_strdup_printf("multifdsend_%d", i);
socket_send_channel_create(multifd_new_send_channel_async, p);
}
+
+ for (i = 0; i < thread_count; i++) {
+ MultiFDSendParams *p = &multifd_send_state->params[i];
+ Error *local_err = NULL;
+ int ret;
+
+ ret = multifd_send_state->ops->send_setup(p, &local_err);
+ if (ret) {
+ error_propagate(errp, local_err);
+ return ret;
+ }
+ }
return 0;
}
@@ -634,6 +788,8 @@ struct {
QemuSemaphore sem_sync;
/* global number of generated multifd packets */
uint64_t packet_num;
+ /* multifd ops */
+ MultiFDMethods *ops;
} *multifd_recv_state;
static void multifd_recv_terminate_threads(Error *err)
@@ -673,7 +829,6 @@ static void multifd_recv_terminate_threads(Error *err)
int multifd_load_cleanup(Error **errp)
{
int i;
- int ret = 0;
if (!migrate_use_multifd()) {
return 0;
@@ -706,6 +861,7 @@ int multifd_load_cleanup(Error **errp)
p->packet_len = 0;
g_free(p->packet);
p->packet = NULL;
+ multifd_recv_state->ops->recv_cleanup(p);
}
qemu_sem_destroy(&multifd_recv_state->sem_sync);
g_free(multifd_recv_state->params);
@@ -713,7 +869,7 @@ int multifd_load_cleanup(Error **errp)
g_free(multifd_recv_state);
multifd_recv_state = NULL;
- return ret;
+ return 0;
}
void multifd_recv_sync_main(void)
@@ -778,6 +934,8 @@ static void *multifd_recv_thread(void *opaque)
used = p->pages->used;
flags = p->flags;
+ /* recv methods don't know how to handle the SYNC flag */
+ p->flags &= ~MULTIFD_FLAG_SYNC;
trace_multifd_recv(p->id, p->packet_num, used, flags,
p->next_packet_size);
p->num_packets++;
@@ -785,8 +943,7 @@ static void *multifd_recv_thread(void *opaque)
qemu_mutex_unlock(&p->mutex);
if (used) {
- ret = qio_channel_readv_all(p->c, p->pages->iov,
- used, &local_err);
+ ret = multifd_recv_state->ops->recv_pages(p, used, &local_err);
if (ret != 0) {
break;
}
@@ -825,6 +982,7 @@ int multifd_load_setup(Error **errp)
multifd_recv_state->params = g_new0(MultiFDRecvParams, thread_count);
atomic_set(&multifd_recv_state->count, 0);
qemu_sem_init(&multifd_recv_state->sem_sync, 0);
+ multifd_recv_state->ops = multifd_ops[migrate_multifd_compression()];
for (i = 0; i < thread_count; i++) {
MultiFDRecvParams *p = &multifd_recv_state->params[i];
@@ -839,6 +997,18 @@ int multifd_load_setup(Error **errp)
p->packet = g_malloc0(p->packet_len);
p->name = g_strdup_printf("multifdrecv_%d", i);
}
+
+ for (i = 0; i < thread_count; i++) {
+ MultiFDRecvParams *p = &multifd_recv_state->params[i];
+ Error *local_err = NULL;
+ int ret;
+
+ ret = multifd_recv_state->ops->recv_setup(p, &local_err);
+ if (ret) {
+ error_propagate(errp, local_err);
+ return ret;
+ }
+ }
return 0;
}
@@ -896,4 +1066,3 @@ bool multifd_recv_new_channel(QIOChannel *ioc, Error **errp)
return atomic_read(&multifd_recv_state->count) ==
migrate_multifd_channels();
}
-
diff --git a/migration/multifd.h b/migration/multifd.h
index d8b0205977..54075ffa7d 100644
--- a/migration/multifd.h
+++ b/migration/multifd.h
@@ -25,6 +25,11 @@ int multifd_queue_page(QEMUFile *f, RAMBlock *block, ram_addr_t offset);
#define MULTIFD_FLAG_SYNC (1 << 0)
+/* We reserve 3 bits for compression methods */
+#define MULTIFD_FLAG_COMPRESSION_MASK (7 << 1)
+/* we need to be compatible. Before compression value was 0 */
+#define MULTIFD_FLAG_NOCOMP (0 << 1)
+
/* This value needs to be a multiple of qemu_target_page_size() */
#define MULTIFD_PACKET_SIZE (512 * 1024)
@@ -96,6 +101,8 @@ typedef struct {
uint64_t num_pages;
/* syncs main thread and channels */
QemuSemaphore sem_sync;
+ /* used for compression methods */
+ void *data;
} MultiFDSendParams;
typedef struct {
@@ -133,7 +140,26 @@ typedef struct {
uint64_t num_pages;
/* syncs main thread and channels */
QemuSemaphore sem_sync;
+ /* used for de-compression methods */
+ void *data;
} MultiFDRecvParams;
+typedef struct {
+ /* Setup for sending side */
+ int (*send_setup)(MultiFDSendParams *p, Error **errp);
+ /* Cleanup for sending side */
+ void (*send_cleanup)(MultiFDSendParams *p, Error **errp);
+ /* Prepare the send packet */
+ int (*send_prepare)(MultiFDSendParams *p, uint32_t used, Error **errp);
+ /* Write the send packet */
+ int (*send_write)(MultiFDSendParams *p, uint32_t used, Error **errp);
+ /* Setup for receiving side */
+ int (*recv_setup)(MultiFDRecvParams *p, Error **errp);
+ /* Cleanup for receiving side */
+ void (*recv_cleanup)(MultiFDRecvParams *p);
+ /* Read all pages */
+ int (*recv_pages)(MultiFDRecvParams *p, uint32_t used, Error **errp);
+} MultiFDMethods;
+
#endif
diff --git a/migration/ram.c b/migration/ram.c
index ed23ed1c7c..73a141bb60 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -43,6 +43,7 @@
#include "page_cache.h"
#include "qemu/error-report.h"
#include "qapi/error.h"
+#include "qapi/qapi-types-migration.h"
#include "qapi/qapi-events-migration.h"
#include "qapi/qmp/qerror.h"
#include "trace.h"
--
2.24.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PULL 04/15] multifd: Add multifd-zlib-level parameter
2020-02-28 9:24 [PULL 00/15] Pull migration patches Juan Quintela
` (2 preceding siblings ...)
2020-02-28 9:24 ` [PULL 03/15] multifd: Make no compression operations into its own structure Juan Quintela
@ 2020-02-28 9:24 ` Juan Quintela
2020-02-28 9:24 ` [PULL 05/15] multifd: Add zlib compression multifd support Juan Quintela
` (11 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: Juan Quintela @ 2020-02-28 9:24 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Fam Zheng, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, qemu-block, Juan Quintela,
Dr. David Alan Gilbert, Markus Armbruster, Alex Bennée,
Stefan Hajnoczi, Paolo Bonzini, Philippe Mathieu-Daudé,
Hailiang Zhang
This parameter specifies the zlib compression level. The next patch
will put it to use.
Signed-off-by: Juan Quintela <quintela@redhat.com>
Acked-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
migration/migration.c | 24 ++++++++++++++++++++++++
migration/migration.h | 1 +
monitor/hmp-cmds.c | 4 ++++
qapi/migration.json | 30 +++++++++++++++++++++++++++---
4 files changed, 56 insertions(+), 3 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index eb7b0a2df0..a09726f679 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -89,6 +89,8 @@
#define DEFAULT_MIGRATE_X_CHECKPOINT_DELAY (200 * 100)
#define DEFAULT_MIGRATE_MULTIFD_CHANNELS 2
#define DEFAULT_MIGRATE_MULTIFD_COMPRESSION MULTIFD_COMPRESSION_NONE
+/* 0: means nocompress, 1: best speed, ... 9: best compress ratio */
+#define DEFAULT_MIGRATE_MULTIFD_ZLIB_LEVEL 1
/* Background transfer rate for postcopy, 0 means unlimited, note
* that page requests can still exceed this limit.
@@ -801,6 +803,8 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
params->multifd_channels = s->parameters.multifd_channels;
params->has_multifd_compression = true;
params->multifd_compression = s->parameters.multifd_compression;
+ params->has_multifd_zlib_level = true;
+ params->multifd_zlib_level = s->parameters.multifd_zlib_level;
params->has_xbzrle_cache_size = true;
params->xbzrle_cache_size = s->parameters.xbzrle_cache_size;
params->has_max_postcopy_bandwidth = true;
@@ -1208,6 +1212,13 @@ static bool migrate_params_check(MigrationParameters *params, Error **errp)
return false;
}
+ if (params->has_multifd_zlib_level &&
+ (params->multifd_zlib_level > 9)) {
+ error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "multifd_zlib_level",
+ "is invalid, it should be in the range of 0 to 9");
+ return false;
+ }
+
if (params->has_xbzrle_cache_size &&
(params->xbzrle_cache_size < qemu_target_page_size() ||
!is_power_of_2(params->xbzrle_cache_size))) {
@@ -2254,6 +2265,15 @@ MultiFDCompression migrate_multifd_compression(void)
return s->parameters.multifd_compression;
}
+int migrate_multifd_zlib_level(void)
+{
+ MigrationState *s;
+
+ s = migrate_get_current();
+
+ return s->parameters.multifd_zlib_level;
+}
+
int migrate_use_xbzrle(void)
{
MigrationState *s;
@@ -3544,6 +3564,9 @@ static Property migration_properties[] = {
DEFINE_PROP_MULTIFD_COMPRESSION("multifd-compression", MigrationState,
parameters.multifd_compression,
DEFAULT_MIGRATE_MULTIFD_COMPRESSION),
+ DEFINE_PROP_UINT8("multifd-zlib-level", MigrationState,
+ parameters.multifd_zlib_level,
+ DEFAULT_MIGRATE_MULTIFD_ZLIB_LEVEL),
DEFINE_PROP_SIZE("xbzrle-cache-size", MigrationState,
parameters.xbzrle_cache_size,
DEFAULT_MIGRATE_XBZRLE_CACHE_SIZE),
@@ -3635,6 +3658,7 @@ static void migration_instance_init(Object *obj)
params->has_block_incremental = true;
params->has_multifd_channels = true;
params->has_multifd_compression = true;
+ params->has_multifd_zlib_level = true;
params->has_xbzrle_cache_size = true;
params->has_max_postcopy_bandwidth = true;
params->has_max_cpu_throttle = true;
diff --git a/migration/migration.h b/migration/migration.h
index 59fea02482..c363ef9334 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -301,6 +301,7 @@ bool migrate_use_multifd(void);
bool migrate_pause_before_switchover(void);
int migrate_multifd_channels(void);
MultiFDCompression migrate_multifd_compression(void);
+int migrate_multifd_zlib_level(void);
int migrate_use_xbzrle(void);
int64_t migrate_xbzrle_cache_size(void);
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index d9b4861f94..409ebb2b03 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1837,6 +1837,10 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
}
p->multifd_compression = compress_type;
break;
+ case MIGRATION_PARAMETER_MULTIFD_ZLIB_LEVEL:
+ p->has_multifd_zlib_level = true;
+ visit_type_int(v, param, &p->multifd_zlib_level, &err);
+ break;
case MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE:
p->has_xbzrle_cache_size = true;
visit_type_size(v, param, &cache_size, &err);
diff --git a/qapi/migration.json b/qapi/migration.json
index 40950ef47e..468a9d92a4 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -602,6 +602,13 @@
# @multifd-compression: Which compression method to use.
# Defaults to none. (Since 5.0)
#
+# @multifd-zlib-level: Set the compression level to be used in live
+# migration, the compression level is an integer between 0
+# and 9, where 0 means no compression, 1 means the best
+# compression speed, and 9 means best compression ratio which
+# will consume more CPU.
+# Defaults to 1. (Since 5.0)
+#
# Since: 2.4
##
{ 'enum': 'MigrationParameter',
@@ -614,7 +621,8 @@
'downtime-limit', 'x-checkpoint-delay', 'block-incremental',
'multifd-channels',
'xbzrle-cache-size', 'max-postcopy-bandwidth',
- 'max-cpu-throttle', 'multifd-compression' ] }
+ 'max-cpu-throttle', 'multifd-compression',
+ 'multifd-zlib-level' ] }
##
# @MigrateSetParameters:
@@ -707,6 +715,13 @@
# @multifd-compression: Which compression method to use.
# Defaults to none. (Since 5.0)
#
+# @multifd-zlib-level: Set the compression level to be used in live
+# migration, the compression level is an integer between 0
+# and 9, where 0 means no compression, 1 means the best
+# compression speed, and 9 means best compression ratio which
+# will consume more CPU.
+# Defaults to 1. (Since 5.0)
+#
# Since: 2.4
##
# TODO either fuse back into MigrationParameters, or make
@@ -733,7 +748,8 @@
'*xbzrle-cache-size': 'size',
'*max-postcopy-bandwidth': 'size',
'*max-cpu-throttle': 'int',
- '*multifd-compression': 'MultiFDCompression' } }
+ '*multifd-compression': 'MultiFDCompression',
+ '*multifd-zlib-level': 'int' } }
##
# @migrate-set-parameters:
@@ -846,6 +862,13 @@
# @multifd-compression: Which compression method to use.
# Defaults to none. (Since 5.0)
#
+# @multifd-zlib-level: Set the compression level to be used in live
+# migration, the compression level is an integer between 0
+# and 9, where 0 means no compression, 1 means the best
+# compression speed, and 9 means best compression ratio which
+# will consume more CPU.
+# Defaults to 1. (Since 5.0)
+#
# Since: 2.4
##
{ 'struct': 'MigrationParameters',
@@ -870,7 +893,8 @@
'*xbzrle-cache-size': 'size',
'*max-postcopy-bandwidth': 'size',
'*max-cpu-throttle': 'uint8',
- '*multifd-compression': 'MultiFDCompression' } }
+ '*multifd-compression': 'MultiFDCompression',
+ '*multifd-zlib-level': 'uint8' } }
##
# @query-migrate-parameters:
--
2.24.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PULL 05/15] multifd: Add zlib compression multifd support
2020-02-28 9:24 [PULL 00/15] Pull migration patches Juan Quintela
` (3 preceding siblings ...)
2020-02-28 9:24 ` [PULL 04/15] multifd: Add multifd-zlib-level parameter Juan Quintela
@ 2020-02-28 9:24 ` Juan Quintela
2020-02-28 9:24 ` [PULL 06/15] configure: Enable test and libs for zstd Juan Quintela
` (10 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: Juan Quintela @ 2020-02-28 9:24 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Fam Zheng, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, qemu-block, Juan Quintela,
Dr. David Alan Gilbert, Markus Armbruster, Alex Bennée,
Stefan Hajnoczi, Paolo Bonzini, Philippe Mathieu-Daudé,
Hailiang Zhang
Signed-off-by: Juan Quintela <quintela@redhat.com>
Acked-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
hw/core/qdev-properties.c | 2 +-
migration/Makefile.objs | 1 +
migration/multifd-zlib.c | 325 +++++++++++++++++++++++++++++++++++
migration/multifd.c | 6 +
migration/multifd.h | 4 +
qapi/migration.json | 3 +-
tests/qtest/migration-test.c | 6 +
7 files changed, 345 insertions(+), 2 deletions(-)
create mode 100644 migration/multifd-zlib.c
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index fa7edac020..db2a7abfb2 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -645,7 +645,7 @@ const PropertyInfo qdev_prop_fdc_drive_type = {
const PropertyInfo qdev_prop_multifd_compression = {
.name = "MultiFDCompression",
.description = "multifd_compression values, "
- "none",
+ "none/zlib",
.enum_table = &MultiFDCompression_lookup,
.get = get_enum,
.set = set_enum,
diff --git a/migration/Makefile.objs b/migration/Makefile.objs
index d3623d5f9b..0308caa5c5 100644
--- a/migration/Makefile.objs
+++ b/migration/Makefile.objs
@@ -8,6 +8,7 @@ common-obj-y += xbzrle.o postcopy-ram.o
common-obj-y += qjson.o
common-obj-y += block-dirty-bitmap.o
common-obj-y += multifd.o
+common-obj-y += multifd-zlib.o
common-obj-$(CONFIG_RDMA) += rdma.o
diff --git a/migration/multifd-zlib.c b/migration/multifd-zlib.c
new file mode 100644
index 0000000000..ab4ba75d75
--- /dev/null
+++ b/migration/multifd-zlib.c
@@ -0,0 +1,325 @@
+/*
+ * Multifd zlib compression implementation
+ *
+ * Copyright (c) 2020 Red Hat Inc
+ *
+ * Authors:
+ * Juan Quintela <quintela@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 "qemu/osdep.h"
+#include <zlib.h>
+#include "qemu/rcu.h"
+#include "exec/target_page.h"
+#include "qapi/error.h"
+#include "migration.h"
+#include "trace.h"
+#include "multifd.h"
+
+struct zlib_data {
+ /* stream for compression */
+ z_stream zs;
+ /* compressed buffer */
+ uint8_t *zbuff;
+ /* size of compressed buffer */
+ uint32_t zbuff_len;
+};
+
+/* Multifd zlib compression */
+
+/**
+ * zlib_send_setup: setup send side
+ *
+ * Setup each channel with zlib compression.
+ *
+ * Returns 0 for success or -1 for error
+ *
+ * @p: Params for the channel that we are using
+ * @errp: pointer to an error
+ */
+static int zlib_send_setup(MultiFDSendParams *p, Error **errp)
+{
+ uint32_t page_count = MULTIFD_PACKET_SIZE / qemu_target_page_size();
+ struct zlib_data *z = g_malloc0(sizeof(struct zlib_data));
+ z_stream *zs = &z->zs;
+
+ zs->zalloc = Z_NULL;
+ zs->zfree = Z_NULL;
+ zs->opaque = Z_NULL;
+ if (deflateInit(zs, migrate_multifd_zlib_level()) != Z_OK) {
+ g_free(z);
+ error_setg(errp, "multifd %d: deflate init failed", p->id);
+ return -1;
+ }
+ /* We will never have more than page_count pages */
+ z->zbuff_len = page_count * qemu_target_page_size();
+ z->zbuff_len *= 2;
+ z->zbuff = g_try_malloc(z->zbuff_len);
+ if (!z->zbuff) {
+ deflateEnd(&z->zs);
+ g_free(z);
+ error_setg(errp, "multifd %d: out of memory for zbuff", p->id);
+ return -1;
+ }
+ p->data = z;
+ return 0;
+}
+
+/**
+ * zlib_send_cleanup: cleanup send side
+ *
+ * Close the channel and return memory.
+ *
+ * @p: Params for the channel that we are using
+ */
+static void zlib_send_cleanup(MultiFDSendParams *p, Error **errp)
+{
+ struct zlib_data *z = p->data;
+
+ deflateEnd(&z->zs);
+ g_free(z->zbuff);
+ z->zbuff = NULL;
+ g_free(p->data);
+ p->data = NULL;
+}
+
+/**
+ * zlib_send_prepare: prepare date to be able to send
+ *
+ * Create a compressed buffer with all the pages that we are going to
+ * send.
+ *
+ * Returns 0 for success or -1 for error
+ *
+ * @p: Params for the channel that we are using
+ * @used: number of pages used
+ */
+static int zlib_send_prepare(MultiFDSendParams *p, uint32_t used, Error **errp)
+{
+ struct iovec *iov = p->pages->iov;
+ struct zlib_data *z = p->data;
+ z_stream *zs = &z->zs;
+ uint32_t out_size = 0;
+ int ret;
+ uint32_t i;
+
+ for (i = 0; i < used; i++) {
+ uint32_t available = z->zbuff_len - out_size;
+ int flush = Z_NO_FLUSH;
+
+ if (i == used - 1) {
+ flush = Z_SYNC_FLUSH;
+ }
+
+ zs->avail_in = iov[i].iov_len;
+ zs->next_in = iov[i].iov_base;
+
+ zs->avail_out = available;
+ zs->next_out = z->zbuff + out_size;
+
+ /*
+ * Welcome to deflate semantics
+ *
+ * We need to loop while:
+ * - return is Z_OK
+ * - there are stuff to be compressed
+ * - there are output space free
+ */
+ do {
+ ret = deflate(zs, flush);
+ } while (ret == Z_OK && zs->avail_in && zs->avail_out);
+ if (ret == Z_OK && zs->avail_in) {
+ error_setg(errp, "multifd %d: deflate failed to compress all input",
+ p->id);
+ return -1;
+ }
+ if (ret != Z_OK) {
+ error_setg(errp, "multifd %d: deflate returned %d instead of Z_OK",
+ p->id, ret);
+ return -1;
+ }
+ out_size += available - zs->avail_out;
+ }
+ p->next_packet_size = out_size;
+ p->flags |= MULTIFD_FLAG_ZLIB;
+
+ return 0;
+}
+
+/**
+ * zlib_send_write: do the actual write of the data
+ *
+ * Do the actual write of the comprresed buffer.
+ *
+ * Returns 0 for success or -1 for error
+ *
+ * @p: Params for the channel that we are using
+ * @used: number of pages used
+ * @errp: pointer to an error
+ */
+static int zlib_send_write(MultiFDSendParams *p, uint32_t used, Error **errp)
+{
+ struct zlib_data *z = p->data;
+
+ return qio_channel_write_all(p->c, (void *)z->zbuff, p->next_packet_size,
+ errp);
+}
+
+/**
+ * zlib_recv_setup: setup receive side
+ *
+ * Create the compressed channel and buffer.
+ *
+ * Returns 0 for success or -1 for error
+ *
+ * @p: Params for the channel that we are using
+ * @errp: pointer to an error
+ */
+static int zlib_recv_setup(MultiFDRecvParams *p, Error **errp)
+{
+ uint32_t page_count = MULTIFD_PACKET_SIZE / qemu_target_page_size();
+ struct zlib_data *z = g_malloc0(sizeof(struct zlib_data));
+ z_stream *zs = &z->zs;
+
+ p->data = z;
+ zs->zalloc = Z_NULL;
+ zs->zfree = Z_NULL;
+ zs->opaque = Z_NULL;
+ zs->avail_in = 0;
+ zs->next_in = Z_NULL;
+ if (inflateInit(zs) != Z_OK) {
+ error_setg(errp, "multifd %d: inflate init failed", p->id);
+ return -1;
+ }
+ /* We will never have more than page_count pages */
+ z->zbuff_len = page_count * qemu_target_page_size();
+ /* We know compression "could" use more space */
+ z->zbuff_len *= 2;
+ z->zbuff = g_try_malloc(z->zbuff_len);
+ if (!z->zbuff) {
+ inflateEnd(zs);
+ error_setg(errp, "multifd %d: out of memory for zbuff", p->id);
+ return -1;
+ }
+ return 0;
+}
+
+/**
+ * zlib_recv_cleanup: setup receive side
+ *
+ * For no compression this function does nothing.
+ *
+ * @p: Params for the channel that we are using
+ */
+static void zlib_recv_cleanup(MultiFDRecvParams *p)
+{
+ struct zlib_data *z = p->data;
+
+ inflateEnd(&z->zs);
+ g_free(z->zbuff);
+ z->zbuff = NULL;
+ g_free(p->data);
+ p->data = NULL;
+}
+
+/**
+ * zlib_recv_pages: read the data from the channel into actual pages
+ *
+ * Read the compressed buffer, and uncompress it into the actual
+ * pages.
+ *
+ * Returns 0 for success or -1 for error
+ *
+ * @p: Params for the channel that we are using
+ * @used: number of pages used
+ * @errp: pointer to an error
+ */
+static int zlib_recv_pages(MultiFDRecvParams *p, uint32_t used, Error **errp)
+{
+ struct zlib_data *z = p->data;
+ z_stream *zs = &z->zs;
+ uint32_t in_size = p->next_packet_size;
+ /* we measure the change of total_out */
+ uint32_t out_size = zs->total_out;
+ uint32_t expected_size = used * qemu_target_page_size();
+ uint32_t flags = p->flags & MULTIFD_FLAG_COMPRESSION_MASK;
+ int ret;
+ int i;
+
+ if (flags != MULTIFD_FLAG_ZLIB) {
+ error_setg(errp, "multifd %d: flags received %x flags expected %x",
+ p->id, flags, MULTIFD_FLAG_ZLIB);
+ return -1;
+ }
+ ret = qio_channel_read_all(p->c, (void *)z->zbuff, in_size, errp);
+
+ if (ret != 0) {
+ return ret;
+ }
+
+ zs->avail_in = in_size;
+ zs->next_in = z->zbuff;
+
+ for (i = 0; i < used; i++) {
+ struct iovec *iov = &p->pages->iov[i];
+ int flush = Z_NO_FLUSH;
+ unsigned long start = zs->total_out;
+
+ if (i == used - 1) {
+ flush = Z_SYNC_FLUSH;
+ }
+
+ zs->avail_out = iov->iov_len;
+ zs->next_out = iov->iov_base;
+
+ /*
+ * Welcome to inflate semantics
+ *
+ * We need to loop while:
+ * - return is Z_OK
+ * - there are input available
+ * - we haven't completed a full page
+ */
+ do {
+ ret = inflate(zs, flush);
+ } while (ret == Z_OK && zs->avail_in
+ && (zs->total_out - start) < iov->iov_len);
+ if (ret == Z_OK && (zs->total_out - start) < iov->iov_len) {
+ error_setg(errp, "multifd %d: inflate generated too few output",
+ p->id);
+ return -1;
+ }
+ if (ret != Z_OK) {
+ error_setg(errp, "multifd %d: inflate returned %d instead of Z_OK",
+ p->id, ret);
+ return -1;
+ }
+ }
+ out_size = zs->total_out - out_size;
+ if (out_size != expected_size) {
+ error_setg(errp, "multifd %d: packet size received %d size expected %d",
+ p->id, out_size, expected_size);
+ return -1;
+ }
+ return 0;
+}
+
+static MultiFDMethods multifd_zlib_ops = {
+ .send_setup = zlib_send_setup,
+ .send_cleanup = zlib_send_cleanup,
+ .send_prepare = zlib_send_prepare,
+ .send_write = zlib_send_write,
+ .recv_setup = zlib_recv_setup,
+ .recv_cleanup = zlib_recv_cleanup,
+ .recv_pages = zlib_recv_pages
+};
+
+static void multifd_zlib_register(void)
+{
+ multifd_register_ops(MULTIFD_COMPRESSION_ZLIB, &multifd_zlib_ops);
+}
+
+migration_init(multifd_zlib_register);
diff --git a/migration/multifd.c b/migration/multifd.c
index 97433e5135..cb6a4a3ab8 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -166,6 +166,12 @@ static MultiFDMethods *multifd_ops[MULTIFD_COMPRESSION__MAX] = {
[MULTIFD_COMPRESSION_NONE] = &multifd_nocomp_ops,
};
+void multifd_register_ops(int method, MultiFDMethods *ops)
+{
+ assert(0 < method && method < MULTIFD_COMPRESSION__MAX);
+ multifd_ops[method] = ops;
+}
+
static int multifd_send_initial_packet(MultiFDSendParams *p, Error **errp)
{
MultiFDInit_t msg = {};
diff --git a/migration/multifd.h b/migration/multifd.h
index 54075ffa7d..c6dad7b990 100644
--- a/migration/multifd.h
+++ b/migration/multifd.h
@@ -23,12 +23,14 @@ void multifd_recv_sync_main(void);
void multifd_send_sync_main(QEMUFile *f);
int multifd_queue_page(QEMUFile *f, RAMBlock *block, ram_addr_t offset);
+/* Multifd Compression flags */
#define MULTIFD_FLAG_SYNC (1 << 0)
/* We reserve 3 bits for compression methods */
#define MULTIFD_FLAG_COMPRESSION_MASK (7 << 1)
/* we need to be compatible. Before compression value was 0 */
#define MULTIFD_FLAG_NOCOMP (0 << 1)
+#define MULTIFD_FLAG_ZLIB (1 << 1)
/* This value needs to be a multiple of qemu_target_page_size() */
#define MULTIFD_PACKET_SIZE (512 * 1024)
@@ -161,5 +163,7 @@ typedef struct {
int (*recv_pages)(MultiFDRecvParams *p, uint32_t used, Error **errp);
} MultiFDMethods;
+void multifd_register_ops(int method, MultiFDMethods *ops);
+
#endif
diff --git a/qapi/migration.json b/qapi/migration.json
index 468a9d92a4..3fb9e59bce 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -494,12 +494,13 @@
# An enumeration of multifd compression methods.
#
# @none: no compression.
+# @zlib: use zlib compression method.
#
# Since: 5.0
#
##
{ 'enum': 'MultiFDCompression',
- 'data': [ 'none' ] }
+ 'data': [ 'none', 'zlib' ] }
##
# @MigrationParameter:
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index f84c157285..90c26e879f 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -1323,6 +1323,11 @@ static void test_multifd_tcp_none(void)
test_multifd_tcp("none");
}
+static void test_multifd_tcp_zlib(void)
+{
+ test_multifd_tcp("zlib");
+}
+
/*
* This test does:
* source target
@@ -1486,6 +1491,7 @@ int main(int argc, char **argv)
qtest_add_func("/migration/auto_converge", test_migrate_auto_converge);
qtest_add_func("/migration/multifd/tcp/none", test_multifd_tcp_none);
qtest_add_func("/migration/multifd/tcp/cancel", test_multifd_tcp_cancel);
+ qtest_add_func("/migration/multifd/tcp/zlib", test_multifd_tcp_zlib);
ret = g_test_run();
--
2.24.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PULL 06/15] configure: Enable test and libs for zstd
2020-02-28 9:24 [PULL 00/15] Pull migration patches Juan Quintela
` (4 preceding siblings ...)
2020-02-28 9:24 ` [PULL 05/15] multifd: Add zlib compression multifd support Juan Quintela
@ 2020-02-28 9:24 ` Juan Quintela
2020-02-29 20:06 ` Richard Henderson
2020-03-17 17:09 ` Peter Maydell
2020-02-28 9:24 ` [PULL 07/15] multifd: Add multifd-zstd-level parameter Juan Quintela
` (9 subsequent siblings)
15 siblings, 2 replies; 27+ messages in thread
From: Juan Quintela @ 2020-02-28 9:24 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Fam Zheng, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, qemu-block, Juan Quintela,
Dr. David Alan Gilbert, Markus Armbruster, Alex Bennée,
Stefan Hajnoczi, Paolo Bonzini, Philippe Mathieu-Daudé,
Hailiang Zhang
Add it to several build systems to make testing good.
Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
.gitlab-ci.yml | 1 +
.travis.yml | 1 +
configure | 30 +++++++++++++++++++
tests/docker/dockerfiles/centos7.docker | 3 +-
.../dockerfiles/fedora-i386-cross.docker | 3 +-
tests/docker/dockerfiles/fedora.docker | 3 +-
tests/docker/dockerfiles/ubuntu.docker | 1 +
tests/docker/dockerfiles/ubuntu1804.docker | 1 +
tests/vm/fedora | 5 +++-
tests/vm/freebsd | 3 ++
tests/vm/netbsd | 3 ++
tests/vm/openbsd | 3 ++
12 files changed, 53 insertions(+), 4 deletions(-)
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index c15e394f09..72f8b8aa51 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -21,6 +21,7 @@ build-system2:
script:
- apt-get install -y -qq libsdl2-dev libgcrypt-dev libbrlapi-dev libaio-dev
libfdt-dev liblzo2-dev librdmacm-dev libibverbs-dev libibumad-dev
+ libzstd-dev
- mkdir build
- cd build
- ../configure --enable-werror --target-list="tricore-softmmu unicore32-softmmu
diff --git a/.travis.yml b/.travis.yml
index f4020dcc6c..9867272177 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -49,6 +49,7 @@ addons:
- libusb-1.0-0-dev
- libvdeplug-dev
- libvte-2.91-dev
+ - libzstd-dev
- sparse
- uuid-dev
- gcovr
diff --git a/configure b/configure
index 48d6f89d57..7b373bc0bb 100755
--- a/configure
+++ b/configure
@@ -449,6 +449,7 @@ lzo=""
snappy=""
bzip2=""
lzfse=""
+zstd=""
guest_agent=""
guest_agent_with_vss="no"
guest_agent_ntddscsi="no"
@@ -1371,6 +1372,10 @@ for opt do
;;
--disable-lzfse) lzfse="no"
;;
+ --disable-zstd) zstd="no"
+ ;;
+ --enable-zstd) zstd="yes"
+ ;;
--enable-guest-agent) guest_agent="yes"
;;
--disable-guest-agent) guest_agent="no"
@@ -1829,6 +1834,8 @@ disabled with --disable-FEATURE, default is enabled if available:
(for reading bzip2-compressed dmg images)
lzfse support of lzfse compression library
(for reading lzfse-compressed dmg images)
+ zstd support for zstd compression library
+ (for migration compression)
seccomp seccomp support
coroutine-pool coroutine freelist (better performance)
glusterfs GlusterFS backend
@@ -2453,6 +2460,24 @@ EOF
fi
fi
+##########################################
+# zstd check
+
+if test "$zstd" != "no" ; then
+ if $pkg_config --exist libzstd ; then
+ zstd_cflags="$($pkg_config --cflags libzstd)"
+ zstd_libs="$($pkg_config --libs libzstd)"
+ LIBS="$zstd_libs $LIBS"
+ QEMU_CFLAGS="$QEMU_CFLAGS $zstd_cflags"
+ zstd="yes"
+ else
+ if test "$zstd" = "yes" ; then
+ feature_not_found "libzstd" "Install libzstd devel"
+ fi
+ zstd="no"
+ fi
+fi
+
##########################################
# libseccomp check
@@ -6668,6 +6693,7 @@ echo "lzo support $lzo"
echo "snappy support $snappy"
echo "bzip2 support $bzip2"
echo "lzfse support $lzfse"
+echo "zstd support $zstd"
echo "NUMA host support $numa"
echo "libxml2 $libxml2"
echo "tcmalloc support $tcmalloc"
@@ -7242,6 +7268,10 @@ if test "$lzfse" = "yes" ; then
echo "LZFSE_LIBS=-llzfse" >> $config_host_mak
fi
+if test "$zstd" = "yes" ; then
+ echo "CONFIG_ZSTD=y" >> $config_host_mak
+fi
+
if test "$libiscsi" = "yes" ; then
echo "CONFIG_LIBISCSI=m" >> $config_host_mak
echo "LIBISCSI_CFLAGS=$libiscsi_cflags" >> $config_host_mak
diff --git a/tests/docker/dockerfiles/centos7.docker b/tests/docker/dockerfiles/centos7.docker
index 562d65be9e..cdd72de7eb 100644
--- a/tests/docker/dockerfiles/centos7.docker
+++ b/tests/docker/dockerfiles/centos7.docker
@@ -33,6 +33,7 @@ ENV PACKAGES \
tar \
vte-devel \
xen-devel \
- zlib-devel
+ zlib-devel \
+ libzstd-devel
RUN yum install -y $PACKAGES
RUN rpm -q $PACKAGES | sort > /packages.txt
diff --git a/tests/docker/dockerfiles/fedora-i386-cross.docker b/tests/docker/dockerfiles/fedora-i386-cross.docker
index 9106cf9ebe..cd16cd1bfa 100644
--- a/tests/docker/dockerfiles/fedora-i386-cross.docker
+++ b/tests/docker/dockerfiles/fedora-i386-cross.docker
@@ -7,7 +7,8 @@ ENV PACKAGES \
gnutls-devel.i686 \
nettle-devel.i686 \
pixman-devel.i686 \
- zlib-devel.i686
+ zlib-devel.i686 \
+ libzstd-devel.i686
RUN dnf install -y $PACKAGES
RUN rpm -q $PACKAGES | sort > /packages.txt
diff --git a/tests/docker/dockerfiles/fedora.docker b/tests/docker/dockerfiles/fedora.docker
index 987a3c170a..a6522228c0 100644
--- a/tests/docker/dockerfiles/fedora.docker
+++ b/tests/docker/dockerfiles/fedora.docker
@@ -92,7 +92,8 @@ ENV PACKAGES \
vte291-devel \
which \
xen-devel \
- zlib-devel
+ zlib-devel \
+ libzstd-devel
ENV QEMU_CONFIGURE_OPTS --python=/usr/bin/python3
RUN dnf install -y $PACKAGES
diff --git a/tests/docker/dockerfiles/ubuntu.docker b/tests/docker/dockerfiles/ubuntu.docker
index 4177f33691..b6c7b41ddd 100644
--- a/tests/docker/dockerfiles/ubuntu.docker
+++ b/tests/docker/dockerfiles/ubuntu.docker
@@ -58,6 +58,7 @@ ENV PACKAGES flex bison \
libvdeplug-dev \
libvte-2.91-dev \
libxen-dev \
+ libzstd-dev \
make \
python3-yaml \
python3-sphinx \
diff --git a/tests/docker/dockerfiles/ubuntu1804.docker b/tests/docker/dockerfiles/ubuntu1804.docker
index 0766f94cf4..1efedeef99 100644
--- a/tests/docker/dockerfiles/ubuntu1804.docker
+++ b/tests/docker/dockerfiles/ubuntu1804.docker
@@ -44,6 +44,7 @@ ENV PACKAGES flex bison \
libvdeplug-dev \
libvte-2.91-dev \
libxen-dev \
+ libzstd-dev \
make \
python3-yaml \
python3-sphinx \
diff --git a/tests/vm/fedora b/tests/vm/fedora
index 4d7d6049f4..4843b4175e 100755
--- a/tests/vm/fedora
+++ b/tests/vm/fedora
@@ -53,7 +53,10 @@ class FedoraVM(basevm.BaseVM):
# libs: audio
'"pkgconfig(libpulse)"',
'"pkgconfig(alsa)"',
- ]
+
+ # libs: migration
+ '"pkgconfig(libzstd)"',
+]
BUILD_SCRIPT = """
set -e;
diff --git a/tests/vm/freebsd b/tests/vm/freebsd
index fb54334696..86770878b6 100755
--- a/tests/vm/freebsd
+++ b/tests/vm/freebsd
@@ -55,6 +55,9 @@ class FreeBSDVM(basevm.BaseVM):
# libs: opengl
"libepoxy",
"mesa-libs",
+
+ # libs: migration
+ "zstd",
]
BUILD_SCRIPT = """
diff --git a/tests/vm/netbsd b/tests/vm/netbsd
index c5069a45f4..55590f4601 100755
--- a/tests/vm/netbsd
+++ b/tests/vm/netbsd
@@ -49,6 +49,9 @@ class NetBSDVM(basevm.BaseVM):
"SDL2",
"gtk3+",
"libxkbcommon",
+
+ # libs: migration
+ "zstd",
]
BUILD_SCRIPT = """
diff --git a/tests/vm/openbsd b/tests/vm/openbsd
index 22cd9513dd..ab6abbedab 100755
--- a/tests/vm/openbsd
+++ b/tests/vm/openbsd
@@ -51,6 +51,9 @@ class OpenBSDVM(basevm.BaseVM):
"sdl2",
"gtk+3",
"libxkbcommon",
+
+ # libs: migration
+ "zstd",
]
BUILD_SCRIPT = """
--
2.24.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PULL 07/15] multifd: Add multifd-zstd-level parameter
2020-02-28 9:24 [PULL 00/15] Pull migration patches Juan Quintela
` (5 preceding siblings ...)
2020-02-28 9:24 ` [PULL 06/15] configure: Enable test and libs for zstd Juan Quintela
@ 2020-02-28 9:24 ` Juan Quintela
2020-02-28 9:24 ` [PULL 08/15] multifd: Add zstd compression multifd support Juan Quintela
` (8 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: Juan Quintela @ 2020-02-28 9:24 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Fam Zheng, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, qemu-block, Juan Quintela,
Dr. David Alan Gilbert, Peter Xu, Markus Armbruster,
Alex Bennée, Stefan Hajnoczi, Paolo Bonzini,
Philippe Mathieu-Daudé,
Hailiang Zhang
This parameter specifies the zstd compression level. The next patch
will put it to use.
Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Acked-by: Markus Armbruster <armbru@redhat.com>
---
migration/migration.c | 24 ++++++++++++++++++++++++
migration/migration.h | 1 +
monitor/hmp-cmds.c | 4 ++++
qapi/migration.json | 29 ++++++++++++++++++++++++++---
4 files changed, 55 insertions(+), 3 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index a09726f679..c1814a6861 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -91,6 +91,8 @@
#define DEFAULT_MIGRATE_MULTIFD_COMPRESSION MULTIFD_COMPRESSION_NONE
/* 0: means nocompress, 1: best speed, ... 9: best compress ratio */
#define DEFAULT_MIGRATE_MULTIFD_ZLIB_LEVEL 1
+/* 0: means nocompress, 1: best speed, ... 20: best compress ratio */
+#define DEFAULT_MIGRATE_MULTIFD_ZSTD_LEVEL 1
/* Background transfer rate for postcopy, 0 means unlimited, note
* that page requests can still exceed this limit.
@@ -805,6 +807,8 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
params->multifd_compression = s->parameters.multifd_compression;
params->has_multifd_zlib_level = true;
params->multifd_zlib_level = s->parameters.multifd_zlib_level;
+ params->has_multifd_zstd_level = true;
+ params->multifd_zstd_level = s->parameters.multifd_zstd_level;
params->has_xbzrle_cache_size = true;
params->xbzrle_cache_size = s->parameters.xbzrle_cache_size;
params->has_max_postcopy_bandwidth = true;
@@ -1219,6 +1223,13 @@ static bool migrate_params_check(MigrationParameters *params, Error **errp)
return false;
}
+ if (params->has_multifd_zstd_level &&
+ (params->multifd_zstd_level > 20)) {
+ error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "multifd_zstd_level",
+ "is invalid, it should be in the range of 0 to 20");
+ return false;
+ }
+
if (params->has_xbzrle_cache_size &&
(params->xbzrle_cache_size < qemu_target_page_size() ||
!is_power_of_2(params->xbzrle_cache_size))) {
@@ -2274,6 +2285,15 @@ int migrate_multifd_zlib_level(void)
return s->parameters.multifd_zlib_level;
}
+int migrate_multifd_zstd_level(void)
+{
+ MigrationState *s;
+
+ s = migrate_get_current();
+
+ return s->parameters.multifd_zstd_level;
+}
+
int migrate_use_xbzrle(void)
{
MigrationState *s;
@@ -3567,6 +3587,9 @@ static Property migration_properties[] = {
DEFINE_PROP_UINT8("multifd-zlib-level", MigrationState,
parameters.multifd_zlib_level,
DEFAULT_MIGRATE_MULTIFD_ZLIB_LEVEL),
+ DEFINE_PROP_UINT8("multifd-zstd-level", MigrationState,
+ parameters.multifd_zstd_level,
+ DEFAULT_MIGRATE_MULTIFD_ZSTD_LEVEL),
DEFINE_PROP_SIZE("xbzrle-cache-size", MigrationState,
parameters.xbzrle_cache_size,
DEFAULT_MIGRATE_XBZRLE_CACHE_SIZE),
@@ -3659,6 +3682,7 @@ static void migration_instance_init(Object *obj)
params->has_multifd_channels = true;
params->has_multifd_compression = true;
params->has_multifd_zlib_level = true;
+ params->has_multifd_zstd_level = true;
params->has_xbzrle_cache_size = true;
params->has_max_postcopy_bandwidth = true;
params->has_max_cpu_throttle = true;
diff --git a/migration/migration.h b/migration/migration.h
index c363ef9334..507284e563 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -302,6 +302,7 @@ bool migrate_pause_before_switchover(void);
int migrate_multifd_channels(void);
MultiFDCompression migrate_multifd_compression(void);
int migrate_multifd_zlib_level(void);
+int migrate_multifd_zstd_level(void);
int migrate_use_xbzrle(void);
int64_t migrate_xbzrle_cache_size(void);
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 409ebb2b03..30313858c2 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1841,6 +1841,10 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
p->has_multifd_zlib_level = true;
visit_type_int(v, param, &p->multifd_zlib_level, &err);
break;
+ case MIGRATION_PARAMETER_MULTIFD_ZSTD_LEVEL:
+ p->has_multifd_zstd_level = true;
+ visit_type_int(v, param, &p->multifd_zstd_level, &err);
+ break;
case MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE:
p->has_xbzrle_cache_size = true;
visit_type_size(v, param, &cache_size, &err);
diff --git a/qapi/migration.json b/qapi/migration.json
index 3fb9e59bce..f2da5cfa0f 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -610,6 +610,13 @@
# will consume more CPU.
# Defaults to 1. (Since 5.0)
#
+# @multifd-zstd-level: Set the compression level to be used in live
+# migration, the compression level is an integer between 0
+# and 20, where 0 means no compression, 1 means the best
+# compression speed, and 20 means best compression ratio which
+# will consume more CPU.
+# Defaults to 1. (Since 5.0)
+#
# Since: 2.4
##
{ 'enum': 'MigrationParameter',
@@ -623,7 +630,7 @@
'multifd-channels',
'xbzrle-cache-size', 'max-postcopy-bandwidth',
'max-cpu-throttle', 'multifd-compression',
- 'multifd-zlib-level' ] }
+ 'multifd-zlib-level' ,'multifd-zstd-level' ] }
##
# @MigrateSetParameters:
@@ -723,6 +730,13 @@
# will consume more CPU.
# Defaults to 1. (Since 5.0)
#
+# @multifd-zstd-level: Set the compression level to be used in live
+# migration, the compression level is an integer between 0
+# and 20, where 0 means no compression, 1 means the best
+# compression speed, and 20 means best compression ratio which
+# will consume more CPU.
+# Defaults to 1. (Since 5.0)
+#
# Since: 2.4
##
# TODO either fuse back into MigrationParameters, or make
@@ -750,7 +764,8 @@
'*max-postcopy-bandwidth': 'size',
'*max-cpu-throttle': 'int',
'*multifd-compression': 'MultiFDCompression',
- '*multifd-zlib-level': 'int' } }
+ '*multifd-zlib-level': 'int',
+ '*multifd-zstd-level': 'int' } }
##
# @migrate-set-parameters:
@@ -870,6 +885,13 @@
# will consume more CPU.
# Defaults to 1. (Since 5.0)
#
+# @multifd-zstd-level: Set the compression level to be used in live
+# migration, the compression level is an integer between 0
+# and 20, where 0 means no compression, 1 means the best
+# compression speed, and 20 means best compression ratio which
+# will consume more CPU.
+# Defaults to 1. (Since 5.0)
+#
# Since: 2.4
##
{ 'struct': 'MigrationParameters',
@@ -895,7 +917,8 @@
'*max-postcopy-bandwidth': 'size',
'*max-cpu-throttle': 'uint8',
'*multifd-compression': 'MultiFDCompression',
- '*multifd-zlib-level': 'uint8' } }
+ '*multifd-zlib-level': 'uint8',
+ '*multifd-zstd-level': 'uint8' } }
##
# @query-migrate-parameters:
--
2.24.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PULL 08/15] multifd: Add zstd compression multifd support
2020-02-28 9:24 [PULL 00/15] Pull migration patches Juan Quintela
` (6 preceding siblings ...)
2020-02-28 9:24 ` [PULL 07/15] multifd: Add multifd-zstd-level parameter Juan Quintela
@ 2020-02-28 9:24 ` Juan Quintela
2020-02-28 9:24 ` [PULL 09/15] migration/vmstate: Remove redundant statement in vmstate_save_state_v() Juan Quintela
` (7 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: Juan Quintela @ 2020-02-28 9:24 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Fam Zheng, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, qemu-block, Juan Quintela,
Dr. David Alan Gilbert, Markus Armbruster, Alex Bennée,
Stefan Hajnoczi, Paolo Bonzini, Philippe Mathieu-Daudé,
Hailiang Zhang
Signed-off-by: Juan Quintela <quintela@redhat.com>
Acked-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
hw/core/qdev-properties.c | 2 +-
migration/Makefile.objs | 1 +
migration/multifd-zstd.c | 339 +++++++++++++++++++++++++++++++++++
migration/multifd.h | 1 +
migration/ram.c | 1 -
qapi/migration.json | 4 +-
tests/qtest/migration-test.c | 10 ++
7 files changed, 355 insertions(+), 3 deletions(-)
create mode 100644 migration/multifd-zstd.c
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index db2a7abfb2..2047114fca 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -645,7 +645,7 @@ const PropertyInfo qdev_prop_fdc_drive_type = {
const PropertyInfo qdev_prop_multifd_compression = {
.name = "MultiFDCompression",
.description = "multifd_compression values, "
- "none/zlib",
+ "none/zlib/zstd",
.enum_table = &MultiFDCompression_lookup,
.get = get_enum,
.set = set_enum,
diff --git a/migration/Makefile.objs b/migration/Makefile.objs
index 0308caa5c5..0fc619e380 100644
--- a/migration/Makefile.objs
+++ b/migration/Makefile.objs
@@ -9,6 +9,7 @@ common-obj-y += qjson.o
common-obj-y += block-dirty-bitmap.o
common-obj-y += multifd.o
common-obj-y += multifd-zlib.o
+common-obj-$(CONFIG_ZSTD) += multifd-zstd.o
common-obj-$(CONFIG_RDMA) += rdma.o
diff --git a/migration/multifd-zstd.c b/migration/multifd-zstd.c
new file mode 100644
index 0000000000..693bddf8c9
--- /dev/null
+++ b/migration/multifd-zstd.c
@@ -0,0 +1,339 @@
+/*
+ * Multifd zlib compression implementation
+ *
+ * Copyright (c) 2020 Red Hat Inc
+ *
+ * Authors:
+ * Juan Quintela <quintela@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 "qemu/osdep.h"
+#include <zstd.h>
+#include "qemu/rcu.h"
+#include "exec/target_page.h"
+#include "qapi/error.h"
+#include "migration.h"
+#include "trace.h"
+#include "multifd.h"
+
+struct zstd_data {
+ /* stream for compression */
+ ZSTD_CStream *zcs;
+ /* stream for decompression */
+ ZSTD_DStream *zds;
+ /* buffers */
+ ZSTD_inBuffer in;
+ ZSTD_outBuffer out;
+ /* compressed buffer */
+ uint8_t *zbuff;
+ /* size of compressed buffer */
+ uint32_t zbuff_len;
+};
+
+/* Multifd zstd compression */
+
+/**
+ * zstd_send_setup: setup send side
+ *
+ * Setup each channel with zstd compression.
+ *
+ * Returns 0 for success or -1 for error
+ *
+ * @p: Params for the channel that we are using
+ * @errp: pointer to an error
+ */
+static int zstd_send_setup(MultiFDSendParams *p, Error **errp)
+{
+ uint32_t page_count = MULTIFD_PACKET_SIZE / qemu_target_page_size();
+ struct zstd_data *z = g_new0(struct zstd_data, 1);
+ int res;
+
+ p->data = z;
+ z->zcs = ZSTD_createCStream();
+ if (!z->zcs) {
+ g_free(z);
+ error_setg(errp, "multifd %d: zstd createCStream failed", p->id);
+ return -1;
+ }
+
+ res = ZSTD_initCStream(z->zcs, migrate_multifd_zstd_level());
+ if (ZSTD_isError(res)) {
+ ZSTD_freeCStream(z->zcs);
+ g_free(z);
+ error_setg(errp, "multifd %d: initCStream failed with error %s",
+ p->id, ZSTD_getErrorName(res));
+ return -1;
+ }
+ /* We will never have more than page_count pages */
+ z->zbuff_len = page_count * qemu_target_page_size();
+ z->zbuff_len *= 2;
+ z->zbuff = g_try_malloc(z->zbuff_len);
+ if (!z->zbuff) {
+ ZSTD_freeCStream(z->zcs);
+ g_free(z);
+ error_setg(errp, "multifd %d: out of memory for zbuff", p->id);
+ return -1;
+ }
+ return 0;
+}
+
+/**
+ * zstd_send_cleanup: cleanup send side
+ *
+ * Close the channel and return memory.
+ *
+ * @p: Params for the channel that we are using
+ */
+static void zstd_send_cleanup(MultiFDSendParams *p, Error **errp)
+{
+ struct zstd_data *z = p->data;
+
+ ZSTD_freeCStream(z->zcs);
+ z->zcs = NULL;
+ g_free(z->zbuff);
+ z->zbuff = NULL;
+ g_free(p->data);
+ p->data = NULL;
+}
+
+/**
+ * zstd_send_prepare: prepare date to be able to send
+ *
+ * Create a compressed buffer with all the pages that we are going to
+ * send.
+ *
+ * Returns 0 for success or -1 for error
+ *
+ * @p: Params for the channel that we are using
+ * @used: number of pages used
+ */
+static int zstd_send_prepare(MultiFDSendParams *p, uint32_t used, Error **errp)
+{
+ struct iovec *iov = p->pages->iov;
+ struct zstd_data *z = p->data;
+ int ret;
+ uint32_t i;
+
+ z->out.dst = z->zbuff;
+ z->out.size = z->zbuff_len;
+ z->out.pos = 0;
+
+ for (i = 0; i < used; i++) {
+ ZSTD_EndDirective flush = ZSTD_e_continue;
+
+ if (i == used - 1) {
+ flush = ZSTD_e_flush;
+ }
+ z->in.src = iov[i].iov_base;
+ z->in.size = iov[i].iov_len;
+ z->in.pos = 0;
+
+ /*
+ * Welcome to compressStream2 semantics
+ *
+ * We need to loop while:
+ * - return is > 0
+ * - there is input available
+ * - there is output space free
+ */
+ do {
+ ret = ZSTD_compressStream2(z->zcs, &z->out, &z->in, flush);
+ } while (ret > 0 && (z->in.size - z->in.pos > 0)
+ && (z->out.size - z->out.pos > 0));
+ if (ret > 0 && (z->in.size - z->in.pos > 0)) {
+ error_setg(errp, "multifd %d: compressStream buffer too small",
+ p->id);
+ return -1;
+ }
+ if (ZSTD_isError(ret)) {
+ error_setg(errp, "multifd %d: compressStream error %s",
+ p->id, ZSTD_getErrorName(ret));
+ return -1;
+ }
+ }
+ p->next_packet_size = z->out.pos;
+ p->flags |= MULTIFD_FLAG_ZSTD;
+
+ return 0;
+}
+
+/**
+ * zstd_send_write: do the actual write of the data
+ *
+ * Do the actual write of the comprresed buffer.
+ *
+ * Returns 0 for success or -1 for error
+ *
+ * @p: Params for the channel that we are using
+ * @used: number of pages used
+ * @errp: pointer to an error
+ */
+static int zstd_send_write(MultiFDSendParams *p, uint32_t used, Error **errp)
+{
+ struct zstd_data *z = p->data;
+
+ return qio_channel_write_all(p->c, (void *)z->zbuff, p->next_packet_size,
+ errp);
+}
+
+/**
+ * zstd_recv_setup: setup receive side
+ *
+ * Create the compressed channel and buffer.
+ *
+ * Returns 0 for success or -1 for error
+ *
+ * @p: Params for the channel that we are using
+ * @errp: pointer to an error
+ */
+static int zstd_recv_setup(MultiFDRecvParams *p, Error **errp)
+{
+ uint32_t page_count = MULTIFD_PACKET_SIZE / qemu_target_page_size();
+ struct zstd_data *z = g_new0(struct zstd_data, 1);
+ int ret;
+
+ p->data = z;
+ z->zds = ZSTD_createDStream();
+ if (!z->zds) {
+ g_free(z);
+ error_setg(errp, "multifd %d: zstd createDStream failed", p->id);
+ return -1;
+ }
+
+ ret = ZSTD_initDStream(z->zds);
+ if (ZSTD_isError(ret)) {
+ ZSTD_freeDStream(z->zds);
+ g_free(z);
+ error_setg(errp, "multifd %d: initDStream failed with error %s",
+ p->id, ZSTD_getErrorName(ret));
+ return -1;
+ }
+
+ /* We will never have more than page_count pages */
+ z->zbuff_len = page_count * qemu_target_page_size();
+ /* We know compression "could" use more space */
+ z->zbuff_len *= 2;
+ z->zbuff = g_try_malloc(z->zbuff_len);
+ if (!z->zbuff) {
+ ZSTD_freeDStream(z->zds);
+ g_free(z);
+ error_setg(errp, "multifd %d: out of memory for zbuff", p->id);
+ return -1;
+ }
+ return 0;
+}
+
+/**
+ * zstd_recv_cleanup: setup receive side
+ *
+ * For no compression this function does nothing.
+ *
+ * @p: Params for the channel that we are using
+ */
+static void zstd_recv_cleanup(MultiFDRecvParams *p)
+{
+ struct zstd_data *z = p->data;
+
+ ZSTD_freeDStream(z->zds);
+ z->zds = NULL;
+ g_free(z->zbuff);
+ z->zbuff = NULL;
+ g_free(p->data);
+ p->data = NULL;
+}
+
+/**
+ * zstd_recv_pages: read the data from the channel into actual pages
+ *
+ * Read the compressed buffer, and uncompress it into the actual
+ * pages.
+ *
+ * Returns 0 for success or -1 for error
+ *
+ * @p: Params for the channel that we are using
+ * @used: number of pages used
+ * @errp: pointer to an error
+ */
+static int zstd_recv_pages(MultiFDRecvParams *p, uint32_t used, Error **errp)
+{
+ uint32_t in_size = p->next_packet_size;
+ uint32_t out_size = 0;
+ uint32_t expected_size = used * qemu_target_page_size();
+ uint32_t flags = p->flags & MULTIFD_FLAG_COMPRESSION_MASK;
+ struct zstd_data *z = p->data;
+ int ret;
+ int i;
+
+ if (flags != MULTIFD_FLAG_ZSTD) {
+ error_setg(errp, "multifd %d: flags received %x flags expected %x",
+ p->id, flags, MULTIFD_FLAG_ZSTD);
+ return -1;
+ }
+ ret = qio_channel_read_all(p->c, (void *)z->zbuff, in_size, errp);
+
+ if (ret != 0) {
+ return ret;
+ }
+
+ z->in.src = z->zbuff;
+ z->in.size = in_size;
+ z->in.pos = 0;
+
+ for (i = 0; i < used; i++) {
+ struct iovec *iov = &p->pages->iov[i];
+
+ z->out.dst = iov->iov_base;
+ z->out.size = iov->iov_len;
+ z->out.pos = 0;
+
+ /*
+ * Welcome to decompressStream semantics
+ *
+ * We need to loop while:
+ * - return is > 0
+ * - there is input available
+ * - we haven't put out a full page
+ */
+ do {
+ ret = ZSTD_decompressStream(z->zds, &z->out, &z->in);
+ } while (ret > 0 && (z->in.size - z->in.pos > 0)
+ && (z->out.pos < iov->iov_len));
+ if (ret > 0 && (z->out.pos < iov->iov_len)) {
+ error_setg(errp, "multifd %d: decompressStream buffer too small",
+ p->id);
+ return -1;
+ }
+ if (ZSTD_isError(ret)) {
+ error_setg(errp, "multifd %d: decompressStream returned %s",
+ p->id, ZSTD_getErrorName(ret));
+ return ret;
+ }
+ out_size += z->out.pos;
+ }
+ if (out_size != expected_size) {
+ error_setg(errp, "multifd %d: packet size received %d size expected %d",
+ p->id, out_size, expected_size);
+ return -1;
+ }
+ return 0;
+}
+
+static MultiFDMethods multifd_zstd_ops = {
+ .send_setup = zstd_send_setup,
+ .send_cleanup = zstd_send_cleanup,
+ .send_prepare = zstd_send_prepare,
+ .send_write = zstd_send_write,
+ .recv_setup = zstd_recv_setup,
+ .recv_cleanup = zstd_recv_cleanup,
+ .recv_pages = zstd_recv_pages
+};
+
+static void multifd_zstd_register(void)
+{
+ multifd_register_ops(MULTIFD_COMPRESSION_ZSTD, &multifd_zstd_ops);
+}
+
+migration_init(multifd_zstd_register);
diff --git a/migration/multifd.h b/migration/multifd.h
index c6dad7b990..448a03d89a 100644
--- a/migration/multifd.h
+++ b/migration/multifd.h
@@ -31,6 +31,7 @@ int multifd_queue_page(QEMUFile *f, RAMBlock *block, ram_addr_t offset);
/* we need to be compatible. Before compression value was 0 */
#define MULTIFD_FLAG_NOCOMP (0 << 1)
#define MULTIFD_FLAG_ZLIB (1 << 1)
+#define MULTIFD_FLAG_ZSTD (2 << 1)
/* This value needs to be a multiple of qemu_target_page_size() */
#define MULTIFD_PACKET_SIZE (512 * 1024)
diff --git a/migration/ram.c b/migration/ram.c
index 73a141bb60..0ef68798d2 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -28,7 +28,6 @@
#include "qemu/osdep.h"
#include "cpu.h"
-#include <zlib.h>
#include "qemu/cutils.h"
#include "qemu/bitops.h"
#include "qemu/bitmap.h"
diff --git a/qapi/migration.json b/qapi/migration.json
index f2da5cfa0f..d44d99cd78 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -495,12 +495,14 @@
#
# @none: no compression.
# @zlib: use zlib compression method.
+# @zstd: use zstd compression method.
#
# Since: 5.0
#
##
{ 'enum': 'MultiFDCompression',
- 'data': [ 'none', 'zlib' ] }
+ 'data': [ 'none', 'zlib',
+ { 'name': 'zstd', 'if': 'defined(CONFIG_ZSTD)' } ] }
##
# @MigrationParameter:
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 90c26e879f..3d6cc83b88 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -1328,6 +1328,13 @@ static void test_multifd_tcp_zlib(void)
test_multifd_tcp("zlib");
}
+#ifdef CONFIG_ZSTD
+static void test_multifd_tcp_zstd(void)
+{
+ test_multifd_tcp("zstd");
+}
+#endif
+
/*
* This test does:
* source target
@@ -1492,6 +1499,9 @@ int main(int argc, char **argv)
qtest_add_func("/migration/multifd/tcp/none", test_multifd_tcp_none);
qtest_add_func("/migration/multifd/tcp/cancel", test_multifd_tcp_cancel);
qtest_add_func("/migration/multifd/tcp/zlib", test_multifd_tcp_zlib);
+#ifdef CONFIG_ZSTD
+ qtest_add_func("/migration/multifd/tcp/zstd", test_multifd_tcp_zstd);
+#endif
ret = g_test_run();
--
2.24.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PULL 09/15] migration/vmstate: Remove redundant statement in vmstate_save_state_v()
2020-02-28 9:24 [PULL 00/15] Pull migration patches Juan Quintela
` (7 preceding siblings ...)
2020-02-28 9:24 ` [PULL 08/15] multifd: Add zstd compression multifd support Juan Quintela
@ 2020-02-28 9:24 ` Juan Quintela
2020-02-28 9:24 ` [PULL 10/15] test-vmstate: Fix memleaks in test_load_qlist Juan Quintela
` (6 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: Juan Quintela @ 2020-02-28 9:24 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Fam Zheng, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, qemu-block, Juan Quintela, Euler Robot,
Dr. David Alan Gilbert, Markus Armbruster, Alex Bennée,
Stefan Hajnoczi, Chen Qun, Paolo Bonzini,
Philippe Mathieu-Daudé,
Hailiang Zhang
From: Chen Qun <kuhn.chenqun@huawei.com>
The "ret" has been assigned in all branches. It didn't need to be
assigned separately.
Clang static code analyzer show warning:
migration/vmstate.c:365:17: warning: Value stored to 'ret' is never read
ret = 0;
^ ~
Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration/vmstate.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/migration/vmstate.c b/migration/vmstate.c
index 7dd8ef66c6..bafa890384 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -362,7 +362,6 @@ int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
}
for (i = 0; i < n_elems; i++) {
void *curr_elem = first_elem + size * i;
- ret = 0;
vmsd_desc_field_start(vmsd, vmdesc_loop, field, i, n_elems);
old_offset = qemu_ftell_fast(f);
--
2.24.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PULL 10/15] test-vmstate: Fix memleaks in test_load_qlist
2020-02-28 9:24 [PULL 00/15] Pull migration patches Juan Quintela
` (8 preceding siblings ...)
2020-02-28 9:24 ` [PULL 09/15] migration/vmstate: Remove redundant statement in vmstate_save_state_v() Juan Quintela
@ 2020-02-28 9:24 ` Juan Quintela
2020-02-28 9:24 ` [PULL 11/15] migration/savevm: release gslist after dump_vmstate_json Juan Quintela
` (5 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: Juan Quintela @ 2020-02-28 9:24 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Fam Zheng, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, qemu-block, Juan Quintela, Euler Robot,
Dr. David Alan Gilbert, Markus Armbruster, Alex Bennée,
Stefan Hajnoczi, Chen Qun, Paolo Bonzini,
Philippe Mathieu-Daudé,
Hailiang Zhang
From: Chen Qun <kuhn.chenqun@huawei.com>
There is memleak in test_load_qlist().It's not a big deal,
but test-vmstate will fail if sanitizers is enabled.
In addition, "ret" is written twice with the same value
in test_gtree_load_iommu().
Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Laurent Vivier <lvivier@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
tests/test-vmstate.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/tests/test-vmstate.c b/tests/test-vmstate.c
index cea363dd69..f7b3868881 100644
--- a/tests/test-vmstate.c
+++ b/tests/test-vmstate.c
@@ -1241,7 +1241,6 @@ static void test_gtree_load_iommu(void)
TestGTreeIOMMU *orig_iommu = create_iommu();
QEMUFile *fsave, *fload;
char eof;
- int ret;
fsave = open_test_file(true);
qemu_put_buffer(fsave, iommu_dump, sizeof(iommu_dump));
@@ -1250,10 +1249,8 @@ static void test_gtree_load_iommu(void)
fload = open_test_file(false);
vmstate_load_state(fload, &vmstate_iommu, dest_iommu, 1);
- ret = qemu_file_get_error(fload);
eof = qemu_get_byte(fload);
- ret = qemu_file_get_error(fload);
- g_assert(!ret);
+ g_assert(!qemu_file_get_error(fload));
g_assert_cmpint(orig_iommu->id, ==, dest_iommu->id);
g_assert_cmpint(eof, ==, QEMU_VM_EOF);
@@ -1395,6 +1392,7 @@ static void test_load_qlist(void)
compare_containers(orig_container, dest_container);
free_container(orig_container);
free_container(dest_container);
+ qemu_fclose(fload);
}
typedef struct TmpTestStruct {
--
2.24.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PULL 11/15] migration/savevm: release gslist after dump_vmstate_json
2020-02-28 9:24 [PULL 00/15] Pull migration patches Juan Quintela
` (9 preceding siblings ...)
2020-02-28 9:24 ` [PULL 10/15] test-vmstate: Fix memleaks in test_load_qlist Juan Quintela
@ 2020-02-28 9:24 ` Juan Quintela
2020-02-28 9:24 ` [PULL 12/15] migration/block: rename BLOCK_SIZE macro Juan Quintela
` (4 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: Juan Quintela @ 2020-02-28 9:24 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Fam Zheng, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, qemu-block, Juan Quintela, Pan Nengyuan,
Dr. David Alan Gilbert, Markus Armbruster, Alex Bennée,
Stefan Hajnoczi, Euler Robot, Paolo Bonzini,
Philippe Mathieu-Daudé,
Hailiang Zhang
From: Pan Nengyuan <pannengyuan@huawei.com>
'list' forgot to free at the end of dump_vmstate_json_to_file(), although it's called only once, but seems like a clean code.
Fix the leak as follow:
Direct leak of 16 byte(s) in 1 object(s) allocated from:
#0 0x7fb946abd768 in __interceptor_malloc (/lib64/libasan.so.5+0xef768)
#1 0x7fb945eca445 in g_malloc (/lib64/libglib-2.0.so.0+0x52445)
#2 0x7fb945ee2066 in g_slice_alloc (/lib64/libglib-2.0.so.0+0x6a066)
#3 0x7fb945ee3139 in g_slist_prepend (/lib64/libglib-2.0.so.0+0x6b139)
#4 0x5585db591581 in object_class_get_list_tramp /mnt/sdb/qemu-new/qemu/qom/object.c:1084
#5 0x5585db590f66 in object_class_foreach_tramp /mnt/sdb/qemu-new/qemu/qom/object.c:1028
#6 0x7fb945eb35f7 in g_hash_table_foreach (/lib64/libglib-2.0.so.0+0x3b5f7)
#7 0x5585db59110c in object_class_foreach /mnt/sdb/qemu-new/qemu/qom/object.c:1038
#8 0x5585db5916b6 in object_class_get_list /mnt/sdb/qemu-new/qemu/qom/object.c:1092
#9 0x5585db335ca0 in dump_vmstate_json_to_file /mnt/sdb/qemu-new/qemu/migration/savevm.c:638
#10 0x5585daa5bcbf in main /mnt/sdb/qemu-new/qemu/vl.c:4420
#11 0x7fb941204812 in __libc_start_main ../csu/libc-start.c:308
#12 0x5585da29420d in _start (/mnt/sdb/qemu-new/qemu/build/x86_64-softmmu/qemu-system-x86_64+0x27f020d)
Indirect leak of 7472 byte(s) in 467 object(s) allocated from:
#0 0x7fb946abd768 in __interceptor_malloc (/lib64/libasan.so.5+0xef768)
#1 0x7fb945eca445 in g_malloc (/lib64/libglib-2.0.so.0+0x52445)
#2 0x7fb945ee2066 in g_slice_alloc (/lib64/libglib-2.0.so.0+0x6a066)
#3 0x7fb945ee3139 in g_slist_prepend (/lib64/libglib-2.0.so.0+0x6b139)
#4 0x5585db591581 in object_class_get_list_tramp /mnt/sdb/qemu-new/qemu/qom/object.c:1084
#5 0x5585db590f66 in object_class_foreach_tramp /mnt/sdb/qemu-new/qemu/qom/object.c:1028
#6 0x7fb945eb35f7 in g_hash_table_foreach (/lib64/libglib-2.0.so.0+0x3b5f7)
#7 0x5585db59110c in object_class_foreach /mnt/sdb/qemu-new/qemu/qom/object.c:1038
#8 0x5585db5916b6 in object_class_get_list /mnt/sdb/qemu-new/qemu/qom/object.c:1092
#9 0x5585db335ca0 in dump_vmstate_json_to_file /mnt/sdb/qemu-new/qemu/migration/savevm.c:638
#10 0x5585daa5bcbf in main /mnt/sdb/qemu-new/qemu/vl.c:4420
#11 0x7fb941204812 in __libc_start_main ../csu/libc-start.c:308
#12 0x5585da29420d in _start (/mnt/sdb/qemu-new/qemu/build/x86_64-softmmu/qemu-system-x86_64+0x27f020d)
Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration/savevm.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/migration/savevm.c b/migration/savevm.c
index 1d4220ece8..c00a6807d9 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -665,6 +665,7 @@ void dump_vmstate_json_to_file(FILE *out_file)
}
fprintf(out_file, "\n}\n");
fclose(out_file);
+ g_slist_free(list);
}
static uint32_t calculate_new_instance_id(const char *idstr)
--
2.24.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PULL 12/15] migration/block: rename BLOCK_SIZE macro
2020-02-28 9:24 [PULL 00/15] Pull migration patches Juan Quintela
` (10 preceding siblings ...)
2020-02-28 9:24 ` [PULL 11/15] migration/savevm: release gslist after dump_vmstate_json Juan Quintela
@ 2020-02-28 9:24 ` Juan Quintela
2022-05-12 16:22 ` Peter Maydell
2020-02-28 9:24 ` [PULL 13/15] migration: fix COLO broken caused by a previous commit Juan Quintela
` (3 subsequent siblings)
15 siblings, 1 reply; 27+ messages in thread
From: Juan Quintela @ 2020-02-28 9:24 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Fam Zheng, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, qemu-block, Juan Quintela,
Dr. David Alan Gilbert, Markus Armbruster, Alex Bennée,
Stefan Hajnoczi, Paolo Bonzini, Philippe Mathieu-Daudé,
Hailiang Zhang
From: Stefan Hajnoczi <stefanha@redhat.com>
Both <linux/fs.h> and <sys/mount.h> define BLOCK_SIZE macros. Avoiding
using that name in block/migration.c.
I noticed this when including <liburing.h> (Linux io_uring) from
"block/aio.h" and compilation failed. Although patches adding that
include haven't been sent yet, it makes sense to rename the macro now in
case someone else stumbles on it in the meantime.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration/block.c | 39 ++++++++++++++++++++-------------------
1 file changed, 20 insertions(+), 19 deletions(-)
diff --git a/migration/block.c b/migration/block.c
index c90288ed29..737b6499f9 100644
--- a/migration/block.c
+++ b/migration/block.c
@@ -27,8 +27,8 @@
#include "migration/vmstate.h"
#include "sysemu/block-backend.h"
-#define BLOCK_SIZE (1 << 20)
-#define BDRV_SECTORS_PER_DIRTY_CHUNK (BLOCK_SIZE >> BDRV_SECTOR_BITS)
+#define BLK_MIG_BLOCK_SIZE (1 << 20)
+#define BDRV_SECTORS_PER_DIRTY_CHUNK (BLK_MIG_BLOCK_SIZE >> BDRV_SECTOR_BITS)
#define BLK_MIG_FLAG_DEVICE_BLOCK 0x01
#define BLK_MIG_FLAG_EOS 0x02
@@ -133,7 +133,7 @@ static void blk_send(QEMUFile *f, BlkMigBlock * blk)
uint64_t flags = BLK_MIG_FLAG_DEVICE_BLOCK;
if (block_mig_state.zero_blocks &&
- buffer_is_zero(blk->buf, BLOCK_SIZE)) {
+ buffer_is_zero(blk->buf, BLK_MIG_BLOCK_SIZE)) {
flags |= BLK_MIG_FLAG_ZERO_BLOCK;
}
@@ -154,7 +154,7 @@ static void blk_send(QEMUFile *f, BlkMigBlock * blk)
return;
}
- qemu_put_buffer(f, blk->buf, BLOCK_SIZE);
+ qemu_put_buffer(f, blk->buf, BLK_MIG_BLOCK_SIZE);
}
int blk_mig_active(void)
@@ -309,7 +309,7 @@ static int mig_save_device_bulk(QEMUFile *f, BlkMigDevState *bmds)
}
blk = g_new(BlkMigBlock, 1);
- blk->buf = g_malloc(BLOCK_SIZE);
+ blk->buf = g_malloc(BLK_MIG_BLOCK_SIZE);
blk->bmds = bmds;
blk->sector = cur_sector;
blk->nr_sectors = nr_sectors;
@@ -350,7 +350,8 @@ static int set_dirty_tracking(void)
QSIMPLEQ_FOREACH(bmds, &block_mig_state.bmds_list, entry) {
bmds->dirty_bitmap = bdrv_create_dirty_bitmap(blk_bs(bmds->blk),
- BLOCK_SIZE, NULL, NULL);
+ BLK_MIG_BLOCK_SIZE,
+ NULL, NULL);
if (!bmds->dirty_bitmap) {
ret = -errno;
goto fail;
@@ -548,7 +549,7 @@ static int mig_save_device_dirty(QEMUFile *f, BlkMigDevState *bmds,
bdrv_dirty_bitmap_unlock(bmds->dirty_bitmap);
blk = g_new(BlkMigBlock, 1);
- blk->buf = g_malloc(BLOCK_SIZE);
+ blk->buf = g_malloc(BLK_MIG_BLOCK_SIZE);
blk->bmds = bmds;
blk->sector = sector;
blk->nr_sectors = nr_sectors;
@@ -770,7 +771,7 @@ static int block_save_iterate(QEMUFile *f, void *opaque)
/* control the rate of transfer */
blk_mig_lock();
- while (block_mig_state.read_done * BLOCK_SIZE <
+ while (block_mig_state.read_done * BLK_MIG_BLOCK_SIZE <
qemu_file_get_rate_limit(f) &&
block_mig_state.submitted < MAX_PARALLEL_IO &&
(block_mig_state.submitted + block_mig_state.read_done) <
@@ -874,13 +875,13 @@ static void block_save_pending(QEMUFile *f, void *opaque, uint64_t max_size,
qemu_mutex_unlock_iothread();
blk_mig_lock();
- pending += block_mig_state.submitted * BLOCK_SIZE +
- block_mig_state.read_done * BLOCK_SIZE;
+ pending += block_mig_state.submitted * BLK_MIG_BLOCK_SIZE +
+ block_mig_state.read_done * BLK_MIG_BLOCK_SIZE;
blk_mig_unlock();
/* Report at least one block pending during bulk phase */
if (pending <= max_size && !block_mig_state.bulk_completed) {
- pending = max_size + BLOCK_SIZE;
+ pending = max_size + BLK_MIG_BLOCK_SIZE;
}
DPRINTF("Enter save live pending %" PRIu64 "\n", pending);
@@ -901,7 +902,7 @@ static int block_load(QEMUFile *f, void *opaque, int version_id)
int nr_sectors;
int ret;
BlockDriverInfo bdi;
- int cluster_size = BLOCK_SIZE;
+ int cluster_size = BLK_MIG_BLOCK_SIZE;
do {
addr = qemu_get_be64(f);
@@ -939,11 +940,11 @@ static int block_load(QEMUFile *f, void *opaque, int version_id)
ret = bdrv_get_info(blk_bs(blk), &bdi);
if (ret == 0 && bdi.cluster_size > 0 &&
- bdi.cluster_size <= BLOCK_SIZE &&
- BLOCK_SIZE % bdi.cluster_size == 0) {
+ bdi.cluster_size <= BLK_MIG_BLOCK_SIZE &&
+ BLK_MIG_BLOCK_SIZE % bdi.cluster_size == 0) {
cluster_size = bdi.cluster_size;
} else {
- cluster_size = BLOCK_SIZE;
+ cluster_size = BLK_MIG_BLOCK_SIZE;
}
}
@@ -962,14 +963,14 @@ static int block_load(QEMUFile *f, void *opaque, int version_id)
int64_t cur_addr;
uint8_t *cur_buf;
- buf = g_malloc(BLOCK_SIZE);
- qemu_get_buffer(f, buf, BLOCK_SIZE);
- for (i = 0; i < BLOCK_SIZE / cluster_size; i++) {
+ buf = g_malloc(BLK_MIG_BLOCK_SIZE);
+ qemu_get_buffer(f, buf, BLK_MIG_BLOCK_SIZE);
+ for (i = 0; i < BLK_MIG_BLOCK_SIZE / cluster_size; i++) {
cur_addr = addr * BDRV_SECTOR_SIZE + i * cluster_size;
cur_buf = buf + i * cluster_size;
if ((!block_mig_state.zero_blocks ||
- cluster_size < BLOCK_SIZE) &&
+ cluster_size < BLK_MIG_BLOCK_SIZE) &&
buffer_is_zero(cur_buf, cluster_size)) {
ret = blk_pwrite_zeroes(blk, cur_addr,
cluster_size,
--
2.24.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PULL 13/15] migration: fix COLO broken caused by a previous commit
2020-02-28 9:24 [PULL 00/15] Pull migration patches Juan Quintela
` (11 preceding siblings ...)
2020-02-28 9:24 ` [PULL 12/15] migration/block: rename BLOCK_SIZE macro Juan Quintela
@ 2020-02-28 9:24 ` Juan Quintela
2020-02-28 9:24 ` [PULL 14/15] migration/colo: wrap incoming checkpoint process into new helper Juan Quintela
` (2 subsequent siblings)
15 siblings, 0 replies; 27+ messages in thread
From: Juan Quintela @ 2020-02-28 9:24 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Fam Zheng, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, qemu-block, Juan Quintela,
Dr. David Alan Gilbert, Markus Armbruster, Alex Bennée,
Stefan Hajnoczi, Paolo Bonzini, Philippe Mathieu-Daudé,
Hailiang Zhang
From: zhanghailiang <zhang.zhanghailiang@huawei.com>
This commit "migration: Create migration_is_running()" broke
COLO. Becuase there is a process broken by this commit.
colo_process_checkpoint
->colo_do_checkpoint_transaction
->migrate_set_block_enabled
->qmp_migrate_set_capabilities
It can be fixed by make COLO process as an exception,
Maybe we need a better way to fix it.
Cc: Juan Quintela <quintela@redhat.com>
Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration/migration.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/migration/migration.c b/migration/migration.c
index c1814a6861..92272250f4 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -876,7 +876,6 @@ bool migration_is_running(int state)
case MIGRATION_STATUS_DEVICE:
case MIGRATION_STATUS_WAIT_UNPLUG:
case MIGRATION_STATUS_CANCELLING:
- case MIGRATION_STATUS_COLO:
return true;
default:
--
2.24.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PULL 14/15] migration/colo: wrap incoming checkpoint process into new helper
2020-02-28 9:24 [PULL 00/15] Pull migration patches Juan Quintela
` (12 preceding siblings ...)
2020-02-28 9:24 ` [PULL 13/15] migration: fix COLO broken caused by a previous commit Juan Quintela
@ 2020-02-28 9:24 ` Juan Quintela
2020-02-28 9:24 ` [PULL 15/15] savevm: Don't call colo_init_ram_cache twice Juan Quintela
2020-02-28 16:01 ` [PULL 00/15] Pull migration patches Peter Maydell
15 siblings, 0 replies; 27+ messages in thread
From: Juan Quintela @ 2020-02-28 9:24 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Fam Zheng, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, qemu-block, Juan Quintela,
Dr. David Alan Gilbert, Markus Armbruster, Alex Bennée,
Stefan Hajnoczi, Paolo Bonzini, Philippe Mathieu-Daudé,
Hailiang Zhang
From: zhanghailiang <zhang.zhanghailiang@huawei.com>
Split checkpoint incoming process into a helper.
Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration/colo.c | 260 ++++++++++++++++++++++++-----------------------
1 file changed, 133 insertions(+), 127 deletions(-)
diff --git a/migration/colo.c b/migration/colo.c
index 2c88aa57a2..93c5a452fb 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -664,13 +664,138 @@ void migrate_start_colo_process(MigrationState *s)
qemu_mutex_lock_iothread();
}
-static void colo_wait_handle_message(QEMUFile *f, int *checkpoint_request,
- Error **errp)
+static void colo_incoming_process_checkpoint(MigrationIncomingState *mis,
+ QEMUFile *fb, QIOChannelBuffer *bioc, Error **errp)
+{
+ uint64_t total_size;
+ uint64_t value;
+ Error *local_err = NULL;
+ int ret;
+
+ qemu_mutex_lock_iothread();
+ vm_stop_force_state(RUN_STATE_COLO);
+ trace_colo_vm_state_change("run", "stop");
+ qemu_mutex_unlock_iothread();
+
+ /* FIXME: This is unnecessary for periodic checkpoint mode */
+ colo_send_message(mis->to_src_file, COLO_MESSAGE_CHECKPOINT_REPLY,
+ &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
+
+ colo_receive_check_message(mis->from_src_file,
+ COLO_MESSAGE_VMSTATE_SEND, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
+
+ qemu_mutex_lock_iothread();
+ cpu_synchronize_all_pre_loadvm();
+ ret = qemu_loadvm_state_main(mis->from_src_file, mis);
+ qemu_mutex_unlock_iothread();
+
+ if (ret < 0) {
+ error_setg(errp, "Load VM's live state (ram) error");
+ return;
+ }
+
+ value = colo_receive_message_value(mis->from_src_file,
+ COLO_MESSAGE_VMSTATE_SIZE, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
+
+ /*
+ * Read VM device state data into channel buffer,
+ * It's better to re-use the memory allocated.
+ * Here we need to handle the channel buffer directly.
+ */
+ if (value > bioc->capacity) {
+ bioc->capacity = value;
+ bioc->data = g_realloc(bioc->data, bioc->capacity);
+ }
+ total_size = qemu_get_buffer(mis->from_src_file, bioc->data, value);
+ if (total_size != value) {
+ error_setg(errp, "Got %" PRIu64 " VMState data, less than expected"
+ " %" PRIu64, total_size, value);
+ return;
+ }
+ bioc->usage = total_size;
+ qio_channel_io_seek(QIO_CHANNEL(bioc), 0, 0, NULL);
+
+ colo_send_message(mis->to_src_file, COLO_MESSAGE_VMSTATE_RECEIVED,
+ &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
+
+ qemu_mutex_lock_iothread();
+ vmstate_loading = true;
+ ret = qemu_load_device_state(fb);
+ if (ret < 0) {
+ error_setg(errp, "COLO: load device state failed");
+ qemu_mutex_unlock_iothread();
+ return;
+ }
+
+#ifdef CONFIG_REPLICATION
+ replication_get_error_all(&local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ qemu_mutex_unlock_iothread();
+ return;
+ }
+
+ /* discard colo disk buffer */
+ replication_do_checkpoint_all(&local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ qemu_mutex_unlock_iothread();
+ return;
+ }
+#else
+ abort();
+#endif
+ /* Notify all filters of all NIC to do checkpoint */
+ colo_notify_filters_event(COLO_EVENT_CHECKPOINT, &local_err);
+
+ if (local_err) {
+ error_propagate(errp, local_err);
+ qemu_mutex_unlock_iothread();
+ return;
+ }
+
+ vmstate_loading = false;
+ vm_start();
+ trace_colo_vm_state_change("stop", "run");
+ qemu_mutex_unlock_iothread();
+
+ if (failover_get_state() == FAILOVER_STATUS_RELAUNCH) {
+ failover_set_state(FAILOVER_STATUS_RELAUNCH,
+ FAILOVER_STATUS_NONE);
+ failover_request_active(NULL);
+ return;
+ }
+
+ colo_send_message(mis->to_src_file, COLO_MESSAGE_VMSTATE_LOADED,
+ &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ }
+}
+
+static void colo_wait_handle_message(MigrationIncomingState *mis,
+ QEMUFile *fb, QIOChannelBuffer *bioc, Error **errp)
{
COLOMessage msg;
Error *local_err = NULL;
- msg = colo_receive_message(f, &local_err);
+ msg = colo_receive_message(mis->from_src_file, &local_err);
if (local_err) {
error_propagate(errp, local_err);
return;
@@ -678,10 +803,9 @@ static void colo_wait_handle_message(QEMUFile *f, int *checkpoint_request,
switch (msg) {
case COLO_MESSAGE_CHECKPOINT_REQUEST:
- *checkpoint_request = 1;
+ colo_incoming_process_checkpoint(mis, fb, bioc, errp);
break;
default:
- *checkpoint_request = 0;
error_setg(errp, "Got unknown COLO message: %d", msg);
break;
}
@@ -692,10 +816,7 @@ void *colo_process_incoming_thread(void *opaque)
MigrationIncomingState *mis = opaque;
QEMUFile *fb = NULL;
QIOChannelBuffer *bioc = NULL; /* Cache incoming device state */
- uint64_t total_size;
- uint64_t value;
Error *local_err = NULL;
- int ret;
rcu_register_thread();
qemu_sem_init(&mis->colo_incoming_sem, 0);
@@ -749,134 +870,19 @@ void *colo_process_incoming_thread(void *opaque)
}
while (mis->state == MIGRATION_STATUS_COLO) {
- int request = 0;
-
- colo_wait_handle_message(mis->from_src_file, &request, &local_err);
+ colo_wait_handle_message(mis, fb, bioc, &local_err);
if (local_err) {
- goto out;
+ error_report_err(local_err);
+ break;
}
- assert(request);
if (failover_get_state() != FAILOVER_STATUS_NONE) {
error_report("failover request");
- goto out;
- }
-
- qemu_mutex_lock_iothread();
- vm_stop_force_state(RUN_STATE_COLO);
- trace_colo_vm_state_change("run", "stop");
- qemu_mutex_unlock_iothread();
-
- /* FIXME: This is unnecessary for periodic checkpoint mode */
- colo_send_message(mis->to_src_file, COLO_MESSAGE_CHECKPOINT_REPLY,
- &local_err);
- if (local_err) {
- goto out;
- }
-
- colo_receive_check_message(mis->from_src_file,
- COLO_MESSAGE_VMSTATE_SEND, &local_err);
- if (local_err) {
- goto out;
- }
-
- qemu_mutex_lock_iothread();
- cpu_synchronize_all_pre_loadvm();
- ret = qemu_loadvm_state_main(mis->from_src_file, mis);
- qemu_mutex_unlock_iothread();
-
- if (ret < 0) {
- error_report("Load VM's live state (ram) error");
- goto out;
- }
-
- value = colo_receive_message_value(mis->from_src_file,
- COLO_MESSAGE_VMSTATE_SIZE, &local_err);
- if (local_err) {
- goto out;
- }
-
- /*
- * Read VM device state data into channel buffer,
- * It's better to re-use the memory allocated.
- * Here we need to handle the channel buffer directly.
- */
- if (value > bioc->capacity) {
- bioc->capacity = value;
- bioc->data = g_realloc(bioc->data, bioc->capacity);
- }
- total_size = qemu_get_buffer(mis->from_src_file, bioc->data, value);
- if (total_size != value) {
- error_report("Got %" PRIu64 " VMState data, less than expected"
- " %" PRIu64, total_size, value);
- goto out;
- }
- bioc->usage = total_size;
- qio_channel_io_seek(QIO_CHANNEL(bioc), 0, 0, NULL);
-
- colo_send_message(mis->to_src_file, COLO_MESSAGE_VMSTATE_RECEIVED,
- &local_err);
- if (local_err) {
- goto out;
- }
-
- qemu_mutex_lock_iothread();
- vmstate_loading = true;
- ret = qemu_load_device_state(fb);
- if (ret < 0) {
- error_report("COLO: load device state failed");
- qemu_mutex_unlock_iothread();
- goto out;
- }
-
-#ifdef CONFIG_REPLICATION
- replication_get_error_all(&local_err);
- if (local_err) {
- qemu_mutex_unlock_iothread();
- goto out;
- }
-
- /* discard colo disk buffer */
- replication_do_checkpoint_all(&local_err);
- if (local_err) {
- qemu_mutex_unlock_iothread();
- goto out;
- }
-#else
- abort();
-#endif
- /* Notify all filters of all NIC to do checkpoint */
- colo_notify_filters_event(COLO_EVENT_CHECKPOINT, &local_err);
-
- if (local_err) {
- qemu_mutex_unlock_iothread();
- goto out;
- }
-
- vmstate_loading = false;
- vm_start();
- trace_colo_vm_state_change("stop", "run");
- qemu_mutex_unlock_iothread();
-
- if (failover_get_state() == FAILOVER_STATUS_RELAUNCH) {
- failover_set_state(FAILOVER_STATUS_RELAUNCH,
- FAILOVER_STATUS_NONE);
- failover_request_active(NULL);
- goto out;
- }
-
- colo_send_message(mis->to_src_file, COLO_MESSAGE_VMSTATE_LOADED,
- &local_err);
- if (local_err) {
- goto out;
+ break;
}
}
out:
vmstate_loading = false;
- /* Throw the unreported error message after exited from loop */
- if (local_err) {
- error_report_err(local_err);
- }
/*
* There are only two reasons we can get here, some error happened
--
2.24.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PULL 15/15] savevm: Don't call colo_init_ram_cache twice
2020-02-28 9:24 [PULL 00/15] Pull migration patches Juan Quintela
` (13 preceding siblings ...)
2020-02-28 9:24 ` [PULL 14/15] migration/colo: wrap incoming checkpoint process into new helper Juan Quintela
@ 2020-02-28 9:24 ` Juan Quintela
2020-02-28 16:01 ` [PULL 00/15] Pull migration patches Peter Maydell
15 siblings, 0 replies; 27+ messages in thread
From: Juan Quintela @ 2020-02-28 9:24 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Fam Zheng, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, qemu-block, Juan Quintela,
Dr. David Alan Gilbert, Markus Armbruster, Alex Bennée,
Stefan Hajnoczi, Paolo Bonzini, Philippe Mathieu-Daudé,
Hailiang Zhang
From: zhanghailiang <zhang.zhanghailiang@huawei.com>
This helper has been called twice which is wrong.
Left the one where called while get COLO enable message
from source side.
Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration/migration.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index 92272250f4..0b2045ccbd 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -489,11 +489,6 @@ static void process_incoming_migration_co(void *opaque)
goto fail;
}
- if (colo_init_ram_cache() < 0) {
- error_report("Init ram cache failed");
- goto fail;
- }
-
qemu_thread_create(&mis->colo_incoming_thread, "COLO incoming",
colo_process_incoming_thread, mis, QEMU_THREAD_JOINABLE);
mis->have_colo_incoming_thread = true;
--
2.24.1
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PULL 00/15] Pull migration patches
2020-02-28 9:24 [PULL 00/15] Pull migration patches Juan Quintela
` (14 preceding siblings ...)
2020-02-28 9:24 ` [PULL 15/15] savevm: Don't call colo_init_ram_cache twice Juan Quintela
@ 2020-02-28 16:01 ` Peter Maydell
15 siblings, 0 replies; 27+ messages in thread
From: Peter Maydell @ 2020-02-28 16:01 UTC (permalink / raw)
To: Juan Quintela
Cc: Laurent Vivier, Fam Zheng, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, Qemu-block, Markus Armbruster,
Philippe Mathieu-Daudé,
QEMU Developers, Dr. David Alan Gilbert, Stefan Hajnoczi,
Paolo Bonzini, Alex Bennée, Hailiang Zhang
On Fri, 28 Feb 2020 at 09:25, Juan Quintela <quintela@redhat.com> wrote:
>
> The following changes since commit 8b6b68e05b43f976714ca1d2afe01a64e1d82cba:
>
> Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging (2020-02-27 19:15:15 +0000)
>
> are available in the Git repository at:
>
> https://github.com/juanquintela/qemu.git tags/pull-migration-pull-request
>
> for you to fetch changes up to f51d0b4178738bba87d796eba7444f6cdb3aa0fd:
>
> savevm: Don't call colo_init_ram_cache twice (2020-02-28 10:13:54 +0100)
>
> ----------------------------------------------------------------
> Migration pull request
>
> - cleanup redundant initilazation (chen)
> - fix memleak in test_load_qlist (chen)
> - several colo fixes (hailiang)
> - multifd compression (juan)
> - release list after use (pan)
> - rename BLOCK_SIZE (stefan)
>
> Please apply.
>
Applied, thanks.
Please update the changelog at https://wiki.qemu.org/ChangeLog/5.0
for any user-visible changes.
-- PMM
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PULL 06/15] configure: Enable test and libs for zstd
2020-02-28 9:24 ` [PULL 06/15] configure: Enable test and libs for zstd Juan Quintela
@ 2020-02-29 20:06 ` Richard Henderson
2020-03-02 8:00 ` Juan Quintela
2020-03-17 17:09 ` Peter Maydell
1 sibling, 1 reply; 27+ messages in thread
From: Richard Henderson @ 2020-02-29 20:06 UTC (permalink / raw)
To: Juan Quintela
Cc: Laurent Vivier, Fam Zheng, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, open list:Block layer core, Markus Armbruster,
Philippe Mathieu-Daudé,
qemu-devel@nongnu.org Developers, Dr. David Alan Gilbert,
Stefan Hajnoczi, Paolo Bonzini, Alex Bennée, Hailiang Zhang
> +##########################################
> +# zstd check
> +
> +if test "$zstd" != "no" ; then
> + if $pkg_config --exist libzstd ; then
The option is spelled --exists. This is generating an error during configure.
r~
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PULL 06/15] configure: Enable test and libs for zstd
2020-02-29 20:06 ` Richard Henderson
@ 2020-03-02 8:00 ` Juan Quintela
2020-03-02 8:32 ` Alex Bennée
0 siblings, 1 reply; 27+ messages in thread
From: Juan Quintela @ 2020-03-02 8:00 UTC (permalink / raw)
To: Richard Henderson
Cc: Laurent Vivier, Fam Zheng, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, open list:Block layer core, Markus Armbruster,
Philippe Mathieu-Daudé,
qemu-devel@nongnu.org Developers, Dr. David Alan Gilbert,
Stefan Hajnoczi, Paolo Bonzini, Alex Bennée, Hailiang Zhang
Richard Henderson <richard.henderson@linaro.org> wrote:
>> +##########################################
>> +# zstd check
>> +
>> +if test "$zstd" != "no" ; then
>> + if $pkg_config --exist libzstd ; then
>
> The option is spelled --exists. This is generating an error during configure.
Ooops, you are right.
Sending a patch. Just wondering why I didn't saw that error :-(
Thanks, Juan.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PULL 06/15] configure: Enable test and libs for zstd
2020-03-02 8:00 ` Juan Quintela
@ 2020-03-02 8:32 ` Alex Bennée
0 siblings, 0 replies; 27+ messages in thread
From: Alex Bennée @ 2020-03-02 8:32 UTC (permalink / raw)
To: quintela
Cc: Laurent Vivier, Fam Zheng, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, open list:Block layer core, Markus Armbruster,
Richard Henderson, qemu-devel@nongnu.org Developers,
Dr. David Alan Gilbert, Stefan Hajnoczi, Paolo Bonzini,
Philippe Mathieu-Daudé,
Hailiang Zhang
Juan Quintela <quintela@redhat.com> writes:
> Richard Henderson <richard.henderson@linaro.org> wrote:
>>> +##########################################
>>> +# zstd check
>>> +
>>> +if test "$zstd" != "no" ; then
>>> + if $pkg_config --exist libzstd ; then
>>
>> The option is spelled --exists. This is generating an error during configure.
>
> Ooops, you are right.
>
> Sending a patch. Just wondering why I didn't saw that error :-(
I was only noticing when re-directing output - otherwise you miss it in
a fairly long scrollback.
>
> Thanks, Juan.
--
Alex Bennée
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PULL 06/15] configure: Enable test and libs for zstd
2020-02-28 9:24 ` [PULL 06/15] configure: Enable test and libs for zstd Juan Quintela
2020-02-29 20:06 ` Richard Henderson
@ 2020-03-17 17:09 ` Peter Maydell
2020-03-17 17:40 ` Juan Quintela
1 sibling, 1 reply; 27+ messages in thread
From: Peter Maydell @ 2020-03-17 17:09 UTC (permalink / raw)
To: Juan Quintela
Cc: Laurent Vivier, Fam Zheng, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, Qemu-block, Markus Armbruster,
Philippe Mathieu-Daudé,
QEMU Developers, Dr. David Alan Gilbert, Stefan Hajnoczi,
Paolo Bonzini, Alex Bennée, Hailiang Zhang
On Fri, 28 Feb 2020 at 09:28, Juan Quintela <quintela@redhat.com> wrote:
>
> Add it to several build systems to make testing good.
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> .gitlab-ci.yml | 1 +
> .travis.yml | 1 +
> configure | 30 +++++++++++++++++++
> tests/docker/dockerfiles/centos7.docker | 3 +-
> .../dockerfiles/fedora-i386-cross.docker | 3 +-
> tests/docker/dockerfiles/fedora.docker | 3 +-
> tests/docker/dockerfiles/ubuntu.docker | 1 +
> tests/docker/dockerfiles/ubuntu1804.docker | 1 +
> tests/vm/fedora | 5 +++-
> tests/vm/freebsd | 3 ++
> tests/vm/netbsd | 3 ++
> tests/vm/openbsd | 3 ++
> 12 files changed, 53 insertions(+), 4 deletions(-)
Hi; this patch changes some .docker files, but it has
put the new line at the bottom of each package list,
rather than at the correct point in the alphabetical
order that the lists are in, for these 3 fedora/centos ones:
> diff --git a/tests/docker/dockerfiles/centos7.docker b/tests/docker/dockerfiles/centos7.docker
> index 562d65be9e..cdd72de7eb 100644
> --- a/tests/docker/dockerfiles/centos7.docker
> +++ b/tests/docker/dockerfiles/centos7.docker
> @@ -33,6 +33,7 @@ ENV PACKAGES \
> tar \
> vte-devel \
> xen-devel \
> - zlib-devel
> + zlib-devel \
> + libzstd-devel
> RUN yum install -y $PACKAGES
> RUN rpm -q $PACKAGES | sort > /packages.txt
> diff --git a/tests/docker/dockerfiles/fedora-i386-cross.docker b/tests/docker/dockerfiles/fedora-i386-cross.docker
> index 9106cf9ebe..cd16cd1bfa 100644
> --- a/tests/docker/dockerfiles/fedora-i386-cross.docker
> +++ b/tests/docker/dockerfiles/fedora-i386-cross.docker
> @@ -7,7 +7,8 @@ ENV PACKAGES \
> gnutls-devel.i686 \
> nettle-devel.i686 \
> pixman-devel.i686 \
> - zlib-devel.i686
> + zlib-devel.i686 \
> + libzstd-devel.i686
>
> RUN dnf install -y $PACKAGES
> RUN rpm -q $PACKAGES | sort > /packages.txt
> diff --git a/tests/docker/dockerfiles/fedora.docker b/tests/docker/dockerfiles/fedora.docker
> index 987a3c170a..a6522228c0 100644
> --- a/tests/docker/dockerfiles/fedora.docker
> +++ b/tests/docker/dockerfiles/fedora.docker
> @@ -92,7 +92,8 @@ ENV PACKAGES \
> vte291-devel \
> which \
> xen-devel \
> - zlib-devel
> + zlib-devel \
> + libzstd-devel
> ENV QEMU_CONFIGURE_OPTS --python=/usr/bin/python3
>
> RUN dnf install -y $PACKAGES
The ubuntu ones are OK though:
> diff --git a/tests/docker/dockerfiles/ubuntu.docker b/tests/docker/dockerfiles/ubuntu.docker
> index 4177f33691..b6c7b41ddd 100644
> --- a/tests/docker/dockerfiles/ubuntu.docker
> +++ b/tests/docker/dockerfiles/ubuntu.docker
> @@ -58,6 +58,7 @@ ENV PACKAGES flex bison \
> libvdeplug-dev \
> libvte-2.91-dev \
> libxen-dev \
> + libzstd-dev \
> make \
> python3-yaml \
> python3-sphinx \
> diff --git a/tests/docker/dockerfiles/ubuntu1804.docker b/tests/docker/dockerfiles/ubuntu1804.docker
> index 0766f94cf4..1efedeef99 100644
> --- a/tests/docker/dockerfiles/ubuntu1804.docker
> +++ b/tests/docker/dockerfiles/ubuntu1804.docker
> @@ -44,6 +44,7 @@ ENV PACKAGES flex bison \
> libvdeplug-dev \
> libvte-2.91-dev \
> libxen-dev \
> + libzstd-dev \
> make \
> python3-yaml \
> python3-sphinx \
Could somebody send a patch that fixes up the ordering,
please?
thanks
-- PMM
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PULL 06/15] configure: Enable test and libs for zstd
2020-03-17 17:09 ` Peter Maydell
@ 2020-03-17 17:40 ` Juan Quintela
0 siblings, 0 replies; 27+ messages in thread
From: Juan Quintela @ 2020-03-17 17:40 UTC (permalink / raw)
To: Peter Maydell
Cc: Laurent Vivier, Fam Zheng, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, Qemu-block, Markus Armbruster,
Philippe Mathieu-Daudé,
QEMU Developers, Dr. David Alan Gilbert, Stefan Hajnoczi,
Paolo Bonzini, Alex Bennée, Hailiang Zhang
Peter Maydell <peter.maydell@linaro.org> wrote:
> On Fri, 28 Feb 2020 at 09:28, Juan Quintela <quintela@redhat.com> wrote:
>>
>> Add it to several build systems to make testing good.
>>
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>> ---
>> .gitlab-ci.yml | 1 +
>> .travis.yml | 1 +
>> configure | 30 +++++++++++++++++++
>> tests/docker/dockerfiles/centos7.docker | 3 +-
>> .../dockerfiles/fedora-i386-cross.docker | 3 +-
>> tests/docker/dockerfiles/fedora.docker | 3 +-
>> tests/docker/dockerfiles/ubuntu.docker | 1 +
>> tests/docker/dockerfiles/ubuntu1804.docker | 1 +
>> tests/vm/fedora | 5 +++-
>> tests/vm/freebsd | 3 ++
>> tests/vm/netbsd | 3 ++
>> tests/vm/openbsd | 3 ++
>> 12 files changed, 53 insertions(+), 4 deletions(-)
>
> Hi; this patch changes some .docker files, but it has
> put the new line at the bottom of each package list,
> rather than at the correct point in the alphabetical
> order that the lists are in, for these 3 fedora/centos ones:
ok.
I mill send a patch for that.
Sorry, Juan.
>
>> diff --git a/tests/docker/dockerfiles/centos7.docker b/tests/docker/dockerfiles/centos7.docker
>> index 562d65be9e..cdd72de7eb 100644
>> --- a/tests/docker/dockerfiles/centos7.docker
>> +++ b/tests/docker/dockerfiles/centos7.docker
>> @@ -33,6 +33,7 @@ ENV PACKAGES \
>> tar \
>> vte-devel \
>> xen-devel \
>> - zlib-devel
>> + zlib-devel \
>> + libzstd-devel
>> RUN yum install -y $PACKAGES
>> RUN rpm -q $PACKAGES | sort > /packages.txt
>> diff --git a/tests/docker/dockerfiles/fedora-i386-cross.docker b/tests/docker/dockerfiles/fedora-i386-cross.docker
>> index 9106cf9ebe..cd16cd1bfa 100644
>> --- a/tests/docker/dockerfiles/fedora-i386-cross.docker
>> +++ b/tests/docker/dockerfiles/fedora-i386-cross.docker
>> @@ -7,7 +7,8 @@ ENV PACKAGES \
>> gnutls-devel.i686 \
>> nettle-devel.i686 \
>> pixman-devel.i686 \
>> - zlib-devel.i686
>> + zlib-devel.i686 \
>> + libzstd-devel.i686
>>
>> RUN dnf install -y $PACKAGES
>> RUN rpm -q $PACKAGES | sort > /packages.txt
>> diff --git a/tests/docker/dockerfiles/fedora.docker b/tests/docker/dockerfiles/fedora.docker
>> index 987a3c170a..a6522228c0 100644
>> --- a/tests/docker/dockerfiles/fedora.docker
>> +++ b/tests/docker/dockerfiles/fedora.docker
>> @@ -92,7 +92,8 @@ ENV PACKAGES \
>> vte291-devel \
>> which \
>> xen-devel \
>> - zlib-devel
>> + zlib-devel \
>> + libzstd-devel
>> ENV QEMU_CONFIGURE_OPTS --python=/usr/bin/python3
>>
>> RUN dnf install -y $PACKAGES
>
> The ubuntu ones are OK though:
>
>> diff --git a/tests/docker/dockerfiles/ubuntu.docker
>> b/tests/docker/dockerfiles/ubuntu.docker
>> index 4177f33691..b6c7b41ddd 100644
>> --- a/tests/docker/dockerfiles/ubuntu.docker
>> +++ b/tests/docker/dockerfiles/ubuntu.docker
>> @@ -58,6 +58,7 @@ ENV PACKAGES flex bison \
>> libvdeplug-dev \
>> libvte-2.91-dev \
>> libxen-dev \
>> + libzstd-dev \
>> make \
>> python3-yaml \
>> python3-sphinx \
>> diff --git a/tests/docker/dockerfiles/ubuntu1804.docker
>> b/tests/docker/dockerfiles/ubuntu1804.docker
>> index 0766f94cf4..1efedeef99 100644
>> --- a/tests/docker/dockerfiles/ubuntu1804.docker
>> +++ b/tests/docker/dockerfiles/ubuntu1804.docker
>> @@ -44,6 +44,7 @@ ENV PACKAGES flex bison \
>> libvdeplug-dev \
>> libvte-2.91-dev \
>> libxen-dev \
>> + libzstd-dev \
>> make \
>> python3-yaml \
>> python3-sphinx \
>
>
> Could somebody send a patch that fixes up the ordering,
> please?
>
> thanks
> -- PMM
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PULL 03/15] multifd: Make no compression operations into its own structure
2020-02-28 9:24 ` [PULL 03/15] multifd: Make no compression operations into its own structure Juan Quintela
@ 2022-04-12 19:04 ` Peter Maydell
2022-05-13 17:56 ` Peter Maydell
2022-07-19 14:52 ` Markus Armbruster
0 siblings, 2 replies; 27+ messages in thread
From: Peter Maydell @ 2022-04-12 19:04 UTC (permalink / raw)
To: Juan Quintela
Cc: Markus Armbruster, qemu-devel, Michael Roth, Dr. David Alan Gilbert
On Fri, 28 Feb 2020 at 09:26, Juan Quintela <quintela@redhat.com> wrote:
>
> It will be used later.
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>
Hi; Coverity thinks there might be a buffer overrun here.
It's probably wrong, but it's not completely obvious why
it can't happen, so an assert somewhere would help...
(This is CID 1487239.)
> +MultiFDCompression migrate_multifd_compression(void)
> +{
> + MigrationState *s;
> +
> + s = migrate_get_current();
> +
> + return s->parameters.multifd_compression;
This function returns an enum of type MultiFDCompression,
whose (autogenerated from QAPI) definition is:
typedef enum MultiFDCompression {
MULTIFD_COMPRESSION_NONE,
MULTIFD_COMPRESSION_ZLIB,
#if defined(CONFIG_ZSTD)
MULTIFD_COMPRESSION_ZSTD,
#endif /* defined(CONFIG_ZSTD) */
MULTIFD_COMPRESSION__MAX,
} MultiFDCompression;
> @@ -604,6 +745,7 @@ int multifd_save_setup(Error **errp)
> multifd_send_state->pages = multifd_pages_init(page_count);
> qemu_sem_init(&multifd_send_state->channels_ready, 0);
> atomic_set(&multifd_send_state->exiting, 0);
> + multifd_send_state->ops = multifd_ops[migrate_multifd_compression()];
Here we take the result of the function and use it as an
array index into multifd_ops, whose definition is
static MultiFDMethods *multifd_ops[MULTIFD_COMPRESSION__MAX] = { ... }
Coverity doesn't see any reason why the return value from
migrate_multifd_compression() can't be MULTIFD_COMPRESSION__MAX,
so it complains that if it is then we are going to index off the
end of the array.
An assert in migrate_multifd_compression() that the value being
returned is within the expected range would probably placate it.
Alternatively, if the qapi type codegen didn't put the __MAX
value as a possible value of the enum type then Coverity
and probably also the compiler wouldn't consider it to be
a possible value of this kind of variable. But that might
have other awkward side-effects.
thanks
-- PMM
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PULL 12/15] migration/block: rename BLOCK_SIZE macro
2020-02-28 9:24 ` [PULL 12/15] migration/block: rename BLOCK_SIZE macro Juan Quintela
@ 2022-05-12 16:22 ` Peter Maydell
0 siblings, 0 replies; 27+ messages in thread
From: Peter Maydell @ 2022-05-12 16:22 UTC (permalink / raw)
To: Juan Quintela; +Cc: qemu-devel, Dr. David Alan Gilbert, Stefan Hajnoczi
On Fri, 28 Feb 2020 at 09:30, Juan Quintela <quintela@redhat.com> wrote:
>
> From: Stefan Hajnoczi <stefanha@redhat.com>
>
> Both <linux/fs.h> and <sys/mount.h> define BLOCK_SIZE macros. Avoiding
> using that name in block/migration.c.
>
> I noticed this when including <liburing.h> (Linux io_uring) from
> "block/aio.h" and compilation failed. Although patches adding that
> include haven't been sent yet, it makes sense to rename the macro now in
> case someone else stumbles on it in the meantime.
A rather old change, and it didn't even introduce the code that
Coverity is complaining about, but this seems as good a point as
any to hang the email off of...
BLK_MIG_BLOCK_SIZE doesn't have a ULL suffix, so it's 32 bits,
and so Coverity complains about places where we multiply some
block count by it and then use that in a 64-bit result, eg here:
> @@ -770,7 +771,7 @@ static int block_save_iterate(QEMUFile *f, void *opaque)
>
> /* control the rate of transfer */
> blk_mig_lock();
> - while (block_mig_state.read_done * BLOCK_SIZE <
> + while (block_mig_state.read_done * BLK_MIG_BLOCK_SIZE <
> qemu_file_get_rate_limit(f) &&
> block_mig_state.submitted < MAX_PARALLEL_IO &&
> (block_mig_state.submitted + block_mig_state.read_done) <
and here:
> @@ -874,13 +875,13 @@ static void block_save_pending(QEMUFile *f, void *opaque, uint64_t max_size,
> qemu_mutex_unlock_iothread();
>
> blk_mig_lock();
> - pending += block_mig_state.submitted * BLOCK_SIZE +
> - block_mig_state.read_done * BLOCK_SIZE;
> + pending += block_mig_state.submitted * BLK_MIG_BLOCK_SIZE +
> + block_mig_state.read_done * BLK_MIG_BLOCK_SIZE;
> blk_mig_unlock();
Putting a suitable cast to ensure the multiply is done at
64 bits would satisfy Coverity.
This is CID 1487136, 1487175.
thanks
-- PMM
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PULL 03/15] multifd: Make no compression operations into its own structure
2022-04-12 19:04 ` Peter Maydell
@ 2022-05-13 17:56 ` Peter Maydell
2022-07-19 14:06 ` Peter Maydell
2022-07-19 14:52 ` Markus Armbruster
1 sibling, 1 reply; 27+ messages in thread
From: Peter Maydell @ 2022-05-13 17:56 UTC (permalink / raw)
To: Juan Quintela
Cc: qemu-devel, Dr. David Alan Gilbert, Markus Armbruster, Michael Roth
Ping! Any opinions, especially from qapi codegen people,
on the right thing to do here?
On Tue, 12 Apr 2022 at 20:04, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Fri, 28 Feb 2020 at 09:26, Juan Quintela <quintela@redhat.com> wrote:
> >
> > It will be used later.
> >
> > Signed-off-by: Juan Quintela <quintela@redhat.com>
> > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> >
>
> Hi; Coverity thinks there might be a buffer overrun here.
> It's probably wrong, but it's not completely obvious why
> it can't happen, so an assert somewhere would help...
> (This is CID 1487239.)
>
> > +MultiFDCompression migrate_multifd_compression(void)
> > +{
> > + MigrationState *s;
> > +
> > + s = migrate_get_current();
> > +
> > + return s->parameters.multifd_compression;
>
> This function returns an enum of type MultiFDCompression,
> whose (autogenerated from QAPI) definition is:
>
> typedef enum MultiFDCompression {
> MULTIFD_COMPRESSION_NONE,
> MULTIFD_COMPRESSION_ZLIB,
> #if defined(CONFIG_ZSTD)
> MULTIFD_COMPRESSION_ZSTD,
> #endif /* defined(CONFIG_ZSTD) */
> MULTIFD_COMPRESSION__MAX,
> } MultiFDCompression;
>
> > @@ -604,6 +745,7 @@ int multifd_save_setup(Error **errp)
> > multifd_send_state->pages = multifd_pages_init(page_count);
> > qemu_sem_init(&multifd_send_state->channels_ready, 0);
> > atomic_set(&multifd_send_state->exiting, 0);
> > + multifd_send_state->ops = multifd_ops[migrate_multifd_compression()];
>
> Here we take the result of the function and use it as an
> array index into multifd_ops, whose definition is
> static MultiFDMethods *multifd_ops[MULTIFD_COMPRESSION__MAX] = { ... }
>
> Coverity doesn't see any reason why the return value from
> migrate_multifd_compression() can't be MULTIFD_COMPRESSION__MAX,
> so it complains that if it is then we are going to index off the
> end of the array.
>
> An assert in migrate_multifd_compression() that the value being
> returned is within the expected range would probably placate it.
>
> Alternatively, if the qapi type codegen didn't put the __MAX
> value as a possible value of the enum type then Coverity
> and probably also the compiler wouldn't consider it to be
> a possible value of this kind of variable. But that might
> have other awkward side-effects.
>
> thanks
> -- PMM
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PULL 03/15] multifd: Make no compression operations into its own structure
2022-05-13 17:56 ` Peter Maydell
@ 2022-07-19 14:06 ` Peter Maydell
0 siblings, 0 replies; 27+ messages in thread
From: Peter Maydell @ 2022-07-19 14:06 UTC (permalink / raw)
To: Juan Quintela
Cc: qemu-devel, Dr. David Alan Gilbert, Markus Armbruster, Michael Roth
Ping^2! I remain unsure how we should resolve this Coverity complaint
and would like opinions from QAPI codegen people.
thanks
-- PMM
On Fri, 13 May 2022 at 18:56, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> Ping! Any opinions, especially from qapi codegen people,
> on the right thing to do here?
>
> On Tue, 12 Apr 2022 at 20:04, Peter Maydell <peter.maydell@linaro.org> wrote:
> >
> > On Fri, 28 Feb 2020 at 09:26, Juan Quintela <quintela@redhat.com> wrote:
> > >
> > > It will be used later.
> > >
> > > Signed-off-by: Juan Quintela <quintela@redhat.com>
> > > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > >
> >
> > Hi; Coverity thinks there might be a buffer overrun here.
> > It's probably wrong, but it's not completely obvious why
> > it can't happen, so an assert somewhere would help...
> > (This is CID 1487239.)
> >
> > > +MultiFDCompression migrate_multifd_compression(void)
> > > +{
> > > + MigrationState *s;
> > > +
> > > + s = migrate_get_current();
> > > +
> > > + return s->parameters.multifd_compression;
> >
> > This function returns an enum of type MultiFDCompression,
> > whose (autogenerated from QAPI) definition is:
> >
> > typedef enum MultiFDCompression {
> > MULTIFD_COMPRESSION_NONE,
> > MULTIFD_COMPRESSION_ZLIB,
> > #if defined(CONFIG_ZSTD)
> > MULTIFD_COMPRESSION_ZSTD,
> > #endif /* defined(CONFIG_ZSTD) */
> > MULTIFD_COMPRESSION__MAX,
> > } MultiFDCompression;
> >
> > > @@ -604,6 +745,7 @@ int multifd_save_setup(Error **errp)
> > > multifd_send_state->pages = multifd_pages_init(page_count);
> > > qemu_sem_init(&multifd_send_state->channels_ready, 0);
> > > atomic_set(&multifd_send_state->exiting, 0);
> > > + multifd_send_state->ops = multifd_ops[migrate_multifd_compression()];
> >
> > Here we take the result of the function and use it as an
> > array index into multifd_ops, whose definition is
> > static MultiFDMethods *multifd_ops[MULTIFD_COMPRESSION__MAX] = { ... }
> >
> > Coverity doesn't see any reason why the return value from
> > migrate_multifd_compression() can't be MULTIFD_COMPRESSION__MAX,
> > so it complains that if it is then we are going to index off the
> > end of the array.
> >
> > An assert in migrate_multifd_compression() that the value being
> > returned is within the expected range would probably placate it.
> >
> > Alternatively, if the qapi type codegen didn't put the __MAX
> > value as a possible value of the enum type then Coverity
> > and probably also the compiler wouldn't consider it to be
> > a possible value of this kind of variable. But that might
> > have other awkward side-effects.
> >
> > thanks
> > -- PMM
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PULL 03/15] multifd: Make no compression operations into its own structure
2022-04-12 19:04 ` Peter Maydell
2022-05-13 17:56 ` Peter Maydell
@ 2022-07-19 14:52 ` Markus Armbruster
1 sibling, 0 replies; 27+ messages in thread
From: Markus Armbruster @ 2022-07-19 14:52 UTC (permalink / raw)
To: Peter Maydell
Cc: Juan Quintela, qemu-devel, Dr. David Alan Gilbert, Michael Roth,
John Snow, Victor Toso, Marc-André Lureau
This fell through the cracks. My apologies.
Peter Maydell <peter.maydell@linaro.org> writes:
> On Fri, 28 Feb 2020 at 09:26, Juan Quintela <quintela@redhat.com> wrote:
>>
>> It will be used later.
>>
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>>
>
> Hi; Coverity thinks there might be a buffer overrun here.
> It's probably wrong,
It is; details below.
> but it's not completely obvious why
> it can't happen, so an assert somewhere would help...
> (This is CID 1487239.)
>
>> +MultiFDCompression migrate_multifd_compression(void)
>> +{
>> + MigrationState *s;
>> +
>> + s = migrate_get_current();
>> +
>> + return s->parameters.multifd_compression;
>
> This function returns an enum of type MultiFDCompression,
> whose (autogenerated from QAPI) definition is:
>
> typedef enum MultiFDCompression {
> MULTIFD_COMPRESSION_NONE,
> MULTIFD_COMPRESSION_ZLIB,
> #if defined(CONFIG_ZSTD)
> MULTIFD_COMPRESSION_ZSTD,
> #endif /* defined(CONFIG_ZSTD) */
> MULTIFD_COMPRESSION__MAX,
> } MultiFDCompression;
>
Generated from
{ 'enum': 'MultiFDCompression',
'data': [ 'none', 'zlib',
{ 'name': 'zstd', 'if': 'CONFIG_ZSTD' } ] }
>> @@ -604,6 +745,7 @@ int multifd_save_setup(Error **errp)
>> multifd_send_state->pages = multifd_pages_init(page_count);
>> qemu_sem_init(&multifd_send_state->channels_ready, 0);
>> atomic_set(&multifd_send_state->exiting, 0);
>> + multifd_send_state->ops = multifd_ops[migrate_multifd_compression()];
>
> Here we take the result of the function and use it as an
> array index into multifd_ops, whose definition is
> static MultiFDMethods *multifd_ops[MULTIFD_COMPRESSION__MAX] = { ... }
>
> Coverity doesn't see any reason why the return value from
> migrate_multifd_compression() can't be MULTIFD_COMPRESSION__MAX,
> so it complains that if it is then we are going to index off the
> end of the array.
Yes.
migrate_multifd_compression() returns
current_migration->parameters.multifd_compression.
.multifd_compression is zero-initialized to MULTIFD_COMPRESSION_NONE,
and modified only by qmp_migrate_set_parameters().
qmp_migrate_set_parameters() can be called on behalf of QMP command
migrate-set-parameters, and on behalf of HMP command
migrate_set_parameter. In either case, the value is the result of
parsing a string with qapi_enum_parse(), via visit_type_enum() and
visit_type_MultiFDCompression() with an input visitor (qobject for QMP,
string for HMP). Never assigns the enum's __MAX.
> An assert in migrate_multifd_compression() that the value being
> returned is within the expected range would probably placate it.
Yes.
> Alternatively, if the qapi type codegen didn't put the __MAX
> value as a possible value of the enum type then Coverity
> and probably also the compiler wouldn't consider it to be
> a possible value of this kind of variable. But that might
> have other awkward side-effects.
Yes. Coding the __MAX as a member of the enum is easy to write and easy
to understand, but gets in the way in places.
We could do something like
typedef enum MultiFDCompression {
MULTIFD_COMPRESSION_NONE,
MULTIFD_COMPRESSION_ZLIB,
#if defined(CONFIG_ZSTD)
MULTIFD_COMPRESSION_ZSTD,
#endif /* defined(CONFIG_ZSTD) */
} MultiFDCompression;
#define MULTIFD_COMPRESSION__MAX ???
where ??? is 3 if defined(CONFIG_ZSTD), else 2. The more conditionals,
the more awkward this gets.
The alternative is holes in the enum, like this:
typedef enum MultiFDCompression {
MULTIFD_COMPRESSION_NONE = 0,
MULTIFD_COMPRESSION_ZLIB = 1,
#if defined(CONFIG_ZSTD)
MULTIFD_COMPRESSION_ZSTD = 2,
#endif /* defined(CONFIG_ZSTD) */
} MultiFDCompression;
#define MULTIFD_COMPRESSION__MAX 3
Also puts holes into the lookup tables.
We'd need to review code to make sure we're not breaking "no holes"
assumptions.
Changing the __MAX from enum member to macro could conceivably break
something, too.
The quick fix is an assertion.
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2022-07-19 14:53 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-28 9:24 [PULL 00/15] Pull migration patches Juan Quintela
2020-02-28 9:24 ` [PULL 01/15] multifd: Add multifd-compression parameter Juan Quintela
2020-02-28 9:24 ` [PULL 02/15] migration: Add support for modules Juan Quintela
2020-02-28 9:24 ` [PULL 03/15] multifd: Make no compression operations into its own structure Juan Quintela
2022-04-12 19:04 ` Peter Maydell
2022-05-13 17:56 ` Peter Maydell
2022-07-19 14:06 ` Peter Maydell
2022-07-19 14:52 ` Markus Armbruster
2020-02-28 9:24 ` [PULL 04/15] multifd: Add multifd-zlib-level parameter Juan Quintela
2020-02-28 9:24 ` [PULL 05/15] multifd: Add zlib compression multifd support Juan Quintela
2020-02-28 9:24 ` [PULL 06/15] configure: Enable test and libs for zstd Juan Quintela
2020-02-29 20:06 ` Richard Henderson
2020-03-02 8:00 ` Juan Quintela
2020-03-02 8:32 ` Alex Bennée
2020-03-17 17:09 ` Peter Maydell
2020-03-17 17:40 ` Juan Quintela
2020-02-28 9:24 ` [PULL 07/15] multifd: Add multifd-zstd-level parameter Juan Quintela
2020-02-28 9:24 ` [PULL 08/15] multifd: Add zstd compression multifd support Juan Quintela
2020-02-28 9:24 ` [PULL 09/15] migration/vmstate: Remove redundant statement in vmstate_save_state_v() Juan Quintela
2020-02-28 9:24 ` [PULL 10/15] test-vmstate: Fix memleaks in test_load_qlist Juan Quintela
2020-02-28 9:24 ` [PULL 11/15] migration/savevm: release gslist after dump_vmstate_json Juan Quintela
2020-02-28 9:24 ` [PULL 12/15] migration/block: rename BLOCK_SIZE macro Juan Quintela
2022-05-12 16:22 ` Peter Maydell
2020-02-28 9:24 ` [PULL 13/15] migration: fix COLO broken caused by a previous commit Juan Quintela
2020-02-28 9:24 ` [PULL 14/15] migration/colo: wrap incoming checkpoint process into new helper Juan Quintela
2020-02-28 9:24 ` [PULL 15/15] savevm: Don't call colo_init_ram_cache twice Juan Quintela
2020-02-28 16:01 ` [PULL 00/15] Pull migration patches Peter Maydell
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.