* [PATCH v5 0/8] Multifd Migration Compression
@ 2020-01-29 11:56 Juan Quintela
2020-01-29 11:56 ` [PATCH v5 1/8] multifd: Add multifd-method parameter Juan Quintela
` (7 more replies)
0 siblings, 8 replies; 39+ messages in thread
From: Juan Quintela @ 2020-01-29 11:56 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, Juan Quintela, Dr. David Alan Gilbert,
Markus Armbruster, Paolo Bonzini
Based-on: 20200129111536.9497-1-quintela@redhat.com
[v5]
- rebased on top of latest pull request
- check zlib/zstd return codes in loops (denis suggestions)
- check that we do the right thing in error cases (I corrupted the
stream, make buffers too small, etc by hand)
[v4]
- create new parameters: multifd-zlib-level & multifd-zstd-level
- use proper "conditionals" for qapi (thanks markus)
- more than half of the patches moved to the migration PULL request
that this series are based on
- method type has moved to one int from a set of flags
- fixed all reviews comments
Please review.
[v3]
- rebased on top of upstream + previous multifd cancel series
- split multifd code into its own file (multifd.[ch])
- split zstd/zlib compression methods (multifd-zstd/zlib.c)
- use qemu module feauture to avoid ifdefs
(my understanding is that zlib needs to be present, but
we setup zstd only if it is not there or is disabled)
- multifd-method: none|zlib|zstd
As far as I can see, there is no easy way to convince qapi that zstd
option could/couldn't be there depending on compliation flags. I
ended just checking in migrate_parameters_check() if it is enabled
and giving an error message otherwise.
Questions:
- I am "reusing" the compress-level parameter for both zstd and zlib,
but it poses a problem:
* zlib values: 1-9 (default: 6?)
* zstd values: 1-19 (default: 3)
So, what should I do:
* create multifd-zstd-level and multifd-zlib-level (easier)
* reuse compress-level, and change its maximum values depending on
multifd-method
* any other good option?
Please, review.
[v2] - rebase on top of previous arguments posted to the list -
introduces zlib compression - introduces zstd compression
Please help if you know anything about zstd/zlib compression.
This puts compression on top of multifd. Advantages about current
compression:
- We copy all pages in a single packet and then compress the whole
thing.
- We reuse the compression stream for all the packets sent through the
same channel.
- We can select nocomp/zlib/zstd levels of compression.
Please, review.
Juan Quintela (8):
multifd: Add multifd-method 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
configure | 30 ++++
hw/core/qdev-properties.c | 13 ++
include/hw/qdev-properties.h | 3 +
include/qemu/module.h | 2 +
migration/Makefile.objs | 2 +
migration/migration.c | 70 ++++++++
migration/migration.h | 3 +
migration/multifd-zlib.c | 325 +++++++++++++++++++++++++++++++++
migration/multifd-zstd.c | 337 +++++++++++++++++++++++++++++++++++
migration/multifd.c | 191 +++++++++++++++++++-
migration/multifd.h | 29 +++
migration/ram.c | 2 +-
monitor/hmp-cmds.c | 21 +++
qapi/migration.json | 80 ++++++++-
tests/qtest/migration-test.c | 30 +++-
vl.c | 1 +
16 files changed, 1123 insertions(+), 16 deletions(-)
create mode 100644 migration/multifd-zlib.c
create mode 100644 migration/multifd-zstd.c
--
2.24.1
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v5 1/8] multifd: Add multifd-method parameter
2020-01-29 11:56 [PATCH v5 0/8] Multifd Migration Compression Juan Quintela
@ 2020-01-29 11:56 ` Juan Quintela
2020-01-30 7:54 ` Markus Armbruster
2020-02-11 18:50 ` Daniel P. Berrangé
2020-01-29 11:56 ` [PATCH v5 2/8] migration: Add support for modules Juan Quintela
` (6 subsequent siblings)
7 siblings, 2 replies; 39+ messages in thread
From: Juan Quintela @ 2020-01-29 11:56 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, Juan Quintela, Dr. David Alan Gilbert,
Markus Armbruster, Paolo Bonzini
This will store the compression method to use. We start with none.
Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
hw/core/qdev-properties.c | 13 +++++++++++++
include/hw/qdev-properties.h | 3 +++
migration/migration.c | 13 +++++++++++++
monitor/hmp-cmds.c | 13 +++++++++++++
qapi/migration.json | 30 +++++++++++++++++++++++++++---
tests/qtest/migration-test.c | 14 ++++++++++----
6 files changed, 79 insertions(+), 7 deletions(-)
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 7f93bfeb88..4442844d37 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,
};
+/* --- MultiFDMethod --- */
+
+const PropertyInfo qdev_prop_multifd_method = {
+ .name = "MultiFDMethod",
+ .description = "multifd_method values, "
+ "none",
+ .enum_table = &MultiFDMethod_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..6871b075a6 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_method;
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,8 @@ 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_METHOD(_n, _s, _f, _d) \
+ DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_multifd_method, MultiFDMethod)
#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 3a21a4686c..cd72bb6e9a 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_METHOD MULTIFD_METHOD_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_method = true;
+ params->multifd_method = s->parameters.multifd_method;
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_method) {
+ dest->multifd_method = params->multifd_method;
+ }
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_method) {
+ s->parameters.multifd_method = params->multifd_method;
+ }
if (params->has_xbzrle_cache_size) {
s->parameters.xbzrle_cache_size = params->xbzrle_cache_size;
xbzrle_cache_resize(params->xbzrle_cache_size, errp);
@@ -3515,6 +3524,9 @@ static Property migration_properties[] = {
DEFINE_PROP_UINT8("multifd-channels", MigrationState,
parameters.multifd_channels,
DEFAULT_MIGRATE_MULTIFD_CHANNELS),
+ DEFINE_PROP_MULTIFD_METHOD("multifd-method", MigrationState,
+ parameters.multifd_method,
+ DEFAULT_MIGRATE_MULTIFD_METHOD),
DEFINE_PROP_SIZE("xbzrle-cache-size", MigrationState,
parameters.xbzrle_cache_size,
DEFAULT_MIGRATE_XBZRLE_CACHE_SIZE),
@@ -3605,6 +3617,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_method = 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 d0e0af893a..16f01d4244 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -39,6 +39,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"
@@ -447,6 +448,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_METHOD),
+ MultiFDMethod_str(params->multifd_method));
monitor_printf(mon, "%s: %" PRIu64 "\n",
MigrationParameter_str(MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE),
params->xbzrle_cache_size);
@@ -1738,6 +1742,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;
+ MultiFDMethod compress_type;
Error *err = NULL;
int val, ret;
@@ -1823,6 +1828,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_METHOD:
+ p->has_multifd_method = true;
+ visit_type_MultiFDMethod(v, param, &compress_type, &err);
+ if (err) {
+ break;
+ }
+ p->multifd_method = 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 b7348d0c8b..96a126751c 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -488,6 +488,19 @@
##
{ 'command': 'query-migrate-capabilities', 'returns': ['MigrationCapabilityStatus']}
+##
+# @MultiFDMethod:
+#
+# An enumeration of multifd compression.
+#
+# @none: no compression.
+#
+# Since: 5.0
+#
+##
+{ 'enum': 'MultiFDMethod',
+ 'data': [ 'none' ] }
+
##
# @MigrationParameter:
#
@@ -586,6 +599,9 @@
# @max-cpu-throttle: maximum cpu throttle percentage.
# Defaults to 99. (Since 3.1)
#
+# @multifd-method: 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-method' ] }
##
# @MigrateSetParameters:
@@ -688,6 +704,9 @@
# @max-cpu-throttle: maximum cpu throttle percentage.
# The default value is 99. (Since 3.1)
#
+# @multifd-method: 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-method': 'MultiFDMethod' } }
##
# @migrate-set-parameters:
@@ -823,6 +843,9 @@
# Defaults to 99.
# (Since 3.1)
#
+# @multifd-method: 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-method': 'MultiFDMethod' } }
##
# @query-migrate-parameters:
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index cf27ebbc9d..d2f9ef38f5 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)
{
@@ -1251,7 +1250,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;
@@ -1275,6 +1274,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-method", method);
+ migrate_set_parameter_str(to, "multifd-method", method);
+
migrate_set_capability(from, "multifd", "true");
migrate_set_capability(to, "multifd", "true");
@@ -1306,6 +1308,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
@@ -1317,7 +1324,6 @@ static void test_multifd_tcp(void)
*
* And see that it works
*/
-
static void test_multifd_tcp_cancel(void)
{
MigrateStart *args = migrate_start_new();
@@ -1467,7 +1473,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] 39+ messages in thread
* [PATCH v5 2/8] migration: Add support for modules
2020-01-29 11:56 [PATCH v5 0/8] Multifd Migration Compression Juan Quintela
2020-01-29 11:56 ` [PATCH v5 1/8] multifd: Add multifd-method parameter Juan Quintela
@ 2020-01-29 11:56 ` Juan Quintela
2020-02-11 10:54 ` Dr. David Alan Gilbert
2020-01-29 11:56 ` [PATCH v5 3/8] multifd: Make no compression operations into its own structure Juan Quintela
` (5 subsequent siblings)
7 siblings, 1 reply; 39+ messages in thread
From: Juan Quintela @ 2020-01-29 11:56 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, Juan Quintela, Dr. David Alan Gilbert,
Markus Armbruster, Paolo Bonzini
So we don't have to compile everything in, or have ifdefs
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
include/qemu/module.h | 2 ++
vl.c | 1 +
2 files changed, 3 insertions(+)
diff --git a/include/qemu/module.h b/include/qemu/module.h
index 65ba596e46..907cb5c0a5 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,
@@ -56,6 +57,7 @@ typedef enum {
#define xen_backend_init(function) module_init(function, \
MODULE_INIT_XEN_BACKEND)
#define libqos_init(function) module_init(function, MODULE_INIT_LIBQOS)
+#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)
diff --git a/vl.c b/vl.c
index b0f52c4d6e..9f8577955a 100644
--- a/vl.c
+++ b/vl.c
@@ -2890,6 +2890,7 @@ int main(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] 39+ messages in thread
* [PATCH v5 3/8] multifd: Make no compression operations into its own structure
2020-01-29 11:56 [PATCH v5 0/8] Multifd Migration Compression Juan Quintela
2020-01-29 11:56 ` [PATCH v5 1/8] multifd: Add multifd-method parameter Juan Quintela
2020-01-29 11:56 ` [PATCH v5 2/8] migration: Add support for modules Juan Quintela
@ 2020-01-29 11:56 ` Juan Quintela
2020-02-07 18:45 ` Dr. David Alan Gilbert
2020-01-29 11:56 ` [PATCH v5 4/8] multifd: Add multifd-zlib-level parameter Juan Quintela
` (4 subsequent siblings)
7 siblings, 1 reply; 39+ messages in thread
From: Juan Quintela @ 2020-01-29 11:56 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, Juan Quintela, Dr. David Alan Gilbert,
Markus Armbruster, Paolo Bonzini
It will be used later.
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration/migration.c | 9 ++
migration/migration.h | 1 +
migration/multifd.c | 185 ++++++++++++++++++++++++++++++++++++++++--
migration/multifd.h | 25 ++++++
migration/ram.c | 1 +
5 files changed, 213 insertions(+), 8 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index cd72bb6e9a..06f6c2d529 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2245,6 +2245,15 @@ int migrate_multifd_channels(void)
return s->parameters.multifd_channels;
}
+MultiFDMethod migrate_multifd_method(void)
+{
+ MigrationState *s;
+
+ s = migrate_get_current();
+
+ return s->parameters.multifd_method;
+}
+
int migrate_use_xbzrle(void)
{
MigrationState *s;
diff --git a/migration/migration.h b/migration/migration.h
index 8473ddfc88..3d23a0852e 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);
+MultiFDMethod migrate_multifd_method(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..1c49c2a665 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_METHOD_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_METHOD__MAX] = {
+ [MULTIFD_METHOD_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_method()];
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_method()];
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..c7fea4914c 100644
--- a/migration/multifd.h
+++ b/migration/multifd.h
@@ -25,6 +25,10 @@ int multifd_queue_page(QEMUFile *f, RAMBlock *block, ram_addr_t offset);
#define MULTIFD_FLAG_SYNC (1 << 0)
+/* We reserve 3 bits for METHODS */
+#define MULTIFD_FLAG_METHOD_MASK (7 << 1)
+#define MULTIFD_FLAG_NOCOMP (1 << 1)
+
/* This value needs to be a multiple of qemu_target_page_size() */
#define MULTIFD_PACKET_SIZE (512 * 1024)
@@ -96,6 +100,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 +139,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] 39+ messages in thread
* [PATCH v5 4/8] multifd: Add multifd-zlib-level parameter
2020-01-29 11:56 [PATCH v5 0/8] Multifd Migration Compression Juan Quintela
` (2 preceding siblings ...)
2020-01-29 11:56 ` [PATCH v5 3/8] multifd: Make no compression operations into its own structure Juan Quintela
@ 2020-01-29 11:56 ` Juan Quintela
2020-01-30 8:03 ` Markus Armbruster
2020-01-29 11:56 ` [PATCH v5 5/8] multifd: Add zlib compression multifd support Juan Quintela
` (3 subsequent siblings)
7 siblings, 1 reply; 39+ messages in thread
From: Juan Quintela @ 2020-01-29 11:56 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, Juan Quintela, Dr. David Alan Gilbert,
Markus Armbruster, Paolo Bonzini
It will indicate which level use for compression.
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration/migration.c | 15 +++++++++++++++
monitor/hmp-cmds.c | 4 ++++
qapi/migration.json | 30 +++++++++++++++++++++++++++---
3 files changed, 46 insertions(+), 3 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index 06f6c2d529..4f88f8e958 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_METHOD MULTIFD_METHOD_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_method = true;
params->multifd_method = s->parameters.multifd_method;
+ 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))) {
@@ -3536,6 +3547,9 @@ static Property migration_properties[] = {
DEFINE_PROP_MULTIFD_METHOD("multifd-method", MigrationState,
parameters.multifd_method,
DEFAULT_MIGRATE_MULTIFD_METHOD),
+ 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),
@@ -3627,6 +3641,7 @@ static void migration_instance_init(Object *obj)
params->has_block_incremental = true;
params->has_multifd_channels = true;
params->has_multifd_method = 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/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 16f01d4244..7f11866446 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1836,6 +1836,10 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
}
p->multifd_method = 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 96a126751c..289dce0da7 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -602,6 +602,13 @@
# @multifd-method: 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-method' ] }
+ 'max-cpu-throttle', 'multifd-method',
+ 'multifd-zlib-level' ] }
##
# @MigrateSetParameters:
@@ -707,6 +715,13 @@
# @multifd-method: 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-method': 'MultiFDMethod' } }
+ '*multifd-method': 'MultiFDMethod',
+ '*multifd-zlib-level': 'int' } }
##
# @migrate-set-parameters:
@@ -846,6 +862,13 @@
# @multifd-method: 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-method': 'MultiFDMethod' } }
+ '*multifd-method': 'MultiFDMethod',
+ '*multifd-zlib-level': 'uint8' } }
##
# @query-migrate-parameters:
--
2.24.1
^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v5 5/8] multifd: Add zlib compression multifd support
2020-01-29 11:56 [PATCH v5 0/8] Multifd Migration Compression Juan Quintela
` (3 preceding siblings ...)
2020-01-29 11:56 ` [PATCH v5 4/8] multifd: Add multifd-zlib-level parameter Juan Quintela
@ 2020-01-29 11:56 ` Juan Quintela
2020-01-30 8:04 ` Markus Armbruster
2020-02-11 18:43 ` Dr. David Alan Gilbert
2020-01-29 11:56 ` [PATCH v5 6/8] configure: Enable test and libs for zstd Juan Quintela
` (2 subsequent siblings)
7 siblings, 2 replies; 39+ messages in thread
From: Juan Quintela @ 2020-01-29 11:56 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, Juan Quintela, Dr. David Alan Gilbert,
Markus Armbruster, Paolo Bonzini
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
hw/core/qdev-properties.c | 2 +-
migration/Makefile.objs | 1 +
migration/migration.c | 9 +
migration/migration.h | 1 +
migration/multifd-zlib.c | 325 +++++++++++++++++++++++++++++++++++
migration/multifd.c | 6 +
migration/multifd.h | 4 +
qapi/migration.json | 3 +-
tests/qtest/migration-test.c | 6 +
9 files changed, 355 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 4442844d37..bf88a50cdf 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_method = {
.name = "MultiFDMethod",
.description = "multifd_method values, "
- "none",
+ "none/zlib",
.enum_table = &MultiFDMethod_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/migration.c b/migration/migration.c
index 4f88f8e958..3b081e8147 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2265,6 +2265,15 @@ MultiFDMethod migrate_multifd_method(void)
return s->parameters.multifd_method;
}
+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;
diff --git a/migration/migration.h b/migration/migration.h
index 3d23a0852e..95e9c196ff 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);
MultiFDMethod migrate_multifd_method(void);
+int migrate_multifd_zlib_level(void);
int migrate_use_xbzrle(void);
int64_t migrate_xbzrle_cache_size(void);
diff --git a/migration/multifd-zlib.c b/migration/multifd-zlib.c
new file mode 100644
index 0000000000..91b3128256
--- /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_METHOD_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_METHOD_ZLIB, &multifd_zlib_ops);
+}
+
+migration_init(multifd_zlib_register);
diff --git a/migration/multifd.c b/migration/multifd.c
index 1c49c2a665..7bb9c3582f 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -166,6 +166,12 @@ static MultiFDMethods *multifd_ops[MULTIFD_METHOD__MAX] = {
[MULTIFD_METHOD_NONE] = &multifd_nocomp_ops,
};
+void multifd_register_ops(int method, MultiFDMethods *ops)
+{
+ assert(0 < method && method < MULTIFD_METHOD__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 c7fea4914c..3fa5132f1d 100644
--- a/migration/multifd.h
+++ b/migration/multifd.h
@@ -23,11 +23,13 @@ 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 METHODS */
#define MULTIFD_FLAG_METHOD_MASK (7 << 1)
#define MULTIFD_FLAG_NOCOMP (1 << 1)
+#define MULTIFD_FLAG_ZLIB (2 << 1)
/* This value needs to be a multiple of qemu_target_page_size() */
#define MULTIFD_PACKET_SIZE (512 * 1024)
@@ -160,5 +162,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 289dce0da7..032ee7d3e6 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -494,12 +494,13 @@
# An enumeration of multifd compression.
#
# @none: no compression.
+# @zlib: use zlib compression method.
#
# Since: 5.0
#
##
{ 'enum': 'MultiFDMethod',
- 'data': [ 'none' ] }
+ 'data': [ 'none', 'zlib' ] }
##
# @MigrationParameter:
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index d2f9ef38f5..8effed205d 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -1313,6 +1313,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
@@ -1475,6 +1480,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] 39+ messages in thread
* [PATCH v5 6/8] configure: Enable test and libs for zstd
2020-01-29 11:56 [PATCH v5 0/8] Multifd Migration Compression Juan Quintela
` (4 preceding siblings ...)
2020-01-29 11:56 ` [PATCH v5 5/8] multifd: Add zlib compression multifd support Juan Quintela
@ 2020-01-29 11:56 ` Juan Quintela
2020-02-11 20:11 ` Daniel P. Berrangé
2020-01-29 11:56 ` [PATCH v5 7/8] multifd: Add multifd-zstd-level parameter Juan Quintela
2020-01-29 11:56 ` [PATCH v5 8/8] multifd: Add zstd compression multifd support Juan Quintela
7 siblings, 1 reply; 39+ messages in thread
From: Juan Quintela @ 2020-01-29 11:56 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, Juan Quintela, Dr. David Alan Gilbert,
Markus Armbruster, Paolo Bonzini
Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
configure | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/configure b/configure
index a72a5def57..7c1dca326f 100755
--- a/configure
+++ b/configure
@@ -448,6 +448,7 @@ lzo=""
snappy=""
bzip2=""
lzfse=""
+zstd=""
guest_agent=""
guest_agent_with_vss="no"
guest_agent_ntddscsi="no"
@@ -1343,6 +1344,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"
@@ -1795,6 +1800,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
@@ -2409,6 +2416,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
@@ -6578,6 +6603,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"
@@ -7143,6 +7169,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
--
2.24.1
^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v5 7/8] multifd: Add multifd-zstd-level parameter
2020-01-29 11:56 [PATCH v5 0/8] Multifd Migration Compression Juan Quintela
` (5 preceding siblings ...)
2020-01-29 11:56 ` [PATCH v5 6/8] configure: Enable test and libs for zstd Juan Quintela
@ 2020-01-29 11:56 ` Juan Quintela
2020-01-30 8:08 ` Markus Armbruster
2020-02-11 18:47 ` Dr. David Alan Gilbert
2020-01-29 11:56 ` [PATCH v5 8/8] multifd: Add zstd compression multifd support Juan Quintela
7 siblings, 2 replies; 39+ messages in thread
From: Juan Quintela @ 2020-01-29 11:56 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, Juan Quintela, Dr. David Alan Gilbert,
Markus Armbruster, Paolo Bonzini
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration/migration.c | 15 +++++++++++++++
monitor/hmp-cmds.c | 4 ++++
qapi/migration.json | 29 ++++++++++++++++++++++++++---
3 files changed, 45 insertions(+), 3 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index 3b081e8147..b690500545 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -91,6 +91,8 @@
#define DEFAULT_MIGRATE_MULTIFD_METHOD MULTIFD_METHOD_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_method = s->parameters.multifd_method;
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))) {
@@ -3559,6 +3570,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),
@@ -3651,6 +3665,7 @@ static void migration_instance_init(Object *obj)
params->has_multifd_channels = true;
params->has_multifd_method = 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/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 7f11866446..87db07694b 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1840,6 +1840,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 032ee7d3e6..bb5cb6b4f4 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-method',
- '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-method': 'MultiFDMethod',
- '*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-method': 'MultiFDMethod',
- '*multifd-zlib-level': 'uint8' } }
+ '*multifd-zlib-level': 'uint8',
+ '*multifd-zstd-level': 'uint8' } }
##
# @query-migrate-parameters:
--
2.24.1
^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH v5 8/8] multifd: Add zstd compression multifd support
2020-01-29 11:56 [PATCH v5 0/8] Multifd Migration Compression Juan Quintela
` (6 preceding siblings ...)
2020-01-29 11:56 ` [PATCH v5 7/8] multifd: Add multifd-zstd-level parameter Juan Quintela
@ 2020-01-29 11:56 ` Juan Quintela
2020-01-30 8:08 ` Markus Armbruster
2020-02-11 20:01 ` Dr. David Alan Gilbert
7 siblings, 2 replies; 39+ messages in thread
From: Juan Quintela @ 2020-01-29 11:56 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, Juan Quintela, Dr. David Alan Gilbert,
Markus Armbruster, Paolo Bonzini
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
hw/core/qdev-properties.c | 2 +-
migration/Makefile.objs | 1 +
migration/migration.c | 9 +
migration/migration.h | 1 +
migration/multifd-zstd.c | 337 +++++++++++++++++++++++++++++++++++
migration/multifd.h | 2 +-
migration/ram.c | 1 -
qapi/migration.json | 4 +-
tests/qtest/migration-test.c | 10 ++
9 files changed, 363 insertions(+), 4 deletions(-)
create mode 100644 migration/multifd-zstd.c
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index bf88a50cdf..9440ca78c3 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_method = {
.name = "MultiFDMethod",
.description = "multifd_method values, "
- "none/zlib",
+ "none/zlib/zstd",
.enum_table = &MultiFDMethod_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/migration.c b/migration/migration.c
index b690500545..aff081128c 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2285,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;
diff --git a/migration/migration.h b/migration/migration.h
index 95e9c196ff..2eb72aee0a 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -302,6 +302,7 @@ bool migrate_pause_before_switchover(void);
int migrate_multifd_channels(void);
MultiFDMethod migrate_multifd_method(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/migration/multifd-zstd.c b/migration/multifd-zstd.c
new file mode 100644
index 0000000000..6383a8a898
--- /dev/null
+++ b/migration/multifd-zstd.c
@@ -0,0 +1,337 @@
+/*
+ * 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", 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) {
+ 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 res;
+
+ 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;
+ }
+
+ res = ZSTD_initDStream(z->zds);
+ if (ZSTD_isError(res)) {
+ ZSTD_freeDStream(z->zds);
+ g_free(z);
+ error_setg(errp, "multifd %d: initDStream 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) {
+ 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_METHOD_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_METHOD_ZSTD, &multifd_zstd_ops);
+}
+
+migration_init(multifd_zstd_register);
diff --git a/migration/multifd.h b/migration/multifd.h
index 3fa5132f1d..621db316c1 100644
--- a/migration/multifd.h
+++ b/migration/multifd.h
@@ -30,6 +30,7 @@ int multifd_queue_page(QEMUFile *f, RAMBlock *block, ram_addr_t offset);
#define MULTIFD_FLAG_METHOD_MASK (7 << 1)
#define MULTIFD_FLAG_NOCOMP (1 << 1)
#define MULTIFD_FLAG_ZLIB (2 << 1)
+#define MULTIFD_FLAG_ZSTD (3 << 1)
/* This value needs to be a multiple of qemu_target_page_size() */
#define MULTIFD_PACKET_SIZE (512 * 1024)
@@ -163,6 +164,5 @@ typedef struct {
} MultiFDMethods;
void multifd_register_ops(int method, MultiFDMethods *ops);
-
#endif
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 bb5cb6b4f4..8ccec7fa79 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': 'MultiFDMethod',
- '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 8effed205d..ec9be28bc9 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -1318,6 +1318,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
@@ -1481,6 +1488,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] 39+ messages in thread
* Re: [PATCH v5 1/8] multifd: Add multifd-method parameter
2020-01-29 11:56 ` [PATCH v5 1/8] multifd: Add multifd-method parameter Juan Quintela
@ 2020-01-30 7:54 ` Markus Armbruster
2020-01-30 9:11 ` Juan Quintela
2020-02-11 18:50 ` Daniel P. Berrangé
1 sibling, 1 reply; 39+ messages in thread
From: Markus Armbruster @ 2020-01-30 7:54 UTC (permalink / raw)
To: Juan Quintela
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, Markus Armbruster, qemu-devel,
Dr. David Alan Gilbert, Paolo Bonzini
Juan Quintela <quintela@redhat.com> writes:
> This will store the compression method to use. We start with none.
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
I don't remember giving my R-by. I gave my Acked-by for
[PATCH v2 06/10] migration: Add multifd-compress parameter
Message-ID: <87d0cku5hq.fsf@dusky.pond.sub.org>
https://lists.nongnu.org/archive/html/qemu-devel/2019-12/msg04153.html
If I did give my R-by for a later revision, let me know.
One small suggestion below.
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
[...]
> diff --git a/qapi/migration.json b/qapi/migration.json
> index b7348d0c8b..96a126751c 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -488,6 +488,19 @@
> ##
> { 'command': 'query-migrate-capabilities', 'returns': ['MigrationCapabilityStatus']}
>
> +##
> +# @MultiFDMethod:
> +#
> +# An enumeration of multifd compression.
Suggest "compression methods."
> +#
> +# @none: no compression.
> +#
> +# Since: 5.0
> +#
> +##
> +{ 'enum': 'MultiFDMethod',
> + 'data': [ 'none' ] }
> +
> ##
> # @MigrationParameter:
> #
> @@ -586,6 +599,9 @@
> # @max-cpu-throttle: maximum cpu throttle percentage.
> # Defaults to 99. (Since 3.1)
> #
> +# @multifd-method: 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-method' ] }
>
> ##
> # @MigrateSetParameters:
> @@ -688,6 +704,9 @@
> # @max-cpu-throttle: maximum cpu throttle percentage.
> # The default value is 99. (Since 3.1)
> #
> +# @multifd-method: 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-method': 'MultiFDMethod' } }
>
> ##
> # @migrate-set-parameters:
> @@ -823,6 +843,9 @@
> # Defaults to 99.
> # (Since 3.1)
> #
> +# @multifd-method: 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-method': 'MultiFDMethod' } }
>
> ##
> # @query-migrate-parameters:
[...]
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v5 4/8] multifd: Add multifd-zlib-level parameter
2020-01-29 11:56 ` [PATCH v5 4/8] multifd: Add multifd-zlib-level parameter Juan Quintela
@ 2020-01-30 8:03 ` Markus Armbruster
2020-01-30 8:56 ` Juan Quintela
2020-02-11 18:57 ` Daniel P. Berrangé
0 siblings, 2 replies; 39+ messages in thread
From: Markus Armbruster @ 2020-01-30 8:03 UTC (permalink / raw)
To: Juan Quintela
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, qemu-devel, Dr. David Alan Gilbert,
Paolo Bonzini
Juan Quintela <quintela@redhat.com> writes:
> It will indicate which level use for compression.
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
This is slightly confusing (there is no zlib compression), unless you
peek at the next patch (which adds zlib compression).
Three ways to make it less confusing:
* Squash the two commits
* Swap them: first add zlib compression with level hardcoded to 1, then
make the level configurable.
* Have the first commit explain itself better. Something like
multifd: Add multifd-zlib-level parameter
This parameter specifies zlib compression level. The next patch
will put it to use.
For QAPI:
Acked-by: Markus Armbruster <armbru@redhat.com>
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v5 5/8] multifd: Add zlib compression multifd support
2020-01-29 11:56 ` [PATCH v5 5/8] multifd: Add zlib compression multifd support Juan Quintela
@ 2020-01-30 8:04 ` Markus Armbruster
2020-02-11 18:43 ` Dr. David Alan Gilbert
1 sibling, 0 replies; 39+ messages in thread
From: Markus Armbruster @ 2020-01-30 8:04 UTC (permalink / raw)
To: Juan Quintela
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, qemu-devel, Dr. David Alan Gilbert,
Paolo Bonzini
Juan Quintela <quintela@redhat.com> writes:
> Signed-off-by: Juan Quintela <quintela@redhat.com>
For QAPI:
Acked-by: Markus Armbruster <armbru@redhat.com>
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v5 7/8] multifd: Add multifd-zstd-level parameter
2020-01-29 11:56 ` [PATCH v5 7/8] multifd: Add multifd-zstd-level parameter Juan Quintela
@ 2020-01-30 8:08 ` Markus Armbruster
2020-02-11 18:47 ` Dr. David Alan Gilbert
1 sibling, 0 replies; 39+ messages in thread
From: Markus Armbruster @ 2020-01-30 8:08 UTC (permalink / raw)
To: Juan Quintela
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, qemu-devel, Dr. David Alan Gilbert,
Paolo Bonzini
Juan Quintela <quintela@redhat.com> writes:
> Signed-off-by: Juan Quintela <quintela@redhat.com>
Same confusion as in PATCH 4; my proposal there applies here, too.
For QAPI:
Acked-by: Markus Armbruster <armbru@redhat.com>
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v5 8/8] multifd: Add zstd compression multifd support
2020-01-29 11:56 ` [PATCH v5 8/8] multifd: Add zstd compression multifd support Juan Quintela
@ 2020-01-30 8:08 ` Markus Armbruster
2020-02-11 20:01 ` Dr. David Alan Gilbert
1 sibling, 0 replies; 39+ messages in thread
From: Markus Armbruster @ 2020-01-30 8:08 UTC (permalink / raw)
To: Juan Quintela
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, qemu-devel, Dr. David Alan Gilbert,
Paolo Bonzini
Juan Quintela <quintela@redhat.com> writes:
> Signed-off-by: Juan Quintela <quintela@redhat.com>
For QAPI:
Acked-by: Markus Armbruster <armbru@redhat.com>
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v5 4/8] multifd: Add multifd-zlib-level parameter
2020-01-30 8:03 ` Markus Armbruster
@ 2020-01-30 8:56 ` Juan Quintela
2020-02-11 18:57 ` Daniel P. Berrangé
1 sibling, 0 replies; 39+ messages in thread
From: Juan Quintela @ 2020-01-30 8:56 UTC (permalink / raw)
To: Markus Armbruster
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, qemu-devel, Dr. David Alan Gilbert,
Paolo Bonzini
Markus Armbruster <armbru@redhat.com> wrote:
> Juan Quintela <quintela@redhat.com> writes:
>
>> It will indicate which level use for compression.
>>
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>
> This is slightly confusing (there is no zlib compression), unless you
> peek at the next patch (which adds zlib compression).
>
> Three ways to make it less confusing:
>
> * Squash the two commits
As a QAPI begginer, I feel it easier to put it in a different patch. It
makes it also easier to add other parameters, just copy whatewer is here.
> * Swap them: first add zlib compression with level hardcoded to 1, then
> make the level configurable.
That could work.
> * Have the first commit explain itself better. Something like
>
> multifd: Add multifd-zlib-level parameter
>
> This parameter specifies zlib compression level. The next patch
> will put it to use.
Will take this approach.
The reason that I put the qapi bits first is that I *know* how to do
them. Once that I got approval for how to add one parameter, I add the
rest exactly the same.
For the rest of the patch, it needs to be developed, and normally needs
more iterations.
>
> For QAPI:
> Acked-by: Markus Armbruster <armbru@redhat.com>
Thanks very much.
Later, Juan.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v5 1/8] multifd: Add multifd-method parameter
2020-01-30 7:54 ` Markus Armbruster
@ 2020-01-30 9:11 ` Juan Quintela
2020-01-30 12:17 ` Markus Armbruster
0 siblings, 1 reply; 39+ messages in thread
From: Juan Quintela @ 2020-01-30 9:11 UTC (permalink / raw)
To: Markus Armbruster
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, qemu-devel, Dr. David Alan Gilbert,
Paolo Bonzini
Markus Armbruster <armbru@redhat.com> wrote:
> Juan Quintela <quintela@redhat.com> writes:
>
>> This will store the compression method to use. We start with none.
>>
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>
> I don't remember giving my R-by. I gave my Acked-by for
> [PATCH v2 06/10] migration: Add multifd-compress parameter
> Message-ID: <87d0cku5hq.fsf@dusky.pond.sub.org>
> https://lists.nongnu.org/archive/html/qemu-devel/2019-12/msg04153.html
> If I did give my R-by for a later revision, let me know.
Hi
Ouch, I *thought* that you got confused. It happens to me all the time.
Apologies.
So, how I should I put that Acked-by: in the commit?
> One small suggestion below.
Thanks.
>> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> [...]
>> diff --git a/qapi/migration.json b/qapi/migration.json
>> index b7348d0c8b..96a126751c 100644
>> --- a/qapi/migration.json
>> +++ b/qapi/migration.json
>> @@ -488,6 +488,19 @@
>> ##
>> { 'command': 'query-migrate-capabilities', 'returns':
>> ['MigrationCapabilityStatus']}
>>
>> +##
>> +# @MultiFDMethod:
>> +#
>> +# An enumeration of multifd compression.
>
> Suggest "compression methods."
will do.
Sorry again for the missunderstanding.
Later, Juan.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v5 1/8] multifd: Add multifd-method parameter
2020-01-30 9:11 ` Juan Quintela
@ 2020-01-30 12:17 ` Markus Armbruster
0 siblings, 0 replies; 39+ messages in thread
From: Markus Armbruster @ 2020-01-30 12:17 UTC (permalink / raw)
To: Juan Quintela
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, Dr. David Alan Gilbert, qemu-devel,
Paolo Bonzini
Juan Quintela <quintela@redhat.com> writes:
> Markus Armbruster <armbru@redhat.com> wrote:
>> Juan Quintela <quintela@redhat.com> writes:
>>
>>> This will store the compression method to use. We start with none.
>>>
>>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>>> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>>
>> I don't remember giving my R-by. I gave my Acked-by for
>> [PATCH v2 06/10] migration: Add multifd-compress parameter
>> Message-ID: <87d0cku5hq.fsf@dusky.pond.sub.org>
>> https://lists.nongnu.org/archive/html/qemu-devel/2019-12/msg04153.html
>
>> If I did give my R-by for a later revision, let me know.
>
> Hi
>
> Ouch, I *thought* that you got confused. It happens to me all the time.
> Apologies.
> So, how I should I put that Acked-by: in the commit?
Replace
Reviewed-by: Markus Armbruster <armbru@redhat.com>
by
Acked-by: Markus Armbruster <armbru@redhat.com>
>> One small suggestion below.
>
> Thanks.
>
>>> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>> [...]
>>> diff --git a/qapi/migration.json b/qapi/migration.json
>>> index b7348d0c8b..96a126751c 100644
>>> --- a/qapi/migration.json
>>> +++ b/qapi/migration.json
>>> @@ -488,6 +488,19 @@
>>> ##
>>> { 'command': 'query-migrate-capabilities', 'returns':
>>> ['MigrationCapabilityStatus']}
>>>
>>> +##
>>> +# @MultiFDMethod:
>>> +#
>>> +# An enumeration of multifd compression.
>>
>> Suggest "compression methods."
>
> will do.
>
> Sorry again for the missunderstanding.
No harm, no foul :)
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v5 3/8] multifd: Make no compression operations into its own structure
2020-01-29 11:56 ` [PATCH v5 3/8] multifd: Make no compression operations into its own structure Juan Quintela
@ 2020-02-07 18:45 ` Dr. David Alan Gilbert
2020-02-11 11:23 ` Juan Quintela
0 siblings, 1 reply; 39+ messages in thread
From: Dr. David Alan Gilbert @ 2020-02-07 18:45 UTC (permalink / raw)
To: Juan Quintela
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, qemu-devel, Markus Armbruster, Paolo Bonzini
* Juan Quintela (quintela@redhat.com) wrote:
> It will be used later.
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> diff --git a/migration/multifd.h b/migration/multifd.h
> index d8b0205977..c7fea4914c 100644
> --- a/migration/multifd.h
> +++ b/migration/multifd.h
> @@ -25,6 +25,10 @@ int multifd_queue_page(QEMUFile *f, RAMBlock *block, ram_addr_t offset);
>
> #define MULTIFD_FLAG_SYNC (1 << 0)
>
> +/* We reserve 3 bits for METHODS */
> +#define MULTIFD_FLAG_METHOD_MASK (7 << 1)
> +#define MULTIFD_FLAG_NOCOMP (1 << 1)
> +
Doesn't the 'NOCOMP' value have to be 0 for it to not break
compatibility with existing multifd?
Dave
> /* This value needs to be a multiple of qemu_target_page_size() */
> #define MULTIFD_PACKET_SIZE (512 * 1024)
>
> @@ -96,6 +100,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 +139,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
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v5 2/8] migration: Add support for modules
2020-01-29 11:56 ` [PATCH v5 2/8] migration: Add support for modules Juan Quintela
@ 2020-02-11 10:54 ` Dr. David Alan Gilbert
2020-02-13 19:38 ` Juan Quintela
0 siblings, 1 reply; 39+ messages in thread
From: Dr. David Alan Gilbert @ 2020-02-11 10:54 UTC (permalink / raw)
To: Juan Quintela
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, qemu-devel, Markus Armbruster, Paolo Bonzini
* Juan Quintela (quintela@redhat.com) wrote:
> So we don't have to compile everything in, or have ifdefs
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
As far as I can tell this matches the way all the rest of the module
stuff works, so:
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
(Although I wish it was documented somewhere).
Dave
> ---
> include/qemu/module.h | 2 ++
> vl.c | 1 +
> 2 files changed, 3 insertions(+)
>
> diff --git a/include/qemu/module.h b/include/qemu/module.h
> index 65ba596e46..907cb5c0a5 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,
> @@ -56,6 +57,7 @@ typedef enum {
> #define xen_backend_init(function) module_init(function, \
> MODULE_INIT_XEN_BACKEND)
> #define libqos_init(function) module_init(function, MODULE_INIT_LIBQOS)
> +#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)
> diff --git a/vl.c b/vl.c
> index b0f52c4d6e..9f8577955a 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2890,6 +2890,7 @@ int main(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
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v5 3/8] multifd: Make no compression operations into its own structure
2020-02-07 18:45 ` Dr. David Alan Gilbert
@ 2020-02-11 11:23 ` Juan Quintela
0 siblings, 0 replies; 39+ messages in thread
From: Juan Quintela @ 2020-02-11 11:23 UTC (permalink / raw)
To: Dr. David Alan Gilbert
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, qemu-devel, Markus Armbruster, Paolo Bonzini
"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> * Juan Quintela (quintela@redhat.com) wrote:
>> It will be used later.
>>
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>
>> diff --git a/migration/multifd.h b/migration/multifd.h
>> index d8b0205977..c7fea4914c 100644
>> --- a/migration/multifd.h
>> +++ b/migration/multifd.h
>> @@ -25,6 +25,10 @@ int multifd_queue_page(QEMUFile *f, RAMBlock *block, ram_addr_t offset);
>>
>> #define MULTIFD_FLAG_SYNC (1 << 0)
>>
>> +/* We reserve 3 bits for METHODS */
>> +#define MULTIFD_FLAG_METHOD_MASK (7 << 1)
>> +#define MULTIFD_FLAG_NOCOMP (1 << 1)
>> +
>
> Doesn't the 'NOCOMP' value have to be 0 for it to not break
> compatibility with existing multifd?
You are right. fixing on next resend.
Thanks.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v5 5/8] multifd: Add zlib compression multifd support
2020-01-29 11:56 ` [PATCH v5 5/8] multifd: Add zlib compression multifd support Juan Quintela
2020-01-30 8:04 ` Markus Armbruster
@ 2020-02-11 18:43 ` Dr. David Alan Gilbert
2020-02-13 20:24 ` Juan Quintela
1 sibling, 1 reply; 39+ messages in thread
From: Dr. David Alan Gilbert @ 2020-02-11 18:43 UTC (permalink / raw)
To: Juan Quintela
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, qemu-devel, Markus Armbruster, Paolo Bonzini
* Juan Quintela (quintela@redhat.com) wrote:
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
> hw/core/qdev-properties.c | 2 +-
> migration/Makefile.objs | 1 +
> migration/migration.c | 9 +
> migration/migration.h | 1 +
> migration/multifd-zlib.c | 325 +++++++++++++++++++++++++++++++++++
> migration/multifd.c | 6 +
> migration/multifd.h | 4 +
> qapi/migration.json | 3 +-
> tests/qtest/migration-test.c | 6 +
> 9 files changed, 355 insertions(+), 2 deletions(-)
> create mode 100644 migration/multifd-zlib.c
Coupld of really minor points below to fix up, but other than those:
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> index 4442844d37..bf88a50cdf 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_method = {
> .name = "MultiFDMethod",
> .description = "multifd_method values, "
> - "none",
> + "none/zlib",
> .enum_table = &MultiFDMethod_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/migration.c b/migration/migration.c
> index 4f88f8e958..3b081e8147 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -2265,6 +2265,15 @@ MultiFDMethod migrate_multifd_method(void)
> return s->parameters.multifd_method;
> }
>
> +int migrate_multifd_zlib_level(void)
> +{
> + MigrationState *s;
> +
> + s = migrate_get_current();
> +
> + return s->parameters.multifd_zlib_level;
> +}
> +
Does this belong in the previous patch?
> int migrate_use_xbzrle(void)
> {
> MigrationState *s;
> diff --git a/migration/migration.h b/migration/migration.h
> index 3d23a0852e..95e9c196ff 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);
> MultiFDMethod migrate_multifd_method(void);
> +int migrate_multifd_zlib_level(void);
>
> int migrate_use_xbzrle(void);
> int64_t migrate_xbzrle_cache_size(void);
> diff --git a/migration/multifd-zlib.c b/migration/multifd-zlib.c
> new file mode 100644
> index 0000000000..91b3128256
> --- /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_METHOD_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) {
Note an extra space there.
> + 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_METHOD_ZLIB, &multifd_zlib_ops);
> +}
> +
> +migration_init(multifd_zlib_register);
> diff --git a/migration/multifd.c b/migration/multifd.c
> index 1c49c2a665..7bb9c3582f 100644
> --- a/migration/multifd.c
> +++ b/migration/multifd.c
> @@ -166,6 +166,12 @@ static MultiFDMethods *multifd_ops[MULTIFD_METHOD__MAX] = {
> [MULTIFD_METHOD_NONE] = &multifd_nocomp_ops,
> };
>
> +void multifd_register_ops(int method, MultiFDMethods *ops)
> +{
> + assert(0 < method && method < MULTIFD_METHOD__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 c7fea4914c..3fa5132f1d 100644
> --- a/migration/multifd.h
> +++ b/migration/multifd.h
> @@ -23,11 +23,13 @@ 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 METHODS */
> #define MULTIFD_FLAG_METHOD_MASK (7 << 1)
> #define MULTIFD_FLAG_NOCOMP (1 << 1)
> +#define MULTIFD_FLAG_ZLIB (2 << 1)
>
> /* This value needs to be a multiple of qemu_target_page_size() */
> #define MULTIFD_PACKET_SIZE (512 * 1024)
> @@ -160,5 +162,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 289dce0da7..032ee7d3e6 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -494,12 +494,13 @@
> # An enumeration of multifd compression.
> #
> # @none: no compression.
> +# @zlib: use zlib compression method.
> #
> # Since: 5.0
> #
> ##
> { 'enum': 'MultiFDMethod',
> - 'data': [ 'none' ] }
> + 'data': [ 'none', 'zlib' ] }
>
> ##
> # @MigrationParameter:
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index d2f9ef38f5..8effed205d 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -1313,6 +1313,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
> @@ -1475,6 +1480,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
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v5 7/8] multifd: Add multifd-zstd-level parameter
2020-01-29 11:56 ` [PATCH v5 7/8] multifd: Add multifd-zstd-level parameter Juan Quintela
2020-01-30 8:08 ` Markus Armbruster
@ 2020-02-11 18:47 ` Dr. David Alan Gilbert
2020-02-13 14:04 ` Markus Armbruster
1 sibling, 1 reply; 39+ messages in thread
From: Dr. David Alan Gilbert @ 2020-02-11 18:47 UTC (permalink / raw)
To: Juan Quintela
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, qemu-devel, Markus Armbruster, Paolo Bonzini
* Juan Quintela (quintela@redhat.com) wrote:
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
> migration/migration.c | 15 +++++++++++++++
> monitor/hmp-cmds.c | 4 ++++
> qapi/migration.json | 29 ++++++++++++++++++++++++++---
> 3 files changed, 45 insertions(+), 3 deletions(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index 3b081e8147..b690500545 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -91,6 +91,8 @@
> #define DEFAULT_MIGRATE_MULTIFD_METHOD MULTIFD_METHOD_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_method = s->parameters.multifd_method;
> 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;
Do we really want different 'multifd_...._level's or just one
'multifd_compress_level' - or even just reuse the existing
'compress-level' parameter.
The only tricky thing about combining them is how to handle
the difference in allowed ranges; When would the right time be
to check it?
Markus/Eric: Any idea?
Dave
> 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))) {
> @@ -3559,6 +3570,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),
> @@ -3651,6 +3665,7 @@ static void migration_instance_init(Object *obj)
> params->has_multifd_channels = true;
> params->has_multifd_method = 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/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index 7f11866446..87db07694b 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -1840,6 +1840,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 032ee7d3e6..bb5cb6b4f4 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-method',
> - '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-method': 'MultiFDMethod',
> - '*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-method': 'MultiFDMethod',
> - '*multifd-zlib-level': 'uint8' } }
> + '*multifd-zlib-level': 'uint8',
> + '*multifd-zstd-level': 'uint8' } }
>
> ##
> # @query-migrate-parameters:
> --
> 2.24.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v5 1/8] multifd: Add multifd-method parameter
2020-01-29 11:56 ` [PATCH v5 1/8] multifd: Add multifd-method parameter Juan Quintela
2020-01-30 7:54 ` Markus Armbruster
@ 2020-02-11 18:50 ` Daniel P. Berrangé
2020-02-13 19:29 ` Juan Quintela
1 sibling, 1 reply; 39+ messages in thread
From: Daniel P. Berrangé @ 2020-02-11 18:50 UTC (permalink / raw)
To: Juan Quintela
Cc: Laurent Vivier, Thomas Huth, Eduardo Habkost, Markus Armbruster,
qemu-devel, Paolo Bonzini, Dr. David Alan Gilbert
On Wed, Jan 29, 2020 at 12:56:48PM +0100, Juan Quintela wrote:
> This will store the compression method to use. We start with none.
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> hw/core/qdev-properties.c | 13 +++++++++++++
> include/hw/qdev-properties.h | 3 +++
> migration/migration.c | 13 +++++++++++++
> monitor/hmp-cmds.c | 13 +++++++++++++
> qapi/migration.json | 30 +++++++++++++++++++++++++++---
> tests/qtest/migration-test.c | 14 ++++++++++----
> 6 files changed, 79 insertions(+), 7 deletions(-)
>
> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> index 7f93bfeb88..4442844d37 100644
> --- a/hw/core/qdev-properties.c
> +++ b/hw/core/qdev-properties.c
> @@ -8,6 +8,7 @@
> @@ -488,6 +488,19 @@
> ##
> { 'command': 'query-migrate-capabilities', 'returns': ['MigrationCapabilityStatus']}
>
> +##
> +# @MultiFDMethod:
> +#
> +# An enumeration of multifd compression.
> +#
> +# @none: no compression.
> +#
> +# Since: 5.0
> +#
> +##
> +{ 'enum': 'MultiFDMethod',
> + 'data': [ 'none' ] }
I feel like "MultiFDMethod" is better called "MultiFDCompression"
> +
> ##
> # @MigrationParameter:
> #
> @@ -586,6 +599,9 @@
> # @max-cpu-throttle: maximum cpu throttle percentage.
> # Defaults to 99. (Since 3.1)
> #
> +# @multifd-method: 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-method' ] }
>
> ##
> # @MigrateSetParameters:
> @@ -688,6 +704,9 @@
> # @max-cpu-throttle: maximum cpu throttle percentage.
> # The default value is 99. (Since 3.1)
> #
> +# @multifd-method: 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-method': 'MultiFDMethod' } }
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v5 4/8] multifd: Add multifd-zlib-level parameter
2020-01-30 8:03 ` Markus Armbruster
2020-01-30 8:56 ` Juan Quintela
@ 2020-02-11 18:57 ` Daniel P. Berrangé
2020-02-13 13:27 ` Markus Armbruster
2020-02-13 16:33 ` Juan Quintela
1 sibling, 2 replies; 39+ messages in thread
From: Daniel P. Berrangé @ 2020-02-11 18:57 UTC (permalink / raw)
To: Markus Armbruster
Cc: Laurent Vivier, Thomas Huth, Eduardo Habkost, Juan Quintela,
qemu-devel, Dr. David Alan Gilbert, Paolo Bonzini
On Thu, Jan 30, 2020 at 09:03:00AM +0100, Markus Armbruster wrote:
> Juan Quintela <quintela@redhat.com> writes:
>
> > It will indicate which level use for compression.
> >
> > Signed-off-by: Juan Quintela <quintela@redhat.com>
>
> This is slightly confusing (there is no zlib compression), unless you
> peek at the next patch (which adds zlib compression).
>
> Three ways to make it less confusing:
>
> * Squash the two commits
>
> * Swap them: first add zlib compression with level hardcoded to 1, then
> make the level configurable.
>
> * Have the first commit explain itself better. Something like
>
> multifd: Add multifd-zlib-level parameter
>
> This parameter specifies zlib compression level. The next patch
> will put it to use.
Wouldn't the "normal" best practice for QAPI design be to use a
enum and discriminated union. eg
{ 'enum': 'MigrationCompression',
'data': ['none', 'zlib'] }
{ 'struct': 'MigrationCompressionParamsZLib',
'data': { 'compression-level' } }
{ 'union': 'MigrationCompressionParams',
'base': { 'mode': 'MigrationCompression' },
'discriminator': 'mode',
'data': {
'zlib': 'MigrationCompressionParamsZLib',
}
Of course this is quite different from how migration parameters are
done today. Maybe it makes sense to stick with the flat list of
migration parameters for consistency & ignore normal QAPI design
practice ?
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v5 8/8] multifd: Add zstd compression multifd support
2020-01-29 11:56 ` [PATCH v5 8/8] multifd: Add zstd compression multifd support Juan Quintela
2020-01-30 8:08 ` Markus Armbruster
@ 2020-02-11 20:01 ` Dr. David Alan Gilbert
2020-02-13 20:39 ` Juan Quintela
1 sibling, 1 reply; 39+ messages in thread
From: Dr. David Alan Gilbert @ 2020-02-11 20:01 UTC (permalink / raw)
To: Juan Quintela
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, qemu-devel, Markus Armbruster, Paolo Bonzini
* Juan Quintela (quintela@redhat.com) wrote:
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
> hw/core/qdev-properties.c | 2 +-
> migration/Makefile.objs | 1 +
> migration/migration.c | 9 +
> migration/migration.h | 1 +
> migration/multifd-zstd.c | 337 +++++++++++++++++++++++++++++++++++
> migration/multifd.h | 2 +-
> migration/ram.c | 1 -
> qapi/migration.json | 4 +-
> tests/qtest/migration-test.c | 10 ++
> 9 files changed, 363 insertions(+), 4 deletions(-)
> create mode 100644 migration/multifd-zstd.c
>
> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> index bf88a50cdf..9440ca78c3 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_method = {
> .name = "MultiFDMethod",
> .description = "multifd_method values, "
> - "none/zlib",
> + "none/zlib/zstd",
> .enum_table = &MultiFDMethod_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/migration.c b/migration/migration.c
> index b690500545..aff081128c 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -2285,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;
> diff --git a/migration/migration.h b/migration/migration.h
> index 95e9c196ff..2eb72aee0a 100644
> --- a/migration/migration.h
> +++ b/migration/migration.h
> @@ -302,6 +302,7 @@ bool migrate_pause_before_switchover(void);
> int migrate_multifd_channels(void);
> MultiFDMethod migrate_multifd_method(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/migration/multifd-zstd.c b/migration/multifd-zstd.c
> new file mode 100644
> index 0000000000..6383a8a898
> --- /dev/null
> +++ b/migration/multifd-zstd.c
> @@ -0,0 +1,337 @@
> +/*
> + * 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", p->id);
It might be useful to print 'res' here - you seem to decode it on the
receive side.
> + 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 res;
> +
> + 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;
> + }
> +
> + res = ZSTD_initDStream(z->zds);
> + if (ZSTD_isError(res)) {
> + ZSTD_freeDStream(z->zds);
> + g_free(z);
> + error_setg(errp, "multifd %d: initDStream 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) {
> + 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_METHOD_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_METHOD_ZSTD, &multifd_zstd_ops);
> +}
> +
> +migration_init(multifd_zstd_register);
> diff --git a/migration/multifd.h b/migration/multifd.h
> index 3fa5132f1d..621db316c1 100644
> --- a/migration/multifd.h
> +++ b/migration/multifd.h
> @@ -30,6 +30,7 @@ int multifd_queue_page(QEMUFile *f, RAMBlock *block, ram_addr_t offset);
> #define MULTIFD_FLAG_METHOD_MASK (7 << 1)
> #define MULTIFD_FLAG_NOCOMP (1 << 1)
> #define MULTIFD_FLAG_ZLIB (2 << 1)
> +#define MULTIFD_FLAG_ZSTD (3 << 1)
>
> /* This value needs to be a multiple of qemu_target_page_size() */
> #define MULTIFD_PACKET_SIZE (512 * 1024)
> @@ -163,6 +164,5 @@ typedef struct {
> } MultiFDMethods;
>
> void multifd_register_ops(int method, MultiFDMethods *ops);
> -
Oddment.
> #endif
>
> 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 bb5cb6b4f4..8ccec7fa79 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': 'MultiFDMethod',
> - '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 8effed205d..ec9be28bc9 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -1318,6 +1318,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
> @@ -1481,6 +1488,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
>
Minor comments above; but other than those it looks OK to me, but I've
not used zstd before:
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v5 6/8] configure: Enable test and libs for zstd
2020-01-29 11:56 ` [PATCH v5 6/8] configure: Enable test and libs for zstd Juan Quintela
@ 2020-02-11 20:11 ` Daniel P. Berrangé
2020-02-13 21:08 ` Juan Quintela
0 siblings, 1 reply; 39+ messages in thread
From: Daniel P. Berrangé @ 2020-02-11 20:11 UTC (permalink / raw)
To: Juan Quintela
Cc: Laurent Vivier, Thomas Huth, Eduardo Habkost, Markus Armbruster,
qemu-devel, Dr. David Alan Gilbert, Paolo Bonzini
On Wed, Jan 29, 2020 at 12:56:53PM +0100, Juan Quintela wrote:
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> configure | 30 ++++++++++++++++++++++++++++++
> 1 file changed, 30 insertions(+)
This is adding a new 3rd party library to QEMU that we've not previously
built and so isn't included in any of our CI platforms.
This commit should be updating at least some of our CI platforms to
request the libzstd library installation to get CI coverage for the
latest patches in this series. Probably the docker files, the VM
installs for FreeBSD at least, travis and gitlab CI.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v5 4/8] multifd: Add multifd-zlib-level parameter
2020-02-11 18:57 ` Daniel P. Berrangé
@ 2020-02-13 13:27 ` Markus Armbruster
2020-02-13 16:33 ` Juan Quintela
1 sibling, 0 replies; 39+ messages in thread
From: Markus Armbruster @ 2020-02-13 13:27 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Laurent Vivier, Thomas Huth, Eduardo Habkost, Juan Quintela,
qemu-devel, Dr. David Alan Gilbert, Paolo Bonzini
Daniel P. Berrangé <berrange@redhat.com> writes:
> On Thu, Jan 30, 2020 at 09:03:00AM +0100, Markus Armbruster wrote:
>> Juan Quintela <quintela@redhat.com> writes:
>>
>> > It will indicate which level use for compression.
>> >
>> > Signed-off-by: Juan Quintela <quintela@redhat.com>
>>
>> This is slightly confusing (there is no zlib compression), unless you
>> peek at the next patch (which adds zlib compression).
>>
>> Three ways to make it less confusing:
>>
>> * Squash the two commits
>>
>> * Swap them: first add zlib compression with level hardcoded to 1, then
>> make the level configurable.
>>
>> * Have the first commit explain itself better. Something like
>>
>> multifd: Add multifd-zlib-level parameter
>>
>> This parameter specifies zlib compression level. The next patch
>> will put it to use.
>
> Wouldn't the "normal" best practice for QAPI design be to use a
> enum and discriminated union. eg
>
> { 'enum': 'MigrationCompression',
> 'data': ['none', 'zlib'] }
>
> { 'struct': 'MigrationCompressionParamsZLib',
> 'data': { 'compression-level' } }
>
> { 'union': 'MigrationCompressionParams',
> 'base': { 'mode': 'MigrationCompression' },
> 'discriminator': 'mode',
> 'data': {
> 'zlib': 'MigrationCompressionParamsZLib',
> }
This expresses the connection between compression mode and level. In
general, we prefer that to a bunch of optional members where the
comments say things like "member A can be present only when member B has
value V", or worse "member A is silently ignored unless member B has
value V". However:
> Of course this is quite different from how migration parameters are
> done today. Maybe it makes sense to stick with the flat list of
> migration parameters for consistency & ignore normal QAPI design
> practice ?
Unsure. It's certainly ugly now. Each parameter is defined in three
places: enum MigrationParameter (for HMP), struct MigrateSetParameters
(for QMP migrate-set-parameters), and struct MigrationParameters (for
QMP query-migrate-parameters).
I don't know how to make this better other than by starting over.
I don't know whether starting over would result in enough of an
improvement to make it worthwhile.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v5 7/8] multifd: Add multifd-zstd-level parameter
2020-02-11 18:47 ` Dr. David Alan Gilbert
@ 2020-02-13 14:04 ` Markus Armbruster
2020-02-13 14:28 ` Dr. David Alan Gilbert
2020-02-13 15:33 ` Juan Quintela
0 siblings, 2 replies; 39+ messages in thread
From: Markus Armbruster @ 2020-02-13 14:04 UTC (permalink / raw)
To: Dr. David Alan Gilbert
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, Juan Quintela, qemu-devel, Paolo Bonzini
"Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:
> * Juan Quintela (quintela@redhat.com) wrote:
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>> migration/migration.c | 15 +++++++++++++++
>> monitor/hmp-cmds.c | 4 ++++
>> qapi/migration.json | 29 ++++++++++++++++++++++++++---
>> 3 files changed, 45 insertions(+), 3 deletions(-)
>>
>> diff --git a/migration/migration.c b/migration/migration.c
>> index 3b081e8147..b690500545 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -91,6 +91,8 @@
>> #define DEFAULT_MIGRATE_MULTIFD_METHOD MULTIFD_METHOD_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_method = s->parameters.multifd_method;
>> 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;
>
> Do we really want different 'multifd_...._level's or just one
> 'multifd_compress_level' - or even just reuse the existing
> 'compress-level' parameter.
compress-level, multifd-zlib-level, and multifd-zstd-level apply
"normal" live migration compression, multifd zlib live migration
compression, and multifd zstd live migration compression, respectively.
Any live migration can only use one of the three compressions.
Correct?
> The only tricky thing about combining them is how to handle
> the difference in allowed ranges; When would the right time be
> to check it?
>
> Markus/Eric: Any idea?
To have an informed opinion, I'd have to dig through the migration
code.
Documentation of admissible range will become a bit awkward, too.
Too many migration parameters...
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v5 7/8] multifd: Add multifd-zstd-level parameter
2020-02-13 14:04 ` Markus Armbruster
@ 2020-02-13 14:28 ` Dr. David Alan Gilbert
2020-02-13 15:33 ` Juan Quintela
1 sibling, 0 replies; 39+ messages in thread
From: Dr. David Alan Gilbert @ 2020-02-13 14:28 UTC (permalink / raw)
To: Markus Armbruster
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, Juan Quintela, qemu-devel, Paolo Bonzini
* Markus Armbruster (armbru@redhat.com) wrote:
> "Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:
>
> > * Juan Quintela (quintela@redhat.com) wrote:
> >> Signed-off-by: Juan Quintela <quintela@redhat.com>
> >> ---
> >> migration/migration.c | 15 +++++++++++++++
> >> monitor/hmp-cmds.c | 4 ++++
> >> qapi/migration.json | 29 ++++++++++++++++++++++++++---
> >> 3 files changed, 45 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/migration/migration.c b/migration/migration.c
> >> index 3b081e8147..b690500545 100644
> >> --- a/migration/migration.c
> >> +++ b/migration/migration.c
> >> @@ -91,6 +91,8 @@
> >> #define DEFAULT_MIGRATE_MULTIFD_METHOD MULTIFD_METHOD_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_method = s->parameters.multifd_method;
> >> 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;
> >
> > Do we really want different 'multifd_...._level's or just one
> > 'multifd_compress_level' - or even just reuse the existing
> > 'compress-level' parameter.
>
> compress-level, multifd-zlib-level, and multifd-zstd-level apply
> "normal" live migration compression, multifd zlib live migration
> compression, and multifd zstd live migration compression, respectively.
>
> Any live migration can only use one of the three compressions.
>
> Correct?
Right.
> > The only tricky thing about combining them is how to handle
> > the difference in allowed ranges; When would the right time be
> > to check it?
> >
> > Markus/Eric: Any idea?
>
> To have an informed opinion, I'd have to dig through the migration
> code.
The tricky part I see is validating settings/parameters;
when someone tries to set a parameter migrate_params_check gets called
and has checks like:
if (params->has_compress_level &&
(params->compress_level > 9)) {
error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "compress_level",
"is invalid, it should be in the range of 0 to 9");
return false;
}
now that's nice because you error when you set the parameter rather
than later when you try and start a migration; the downside now
though is you get more complex interaction between parameters and
capabilities - so for example if you set the multifd-compression type
parameter to 'zstd' and *then* set the single compression level
it'll be fine taking '20' as a compresison level, but if you were
to set the compression level to 20 and then set the type to 'zstd'
it might error because with zlib you can't have 20.
> Documentation of admissible range will become a bit awkward, too.
>
> Too many migration parameters...
Nod; which why I was trying to make it 1.
Dave
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v5 7/8] multifd: Add multifd-zstd-level parameter
2020-02-13 14:04 ` Markus Armbruster
2020-02-13 14:28 ` Dr. David Alan Gilbert
@ 2020-02-13 15:33 ` Juan Quintela
2020-02-14 8:49 ` Markus Armbruster
1 sibling, 1 reply; 39+ messages in thread
From: Juan Quintela @ 2020-02-13 15:33 UTC (permalink / raw)
To: Markus Armbruster
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, qemu-devel, Dr. David Alan Gilbert,
Paolo Bonzini
Markus Armbruster <armbru@redhat.com> wrote:
> "Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:
>
>> * Juan Quintela (quintela@redhat.com) wrote:
>>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>>> ---
>>> migration/migration.c | 15 +++++++++++++++
>>> monitor/hmp-cmds.c | 4 ++++
>>> qapi/migration.json | 29 ++++++++++++++++++++++++++---
>>> 3 files changed, 45 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/migration/migration.c b/migration/migration.c
>>> index 3b081e8147..b690500545 100644
>>> --- a/migration/migration.c
>>> +++ b/migration/migration.c
>>> @@ -91,6 +91,8 @@
>>> #define DEFAULT_MIGRATE_MULTIFD_METHOD MULTIFD_METHOD_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_method = s->parameters.multifd_method;
>>> 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;
>>
>> Do we really want different 'multifd_...._level's or just one
>> 'multifd_compress_level' - or even just reuse the existing
>> 'compress-level' parameter.
>
> compress-level,
possible values: 1 to 9 deprecated
> multifd-zlib-level
possible values: 1 to 9, default 1
(zlib default is -1, let the lib decide)
, and multifd-zstd-level apply
possible values: 1 to 19, default 1
(zstd default is 3)
> "normal" live migration compression, multifd zlib live migration
> compression, and multifd zstd live migration compression, respectively.
>
> Any live migration can only use one of the three compressions.
>
> Correct?
Yeap.
>> The only tricky thing about combining them is how to handle
>> the difference in allowed ranges; When would the right time be
>> to check it?
>>
>> Markus/Eric: Any idea?
>
> To have an informed opinion, I'd have to dig through the migration
> code.
Problem is _how_ to setup them. if we setup zstd compression method,
put the value at 19, and then setup zlib compression method, what should
we do?
Truncate to 9?
Give one error?
Don't allow the zlib setup?
Too complicated.
> Documentation of admissible range will become a bit awkward, too.
>
> Too many migration parameters...
Sure, but the other option is taking a value and live with it.
I am all for leaving the library default and call it a day.
Later, Juan.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v5 4/8] multifd: Add multifd-zlib-level parameter
2020-02-11 18:57 ` Daniel P. Berrangé
2020-02-13 13:27 ` Markus Armbruster
@ 2020-02-13 16:33 ` Juan Quintela
1 sibling, 0 replies; 39+ messages in thread
From: Juan Quintela @ 2020-02-13 16:33 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Laurent Vivier, Thomas Huth, Eduardo Habkost, qemu-devel,
Markus Armbruster, Paolo Bonzini, Dr. David Alan Gilbert
Daniel P. Berrangé <berrange@redhat.com> wrote:
> On Thu, Jan 30, 2020 at 09:03:00AM +0100, Markus Armbruster wrote:
>> Juan Quintela <quintela@redhat.com> writes:
>>
>> > It will indicate which level use for compression.
>> >
>> > Signed-off-by: Juan Quintela <quintela@redhat.com>
>>
>> This is slightly confusing (there is no zlib compression), unless you
>> peek at the next patch (which adds zlib compression).
>>
>> Three ways to make it less confusing:
>>
>> * Squash the two commits
>>
>> * Swap them: first add zlib compression with level hardcoded to 1, then
>> make the level configurable.
>>
>> * Have the first commit explain itself better. Something like
>>
>> multifd: Add multifd-zlib-level parameter
>>
>> This parameter specifies zlib compression level. The next patch
>> will put it to use.
>
> Wouldn't the "normal" best practice for QAPI design be to use a
> enum and discriminated union. eg
>
> { 'enum': 'MigrationCompression',
> 'data': ['none', 'zlib'] }
>
> { 'struct': 'MigrationCompressionParamsZLib',
> 'data': { 'compression-level' } }
>
> { 'union': 'MigrationCompressionParams',
> 'base': { 'mode': 'MigrationCompression' },
> 'discriminator': 'mode',
> 'data': {
> 'zlib': 'MigrationCompressionParamsZLib',
> }
How is this translate into HMP?
Markus says to start over, so lets see the dependencies:
Announce: Allawys there
announce-initial
announce-max
announce-rounds
announce-step
Osd compression (deprecated)
compress-level
compress-threads
compress-wait-thread
decompress-threads
cpu-throttles-initial
cpu-throottle-incroment
max-cpu-throotle
tls-creds
tls-hostname
tls-auth
Real params
max-bandwidth
downtime-limit
colo
x-checkpoint-delay
block-incremental
multifd-channels
xbzrle-cache-size
max-postcopy-bandwidth
New things:
- multifd method
- multifd-zlib-level
- multifd-zstd-level
What is a good way to define them?
Why do I ask, because the current method is as bad as it can be.
To add a new parameter:
- for qapi, add it in three places (as Markus said)
- go to hmp-cmds.c and do things by hand
- qemu_migrate_set_parameters
- migrate_params_check
- migrate_params_apply
(last three functions are almost identical in structure, not in
content).
So, if you can give me something that is _easier_ of maintaining, I am
all ears.
Later, Juan.
>
> Of course this is quite different from how migration parameters are
> done today. Maybe it makes sense to stick with the flat list of
> migration parameters for consistency & ignore normal QAPI design
> practice ?
>
>
> Regards,
> Daniel
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v5 1/8] multifd: Add multifd-method parameter
2020-02-11 18:50 ` Daniel P. Berrangé
@ 2020-02-13 19:29 ` Juan Quintela
0 siblings, 0 replies; 39+ messages in thread
From: Juan Quintela @ 2020-02-13 19:29 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Laurent Vivier, Thomas Huth, Eduardo Habkost, Markus Armbruster,
qemu-devel, Paolo Bonzini, Dr. David Alan Gilbert
Daniel P. Berrangé <berrange@redhat.com> wrote:
> On Wed, Jan 29, 2020 at 12:56:48PM +0100, Juan Quintela wrote:
>> This will store the compression method to use. We start with none.
>>
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>> ---
>> hw/core/qdev-properties.c | 13 +++++++++++++
>> include/hw/qdev-properties.h | 3 +++
>> migration/migration.c | 13 +++++++++++++
>> monitor/hmp-cmds.c | 13 +++++++++++++
>> qapi/migration.json | 30 +++++++++++++++++++++++++++---
>> tests/qtest/migration-test.c | 14 ++++++++++----
>> 6 files changed, 79 insertions(+), 7 deletions(-)
>>
>> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
>> index 7f93bfeb88..4442844d37 100644
>> --- a/hw/core/qdev-properties.c
>> +++ b/hw/core/qdev-properties.c
>> @@ -8,6 +8,7 @@
>
>> @@ -488,6 +488,19 @@
>> ##
>> { 'command': 'query-migrate-capabilities', 'returns': ['MigrationCapabilityStatus']}
>>
>> +##
>> +# @MultiFDMethod:
>> +#
>> +# An enumeration of multifd compression.
>> +#
>> +# @none: no compression.
>> +#
>> +# Since: 5.0
>> +#
>> +##
>> +{ 'enum': 'MultiFDMethod',
>> + 'data': [ 'none' ] }
>
> I feel like "MultiFDMethod" is better called "MultiFDCompression"
Changed.
Thanks, Juan.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v5 2/8] migration: Add support for modules
2020-02-11 10:54 ` Dr. David Alan Gilbert
@ 2020-02-13 19:38 ` Juan Quintela
0 siblings, 0 replies; 39+ messages in thread
From: Juan Quintela @ 2020-02-13 19:38 UTC (permalink / raw)
To: Dr. David Alan Gilbert
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, qemu-devel, Markus Armbruster, Paolo Bonzini
"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> * Juan Quintela (quintela@redhat.com) wrote:
>> So we don't have to compile everything in, or have ifdefs
>>
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>
> As far as I can tell this matches the way all the rest of the module
> stuff works, so:
>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Thanks
> (Although I wish it was documented somewhere).
I had to learn to use it with grep :-p
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v5 5/8] multifd: Add zlib compression multifd support
2020-02-11 18:43 ` Dr. David Alan Gilbert
@ 2020-02-13 20:24 ` Juan Quintela
0 siblings, 0 replies; 39+ messages in thread
From: Juan Quintela @ 2020-02-13 20:24 UTC (permalink / raw)
To: Dr. David Alan Gilbert
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, qemu-devel, Markus Armbruster, Paolo Bonzini
"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> * Juan Quintela (quintela@redhat.com) wrote:
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>> hw/core/qdev-properties.c | 2 +-
>> migration/Makefile.objs | 1 +
>> migration/migration.c | 9 +
>> migration/migration.h | 1 +
>> migration/multifd-zlib.c | 325 +++++++++++++++++++++++++++++++++++
>> migration/multifd.c | 6 +
>> migration/multifd.h | 4 +
>> qapi/migration.json | 3 +-
>> tests/qtest/migration-test.c | 6 +
>> 9 files changed, 355 insertions(+), 2 deletions(-)
>> create mode 100644 migration/multifd-zlib.c
>
> Coupld of really minor points below to fix up, but other than those:
>
>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>
>> }
>>
>> +int migrate_multifd_zlib_level(void)
>> +{
>> + MigrationState *s;
>> +
>> + s = migrate_get_current();
>> +
>> + return s->parameters.multifd_zlib_level;
>> +}
>> +
>
> Does this belong in the previous patch?
It is used only here. Should not make any difference.
Anyways, changing it.
>
>> int migrate_use_xbzrle(void)
>> {
>> MigrationState *s;
>> diff --git a/migration/migration.h b/migration/migration.h
>> index 3d23a0852e..95e9c196ff 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);
>> MultiFDMethod migrate_multifd_method(void);
>> +int migrate_multifd_zlib_level(void);
>> + int flush = Z_NO_FLUSH;
>> + unsigned long start = zs->total_out;
>> +
>> + if (i == used - 1) {
>
> Note an extra space there.
>
Fixed, thanks.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v5 8/8] multifd: Add zstd compression multifd support
2020-02-11 20:01 ` Dr. David Alan Gilbert
@ 2020-02-13 20:39 ` Juan Quintela
0 siblings, 0 replies; 39+ messages in thread
From: Juan Quintela @ 2020-02-13 20:39 UTC (permalink / raw)
To: Dr. David Alan Gilbert
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, qemu-devel, Markus Armbruster, Paolo Bonzini
"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> * Juan Quintela (quintela@redhat.com) wrote:
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>> hw/core/qdev-properties.c | 2 +-
>> migration/Makefile.objs | 1 +
>> migration/migration.c | 9 +
>> migration/migration.h | 1 +
>> migration/multifd-zstd.c | 337 +++++++++++++++++++++++++++++++++++
>> migration/multifd.h | 2 +-
>> migration/ram.c | 1 -
>> qapi/migration.json | 4 +-
>> tests/qtest/migration-test.c | 10 ++
>> 9 files changed, 363 insertions(+), 4 deletions(-)
>> create mode 100644 migration/multifd-zstd.c
>> + 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", p->id);
>
> It might be useful to print 'res' here - you seem to decode it on the
> receive side.
Fixed all callers.
>> @@ -163,6 +164,5 @@ typedef struct {
>> } MultiFDMethods;
>>
>> void multifd_register_ops(int method, MultiFDMethods *ops);
>> -
>
> Oddment.
oops. Removed the chunk.
>
>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Thanks.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v5 6/8] configure: Enable test and libs for zstd
2020-02-11 20:11 ` Daniel P. Berrangé
@ 2020-02-13 21:08 ` Juan Quintela
2020-02-14 10:26 ` Daniel P. Berrangé
0 siblings, 1 reply; 39+ messages in thread
From: Juan Quintela @ 2020-02-13 21:08 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Laurent Vivier, Thomas Huth, Eduardo Habkost, Markus Armbruster,
qemu-devel, Dr. David Alan Gilbert, Paolo Bonzini
Daniel P. Berrangé <berrange@redhat.com> wrote:
> On Wed, Jan 29, 2020 at 12:56:53PM +0100, Juan Quintela wrote:
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>> ---
>> configure | 30 ++++++++++++++++++++++++++++++
>> 1 file changed, 30 insertions(+)
>
> This is adding a new 3rd party library to QEMU that we've not previously
> built and so isn't included in any of our CI platforms.
Ok.
Learning how one does that.
> This commit should be updating at least some of our CI platforms to
> request the libzstd library installation to get CI coverage for the
> latest patches in this series.
> Probably the docker files,
I added it in all debian/centos/fedora files that zlib-dev or xen-dev
> the VM installs for FreeBSD at least,
A fast google finds that library is called "zstd" and that it includes
the includes (put intended)
tests/vm/freebsd
Once there, include it in fedora
> travis and
I added it to .travis.yml
> gitlab CI.
gitlab-ci.yml (just when we compile x86_64-softmmu)
I have something like this, but net real clue how to test that I haven't
broken anything:
commit 59d8694dfcfc3d5ef36bc72a5c02bedaa3a6a6ec
Author: Juan Quintela <quintela@redhat.com>
Date: Thu Feb 13 22:06:16 2020 +0100
Use zstd packages
Signed-off-by: Juan Quintela <quintela@redhat.com>
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 5887055951..dd17301f3b 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -48,6 +48,7 @@ addons:
- libusb-1.0-0-dev
- libvdeplug-dev
- libvte-2.91-dev
+ - libzstd-dev
- sparse
- uuid-dev
- gcovr
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 = """
^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH v5 7/8] multifd: Add multifd-zstd-level parameter
2020-02-13 15:33 ` Juan Quintela
@ 2020-02-14 8:49 ` Markus Armbruster
2020-02-14 18:50 ` Dr. David Alan Gilbert
0 siblings, 1 reply; 39+ messages in thread
From: Markus Armbruster @ 2020-02-14 8:49 UTC (permalink / raw)
To: Juan Quintela
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, Dr. David Alan Gilbert, qemu-devel,
Paolo Bonzini
Juan Quintela <quintela@redhat.com> writes:
> Markus Armbruster <armbru@redhat.com> wrote:
>> "Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:
>>
>>> * Juan Quintela (quintela@redhat.com) wrote:
>>>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>>>> ---
>>>> migration/migration.c | 15 +++++++++++++++
>>>> monitor/hmp-cmds.c | 4 ++++
>>>> qapi/migration.json | 29 ++++++++++++++++++++++++++---
>>>> 3 files changed, 45 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/migration/migration.c b/migration/migration.c
>>>> index 3b081e8147..b690500545 100644
>>>> --- a/migration/migration.c
>>>> +++ b/migration/migration.c
>>>> @@ -91,6 +91,8 @@
>>>> #define DEFAULT_MIGRATE_MULTIFD_METHOD MULTIFD_METHOD_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_method = s->parameters.multifd_method;
>>>> 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;
>>>
>>> Do we really want different 'multifd_...._level's or just one
>>> 'multifd_compress_level' - or even just reuse the existing
>>> 'compress-level' parameter.
>>
>> compress-level,
>
> possible values: 1 to 9 deprecated
>
>> multifd-zlib-level
>
> possible values: 1 to 9, default 1
> (zlib default is -1, let the lib decide)
>
> , and multifd-zstd-level apply
>
> possible values: 1 to 19, default 1
> (zstd default is 3)
>
>> "normal" live migration compression, multifd zlib live migration
>> compression, and multifd zstd live migration compression, respectively.
>>
>> Any live migration can only use one of the three compressions.
>>
>> Correct?
>
> Yeap.
>
>>> The only tricky thing about combining them is how to handle
>>> the difference in allowed ranges; When would the right time be
>>> to check it?
>>>
>>> Markus/Eric: Any idea?
>>
>> To have an informed opinion, I'd have to dig through the migration
>> code.
>
> Problem is _how_ to setup them. if we setup zstd compression method,
> put the value at 19, and then setup zlib compression method, what should
> we do?
>
> Truncate to 9?
> Give one error?
> Don't allow the zlib setup?
>
> Too complicated.
The interface pretends the parameters are all independent: you get to
set them one by one.
They are in fact not independent, and this now leads to difficulties.
To avoid them, the interface should let you specify a desired
configuration all at once, and its implementation should then do what it
takes to get from here to there.
Example: current state is multifd compression method "zstd", compression
level is 19. User wants to switch to method "zlib" level 9 the obvious
way
With old interface:
step 1: set method to "zlib"
step 2: set level to 9
Problem: after step 1, we have method "zlib" with invalid level 19.
Workaround: swap the steps.
Note that the workaround only works because the set of levels both
methods support is non-empty. We might still come up with more
complicated parameter combinations where that is not the case.
With new interface:
set compression to "zlib" level 9
The new interface require us to specify a QAPI type capable of holding
the complete migration configuration.
The stupid way is to throw all migration parameters into a struct, and
make the ones optional that apply only when others have certain values.
Thus, the "applies only when" bits are semantical, documented in
comments, and enforced by C code.
With a bit more care, we can make "applies only when" syntactical
instead. Examples:
@max-bandwidth and @downtime-limit always apply. They go straight
into the struct.
@tls-creds, @tls-hostname, @tls-authz apply only when TLS is enabled
by setting @tls-creds. Have an optional member @tls, which is a
struct with mandatory member @creds, optional members @hostname,
@authz.
@multifd-zlib-level applies when @multifd-method is "zlib", and
@multifd-zstd-level applies when it's "zstd". Have a union
@multifd-compression, cases "none", "zlib" and "zstd", where each
case's members are the parameters applying in that case.
Please note the purpose of these examples is to show how things can be
done in the schema. I'm not trying to tell you how these specific
things *should* be done.
The resulting type is perfectly suited as return value of a query
command. It's awkward as argument of a "specify desired configuration"
command, because it requires the user to specify *complete*
configuration. If we want to support omitting the parts of it we don't
want to change, we have to make more members optional. Imprecise for
the query, where we now have to specify "always present" in comments.
Usually less bad than duplicating a complex type.
>> Documentation of admissible range will become a bit awkward, too.
>>
>> Too many migration parameters...
>
> Sure, but the other option is taking a value and live with it.
> I am all for leaving the library default and call it a day.
>
> Later, Juan.
Hope this helps some.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v5 6/8] configure: Enable test and libs for zstd
2020-02-13 21:08 ` Juan Quintela
@ 2020-02-14 10:26 ` Daniel P. Berrangé
0 siblings, 0 replies; 39+ messages in thread
From: Daniel P. Berrangé @ 2020-02-14 10:26 UTC (permalink / raw)
To: Juan Quintela
Cc: Laurent Vivier, Thomas Huth, Eduardo Habkost, Markus Armbruster,
qemu-devel, Dr. David Alan Gilbert, Paolo Bonzini
On Thu, Feb 13, 2020 at 10:08:59PM +0100, Juan Quintela wrote:
> Daniel P. Berrangé <berrange@redhat.com> wrote:
> > On Wed, Jan 29, 2020 at 12:56:53PM +0100, Juan Quintela wrote:
> >> Signed-off-by: Juan Quintela <quintela@redhat.com>
> >> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> >> ---
> >> configure | 30 ++++++++++++++++++++++++++++++
> >> 1 file changed, 30 insertions(+)
> >
> > This is adding a new 3rd party library to QEMU that we've not previously
> > built and so isn't included in any of our CI platforms.
>
> Ok.
>
> Learning how one does that.
>
> > This commit should be updating at least some of our CI platforms to
> > request the libzstd library installation to get CI coverage for the
> > latest patches in this series.
>
> > Probably the docker files,
>
> I added it in all debian/centos/fedora files that zlib-dev or xen-dev
>
> > the VM installs for FreeBSD at least,
>
> A fast google finds that library is called "zstd" and that it includes
> the includes (put intended)
>
> tests/vm/freebsd
>
> Once there, include it in fedora
>
> > travis and
>
> I added it to .travis.yml
>
> > gitlab CI.
>
> gitlab-ci.yml (just when we compile x86_64-softmmu)
>
> I have something like this, but net real clue how to test that I haven't
> broken anything:
>
> commit 59d8694dfcfc3d5ef36bc72a5c02bedaa3a6a6ec
> Author: Juan Quintela <quintela@redhat.com>
> Date: Thu Feb 13 22:06:16 2020 +0100
>
> Use zstd packages
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
>
> 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 5887055951..dd17301f3b 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -48,6 +48,7 @@ addons:
> - libusb-1.0-0-dev
> - libvdeplug-dev
> - libvte-2.91-dev
> + - libzstd-dev
> - sparse
> - uuid-dev
> - gcovr
> 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 = """
This looks good to me, thanks.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v5 7/8] multifd: Add multifd-zstd-level parameter
2020-02-14 8:49 ` Markus Armbruster
@ 2020-02-14 18:50 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 39+ messages in thread
From: Dr. David Alan Gilbert @ 2020-02-14 18:50 UTC (permalink / raw)
To: Markus Armbruster
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, Juan Quintela, qemu-devel, Paolo Bonzini
* Markus Armbruster (armbru@redhat.com) wrote:
> Juan Quintela <quintela@redhat.com> writes:
>
> > Markus Armbruster <armbru@redhat.com> wrote:
> >> "Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:
> >>
> >>> * Juan Quintela (quintela@redhat.com) wrote:
> >>>> Signed-off-by: Juan Quintela <quintela@redhat.com>
> >>>> ---
> >>>> migration/migration.c | 15 +++++++++++++++
> >>>> monitor/hmp-cmds.c | 4 ++++
> >>>> qapi/migration.json | 29 ++++++++++++++++++++++++++---
> >>>> 3 files changed, 45 insertions(+), 3 deletions(-)
> >>>>
> >>>> diff --git a/migration/migration.c b/migration/migration.c
> >>>> index 3b081e8147..b690500545 100644
> >>>> --- a/migration/migration.c
> >>>> +++ b/migration/migration.c
> >>>> @@ -91,6 +91,8 @@
> >>>> #define DEFAULT_MIGRATE_MULTIFD_METHOD MULTIFD_METHOD_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_method = s->parameters.multifd_method;
> >>>> 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;
> >>>
> >>> Do we really want different 'multifd_...._level's or just one
> >>> 'multifd_compress_level' - or even just reuse the existing
> >>> 'compress-level' parameter.
> >>
> >> compress-level,
> >
> > possible values: 1 to 9 deprecated
> >
> >> multifd-zlib-level
> >
> > possible values: 1 to 9, default 1
> > (zlib default is -1, let the lib decide)
> >
> > , and multifd-zstd-level apply
> >
> > possible values: 1 to 19, default 1
> > (zstd default is 3)
> >
> >> "normal" live migration compression, multifd zlib live migration
> >> compression, and multifd zstd live migration compression, respectively.
> >>
> >> Any live migration can only use one of the three compressions.
> >>
> >> Correct?
> >
> > Yeap.
> >
> >>> The only tricky thing about combining them is how to handle
> >>> the difference in allowed ranges; When would the right time be
> >>> to check it?
> >>>
> >>> Markus/Eric: Any idea?
> >>
> >> To have an informed opinion, I'd have to dig through the migration
> >> code.
> >
> > Problem is _how_ to setup them. if we setup zstd compression method,
> > put the value at 19, and then setup zlib compression method, what should
> > we do?
> >
> > Truncate to 9?
> > Give one error?
> > Don't allow the zlib setup?
> >
> > Too complicated.
>
> The interface pretends the parameters are all independent: you get to
> set them one by one.
>
> They are in fact not independent, and this now leads to difficulties.
>
> To avoid them, the interface should let you specify a desired
> configuration all at once, and its implementation should then do what it
> takes to get from here to there.
>
> Example: current state is multifd compression method "zstd", compression
> level is 19. User wants to switch to method "zlib" level 9 the obvious
> way
>
> With old interface:
> step 1: set method to "zlib"
> step 2: set level to 9
> Problem: after step 1, we have method "zlib" with invalid level 19.
> Workaround: swap the steps.
>
> Note that the workaround only works because the set of levels both
> methods support is non-empty. We might still come up with more
> complicated parameter combinations where that is not the case.
>
> With new interface:
> set compression to "zlib" level 9
>
> The new interface require us to specify a QAPI type capable of holding
> the complete migration configuration.
>
> The stupid way is to throw all migration parameters into a struct, and
> make the ones optional that apply only when others have certain values.
> Thus, the "applies only when" bits are semantical, documented in
> comments, and enforced by C code.
I realised we already have that stupid struct! It's just
MigrationParameters - it has all the parameters as optional values,
and QMP's MigrateSetParameters already takes that - so you can already
provide both the type and the level at the same time; although there's
no semantic correlation between them.
Dave
> With a bit more care, we can make "applies only when" syntactical
> instead. Examples:
>
> @max-bandwidth and @downtime-limit always apply. They go straight
> into the struct.
>
> @tls-creds, @tls-hostname, @tls-authz apply only when TLS is enabled
> by setting @tls-creds. Have an optional member @tls, which is a
> struct with mandatory member @creds, optional members @hostname,
> @authz.
>
> @multifd-zlib-level applies when @multifd-method is "zlib", and
> @multifd-zstd-level applies when it's "zstd". Have a union
> @multifd-compression, cases "none", "zlib" and "zstd", where each
> case's members are the parameters applying in that case.
>
> Please note the purpose of these examples is to show how things can be
> done in the schema. I'm not trying to tell you how these specific
> things *should* be done.
>
> The resulting type is perfectly suited as return value of a query
> command. It's awkward as argument of a "specify desired configuration"
> command, because it requires the user to specify *complete*
> configuration. If we want to support omitting the parts of it we don't
> want to change, we have to make more members optional. Imprecise for
> the query, where we now have to specify "always present" in comments.
> Usually less bad than duplicating a complex type.
>
> >> Documentation of admissible range will become a bit awkward, too.
> >>
> >> Too many migration parameters...
> >
> > Sure, but the other option is taking a value and live with it.
> > I am all for leaving the library default and call it a day.
> >
> > Later, Juan.
>
> Hope this helps some.
>
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 39+ messages in thread
end of thread, other threads:[~2020-02-14 18:52 UTC | newest]
Thread overview: 39+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-29 11:56 [PATCH v5 0/8] Multifd Migration Compression Juan Quintela
2020-01-29 11:56 ` [PATCH v5 1/8] multifd: Add multifd-method parameter Juan Quintela
2020-01-30 7:54 ` Markus Armbruster
2020-01-30 9:11 ` Juan Quintela
2020-01-30 12:17 ` Markus Armbruster
2020-02-11 18:50 ` Daniel P. Berrangé
2020-02-13 19:29 ` Juan Quintela
2020-01-29 11:56 ` [PATCH v5 2/8] migration: Add support for modules Juan Quintela
2020-02-11 10:54 ` Dr. David Alan Gilbert
2020-02-13 19:38 ` Juan Quintela
2020-01-29 11:56 ` [PATCH v5 3/8] multifd: Make no compression operations into its own structure Juan Quintela
2020-02-07 18:45 ` Dr. David Alan Gilbert
2020-02-11 11:23 ` Juan Quintela
2020-01-29 11:56 ` [PATCH v5 4/8] multifd: Add multifd-zlib-level parameter Juan Quintela
2020-01-30 8:03 ` Markus Armbruster
2020-01-30 8:56 ` Juan Quintela
2020-02-11 18:57 ` Daniel P. Berrangé
2020-02-13 13:27 ` Markus Armbruster
2020-02-13 16:33 ` Juan Quintela
2020-01-29 11:56 ` [PATCH v5 5/8] multifd: Add zlib compression multifd support Juan Quintela
2020-01-30 8:04 ` Markus Armbruster
2020-02-11 18:43 ` Dr. David Alan Gilbert
2020-02-13 20:24 ` Juan Quintela
2020-01-29 11:56 ` [PATCH v5 6/8] configure: Enable test and libs for zstd Juan Quintela
2020-02-11 20:11 ` Daniel P. Berrangé
2020-02-13 21:08 ` Juan Quintela
2020-02-14 10:26 ` Daniel P. Berrangé
2020-01-29 11:56 ` [PATCH v5 7/8] multifd: Add multifd-zstd-level parameter Juan Quintela
2020-01-30 8:08 ` Markus Armbruster
2020-02-11 18:47 ` Dr. David Alan Gilbert
2020-02-13 14:04 ` Markus Armbruster
2020-02-13 14:28 ` Dr. David Alan Gilbert
2020-02-13 15:33 ` Juan Quintela
2020-02-14 8:49 ` Markus Armbruster
2020-02-14 18:50 ` Dr. David Alan Gilbert
2020-01-29 11:56 ` [PATCH v5 8/8] multifd: Add zstd compression multifd support Juan Quintela
2020-01-30 8:08 ` Markus Armbruster
2020-02-11 20:01 ` Dr. David Alan Gilbert
2020-02-13 20:39 ` Juan Quintela
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).