* [PULL 01/26] tests/qtest/migration: Add 'to' object into migrate_qmp()
2024-04-23 22:37 [PULL 00/26] Migration 20240423 patches Peter Xu
@ 2024-04-23 22:37 ` Peter Xu
2024-04-23 22:37 ` [PULL 02/26] tests/qtest/migration: Replace connect_uri and move migrate_get_socket_address inside migrate_qmp Peter Xu
` (25 subsequent siblings)
26 siblings, 0 replies; 28+ messages in thread
From: Peter Xu @ 2024-04-23 22:37 UTC (permalink / raw)
To: qemu-devel; +Cc: peterx, Prasad Pandit, Peter Maydell, Fabiano Rosas, Het Gala
From: Het Gala <het.gala@nutanix.com>
Add the 'to' object into migrate_qmp(), so we can use
migrate_get_socket_address() inside migrate_qmp() to get
the port value. This is not applied to other migrate_qmp*
because they don't need the port.
Signed-off-by: Het Gala <het.gala@nutanix.com>
Suggested-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20240312202634.63349-2-het.gala@nutanix.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
tests/qtest/migration-helpers.h | 5 +++--
tests/qtest/migration-helpers.c | 3 ++-
tests/qtest/migration-test.c | 28 ++++++++++++++--------------
3 files changed, 19 insertions(+), 17 deletions(-)
diff --git a/tests/qtest/migration-helpers.h b/tests/qtest/migration-helpers.h
index 3bf7ded1b9..e16a34c796 100644
--- a/tests/qtest/migration-helpers.h
+++ b/tests/qtest/migration-helpers.h
@@ -25,8 +25,9 @@ typedef struct QTestMigrationState {
bool migrate_watch_for_events(QTestState *who, const char *name,
QDict *event, void *opaque);
-G_GNUC_PRINTF(3, 4)
-void migrate_qmp(QTestState *who, const char *uri, const char *fmt, ...);
+G_GNUC_PRINTF(4, 5)
+void migrate_qmp(QTestState *who, QTestState *to, const char *uri,
+ const char *fmt, ...);
G_GNUC_PRINTF(3, 4)
void migrate_incoming_qmp(QTestState *who, const char *uri,
diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
index e451dbdbed..b6206a04fb 100644
--- a/tests/qtest/migration-helpers.c
+++ b/tests/qtest/migration-helpers.c
@@ -68,7 +68,8 @@ void migrate_qmp_fail(QTestState *who, const char *uri, const char *fmt, ...)
* Arguments are built from @fmt... (formatted like
* qobject_from_jsonf_nofail()) with "uri": @uri spliced in.
*/
-void migrate_qmp(QTestState *who, const char *uri, const char *fmt, ...)
+void migrate_qmp(QTestState *who, QTestState *to, const char *uri,
+ const char *fmt, ...)
{
va_list ap;
QDict *args;
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 1d2cee87ea..39c393b7d9 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -1350,7 +1350,7 @@ static int migrate_postcopy_prepare(QTestState **from_ptr,
wait_for_suspend(from, &src_state);
g_autofree char *uri = migrate_get_socket_address(to, "socket-address");
- migrate_qmp(from, uri, "{}");
+ migrate_qmp(from, to, uri, "{}");
migrate_wait_for_dirty_mem(from, to);
@@ -1500,7 +1500,7 @@ static void postcopy_recover_fail(QTestState *from, QTestState *to)
g_assert_cmpint(ret, ==, 1);
migrate_recover(to, "fd:fd-mig");
- migrate_qmp(from, "fd:fd-mig", "{'resume': true}");
+ migrate_qmp(from, to, "fd:fd-mig", "{'resume': true}");
/*
* Make sure both QEMU instances will go into RECOVER stage, then test
@@ -1588,7 +1588,7 @@ static void test_postcopy_recovery_common(MigrateCommon *args)
* Try to rebuild the migration channel using the resume flag and
* the newly created channel
*/
- migrate_qmp(from, uri, "{'resume': true}");
+ migrate_qmp(from, to, uri, "{'resume': true}");
/* Restore the postcopy bandwidth to unlimited */
migrate_set_parameter_int(from, "max-postcopy-bandwidth", 0);
@@ -1669,7 +1669,7 @@ static void test_baddest(void)
if (test_migrate_start(&from, &to, "tcp:127.0.0.1:0", &args)) {
return;
}
- migrate_qmp(from, "tcp:127.0.0.1:0", "{}");
+ migrate_qmp(from, to, "tcp:127.0.0.1:0", "{}");
wait_for_migration_fail(from, false);
test_migrate_end(from, to, false);
}
@@ -1708,7 +1708,7 @@ static void test_analyze_script(void)
uri = g_strdup_printf("exec:cat > %s", file);
migrate_ensure_converge(from);
- migrate_qmp(from, uri, "{}");
+ migrate_qmp(from, to, uri, "{}");
wait_for_migration_complete(from);
pid = fork();
@@ -1777,7 +1777,7 @@ static void test_precopy_common(MigrateCommon *args)
goto finish;
}
- migrate_qmp(from, connect_uri, "{}");
+ migrate_qmp(from, to, connect_uri, "{}");
if (args->result != MIG_TEST_SUCCEED) {
bool allow_active = args->result == MIG_TEST_FAIL;
@@ -1873,7 +1873,7 @@ static void test_file_common(MigrateCommon *args, bool stop_src)
goto finish;
}
- migrate_qmp(from, connect_uri, "{}");
+ migrate_qmp(from, to, connect_uri, "{}");
wait_for_migration_complete(from);
/*
@@ -2029,7 +2029,7 @@ static void test_ignore_shared(void)
/* Wait for the first serial output from the source */
wait_for_serial("src_serial");
- migrate_qmp(from, uri, "{}");
+ migrate_qmp(from, to, uri, "{}");
migrate_wait_for_dirty_mem(from, to);
@@ -2568,7 +2568,7 @@ static void do_test_validate_uuid(MigrateStart *args, bool should_fail)
/* Wait for the first serial output from the source */
wait_for_serial("src_serial");
- migrate_qmp(from, uri, "{}");
+ migrate_qmp(from, to, uri, "{}");
if (should_fail) {
qtest_set_expected_status(to, EXIT_FAILURE);
@@ -2671,7 +2671,7 @@ static void test_migrate_auto_converge(void)
/* Wait for the first serial output from the source */
wait_for_serial("src_serial");
- migrate_qmp(from, uri, "{}");
+ migrate_qmp(from, to, uri, "{}");
/* Wait for throttling begins */
percentage = 0;
@@ -3040,7 +3040,7 @@ static void test_multifd_tcp_cancel(void)
uri = migrate_get_socket_address(to, "socket-address");
- migrate_qmp(from, uri, "{}");
+ migrate_qmp(from, to, uri, "{}");
migrate_wait_for_dirty_mem(from, to);
@@ -3072,7 +3072,7 @@ static void test_multifd_tcp_cancel(void)
migrate_ensure_non_converge(from);
- migrate_qmp(from, uri, "{}");
+ migrate_qmp(from, to2, uri, "{}");
migrate_wait_for_dirty_mem(from, to2);
@@ -3405,7 +3405,7 @@ static void test_migrate_dirty_limit(void)
migrate_dirty_limit_wait_showup(from, dirtylimit_period, dirtylimit_value);
/* Start migrate */
- migrate_qmp(from, uri, "{}");
+ migrate_qmp(from, to, uri, "{}");
/* Wait for dirty limit throttle begin */
throttle_us_per_full = 0;
@@ -3446,7 +3446,7 @@ static void test_migrate_dirty_limit(void)
}
/* Start migrate */
- migrate_qmp(from, uri, "{}");
+ migrate_qmp(from, to, uri, "{}");
/* Wait for dirty limit throttle begin */
throttle_us_per_full = 0;
--
2.44.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PULL 02/26] tests/qtest/migration: Replace connect_uri and move migrate_get_socket_address inside migrate_qmp
2024-04-23 22:37 [PULL 00/26] Migration 20240423 patches Peter Xu
2024-04-23 22:37 ` [PULL 01/26] tests/qtest/migration: Add 'to' object into migrate_qmp() Peter Xu
@ 2024-04-23 22:37 ` Peter Xu
2024-04-23 22:37 ` [PULL 03/26] tests/qtest/migration: Replace migrate_get_connect_uri inplace of migrate_get_socket_address Peter Xu
` (24 subsequent siblings)
26 siblings, 0 replies; 28+ messages in thread
From: Peter Xu @ 2024-04-23 22:37 UTC (permalink / raw)
To: qemu-devel; +Cc: peterx, Prasad Pandit, Peter Maydell, Fabiano Rosas, Het Gala
From: Het Gala <het.gala@nutanix.com>
Move the calls to migrate_get_socket_address() into migrate_qmp().
Get rid of connect_uri and replace it with args->connect_uri only
because 'to' object will help to generate connect_uri with the
correct port number.
Signed-off-by: Het Gala <het.gala@nutanix.com>
Suggested-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20240312202634.63349-3-het.gala@nutanix.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
tests/qtest/migration-helpers.c | 54 ++++++++++++++++++++-
tests/qtest/migration-test.c | 83 ++++-----------------------------
2 files changed, 63 insertions(+), 74 deletions(-)
diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
index b6206a04fb..3e8c19c4de 100644
--- a/tests/qtest/migration-helpers.c
+++ b/tests/qtest/migration-helpers.c
@@ -13,6 +13,9 @@
#include "qemu/osdep.h"
#include "qemu/ctype.h"
#include "qapi/qmp/qjson.h"
+#include "qapi/qapi-visit-sockets.h"
+#include "qapi/qobject-input-visitor.h"
+#include "qapi/error.h"
#include "migration-helpers.h"
@@ -24,6 +27,51 @@
*/
#define MIGRATION_STATUS_WAIT_TIMEOUT 120
+static char *SocketAddress_to_str(SocketAddress *addr)
+{
+ switch (addr->type) {
+ case SOCKET_ADDRESS_TYPE_INET:
+ return g_strdup_printf("tcp:%s:%s",
+ addr->u.inet.host,
+ addr->u.inet.port);
+ case SOCKET_ADDRESS_TYPE_UNIX:
+ return g_strdup_printf("unix:%s",
+ addr->u.q_unix.path);
+ case SOCKET_ADDRESS_TYPE_FD:
+ return g_strdup_printf("fd:%s", addr->u.fd.str);
+ case SOCKET_ADDRESS_TYPE_VSOCK:
+ return g_strdup_printf("tcp:%s:%s",
+ addr->u.vsock.cid,
+ addr->u.vsock.port);
+ default:
+ return g_strdup("unknown address type");
+ }
+}
+
+static char *
+migrate_get_socket_address(QTestState *who, const char *parameter)
+{
+ QDict *rsp;
+ char *result;
+ SocketAddressList *addrs;
+ Visitor *iv = NULL;
+ QObject *object;
+
+ rsp = migrate_query(who);
+ object = qdict_get(rsp, parameter);
+
+ iv = qobject_input_visitor_new(object);
+ visit_type_SocketAddressList(iv, NULL, &addrs, &error_abort);
+ visit_free(iv);
+
+ /* we are only using a single address */
+ result = SocketAddress_to_str(addrs->value);
+
+ qapi_free_SocketAddressList(addrs);
+ qobject_unref(rsp);
+ return result;
+}
+
bool migrate_watch_for_events(QTestState *who, const char *name,
QDict *event, void *opaque)
{
@@ -73,13 +121,17 @@ void migrate_qmp(QTestState *who, QTestState *to, const char *uri,
{
va_list ap;
QDict *args;
+ g_autofree char *connect_uri = NULL;
va_start(ap, fmt);
args = qdict_from_vjsonf_nofail(fmt, ap);
va_end(ap);
g_assert(!qdict_haskey(args, "uri"));
- qdict_put_str(args, "uri", uri);
+ if (!uri) {
+ connect_uri = migrate_get_socket_address(to, "socket-address");
+ }
+ qdict_put_str(args, "uri", uri ? uri : connect_uri);
qtest_qmp_assert_success(who,
"{ 'execute': 'migrate', 'arguments': %p}", args);
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 39c393b7d9..0c76fe2615 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -13,16 +13,12 @@
#include "qemu/osdep.h"
#include "libqtest.h"
-#include "qapi/error.h"
#include "qapi/qmp/qdict.h"
#include "qemu/module.h"
#include "qemu/option.h"
#include "qemu/range.h"
#include "qemu/sockets.h"
#include "chardev/char.h"
-#include "qapi/qapi-visit-sockets.h"
-#include "qapi/qobject-input-visitor.h"
-#include "qapi/qobject-output-visitor.h"
#include "crypto/tlscredspsk.h"
#include "qapi/qmp/qlist.h"
@@ -369,50 +365,6 @@ static void cleanup(const char *filename)
unlink(path);
}
-static char *SocketAddress_to_str(SocketAddress *addr)
-{
- switch (addr->type) {
- case SOCKET_ADDRESS_TYPE_INET:
- return g_strdup_printf("tcp:%s:%s",
- addr->u.inet.host,
- addr->u.inet.port);
- case SOCKET_ADDRESS_TYPE_UNIX:
- return g_strdup_printf("unix:%s",
- addr->u.q_unix.path);
- case SOCKET_ADDRESS_TYPE_FD:
- return g_strdup_printf("fd:%s", addr->u.fd.str);
- case SOCKET_ADDRESS_TYPE_VSOCK:
- return g_strdup_printf("tcp:%s:%s",
- addr->u.vsock.cid,
- addr->u.vsock.port);
- default:
- return g_strdup("unknown address type");
- }
-}
-
-static char *migrate_get_socket_address(QTestState *who, const char *parameter)
-{
- QDict *rsp;
- char *result;
- SocketAddressList *addrs;
- Visitor *iv = NULL;
- QObject *object;
-
- rsp = migrate_query(who);
- object = qdict_get(rsp, parameter);
-
- iv = qobject_input_visitor_new(object);
- visit_type_SocketAddressList(iv, NULL, &addrs, &error_abort);
- visit_free(iv);
-
- /* we are only using a single address */
- result = SocketAddress_to_str(addrs->value);
-
- qapi_free_SocketAddressList(addrs);
- qobject_unref(rsp);
- return result;
-}
-
static long long migrate_get_parameter_int(QTestState *who,
const char *parameter)
{
@@ -1349,8 +1301,7 @@ static int migrate_postcopy_prepare(QTestState **from_ptr,
wait_for_serial("src_serial");
wait_for_suspend(from, &src_state);
- g_autofree char *uri = migrate_get_socket_address(to, "socket-address");
- migrate_qmp(from, to, uri, "{}");
+ migrate_qmp(from, to, NULL, "{}");
migrate_wait_for_dirty_mem(from, to);
@@ -1733,7 +1684,6 @@ static void test_precopy_common(MigrateCommon *args)
{
QTestState *from, *to;
void *data_hook = NULL;
- g_autofree char *connect_uri = NULL;
if (test_migrate_start(&from, &to, args->listen_uri, &args->start)) {
return;
@@ -1766,18 +1716,12 @@ static void test_precopy_common(MigrateCommon *args)
}
}
- if (!args->connect_uri) {
- connect_uri = migrate_get_socket_address(to, "socket-address");
- } else {
- connect_uri = g_strdup(args->connect_uri);
- }
-
if (args->result == MIG_TEST_QMP_ERROR) {
- migrate_qmp_fail(from, connect_uri, "{}");
+ migrate_qmp_fail(from, args->connect_uri, "{}");
goto finish;
}
- migrate_qmp(from, to, connect_uri, "{}");
+ migrate_qmp(from, to, args->connect_uri, "{}");
if (args->result != MIG_TEST_SUCCEED) {
bool allow_active = args->result == MIG_TEST_FAIL;
@@ -1843,7 +1787,6 @@ static void test_file_common(MigrateCommon *args, bool stop_src)
{
QTestState *from, *to;
void *data_hook = NULL;
- g_autofree char *connect_uri = g_strdup(args->connect_uri);
if (test_migrate_start(&from, &to, args->listen_uri, &args->start)) {
return;
@@ -1869,18 +1812,18 @@ static void test_file_common(MigrateCommon *args, bool stop_src)
}
if (args->result == MIG_TEST_QMP_ERROR) {
- migrate_qmp_fail(from, connect_uri, "{}");
+ migrate_qmp_fail(from, args->connect_uri, "{}");
goto finish;
}
- migrate_qmp(from, to, connect_uri, "{}");
+ migrate_qmp(from, to, args->connect_uri, "{}");
wait_for_migration_complete(from);
/*
* We need to wait for the source to finish before starting the
* destination.
*/
- migrate_incoming_qmp(to, connect_uri, "{}");
+ migrate_incoming_qmp(to, args->connect_uri, "{}");
wait_for_migration_complete(to);
if (stop_src) {
@@ -3017,7 +2960,6 @@ static void test_multifd_tcp_cancel(void)
.hide_stderr = true,
};
QTestState *from, *to, *to2;
- g_autofree char *uri = NULL;
if (test_migrate_start(&from, &to, "defer", &args)) {
return;
@@ -3038,9 +2980,7 @@ static void test_multifd_tcp_cancel(void)
/* Wait for the first serial output from the source */
wait_for_serial("src_serial");
- uri = migrate_get_socket_address(to, "socket-address");
-
- migrate_qmp(from, to, uri, "{}");
+ migrate_qmp(from, to, NULL, "{}");
migrate_wait_for_dirty_mem(from, to);
@@ -3065,14 +3005,11 @@ static void test_multifd_tcp_cancel(void)
/* Start incoming migration from the 1st socket */
migrate_incoming_qmp(to2, "tcp:127.0.0.1:0", "{}");
- g_free(uri);
- uri = migrate_get_socket_address(to2, "socket-address");
-
wait_for_migration_status(from, "cancelled", NULL);
migrate_ensure_non_converge(from);
- migrate_qmp(from, to2, uri, "{}");
+ migrate_qmp(from, to2, NULL, "{}");
migrate_wait_for_dirty_mem(from, to2);
@@ -3405,7 +3342,7 @@ static void test_migrate_dirty_limit(void)
migrate_dirty_limit_wait_showup(from, dirtylimit_period, dirtylimit_value);
/* Start migrate */
- migrate_qmp(from, to, uri, "{}");
+ migrate_qmp(from, to, args.connect_uri, "{}");
/* Wait for dirty limit throttle begin */
throttle_us_per_full = 0;
@@ -3446,7 +3383,7 @@ static void test_migrate_dirty_limit(void)
}
/* Start migrate */
- migrate_qmp(from, to, uri, "{}");
+ migrate_qmp(from, to, args.connect_uri, "{}");
/* Wait for dirty limit throttle begin */
throttle_us_per_full = 0;
--
2.44.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PULL 03/26] tests/qtest/migration: Replace migrate_get_connect_uri inplace of migrate_get_socket_address
2024-04-23 22:37 [PULL 00/26] Migration 20240423 patches Peter Xu
2024-04-23 22:37 ` [PULL 01/26] tests/qtest/migration: Add 'to' object into migrate_qmp() Peter Xu
2024-04-23 22:37 ` [PULL 02/26] tests/qtest/migration: Replace connect_uri and move migrate_get_socket_address inside migrate_qmp Peter Xu
@ 2024-04-23 22:37 ` Peter Xu
2024-04-23 22:37 ` [PULL 04/26] tests/qtest/migration: Add channels parameter in migrate_qmp_fail Peter Xu
` (23 subsequent siblings)
26 siblings, 0 replies; 28+ messages in thread
From: Peter Xu @ 2024-04-23 22:37 UTC (permalink / raw)
To: qemu-devel; +Cc: peterx, Prasad Pandit, Peter Maydell, Fabiano Rosas, Het Gala
From: Het Gala <het.gala@nutanix.com>
Refactor migrate_get_socket_address to internally utilize 'socket-address'
parameter, reducing redundancy in the function definition.
migrate_get_socket_address implicitly converts SocketAddress into str.
Move migrate_get_socket_address inside migrate_get_connect_uri which
should return the uri string instead.
Signed-off-by: Het Gala <het.gala@nutanix.com>
Suggested-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20240312202634.63349-4-het.gala@nutanix.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
tests/qtest/migration-helpers.c | 29 +++++++++++++++++++----------
1 file changed, 19 insertions(+), 10 deletions(-)
diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
index 3e8c19c4de..8806dc841e 100644
--- a/tests/qtest/migration-helpers.c
+++ b/tests/qtest/migration-helpers.c
@@ -48,28 +48,37 @@ static char *SocketAddress_to_str(SocketAddress *addr)
}
}
-static char *
-migrate_get_socket_address(QTestState *who, const char *parameter)
+static SocketAddress *migrate_get_socket_address(QTestState *who)
{
QDict *rsp;
- char *result;
SocketAddressList *addrs;
+ SocketAddress *addr;
Visitor *iv = NULL;
QObject *object;
rsp = migrate_query(who);
- object = qdict_get(rsp, parameter);
+ object = qdict_get(rsp, "socket-address");
iv = qobject_input_visitor_new(object);
visit_type_SocketAddressList(iv, NULL, &addrs, &error_abort);
+ addr = addrs->value;
visit_free(iv);
- /* we are only using a single address */
- result = SocketAddress_to_str(addrs->value);
-
- qapi_free_SocketAddressList(addrs);
qobject_unref(rsp);
- return result;
+ return addr;
+}
+
+static char *
+migrate_get_connect_uri(QTestState *who)
+{
+ SocketAddress *addrs;
+ char *connect_uri;
+
+ addrs = migrate_get_socket_address(who);
+ connect_uri = SocketAddress_to_str(addrs);
+
+ qapi_free_SocketAddress(addrs);
+ return connect_uri;
}
bool migrate_watch_for_events(QTestState *who, const char *name,
@@ -129,7 +138,7 @@ void migrate_qmp(QTestState *who, QTestState *to, const char *uri,
g_assert(!qdict_haskey(args, "uri"));
if (!uri) {
- connect_uri = migrate_get_socket_address(to, "socket-address");
+ connect_uri = migrate_get_connect_uri(to);
}
qdict_put_str(args, "uri", uri ? uri : connect_uri);
--
2.44.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PULL 04/26] tests/qtest/migration: Add channels parameter in migrate_qmp_fail
2024-04-23 22:37 [PULL 00/26] Migration 20240423 patches Peter Xu
` (2 preceding siblings ...)
2024-04-23 22:37 ` [PULL 03/26] tests/qtest/migration: Replace migrate_get_connect_uri inplace of migrate_get_socket_address Peter Xu
@ 2024-04-23 22:37 ` Peter Xu
2024-04-23 22:37 ` [PULL 05/26] tests/qtest/migration: Add migrate_set_ports into migrate_qmp to update migration port value Peter Xu
` (22 subsequent siblings)
26 siblings, 0 replies; 28+ messages in thread
From: Peter Xu @ 2024-04-23 22:37 UTC (permalink / raw)
To: qemu-devel; +Cc: peterx, Prasad Pandit, Peter Maydell, Fabiano Rosas, Het Gala
From: Het Gala <het.gala@nutanix.com>
Alter migrate_qmp_fail() to allow both uri and channels
independently. For channels, convert string to a Dict.
No dealing with migrate_get_socket_address() here because
we will fail before starting the migration anyway.
Signed-off-by: Het Gala <het.gala@nutanix.com>
Suggested-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20240312202634.63349-5-het.gala@nutanix.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
tests/qtest/migration-helpers.h | 5 +++--
tests/qtest/migration-helpers.c | 13 +++++++++++--
tests/qtest/migration-test.c | 4 ++--
3 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/tests/qtest/migration-helpers.h b/tests/qtest/migration-helpers.h
index e16a34c796..4e664148a5 100644
--- a/tests/qtest/migration-helpers.h
+++ b/tests/qtest/migration-helpers.h
@@ -33,8 +33,9 @@ G_GNUC_PRINTF(3, 4)
void migrate_incoming_qmp(QTestState *who, const char *uri,
const char *fmt, ...);
-G_GNUC_PRINTF(3, 4)
-void migrate_qmp_fail(QTestState *who, const char *uri, const char *fmt, ...);
+G_GNUC_PRINTF(4, 5)
+void migrate_qmp_fail(QTestState *who, const char *uri,
+ const char *channels, const char *fmt, ...);
void migrate_set_capability(QTestState *who, const char *capability,
bool value);
diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
index 8806dc841e..f215f44467 100644
--- a/tests/qtest/migration-helpers.c
+++ b/tests/qtest/migration-helpers.c
@@ -100,7 +100,8 @@ bool migrate_watch_for_events(QTestState *who, const char *name,
return false;
}
-void migrate_qmp_fail(QTestState *who, const char *uri, const char *fmt, ...)
+void migrate_qmp_fail(QTestState *who, const char *uri,
+ const char *channels, const char *fmt, ...)
{
va_list ap;
QDict *args, *err;
@@ -110,7 +111,15 @@ void migrate_qmp_fail(QTestState *who, const char *uri, const char *fmt, ...)
va_end(ap);
g_assert(!qdict_haskey(args, "uri"));
- qdict_put_str(args, "uri", uri);
+ if (uri) {
+ qdict_put_str(args, "uri", uri);
+ }
+
+ g_assert(!qdict_haskey(args, "channels"));
+ if (channels) {
+ QObject *channels_obj = qobject_from_json(channels, &error_abort);
+ qdict_put_obj(args, "channels", channels_obj);
+ }
err = qtest_qmp_assert_failure_ref(
who, "{ 'execute': 'migrate', 'arguments': %p}", args);
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 0c76fe2615..763ff27f33 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -1717,7 +1717,7 @@ static void test_precopy_common(MigrateCommon *args)
}
if (args->result == MIG_TEST_QMP_ERROR) {
- migrate_qmp_fail(from, args->connect_uri, "{}");
+ migrate_qmp_fail(from, args->connect_uri, NULL, "{}");
goto finish;
}
@@ -1812,7 +1812,7 @@ static void test_file_common(MigrateCommon *args, bool stop_src)
}
if (args->result == MIG_TEST_QMP_ERROR) {
- migrate_qmp_fail(from, args->connect_uri, "{}");
+ migrate_qmp_fail(from, args->connect_uri, NULL, "{}");
goto finish;
}
--
2.44.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PULL 05/26] tests/qtest/migration: Add migrate_set_ports into migrate_qmp to update migration port value
2024-04-23 22:37 [PULL 00/26] Migration 20240423 patches Peter Xu
` (3 preceding siblings ...)
2024-04-23 22:37 ` [PULL 04/26] tests/qtest/migration: Add channels parameter in migrate_qmp_fail Peter Xu
@ 2024-04-23 22:37 ` Peter Xu
2024-04-23 22:37 ` [PULL 06/26] tests/qtest/migration: Add channels parameter in migrate_qmp Peter Xu
` (21 subsequent siblings)
26 siblings, 0 replies; 28+ messages in thread
From: Peter Xu @ 2024-04-23 22:37 UTC (permalink / raw)
To: qemu-devel; +Cc: peterx, Prasad Pandit, Peter Maydell, Fabiano Rosas, Het Gala
From: Het Gala <het.gala@nutanix.com>
migrate_get_connect_qdict gets qdict with the dst QEMU parameters.
migrate_set_ports() from list of channels reads each QDict for port,
and fills the port with correct value in case it was 0 in the test.
Signed-off-by: Het Gala <het.gala@nutanix.com>
Suggested-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20240312202634.63349-6-het.gala@nutanix.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
tests/qtest/migration-helpers.c | 75 +++++++++++++++++++++++++++++++++
1 file changed, 75 insertions(+)
diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
index f215f44467..a330ef9c7f 100644
--- a/tests/qtest/migration-helpers.c
+++ b/tests/qtest/migration-helpers.c
@@ -16,6 +16,8 @@
#include "qapi/qapi-visit-sockets.h"
#include "qapi/qobject-input-visitor.h"
#include "qapi/error.h"
+#include "qapi/qmp/qlist.h"
+#include "qemu/cutils.h"
#include "migration-helpers.h"
@@ -48,6 +50,37 @@ static char *SocketAddress_to_str(SocketAddress *addr)
}
}
+static QDict *SocketAddress_to_qdict(SocketAddress *addr)
+{
+ QDict *dict = qdict_new();
+
+ switch (addr->type) {
+ case SOCKET_ADDRESS_TYPE_INET:
+ qdict_put_str(dict, "type", "inet");
+ qdict_put_str(dict, "host", addr->u.inet.host);
+ qdict_put_str(dict, "port", addr->u.inet.port);
+ break;
+ case SOCKET_ADDRESS_TYPE_UNIX:
+ qdict_put_str(dict, "type", "unix");
+ qdict_put_str(dict, "path", addr->u.q_unix.path);
+ break;
+ case SOCKET_ADDRESS_TYPE_FD:
+ qdict_put_str(dict, "type", "fd");
+ qdict_put_str(dict, "str", addr->u.fd.str);
+ break;
+ case SOCKET_ADDRESS_TYPE_VSOCK:
+ qdict_put_str(dict, "type", "vsock");
+ qdict_put_str(dict, "cid", addr->u.vsock.cid);
+ qdict_put_str(dict, "port", addr->u.vsock.port);
+ break;
+ default:
+ g_assert_not_reached();
+ break;
+ }
+
+ return dict;
+}
+
static SocketAddress *migrate_get_socket_address(QTestState *who)
{
QDict *rsp;
@@ -81,6 +114,46 @@ migrate_get_connect_uri(QTestState *who)
return connect_uri;
}
+static QDict *
+migrate_get_connect_qdict(QTestState *who)
+{
+ SocketAddress *addrs;
+ QDict *connect_qdict;
+
+ addrs = migrate_get_socket_address(who);
+ connect_qdict = SocketAddress_to_qdict(addrs);
+
+ qapi_free_SocketAddress(addrs);
+ return connect_qdict;
+}
+
+static void migrate_set_ports(QTestState *to, QList *channel_list)
+{
+ QDict *addr;
+ QListEntry *entry;
+ const char *addr_port = NULL;
+
+ if (channel_list == NULL) {
+ return;
+ }
+
+ addr = migrate_get_connect_qdict(to);
+
+ QLIST_FOREACH_ENTRY(channel_list, entry) {
+ QDict *channel = qobject_to(QDict, qlist_entry_obj(entry));
+ QDict *addrdict = qdict_get_qdict(channel, "addr");
+
+ if (qdict_haskey(addrdict, "port") &&
+ qdict_haskey(addr, "port") &&
+ (strcmp(qdict_get_str(addrdict, "port"), "0") == 0)) {
+ addr_port = qdict_get_str(addr, "port");
+ qdict_put_str(addrdict, "port", g_strdup(addr_port));
+ }
+ }
+
+ qobject_unref(addr);
+}
+
bool migrate_watch_for_events(QTestState *who, const char *name,
QDict *event, void *opaque)
{
@@ -139,6 +212,7 @@ void migrate_qmp(QTestState *who, QTestState *to, const char *uri,
{
va_list ap;
QDict *args;
+ QList *channel_list = NULL;
g_autofree char *connect_uri = NULL;
va_start(ap, fmt);
@@ -149,6 +223,7 @@ void migrate_qmp(QTestState *who, QTestState *to, const char *uri,
if (!uri) {
connect_uri = migrate_get_connect_uri(to);
}
+ migrate_set_ports(to, channel_list);
qdict_put_str(args, "uri", uri ? uri : connect_uri);
qtest_qmp_assert_success(who,
--
2.44.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PULL 06/26] tests/qtest/migration: Add channels parameter in migrate_qmp
2024-04-23 22:37 [PULL 00/26] Migration 20240423 patches Peter Xu
` (4 preceding siblings ...)
2024-04-23 22:37 ` [PULL 05/26] tests/qtest/migration: Add migrate_set_ports into migrate_qmp to update migration port value Peter Xu
@ 2024-04-23 22:37 ` Peter Xu
2024-04-23 22:37 ` [PULL 07/26] tests/qtest/migration: Add multifd_tcp_plain test using list of channels instead of uri Peter Xu
` (20 subsequent siblings)
26 siblings, 0 replies; 28+ messages in thread
From: Peter Xu @ 2024-04-23 22:37 UTC (permalink / raw)
To: qemu-devel; +Cc: peterx, Prasad Pandit, Peter Maydell, Fabiano Rosas, Het Gala
From: Het Gala <het.gala@nutanix.com>
Alter migrate_qmp() to allow use of channels parameter, but only
fill the uri with correct port number if there are no channels.
Here we don't want to allow the wrong cases of having both or
none (ex: migrate_qmp_fail).
Signed-off-by: Het Gala <het.gala@nutanix.com>
Suggested-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20240312202634.63349-7-het.gala@nutanix.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
tests/qtest/migration-helpers.h | 4 ++--
tests/qtest/migration-helpers.c | 22 +++++++++++++---------
tests/qtest/migration-test.c | 28 ++++++++++++++--------------
3 files changed, 29 insertions(+), 25 deletions(-)
diff --git a/tests/qtest/migration-helpers.h b/tests/qtest/migration-helpers.h
index 4e664148a5..1339835698 100644
--- a/tests/qtest/migration-helpers.h
+++ b/tests/qtest/migration-helpers.h
@@ -25,9 +25,9 @@ typedef struct QTestMigrationState {
bool migrate_watch_for_events(QTestState *who, const char *name,
QDict *event, void *opaque);
-G_GNUC_PRINTF(4, 5)
+G_GNUC_PRINTF(5, 6)
void migrate_qmp(QTestState *who, QTestState *to, const char *uri,
- const char *fmt, ...);
+ const char *channels, const char *fmt, ...);
G_GNUC_PRINTF(3, 4)
void migrate_incoming_qmp(QTestState *who, const char *uri,
diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
index a330ef9c7f..3b72cad6c1 100644
--- a/tests/qtest/migration-helpers.c
+++ b/tests/qtest/migration-helpers.c
@@ -133,10 +133,6 @@ static void migrate_set_ports(QTestState *to, QList *channel_list)
QListEntry *entry;
const char *addr_port = NULL;
- if (channel_list == NULL) {
- return;
- }
-
addr = migrate_get_connect_qdict(to);
QLIST_FOREACH_ENTRY(channel_list, entry) {
@@ -208,11 +204,10 @@ void migrate_qmp_fail(QTestState *who, const char *uri,
* qobject_from_jsonf_nofail()) with "uri": @uri spliced in.
*/
void migrate_qmp(QTestState *who, QTestState *to, const char *uri,
- const char *fmt, ...)
+ const char *channels, const char *fmt, ...)
{
va_list ap;
QDict *args;
- QList *channel_list = NULL;
g_autofree char *connect_uri = NULL;
va_start(ap, fmt);
@@ -220,11 +215,20 @@ void migrate_qmp(QTestState *who, QTestState *to, const char *uri,
va_end(ap);
g_assert(!qdict_haskey(args, "uri"));
- if (!uri) {
+ if (uri) {
+ qdict_put_str(args, "uri", uri);
+ } else if (!channels) {
connect_uri = migrate_get_connect_uri(to);
+ qdict_put_str(args, "uri", connect_uri);
+ }
+
+ g_assert(!qdict_haskey(args, "channels"));
+ if (channels) {
+ QObject *channels_obj = qobject_from_json(channels, &error_abort);
+ QList *channel_list = qobject_to(QList, channels_obj);
+ migrate_set_ports(to, channel_list);
+ qdict_put_obj(args, "channels", channels_obj);
}
- migrate_set_ports(to, channel_list);
- qdict_put_str(args, "uri", uri ? uri : connect_uri);
qtest_qmp_assert_success(who,
"{ 'execute': 'migrate', 'arguments': %p}", args);
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 763ff27f33..af5e7dc6d6 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -1301,7 +1301,7 @@ static int migrate_postcopy_prepare(QTestState **from_ptr,
wait_for_serial("src_serial");
wait_for_suspend(from, &src_state);
- migrate_qmp(from, to, NULL, "{}");
+ migrate_qmp(from, to, NULL, NULL, "{}");
migrate_wait_for_dirty_mem(from, to);
@@ -1451,7 +1451,7 @@ static void postcopy_recover_fail(QTestState *from, QTestState *to)
g_assert_cmpint(ret, ==, 1);
migrate_recover(to, "fd:fd-mig");
- migrate_qmp(from, to, "fd:fd-mig", "{'resume': true}");
+ migrate_qmp(from, to, "fd:fd-mig", NULL, "{'resume': true}");
/*
* Make sure both QEMU instances will go into RECOVER stage, then test
@@ -1539,7 +1539,7 @@ static void test_postcopy_recovery_common(MigrateCommon *args)
* Try to rebuild the migration channel using the resume flag and
* the newly created channel
*/
- migrate_qmp(from, to, uri, "{'resume': true}");
+ migrate_qmp(from, to, uri, NULL, "{'resume': true}");
/* Restore the postcopy bandwidth to unlimited */
migrate_set_parameter_int(from, "max-postcopy-bandwidth", 0);
@@ -1620,7 +1620,7 @@ static void test_baddest(void)
if (test_migrate_start(&from, &to, "tcp:127.0.0.1:0", &args)) {
return;
}
- migrate_qmp(from, to, "tcp:127.0.0.1:0", "{}");
+ migrate_qmp(from, to, "tcp:127.0.0.1:0", NULL, "{}");
wait_for_migration_fail(from, false);
test_migrate_end(from, to, false);
}
@@ -1659,7 +1659,7 @@ static void test_analyze_script(void)
uri = g_strdup_printf("exec:cat > %s", file);
migrate_ensure_converge(from);
- migrate_qmp(from, to, uri, "{}");
+ migrate_qmp(from, to, uri, NULL, "{}");
wait_for_migration_complete(from);
pid = fork();
@@ -1721,7 +1721,7 @@ static void test_precopy_common(MigrateCommon *args)
goto finish;
}
- migrate_qmp(from, to, args->connect_uri, "{}");
+ migrate_qmp(from, to, args->connect_uri, NULL, "{}");
if (args->result != MIG_TEST_SUCCEED) {
bool allow_active = args->result == MIG_TEST_FAIL;
@@ -1816,7 +1816,7 @@ static void test_file_common(MigrateCommon *args, bool stop_src)
goto finish;
}
- migrate_qmp(from, to, args->connect_uri, "{}");
+ migrate_qmp(from, to, args->connect_uri, NULL, "{}");
wait_for_migration_complete(from);
/*
@@ -1972,7 +1972,7 @@ static void test_ignore_shared(void)
/* Wait for the first serial output from the source */
wait_for_serial("src_serial");
- migrate_qmp(from, to, uri, "{}");
+ migrate_qmp(from, to, uri, NULL, "{}");
migrate_wait_for_dirty_mem(from, to);
@@ -2511,7 +2511,7 @@ static void do_test_validate_uuid(MigrateStart *args, bool should_fail)
/* Wait for the first serial output from the source */
wait_for_serial("src_serial");
- migrate_qmp(from, to, uri, "{}");
+ migrate_qmp(from, to, uri, NULL, "{}");
if (should_fail) {
qtest_set_expected_status(to, EXIT_FAILURE);
@@ -2614,7 +2614,7 @@ static void test_migrate_auto_converge(void)
/* Wait for the first serial output from the source */
wait_for_serial("src_serial");
- migrate_qmp(from, to, uri, "{}");
+ migrate_qmp(from, to, uri, NULL, "{}");
/* Wait for throttling begins */
percentage = 0;
@@ -2980,7 +2980,7 @@ static void test_multifd_tcp_cancel(void)
/* Wait for the first serial output from the source */
wait_for_serial("src_serial");
- migrate_qmp(from, to, NULL, "{}");
+ migrate_qmp(from, to, NULL, NULL, "{}");
migrate_wait_for_dirty_mem(from, to);
@@ -3009,7 +3009,7 @@ static void test_multifd_tcp_cancel(void)
migrate_ensure_non_converge(from);
- migrate_qmp(from, to2, NULL, "{}");
+ migrate_qmp(from, to2, NULL, NULL, "{}");
migrate_wait_for_dirty_mem(from, to2);
@@ -3342,7 +3342,7 @@ static void test_migrate_dirty_limit(void)
migrate_dirty_limit_wait_showup(from, dirtylimit_period, dirtylimit_value);
/* Start migrate */
- migrate_qmp(from, to, args.connect_uri, "{}");
+ migrate_qmp(from, to, args.connect_uri, NULL, "{}");
/* Wait for dirty limit throttle begin */
throttle_us_per_full = 0;
@@ -3383,7 +3383,7 @@ static void test_migrate_dirty_limit(void)
}
/* Start migrate */
- migrate_qmp(from, to, args.connect_uri, "{}");
+ migrate_qmp(from, to, args.connect_uri, NULL, "{}");
/* Wait for dirty limit throttle begin */
throttle_us_per_full = 0;
--
2.44.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PULL 07/26] tests/qtest/migration: Add multifd_tcp_plain test using list of channels instead of uri
2024-04-23 22:37 [PULL 00/26] Migration 20240423 patches Peter Xu
` (5 preceding siblings ...)
2024-04-23 22:37 ` [PULL 06/26] tests/qtest/migration: Add channels parameter in migrate_qmp Peter Xu
@ 2024-04-23 22:37 ` Peter Xu
2024-04-23 22:37 ` [PULL 08/26] tests/qtest/migration: Add negative tests to validate migration QAPIs Peter Xu
` (19 subsequent siblings)
26 siblings, 0 replies; 28+ messages in thread
From: Peter Xu @ 2024-04-23 22:37 UTC (permalink / raw)
To: qemu-devel; +Cc: peterx, Prasad Pandit, Peter Maydell, Fabiano Rosas, Het Gala
From: Het Gala <het.gala@nutanix.com>
Add a positive test to check multifd live migration but this time
using list of channels (restricted to 1) as the starting point
instead of simple uri string.
Signed-off-by: Het Gala <het.gala@nutanix.com>
Suggested-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20240312202634.63349-8-het.gala@nutanix.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
tests/qtest/migration-test.c | 32 ++++++++++++++++++++++++++++----
1 file changed, 28 insertions(+), 4 deletions(-)
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index af5e7dc6d6..be67a92536 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -655,6 +655,13 @@ typedef struct {
*/
const char *connect_uri;
+ /*
+ * Optional: JSON-formatted list of src QEMU URIs. If a port is
+ * defined as '0' in any QDict key a value of '0' will be
+ * automatically converted to the correct destination port.
+ */
+ const char *connect_channels;
+
/* Optional: callback to run at start to set migration parameters */
TestMigrateStartHook start_hook;
/* Optional: callback to run at finish to cleanup */
@@ -1721,7 +1728,7 @@ static void test_precopy_common(MigrateCommon *args)
goto finish;
}
- migrate_qmp(from, to, args->connect_uri, NULL, "{}");
+ migrate_qmp(from, to, args->connect_uri, args->connect_channels, "{}");
if (args->result != MIG_TEST_SUCCEED) {
bool allow_active = args->result == MIG_TEST_FAIL;
@@ -2721,7 +2728,7 @@ test_migrate_precopy_tcp_multifd_zstd_start(QTestState *from,
}
#endif /* CONFIG_ZSTD */
-static void test_multifd_tcp_none(void)
+static void test_multifd_tcp_uri_none(void)
{
MigrateCommon args = {
.listen_uri = "defer",
@@ -2766,6 +2773,21 @@ static void test_multifd_tcp_no_zero_page(void)
test_precopy_common(&args);
}
+static void test_multifd_tcp_channels_none(void)
+{
+ MigrateCommon args = {
+ .listen_uri = "defer",
+ .start_hook = test_migrate_precopy_tcp_multifd_start,
+ .live = true,
+ .connect_channels = "[ { 'channel-type': 'main',"
+ " 'addr': { 'transport': 'socket',"
+ " 'type': 'inet',"
+ " 'host': '127.0.0.1',"
+ " 'port': '0' } } ]",
+ };
+ test_precopy_common(&args);
+}
+
static void test_multifd_tcp_zlib(void)
{
MigrateCommon args = {
@@ -3669,8 +3691,10 @@ int main(int argc, char **argv)
test_migrate_dirty_limit);
}
}
- migration_test_add("/migration/multifd/tcp/plain/none",
- test_multifd_tcp_none);
+ migration_test_add("/migration/multifd/tcp/uri/plain/none",
+ test_multifd_tcp_uri_none);
+ migration_test_add("/migration/multifd/tcp/channels/plain/none",
+ test_multifd_tcp_channels_none);
migration_test_add("/migration/multifd/tcp/plain/zero-page/legacy",
test_multifd_tcp_zero_page_legacy);
migration_test_add("/migration/multifd/tcp/plain/zero-page/none",
--
2.44.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PULL 08/26] tests/qtest/migration: Add negative tests to validate migration QAPIs
2024-04-23 22:37 [PULL 00/26] Migration 20240423 patches Peter Xu
` (6 preceding siblings ...)
2024-04-23 22:37 ` [PULL 07/26] tests/qtest/migration: Add multifd_tcp_plain test using list of channels instead of uri Peter Xu
@ 2024-04-23 22:37 ` Peter Xu
2024-04-23 22:37 ` [PULL 09/26] tests/qtest/migration: Fix typo for vsock in SocketAddress_to_str Peter Xu
` (18 subsequent siblings)
26 siblings, 0 replies; 28+ messages in thread
From: Peter Xu @ 2024-04-23 22:37 UTC (permalink / raw)
To: qemu-devel; +Cc: peterx, Prasad Pandit, Peter Maydell, Fabiano Rosas, Het Gala
From: Het Gala <het.gala@nutanix.com>
Migration QAPI arguments - uri and channels are mutually exhaustive.
Add negative validation tests, one with both arguments present and
one with none present.
Signed-off-by: Het Gala <het.gala@nutanix.com>
Suggested-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20240312202634.63349-9-het.gala@nutanix.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
tests/qtest/migration-test.c | 55 +++++++++++++++++++++++++++++++++++-
1 file changed, 54 insertions(+), 1 deletion(-)
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index be67a92536..5d6d8cd634 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -1724,7 +1724,7 @@ static void test_precopy_common(MigrateCommon *args)
}
if (args->result == MIG_TEST_QMP_ERROR) {
- migrate_qmp_fail(from, args->connect_uri, NULL, "{}");
+ migrate_qmp_fail(from, args->connect_uri, args->connect_channels, "{}");
goto finish;
}
@@ -2571,6 +2571,55 @@ static void test_validate_uuid_dst_not_set(void)
do_test_validate_uuid(&args, false);
}
+static void do_test_validate_uri_channel(MigrateCommon *args)
+{
+ QTestState *from, *to;
+
+ if (test_migrate_start(&from, &to, args->listen_uri, &args->start)) {
+ return;
+ }
+
+ /* Wait for the first serial output from the source */
+ wait_for_serial("src_serial");
+
+ /*
+ * 'uri' and 'channels' validation is checked even before the migration
+ * starts.
+ */
+ migrate_qmp_fail(from, args->connect_uri, args->connect_channels, "{}");
+ test_migrate_end(from, to, false);
+}
+
+static void test_validate_uri_channels_both_set(void)
+{
+ MigrateCommon args = {
+ .start = {
+ .hide_stderr = true,
+ },
+ .listen_uri = "defer",
+ .connect_uri = "tcp:127.0.0.1:0",
+ .connect_channels = "[ { 'channel-type': 'main',"
+ " 'addr': { 'transport': 'socket',"
+ " 'type': 'inet',"
+ " 'host': '127.0.0.1',"
+ " 'port': '0' } } ]",
+ };
+
+ do_test_validate_uri_channel(&args);
+}
+
+static void test_validate_uri_channels_none_set(void)
+{
+ MigrateCommon args = {
+ .start = {
+ .hide_stderr = true,
+ },
+ .listen_uri = "defer",
+ };
+
+ do_test_validate_uri_channel(&args);
+}
+
/*
* The way auto_converge works, we need to do too many passes to
* run this test. Auto_converge logic is only run once every
@@ -3679,6 +3728,10 @@ int main(int argc, char **argv)
test_validate_uuid_src_not_set);
migration_test_add("/migration/validate_uuid_dst_not_set",
test_validate_uuid_dst_not_set);
+ migration_test_add("/migration/validate_uri/channels/both_set",
+ test_validate_uri_channels_both_set);
+ migration_test_add("/migration/validate_uri/channels/none_set",
+ test_validate_uri_channels_none_set);
/*
* See explanation why this test is slow on function definition
*/
--
2.44.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PULL 09/26] tests/qtest/migration: Fix typo for vsock in SocketAddress_to_str
2024-04-23 22:37 [PULL 00/26] Migration 20240423 patches Peter Xu
` (7 preceding siblings ...)
2024-04-23 22:37 ` [PULL 08/26] tests/qtest/migration: Add negative tests to validate migration QAPIs Peter Xu
@ 2024-04-23 22:37 ` Peter Xu
2024-04-23 22:37 ` [PULL 10/26] s390/stattrib: Add Error** argument to set_migrationmode() handler Peter Xu
` (17 subsequent siblings)
26 siblings, 0 replies; 28+ messages in thread
From: Peter Xu @ 2024-04-23 22:37 UTC (permalink / raw)
To: qemu-devel; +Cc: peterx, Prasad Pandit, Peter Maydell, Fabiano Rosas, Het Gala
From: Het Gala <het.gala@nutanix.com>
Signed-off-by: Het Gala <het.gala@nutanix.com>
Link: https://lore.kernel.org/r/20240319204840.211632-2-het.gala@nutanix.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
tests/qtest/migration-helpers.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
index 3b72cad6c1..ce6d6615b5 100644
--- a/tests/qtest/migration-helpers.c
+++ b/tests/qtest/migration-helpers.c
@@ -42,7 +42,7 @@ static char *SocketAddress_to_str(SocketAddress *addr)
case SOCKET_ADDRESS_TYPE_FD:
return g_strdup_printf("fd:%s", addr->u.fd.str);
case SOCKET_ADDRESS_TYPE_VSOCK:
- return g_strdup_printf("tcp:%s:%s",
+ return g_strdup_printf("vsock:%s:%s",
addr->u.vsock.cid,
addr->u.vsock.port);
default:
--
2.44.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PULL 10/26] s390/stattrib: Add Error** argument to set_migrationmode() handler
2024-04-23 22:37 [PULL 00/26] Migration 20240423 patches Peter Xu
` (8 preceding siblings ...)
2024-04-23 22:37 ` [PULL 09/26] tests/qtest/migration: Fix typo for vsock in SocketAddress_to_str Peter Xu
@ 2024-04-23 22:37 ` Peter Xu
2024-04-23 22:37 ` [PULL 11/26] vfio: Always report an error in vfio_save_setup() Peter Xu
` (16 subsequent siblings)
26 siblings, 0 replies; 28+ messages in thread
From: Peter Xu @ 2024-04-23 22:37 UTC (permalink / raw)
To: qemu-devel
Cc: peterx, Prasad Pandit, Peter Maydell, Fabiano Rosas,
Cédric Le Goater, Halil Pasic, Christian Borntraeger,
Thomas Huth
From: Cédric Le Goater <clg@redhat.com>
This will prepare ground for future changes adding an Error** argument
to the save_setup() handler. We need to make sure that on failure,
set_migrationmode() always sets a new error. See the Rules section in
qapi/error.h.
Cc: Halil Pasic <pasic@linux.ibm.com>
Cc: Christian Borntraeger <borntraeger@linux.ibm.com>
Cc: Thomas Huth <thuth@redhat.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Cédric Le Goater <clg@redhat.com>
Link: https://lore.kernel.org/r/20240320064911.545001-2-clg@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
include/hw/s390x/storage-attributes.h | 2 +-
hw/s390x/s390-stattrib-kvm.c | 12 ++++++++++--
hw/s390x/s390-stattrib.c | 15 ++++++++++-----
3 files changed, 21 insertions(+), 8 deletions(-)
diff --git a/include/hw/s390x/storage-attributes.h b/include/hw/s390x/storage-attributes.h
index 5239eb538c..8921a04d51 100644
--- a/include/hw/s390x/storage-attributes.h
+++ b/include/hw/s390x/storage-attributes.h
@@ -39,7 +39,7 @@ struct S390StAttribClass {
int (*set_stattr)(S390StAttribState *sa, uint64_t start_gfn,
uint32_t count, uint8_t *values);
void (*synchronize)(S390StAttribState *sa);
- int (*set_migrationmode)(S390StAttribState *sa, bool value);
+ int (*set_migrationmode)(S390StAttribState *sa, bool value, Error **errp);
int (*get_active)(S390StAttribState *sa);
long long (*get_dirtycount)(S390StAttribState *sa);
};
diff --git a/hw/s390x/s390-stattrib-kvm.c b/hw/s390x/s390-stattrib-kvm.c
index 24cd01382e..eeaa811098 100644
--- a/hw/s390x/s390-stattrib-kvm.c
+++ b/hw/s390x/s390-stattrib-kvm.c
@@ -17,6 +17,7 @@
#include "sysemu/kvm.h"
#include "exec/ram_addr.h"
#include "kvm/kvm_s390x.h"
+#include "qapi/error.h"
Object *kvm_s390_stattrib_create(void)
{
@@ -137,14 +138,21 @@ static void kvm_s390_stattrib_synchronize(S390StAttribState *sa)
}
}
-static int kvm_s390_stattrib_set_migrationmode(S390StAttribState *sa, bool val)
+static int kvm_s390_stattrib_set_migrationmode(S390StAttribState *sa, bool val,
+ Error **errp)
{
struct kvm_device_attr attr = {
.group = KVM_S390_VM_MIGRATION,
.attr = val,
.addr = 0,
};
- return kvm_vm_ioctl(kvm_state, KVM_SET_DEVICE_ATTR, &attr);
+ int r;
+
+ r = kvm_vm_ioctl(kvm_state, KVM_SET_DEVICE_ATTR, &attr);
+ if (r) {
+ error_setg_errno(errp, -r, "setting KVM_S390_VM_MIGRATION failed");
+ }
+ return r;
}
static long long kvm_s390_stattrib_get_dirtycount(S390StAttribState *sa)
diff --git a/hw/s390x/s390-stattrib.c b/hw/s390x/s390-stattrib.c
index c483b62a9b..b743e8a2fe 100644
--- a/hw/s390x/s390-stattrib.c
+++ b/hw/s390x/s390-stattrib.c
@@ -60,11 +60,13 @@ void hmp_migrationmode(Monitor *mon, const QDict *qdict)
S390StAttribState *sas = s390_get_stattrib_device();
S390StAttribClass *sac = S390_STATTRIB_GET_CLASS(sas);
uint64_t what = qdict_get_int(qdict, "mode");
+ Error *local_err = NULL;
int r;
- r = sac->set_migrationmode(sas, what);
+ r = sac->set_migrationmode(sas, what, &local_err);
if (r < 0) {
- monitor_printf(mon, "Error: %s", strerror(-r));
+ monitor_printf(mon, "Error: %s", error_get_pretty(local_err));
+ error_free(local_err);
}
}
@@ -170,13 +172,15 @@ static int cmma_save_setup(QEMUFile *f, void *opaque)
{
S390StAttribState *sas = S390_STATTRIB(opaque);
S390StAttribClass *sac = S390_STATTRIB_GET_CLASS(sas);
+ Error *local_err = NULL;
int res;
/*
* Signal that we want to start a migration, thus needing PGSTE dirty
* tracking.
*/
- res = sac->set_migrationmode(sas, 1);
+ res = sac->set_migrationmode(sas, true, &local_err);
if (res) {
+ error_report_err(local_err);
return res;
}
qemu_put_be64(f, STATTR_FLAG_EOS);
@@ -260,7 +264,7 @@ static void cmma_save_cleanup(void *opaque)
{
S390StAttribState *sas = S390_STATTRIB(opaque);
S390StAttribClass *sac = S390_STATTRIB_GET_CLASS(sas);
- sac->set_migrationmode(sas, 0);
+ sac->set_migrationmode(sas, false, NULL);
}
static bool cmma_active(void *opaque)
@@ -293,7 +297,8 @@ static long long qemu_s390_get_dirtycount_stub(S390StAttribState *sa)
{
return 0;
}
-static int qemu_s390_set_migrationmode_stub(S390StAttribState *sa, bool value)
+static int qemu_s390_set_migrationmode_stub(S390StAttribState *sa, bool value,
+ Error **errp)
{
return 0;
}
--
2.44.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PULL 11/26] vfio: Always report an error in vfio_save_setup()
2024-04-23 22:37 [PULL 00/26] Migration 20240423 patches Peter Xu
` (9 preceding siblings ...)
2024-04-23 22:37 ` [PULL 10/26] s390/stattrib: Add Error** argument to set_migrationmode() handler Peter Xu
@ 2024-04-23 22:37 ` Peter Xu
2024-04-23 22:37 ` [PULL 12/26] migration: Always report an error in block_save_setup() Peter Xu
` (15 subsequent siblings)
26 siblings, 0 replies; 28+ messages in thread
From: Peter Xu @ 2024-04-23 22:37 UTC (permalink / raw)
To: qemu-devel
Cc: peterx, Prasad Pandit, Peter Maydell, Fabiano Rosas,
Cédric Le Goater, Eric Auger
From: Cédric Le Goater <clg@redhat.com>
This will prepare ground for future changes adding an Error** argument
to the save_setup() handler. We need to make sure that on failure,
vfio_save_setup() always sets a new error.
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Cédric Le Goater <clg@redhat.com>
Link: https://lore.kernel.org/r/20240320064911.545001-3-clg@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
hw/vfio/migration.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index 1149c6b374..bf5a29ddc1 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -381,6 +381,7 @@ static int vfio_save_setup(QEMUFile *f, void *opaque)
VFIODevice *vbasedev = opaque;
VFIOMigration *migration = vbasedev->migration;
uint64_t stop_copy_size = VFIO_MIG_DEFAULT_DATA_BUFFER_SIZE;
+ int ret;
qemu_put_be64(f, VFIO_MIG_FLAG_DEV_SETUP_STATE);
@@ -395,13 +396,13 @@ static int vfio_save_setup(QEMUFile *f, void *opaque)
}
if (vfio_precopy_supported(vbasedev)) {
- int ret;
-
switch (migration->device_state) {
case VFIO_DEVICE_STATE_RUNNING:
ret = vfio_migration_set_state(vbasedev, VFIO_DEVICE_STATE_PRE_COPY,
VFIO_DEVICE_STATE_RUNNING);
if (ret) {
+ error_report("%s: Failed to set new PRE_COPY state",
+ vbasedev->name);
return ret;
}
@@ -412,6 +413,8 @@ static int vfio_save_setup(QEMUFile *f, void *opaque)
/* vfio_save_complete_precopy() will go to STOP_COPY */
break;
default:
+ error_report("%s: Invalid device state %d", vbasedev->name,
+ migration->device_state);
return -EINVAL;
}
}
@@ -420,7 +423,13 @@ static int vfio_save_setup(QEMUFile *f, void *opaque)
qemu_put_be64(f, VFIO_MIG_FLAG_END_OF_STATE);
- return qemu_file_get_error(f);
+ ret = qemu_file_get_error(f);
+ if (ret < 0) {
+ error_report("%s: save setup failed : %s", vbasedev->name,
+ strerror(-ret));
+ }
+
+ return ret;
}
static void vfio_save_cleanup(void *opaque)
--
2.44.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PULL 12/26] migration: Always report an error in block_save_setup()
2024-04-23 22:37 [PULL 00/26] Migration 20240423 patches Peter Xu
` (10 preceding siblings ...)
2024-04-23 22:37 ` [PULL 11/26] vfio: Always report an error in vfio_save_setup() Peter Xu
@ 2024-04-23 22:37 ` Peter Xu
2024-04-23 22:38 ` [PULL 13/26] migration: Always report an error in ram_save_setup() Peter Xu
` (14 subsequent siblings)
26 siblings, 0 replies; 28+ messages in thread
From: Peter Xu @ 2024-04-23 22:37 UTC (permalink / raw)
To: qemu-devel
Cc: peterx, Prasad Pandit, Peter Maydell, Fabiano Rosas,
Cédric Le Goater, Stefan Hajnoczi
From: Cédric Le Goater <clg@redhat.com>
This will prepare ground for future changes adding an Error** argument
to the save_setup() handler. We need to make sure that on failure,
block_save_setup() always sets a new error.
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Cédric Le Goater <clg@redhat.com>
Link: https://lore.kernel.org/r/20240320064911.545001-4-clg@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/block.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/migration/block.c b/migration/block.c
index 2b9054889a..f8a11beb37 100644
--- a/migration/block.c
+++ b/migration/block.c
@@ -367,7 +367,7 @@ static void unset_dirty_tracking(void)
}
}
-static int init_blk_migration(QEMUFile *f)
+static int init_blk_migration(QEMUFile *f, Error **errp)
{
BlockDriverState *bs;
BlkMigDevState *bmds;
@@ -378,7 +378,6 @@ static int init_blk_migration(QEMUFile *f)
BlkMigDevState *bmds;
BlockDriverState *bs;
} *bmds_bs;
- Error *local_err = NULL;
int ret;
GRAPH_RDLOCK_GUARD_MAINLOOP();
@@ -406,6 +405,8 @@ static int init_blk_migration(QEMUFile *f)
continue;
}
if (sectors < 0) {
+ error_setg(errp, "Error getting length of block device %s",
+ bdrv_get_device_name(bs));
ret = sectors;
bdrv_next_cleanup(&it);
goto out;
@@ -442,9 +443,8 @@ static int init_blk_migration(QEMUFile *f)
bs = bmds_bs[i].bs;
if (bmds) {
- ret = blk_insert_bs(bmds->blk, bs, &local_err);
+ ret = blk_insert_bs(bmds->blk, bs, errp);
if (ret < 0) {
- error_report_err(local_err);
goto out;
}
@@ -714,6 +714,7 @@ static void block_migration_cleanup(void *opaque)
static int block_save_setup(QEMUFile *f, void *opaque)
{
int ret;
+ Error *local_err = NULL;
trace_migration_block_save("setup", block_mig_state.submitted,
block_mig_state.transferred);
@@ -721,18 +722,27 @@ static int block_save_setup(QEMUFile *f, void *opaque)
warn_report("block migration is deprecated;"
" use blockdev-mirror with NBD instead");
- ret = init_blk_migration(f);
+ ret = init_blk_migration(f, &local_err);
if (ret < 0) {
+ error_report_err(local_err);
return ret;
}
/* start track dirty blocks */
ret = set_dirty_tracking();
if (ret) {
+ error_setg_errno(&local_err, -ret,
+ "Failed to start block dirty tracking");
+ error_report_err(local_err);
return ret;
}
ret = flush_blks(f);
+ if (ret) {
+ error_setg_errno(&local_err, -ret, "Flushing block failed");
+ error_report_err(local_err);
+ return ret;
+ }
blk_mig_reset_dirty_cursor();
qemu_put_be64(f, BLK_MIG_FLAG_EOS);
--
2.44.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PULL 13/26] migration: Always report an error in ram_save_setup()
2024-04-23 22:37 [PULL 00/26] Migration 20240423 patches Peter Xu
` (11 preceding siblings ...)
2024-04-23 22:37 ` [PULL 12/26] migration: Always report an error in block_save_setup() Peter Xu
@ 2024-04-23 22:38 ` Peter Xu
2024-04-23 22:38 ` [PULL 14/26] migration: Add Error** argument to vmstate_save() Peter Xu
` (13 subsequent siblings)
26 siblings, 0 replies; 28+ messages in thread
From: Peter Xu @ 2024-04-23 22:38 UTC (permalink / raw)
To: qemu-devel
Cc: peterx, Prasad Pandit, Peter Maydell, Fabiano Rosas,
Cédric Le Goater
From: Cédric Le Goater <clg@redhat.com>
This will prepare ground for future changes adding an Error** argument
to the save_setup() handler. We need to make sure that on failure,
ram_save_setup() sets a new error.
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Cédric Le Goater <clg@redhat.com>
Link: https://lore.kernel.org/r/20240320064911.545001-5-clg@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/ram.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/migration/ram.c b/migration/ram.c
index 8deb84984f..44d7073730 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -3074,12 +3074,14 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
int ret, max_hg_page_size;
if (compress_threads_save_setup()) {
+ error_report("%s: failed to start compress threads", __func__);
return -1;
}
/* migration has already setup the bitmap, reuse it. */
if (!migration_in_colo_state()) {
if (ram_init_all(rsp) != 0) {
+ error_report("%s: failed to setup RAM for migration", __func__);
compress_threads_save_cleanup();
return -1;
}
@@ -3116,12 +3118,14 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
ret = rdma_registration_start(f, RAM_CONTROL_SETUP);
if (ret < 0) {
+ error_report("%s: failed to start RDMA registration", __func__);
qemu_file_set_error(f, ret);
return ret;
}
ret = rdma_registration_stop(f, RAM_CONTROL_SETUP);
if (ret < 0) {
+ error_report("%s: failed to stop RDMA registration", __func__);
qemu_file_set_error(f, ret);
return ret;
}
@@ -3138,6 +3142,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
ret = multifd_send_sync_main();
bql_lock();
if (ret < 0) {
+ error_report("%s: multifd synchronization failed", __func__);
return ret;
}
@@ -3147,7 +3152,11 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
}
qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
- return qemu_fflush(f);
+ ret = qemu_fflush(f);
+ if (ret < 0) {
+ error_report("%s failed : %s", __func__, strerror(-ret));
+ }
+ return ret;
}
static void ram_save_file_bmap(QEMUFile *f)
--
2.44.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PULL 14/26] migration: Add Error** argument to vmstate_save()
2024-04-23 22:37 [PULL 00/26] Migration 20240423 patches Peter Xu
` (12 preceding siblings ...)
2024-04-23 22:38 ` [PULL 13/26] migration: Always report an error in ram_save_setup() Peter Xu
@ 2024-04-23 22:38 ` Peter Xu
2024-04-23 22:38 ` [PULL 15/26] migration: Add Error** argument to qemu_savevm_state_setup() Peter Xu
` (12 subsequent siblings)
26 siblings, 0 replies; 28+ messages in thread
From: Peter Xu @ 2024-04-23 22:38 UTC (permalink / raw)
To: qemu-devel
Cc: peterx, Prasad Pandit, Peter Maydell, Fabiano Rosas,
Cédric Le Goater, Prasad Pandit
From: Cédric Le Goater <clg@redhat.com>
This will prepare ground for future changes adding an Error** argument
to qemu_savevm_state_setup().
Reviewed-by: Prasad Pandit <pjp@fedoraproject.org>
Signed-off-by: Cédric Le Goater <clg@redhat.com>
Link: https://lore.kernel.org/r/20240320064911.545001-6-clg@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/savevm.c | 26 ++++++++++++++++----------
1 file changed, 16 insertions(+), 10 deletions(-)
diff --git a/migration/savevm.c b/migration/savevm.c
index e7c1215671..6252fc37b7 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1009,11 +1009,10 @@ static void save_section_footer(QEMUFile *f, SaveStateEntry *se)
}
}
-static int vmstate_save(QEMUFile *f, SaveStateEntry *se, JSONWriter *vmdesc)
+static int vmstate_save(QEMUFile *f, SaveStateEntry *se, JSONWriter *vmdesc,
+ Error **errp)
{
int ret;
- Error *local_err = NULL;
- MigrationState *s = migrate_get_current();
if ((!se->ops || !se->ops->save_state) && !se->vmsd) {
return 0;
@@ -1035,10 +1034,9 @@ static int vmstate_save(QEMUFile *f, SaveStateEntry *se, JSONWriter *vmdesc)
if (!se->vmsd) {
vmstate_save_old_style(f, se, vmdesc);
} else {
- ret = vmstate_save_state_with_err(f, se->vmsd, se->opaque, vmdesc, &local_err);
+ ret = vmstate_save_state_with_err(f, se->vmsd, se->opaque, vmdesc,
+ errp);
if (ret) {
- migrate_set_error(s, local_err);
- error_report_err(local_err);
return ret;
}
}
@@ -1325,8 +1323,10 @@ void qemu_savevm_state_setup(QEMUFile *f)
trace_savevm_state_setup();
QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
if (se->vmsd && se->vmsd->early_setup) {
- ret = vmstate_save(f, se, ms->vmdesc);
+ ret = vmstate_save(f, se, ms->vmdesc, &local_err);
if (ret) {
+ migrate_set_error(ms, local_err);
+ error_report_err(local_err);
qemu_file_set_error(f, ret);
break;
}
@@ -1542,6 +1542,7 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
JSONWriter *vmdesc = ms->vmdesc;
int vmdesc_len;
SaveStateEntry *se;
+ Error *local_err = NULL;
int ret;
QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
@@ -1552,8 +1553,10 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
start_ts_each = qemu_clock_get_us(QEMU_CLOCK_REALTIME);
- ret = vmstate_save(f, se, vmdesc);
+ ret = vmstate_save(f, se, vmdesc, &local_err);
if (ret) {
+ migrate_set_error(ms, local_err);
+ error_report_err(local_err);
qemu_file_set_error(f, ret);
return ret;
}
@@ -1568,7 +1571,6 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
* bdrv_activate_all() on the other end won't fail. */
ret = bdrv_inactivate_all();
if (ret) {
- Error *local_err = NULL;
error_setg(&local_err, "%s: bdrv_inactivate_all() failed (%d)",
__func__, ret);
migrate_set_error(ms, local_err);
@@ -1764,6 +1766,8 @@ void qemu_savevm_live_state(QEMUFile *f)
int qemu_save_device_state(QEMUFile *f)
{
+ MigrationState *ms = migrate_get_current();
+ Error *local_err = NULL;
SaveStateEntry *se;
if (!migration_in_colo_state()) {
@@ -1778,8 +1782,10 @@ int qemu_save_device_state(QEMUFile *f)
if (se->is_ram) {
continue;
}
- ret = vmstate_save(f, se, NULL);
+ ret = vmstate_save(f, se, NULL, &local_err);
if (ret) {
+ migrate_set_error(ms, local_err);
+ error_report_err(local_err);
return ret;
}
}
--
2.44.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PULL 15/26] migration: Add Error** argument to qemu_savevm_state_setup()
2024-04-23 22:37 [PULL 00/26] Migration 20240423 patches Peter Xu
` (13 preceding siblings ...)
2024-04-23 22:38 ` [PULL 14/26] migration: Add Error** argument to vmstate_save() Peter Xu
@ 2024-04-23 22:38 ` Peter Xu
2024-04-23 22:38 ` [PULL 16/26] migration: Add Error** argument to .save_setup() handler Peter Xu
` (11 subsequent siblings)
26 siblings, 0 replies; 28+ messages in thread
From: Peter Xu @ 2024-04-23 22:38 UTC (permalink / raw)
To: qemu-devel
Cc: peterx, Prasad Pandit, Peter Maydell, Fabiano Rosas,
Cédric Le Goater
From: Cédric Le Goater <clg@redhat.com>
This prepares ground for the changes coming next which add an Error**
argument to the .save_setup() handler. Callers of qemu_savevm_state_setup()
now handle the error and fail earlier setting the migration state from
MIGRATION_STATUS_SETUP to MIGRATION_STATUS_FAILED.
In qemu_savevm_state(), move the cleanup to preserve the error
reported by .save_setup() handlers.
Since the previous behavior was to ignore errors at this step of
migration, this change should be examined closely to check that
cleanups are still correctly done.
Signed-off-by: Cédric Le Goater <clg@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Link: https://lore.kernel.org/r/20240320064911.545001-7-clg@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/savevm.h | 2 +-
migration/migration.c | 33 +++++++++++++++++++++++++++++++--
migration/savevm.c | 26 +++++++++++++++-----------
3 files changed, 47 insertions(+), 14 deletions(-)
diff --git a/migration/savevm.h b/migration/savevm.h
index 74669733dd..9ec96a995c 100644
--- a/migration/savevm.h
+++ b/migration/savevm.h
@@ -32,7 +32,7 @@
bool qemu_savevm_state_blocked(Error **errp);
void qemu_savevm_non_migratable_list(strList **reasons);
int qemu_savevm_state_prepare(Error **errp);
-void qemu_savevm_state_setup(QEMUFile *f);
+int qemu_savevm_state_setup(QEMUFile *f, Error **errp);
bool qemu_savevm_state_guest_unplug_pending(void);
int qemu_savevm_state_resume_prepare(MigrationState *s);
void qemu_savevm_state_header(QEMUFile *f);
diff --git a/migration/migration.c b/migration/migration.c
index 86bf76e925..696762bc64 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -3431,6 +3431,8 @@ static void *migration_thread(void *opaque)
int64_t setup_start = qemu_clock_get_ms(QEMU_CLOCK_HOST);
MigThrError thr_error;
bool urgent = false;
+ Error *local_err = NULL;
+ int ret;
thread = migration_threads_add("live_migration", qemu_get_thread_id());
@@ -3474,12 +3476,24 @@ static void *migration_thread(void *opaque)
}
bql_lock();
- qemu_savevm_state_setup(s->to_dst_file);
+ ret = qemu_savevm_state_setup(s->to_dst_file, &local_err);
bql_unlock();
qemu_savevm_wait_unplug(s, MIGRATION_STATUS_SETUP,
MIGRATION_STATUS_ACTIVE);
+ /*
+ * Handle SETUP failures after waiting for virtio-net-failover
+ * devices to unplug. This to preserve migration state transitions.
+ */
+ if (ret) {
+ migrate_set_error(s, local_err);
+ error_free(local_err);
+ migrate_set_state(&s->state, MIGRATION_STATUS_ACTIVE,
+ MIGRATION_STATUS_FAILED);
+ goto out;
+ }
+
s->setup_time = qemu_clock_get_ms(QEMU_CLOCK_HOST) - setup_start;
trace_migration_thread_setup_complete();
@@ -3553,6 +3567,8 @@ static void *bg_migration_thread(void *opaque)
MigThrError thr_error;
QEMUFile *fb;
bool early_fail = true;
+ Error *local_err = NULL;
+ int ret;
rcu_register_thread();
object_ref(OBJECT(s));
@@ -3586,12 +3602,24 @@ static void *bg_migration_thread(void *opaque)
bql_lock();
qemu_savevm_state_header(s->to_dst_file);
- qemu_savevm_state_setup(s->to_dst_file);
+ ret = qemu_savevm_state_setup(s->to_dst_file, &local_err);
bql_unlock();
qemu_savevm_wait_unplug(s, MIGRATION_STATUS_SETUP,
MIGRATION_STATUS_ACTIVE);
+ /*
+ * Handle SETUP failures after waiting for virtio-net-failover
+ * devices to unplug. This to preserve migration state transitions.
+ */
+ if (ret) {
+ migrate_set_error(s, local_err);
+ error_free(local_err);
+ migrate_set_state(&s->state, MIGRATION_STATUS_ACTIVE,
+ MIGRATION_STATUS_FAILED);
+ goto fail_setup;
+ }
+
s->setup_time = qemu_clock_get_ms(QEMU_CLOCK_HOST) - setup_start;
trace_migration_thread_setup_complete();
@@ -3660,6 +3688,7 @@ fail:
bql_unlock();
}
+fail_setup:
bg_migration_iteration_finish(s);
qemu_fclose(fb);
diff --git a/migration/savevm.c b/migration/savevm.c
index 6252fc37b7..327e9b346e 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1310,11 +1310,11 @@ int qemu_savevm_state_prepare(Error **errp)
return 0;
}
-void qemu_savevm_state_setup(QEMUFile *f)
+int qemu_savevm_state_setup(QEMUFile *f, Error **errp)
{
+ ERRP_GUARD();
MigrationState *ms = migrate_get_current();
SaveStateEntry *se;
- Error *local_err = NULL;
int ret = 0;
json_writer_int64(ms->vmdesc, "page_size", qemu_target_page_size());
@@ -1323,10 +1323,9 @@ void qemu_savevm_state_setup(QEMUFile *f)
trace_savevm_state_setup();
QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
if (se->vmsd && se->vmsd->early_setup) {
- ret = vmstate_save(f, se, ms->vmdesc, &local_err);
+ ret = vmstate_save(f, se, ms->vmdesc, errp);
if (ret) {
- migrate_set_error(ms, local_err);
- error_report_err(local_err);
+ migrate_set_error(ms, *errp);
qemu_file_set_error(f, ret);
break;
}
@@ -1346,18 +1345,19 @@ void qemu_savevm_state_setup(QEMUFile *f)
ret = se->ops->save_setup(f, se->opaque);
save_section_footer(f, se);
if (ret < 0) {
+ error_setg(errp, "failed to setup SaveStateEntry with id(name): "
+ "%d(%s): %d", se->section_id, se->idstr, ret);
qemu_file_set_error(f, ret);
break;
}
}
if (ret) {
- return;
+ return ret;
}
- if (precopy_notify(PRECOPY_NOTIFY_SETUP, &local_err)) {
- error_report_err(local_err);
- }
+ /* TODO: Should we check that errp is set in case of failure ? */
+ return precopy_notify(PRECOPY_NOTIFY_SETUP, errp);
}
int qemu_savevm_state_resume_prepare(MigrationState *s)
@@ -1725,7 +1725,10 @@ static int qemu_savevm_state(QEMUFile *f, Error **errp)
ms->to_dst_file = f;
qemu_savevm_state_header(f);
- qemu_savevm_state_setup(f);
+ ret = qemu_savevm_state_setup(f, errp);
+ if (ret) {
+ goto cleanup;
+ }
while (qemu_file_get_error(f) == 0) {
if (qemu_savevm_state_iterate(f, false) > 0) {
@@ -1738,10 +1741,11 @@ static int qemu_savevm_state(QEMUFile *f, Error **errp)
qemu_savevm_state_complete_precopy(f, false, false);
ret = qemu_file_get_error(f);
}
- qemu_savevm_state_cleanup();
if (ret != 0) {
error_setg_errno(errp, -ret, "Error while writing VM state");
}
+cleanup:
+ qemu_savevm_state_cleanup();
if (ret != 0) {
status = MIGRATION_STATUS_FAILED;
--
2.44.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PULL 16/26] migration: Add Error** argument to .save_setup() handler
2024-04-23 22:37 [PULL 00/26] Migration 20240423 patches Peter Xu
` (14 preceding siblings ...)
2024-04-23 22:38 ` [PULL 15/26] migration: Add Error** argument to qemu_savevm_state_setup() Peter Xu
@ 2024-04-23 22:38 ` Peter Xu
2024-04-23 22:38 ` [PULL 17/26] migration: Add Error** argument to .load_setup() handler Peter Xu
` (10 subsequent siblings)
26 siblings, 0 replies; 28+ messages in thread
From: Peter Xu @ 2024-04-23 22:38 UTC (permalink / raw)
To: qemu-devel
Cc: peterx, Prasad Pandit, Peter Maydell, Fabiano Rosas,
Cédric Le Goater, Nicholas Piggin, Harsh Prateek Bora,
Halil Pasic, Thomas Huth, Eric Blake,
Vladimir Sementsov-Ogievskiy, John Snow, Stefan Hajnoczi,
Philippe Mathieu-Daudé
From: Cédric Le Goater <clg@redhat.com>
The purpose is to record a potential error in the migration stream if
qemu_savevm_state_setup() fails. Most of the current .save_setup()
handlers can be modified to use the Error argument instead of managing
their own and calling locally error_report().
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Harsh Prateek Bora <harshpb@linux.ibm.com>
Cc: Halil Pasic <pasic@linux.ibm.com>
Cc: Thomas Huth <thuth@redhat.com>
Cc: Eric Blake <eblake@redhat.com>
Cc: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Cc: John Snow <jsnow@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Signed-off-by: Cédric Le Goater <clg@redhat.com>
Link: https://lore.kernel.org/r/20240320064911.545001-8-clg@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
include/migration/register.h | 3 ++-
hw/ppc/spapr.c | 2 +-
hw/s390x/s390-stattrib.c | 6 ++----
hw/vfio/migration.c | 17 ++++++++---------
migration/block-dirty-bitmap.c | 4 +++-
migration/block.c | 13 ++++---------
migration/ram.c | 15 ++++++++-------
migration/savevm.c | 4 +---
8 files changed, 29 insertions(+), 35 deletions(-)
diff --git a/include/migration/register.h b/include/migration/register.h
index d7b70a8be6..64fc7c1103 100644
--- a/include/migration/register.h
+++ b/include/migration/register.h
@@ -60,10 +60,11 @@ typedef struct SaveVMHandlers {
*
* @f: QEMUFile where to send the data
* @opaque: data pointer passed to register_savevm_live()
+ * @errp: pointer to Error*, to store an error if it happens.
*
* Returns zero to indicate success and negative for error
*/
- int (*save_setup)(QEMUFile *f, void *opaque);
+ int (*save_setup)(QEMUFile *f, void *opaque, Error **errp);
/**
* @save_cleanup
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index e9bc97fee0..823164e81c 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2172,7 +2172,7 @@ static const VMStateDescription vmstate_spapr = {
}
};
-static int htab_save_setup(QEMUFile *f, void *opaque)
+static int htab_save_setup(QEMUFile *f, void *opaque, Error **errp)
{
SpaprMachineState *spapr = opaque;
diff --git a/hw/s390x/s390-stattrib.c b/hw/s390x/s390-stattrib.c
index b743e8a2fe..bc04187b2b 100644
--- a/hw/s390x/s390-stattrib.c
+++ b/hw/s390x/s390-stattrib.c
@@ -168,19 +168,17 @@ static int cmma_load(QEMUFile *f, void *opaque, int version_id)
return ret;
}
-static int cmma_save_setup(QEMUFile *f, void *opaque)
+static int cmma_save_setup(QEMUFile *f, void *opaque, Error **errp)
{
S390StAttribState *sas = S390_STATTRIB(opaque);
S390StAttribClass *sac = S390_STATTRIB_GET_CLASS(sas);
- Error *local_err = NULL;
int res;
/*
* Signal that we want to start a migration, thus needing PGSTE dirty
* tracking.
*/
- res = sac->set_migrationmode(sas, true, &local_err);
+ res = sac->set_migrationmode(sas, true, errp);
if (res) {
- error_report_err(local_err);
return res;
}
qemu_put_be64(f, STATTR_FLAG_EOS);
diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index bf5a29ddc1..5763c0b683 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -376,7 +376,7 @@ static int vfio_save_prepare(void *opaque, Error **errp)
return 0;
}
-static int vfio_save_setup(QEMUFile *f, void *opaque)
+static int vfio_save_setup(QEMUFile *f, void *opaque, Error **errp)
{
VFIODevice *vbasedev = opaque;
VFIOMigration *migration = vbasedev->migration;
@@ -390,8 +390,8 @@ static int vfio_save_setup(QEMUFile *f, void *opaque)
stop_copy_size);
migration->data_buffer = g_try_malloc0(migration->data_buffer_size);
if (!migration->data_buffer) {
- error_report("%s: Failed to allocate migration data buffer",
- vbasedev->name);
+ error_setg(errp, "%s: Failed to allocate migration data buffer",
+ vbasedev->name);
return -ENOMEM;
}
@@ -401,8 +401,8 @@ static int vfio_save_setup(QEMUFile *f, void *opaque)
ret = vfio_migration_set_state(vbasedev, VFIO_DEVICE_STATE_PRE_COPY,
VFIO_DEVICE_STATE_RUNNING);
if (ret) {
- error_report("%s: Failed to set new PRE_COPY state",
- vbasedev->name);
+ error_setg(errp, "%s: Failed to set new PRE_COPY state",
+ vbasedev->name);
return ret;
}
@@ -413,8 +413,8 @@ static int vfio_save_setup(QEMUFile *f, void *opaque)
/* vfio_save_complete_precopy() will go to STOP_COPY */
break;
default:
- error_report("%s: Invalid device state %d", vbasedev->name,
- migration->device_state);
+ error_setg(errp, "%s: Invalid device state %d", vbasedev->name,
+ migration->device_state);
return -EINVAL;
}
}
@@ -425,8 +425,7 @@ static int vfio_save_setup(QEMUFile *f, void *opaque)
ret = qemu_file_get_error(f);
if (ret < 0) {
- error_report("%s: save setup failed : %s", vbasedev->name,
- strerror(-ret));
+ error_setg_errno(errp, -ret, "%s: save setup failed", vbasedev->name);
}
return ret;
diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
index 2708abf3d7..542a8c297b 100644
--- a/migration/block-dirty-bitmap.c
+++ b/migration/block-dirty-bitmap.c
@@ -1213,12 +1213,14 @@ fail:
return ret;
}
-static int dirty_bitmap_save_setup(QEMUFile *f, void *opaque)
+static int dirty_bitmap_save_setup(QEMUFile *f, void *opaque, Error **errp)
{
DBMSaveState *s = &((DBMState *)opaque)->save;
SaveBitmapState *dbms = NULL;
if (init_dirty_bitmap_migration(s) < 0) {
+ error_setg(errp,
+ "Failed to initialize dirty tracking bitmap for blocks");
return -1;
}
diff --git a/migration/block.c b/migration/block.c
index f8a11beb37..bae6e94891 100644
--- a/migration/block.c
+++ b/migration/block.c
@@ -711,10 +711,9 @@ static void block_migration_cleanup(void *opaque)
blk_mig_unlock();
}
-static int block_save_setup(QEMUFile *f, void *opaque)
+static int block_save_setup(QEMUFile *f, void *opaque, Error **errp)
{
int ret;
- Error *local_err = NULL;
trace_migration_block_save("setup", block_mig_state.submitted,
block_mig_state.transferred);
@@ -722,25 +721,21 @@ static int block_save_setup(QEMUFile *f, void *opaque)
warn_report("block migration is deprecated;"
" use blockdev-mirror with NBD instead");
- ret = init_blk_migration(f, &local_err);
+ ret = init_blk_migration(f, errp);
if (ret < 0) {
- error_report_err(local_err);
return ret;
}
/* start track dirty blocks */
ret = set_dirty_tracking();
if (ret) {
- error_setg_errno(&local_err, -ret,
- "Failed to start block dirty tracking");
- error_report_err(local_err);
+ error_setg_errno(errp, -ret, "Failed to start block dirty tracking");
return ret;
}
ret = flush_blks(f);
if (ret) {
- error_setg_errno(&local_err, -ret, "Flushing block failed");
- error_report_err(local_err);
+ error_setg_errno(errp, -ret, "Flushing block failed");
return ret;
}
blk_mig_reset_dirty_cursor();
diff --git a/migration/ram.c b/migration/ram.c
index 44d7073730..6ea5a06e00 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -3066,22 +3066,23 @@ static bool mapped_ram_read_header(QEMUFile *file, MappedRamHeader *header,
*
* @f: QEMUFile where to send the data
* @opaque: RAMState pointer
+ * @errp: pointer to Error*, to store an error if it happens.
*/
-static int ram_save_setup(QEMUFile *f, void *opaque)
+static int ram_save_setup(QEMUFile *f, void *opaque, Error **errp)
{
RAMState **rsp = opaque;
RAMBlock *block;
int ret, max_hg_page_size;
if (compress_threads_save_setup()) {
- error_report("%s: failed to start compress threads", __func__);
+ error_setg(errp, "%s: failed to start compress threads", __func__);
return -1;
}
/* migration has already setup the bitmap, reuse it. */
if (!migration_in_colo_state()) {
if (ram_init_all(rsp) != 0) {
- error_report("%s: failed to setup RAM for migration", __func__);
+ error_setg(errp, "%s: failed to setup RAM for migration", __func__);
compress_threads_save_cleanup();
return -1;
}
@@ -3118,14 +3119,14 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
ret = rdma_registration_start(f, RAM_CONTROL_SETUP);
if (ret < 0) {
- error_report("%s: failed to start RDMA registration", __func__);
+ error_setg(errp, "%s: failed to start RDMA registration", __func__);
qemu_file_set_error(f, ret);
return ret;
}
ret = rdma_registration_stop(f, RAM_CONTROL_SETUP);
if (ret < 0) {
- error_report("%s: failed to stop RDMA registration", __func__);
+ error_setg(errp, "%s: failed to stop RDMA registration", __func__);
qemu_file_set_error(f, ret);
return ret;
}
@@ -3142,7 +3143,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
ret = multifd_send_sync_main();
bql_lock();
if (ret < 0) {
- error_report("%s: multifd synchronization failed", __func__);
+ error_setg(errp, "%s: multifd synchronization failed", __func__);
return ret;
}
@@ -3154,7 +3155,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
ret = qemu_fflush(f);
if (ret < 0) {
- error_report("%s failed : %s", __func__, strerror(-ret));
+ error_setg_errno(errp, -ret, "%s failed", __func__);
}
return ret;
}
diff --git a/migration/savevm.c b/migration/savevm.c
index 327e9b346e..a2679ba0b8 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1342,11 +1342,9 @@ int qemu_savevm_state_setup(QEMUFile *f, Error **errp)
}
save_section_header(f, se, QEMU_VM_SECTION_START);
- ret = se->ops->save_setup(f, se->opaque);
+ ret = se->ops->save_setup(f, se->opaque, errp);
save_section_footer(f, se);
if (ret < 0) {
- error_setg(errp, "failed to setup SaveStateEntry with id(name): "
- "%d(%s): %d", se->section_id, se->idstr, ret);
qemu_file_set_error(f, ret);
break;
}
--
2.44.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PULL 17/26] migration: Add Error** argument to .load_setup() handler
2024-04-23 22:37 [PULL 00/26] Migration 20240423 patches Peter Xu
` (15 preceding siblings ...)
2024-04-23 22:38 ` [PULL 16/26] migration: Add Error** argument to .save_setup() handler Peter Xu
@ 2024-04-23 22:38 ` Peter Xu
2024-04-23 22:38 ` [PULL 18/26] memory: Add Error** argument to .log_global_start() handler Peter Xu
` (9 subsequent siblings)
26 siblings, 0 replies; 28+ messages in thread
From: Peter Xu @ 2024-04-23 22:38 UTC (permalink / raw)
To: qemu-devel
Cc: peterx, Prasad Pandit, Peter Maydell, Fabiano Rosas,
Cédric Le Goater, Philippe Mathieu-Daudé
From: Cédric Le Goater <clg@redhat.com>
This will be useful to report errors at a higher level, mostly in VFIO
today.
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Cédric Le Goater <clg@redhat.com>
Link: https://lore.kernel.org/r/20240320064911.545001-9-clg@redhat.com
[peterx: drop comment for ERRP_GUARD, per Markus]
Signed-off-by: Peter Xu <peterx@redhat.com>
---
include/migration/register.h | 3 ++-
hw/vfio/migration.c | 9 +++++++--
migration/ram.c | 3 ++-
migration/savevm.c | 11 +++++++----
4 files changed, 18 insertions(+), 8 deletions(-)
diff --git a/include/migration/register.h b/include/migration/register.h
index 64fc7c1103..f60e797894 100644
--- a/include/migration/register.h
+++ b/include/migration/register.h
@@ -234,10 +234,11 @@ typedef struct SaveVMHandlers {
*
* @f: QEMUFile where to receive the data
* @opaque: data pointer passed to register_savevm_live()
+ * @errp: pointer to Error*, to store an error if it happens.
*
* Returns zero to indicate success and negative for error
*/
- int (*load_setup)(QEMUFile *f, void *opaque);
+ int (*load_setup)(QEMUFile *f, void *opaque, Error **errp);
/**
* @load_cleanup
diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index 5763c0b683..06ae40969b 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -588,12 +588,17 @@ static void vfio_save_state(QEMUFile *f, void *opaque)
}
}
-static int vfio_load_setup(QEMUFile *f, void *opaque)
+static int vfio_load_setup(QEMUFile *f, void *opaque, Error **errp)
{
VFIODevice *vbasedev = opaque;
+ int ret;
- return vfio_migration_set_state(vbasedev, VFIO_DEVICE_STATE_RESUMING,
+ ret = vfio_migration_set_state(vbasedev, VFIO_DEVICE_STATE_RESUMING,
vbasedev->migration->device_state);
+ if (ret) {
+ error_setg(errp, "%s: Failed to set RESUMING state", vbasedev->name);
+ }
+ return ret;
}
static int vfio_load_cleanup(void *opaque)
diff --git a/migration/ram.c b/migration/ram.c
index 6ea5a06e00..4cd4f0158c 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -3704,8 +3704,9 @@ void colo_release_ram_cache(void)
*
* @f: QEMUFile where to receive the data
* @opaque: RAMState pointer
+ * @errp: pointer to Error*, to store an error if it happens.
*/
-static int ram_load_setup(QEMUFile *f, void *opaque)
+static int ram_load_setup(QEMUFile *f, void *opaque, Error **errp)
{
xbzrle_load_setup();
ramblock_recv_map_init();
diff --git a/migration/savevm.c b/migration/savevm.c
index a2679ba0b8..5d200cf42a 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2768,8 +2768,9 @@ static void qemu_loadvm_state_switchover_ack_needed(MigrationIncomingState *mis)
trace_loadvm_state_switchover_ack_needed(mis->switchover_ack_pending_num);
}
-static int qemu_loadvm_state_setup(QEMUFile *f)
+static int qemu_loadvm_state_setup(QEMUFile *f, Error **errp)
{
+ ERRP_GUARD();
SaveStateEntry *se;
int ret;
@@ -2784,10 +2785,11 @@ static int qemu_loadvm_state_setup(QEMUFile *f)
}
}
- ret = se->ops->load_setup(f, se->opaque);
+ ret = se->ops->load_setup(f, se->opaque, errp);
if (ret < 0) {
+ error_prepend(errp, "Load state of device %s failed: ",
+ se->idstr);
qemu_file_set_error(f, ret);
- error_report("Load state of device %s failed", se->idstr);
return ret;
}
}
@@ -2968,7 +2970,8 @@ int qemu_loadvm_state(QEMUFile *f)
return ret;
}
- if (qemu_loadvm_state_setup(f) != 0) {
+ if (qemu_loadvm_state_setup(f, &local_err) != 0) {
+ error_report_err(local_err);
return -EINVAL;
}
--
2.44.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PULL 18/26] memory: Add Error** argument to .log_global_start() handler
2024-04-23 22:37 [PULL 00/26] Migration 20240423 patches Peter Xu
` (16 preceding siblings ...)
2024-04-23 22:38 ` [PULL 17/26] migration: Add Error** argument to .load_setup() handler Peter Xu
@ 2024-04-23 22:38 ` Peter Xu
2024-04-23 22:38 ` [PULL 19/26] migration: Introduce ram_bitmaps_destroy() Peter Xu
` (8 subsequent siblings)
26 siblings, 0 replies; 28+ messages in thread
From: Peter Xu @ 2024-04-23 22:38 UTC (permalink / raw)
To: qemu-devel
Cc: peterx, Prasad Pandit, Peter Maydell, Fabiano Rosas,
Cédric Le Goater, Stefano Stabellini, Anthony Perard,
Paul Durrant, Michael S. Tsirkin, Paolo Bonzini,
David Hildenbrand
From: Cédric Le Goater <clg@redhat.com>
Modify all .log_global_start() handlers to take an Error** parameter
and return a bool. Adapt memory_global_dirty_log_start() to interrupt
on the first error the loop on handlers. In such case, a rollback is
performed to stop dirty logging on all listeners where it was
previously enabled.
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Paul Durrant <paul@xen.org>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: David Hildenbrand <david@redhat.com>
Signed-off-by: Cédric Le Goater <clg@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Link: https://lore.kernel.org/r/20240320064911.545001-10-clg@redhat.com
[peterx: modify & enrich the comment for listener_add_address_space() ]
Signed-off-by: Peter Xu <peterx@redhat.com>
---
include/exec/memory.h | 5 ++++-
hw/i386/xen/xen-hvm.c | 3 ++-
hw/vfio/common.c | 4 +++-
hw/virtio/vhost.c | 3 ++-
system/memory.c | 41 +++++++++++++++++++++++++++++++++++++++--
5 files changed, 50 insertions(+), 6 deletions(-)
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 8626a355b3..5555567bc4 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -998,8 +998,11 @@ struct MemoryListener {
* active at that time.
*
* @listener: The #MemoryListener.
+ * @errp: pointer to Error*, to store an error if it happens.
+ *
+ * Return: true on success, else false setting @errp with error.
*/
- void (*log_global_start)(MemoryListener *listener);
+ bool (*log_global_start)(MemoryListener *listener, Error **errp);
/**
* @log_global_stop:
diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index 7745cb3963..f6e9a1bc86 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -457,11 +457,12 @@ static void xen_log_sync(MemoryListener *listener, MemoryRegionSection *section)
int128_get64(section->size));
}
-static void xen_log_global_start(MemoryListener *listener)
+static bool xen_log_global_start(MemoryListener *listener, Error **errp)
{
if (xen_enabled()) {
xen_in_migration = true;
}
+ return true;
}
static void xen_log_global_stop(MemoryListener *listener)
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 011ceaab89..8f9cbdc026 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -1066,7 +1066,8 @@ out:
return ret;
}
-static void vfio_listener_log_global_start(MemoryListener *listener)
+static bool vfio_listener_log_global_start(MemoryListener *listener,
+ Error **errp)
{
VFIOContainerBase *bcontainer = container_of(listener, VFIOContainerBase,
listener);
@@ -1083,6 +1084,7 @@ static void vfio_listener_log_global_start(MemoryListener *listener)
ret, strerror(-ret));
vfio_set_migration_error(ret);
}
+ return !ret;
}
static void vfio_listener_log_global_stop(MemoryListener *listener)
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index f50180e60e..4acd77e890 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1044,7 +1044,7 @@ check_dev_state:
return r;
}
-static void vhost_log_global_start(MemoryListener *listener)
+static bool vhost_log_global_start(MemoryListener *listener, Error **errp)
{
int r;
@@ -1052,6 +1052,7 @@ static void vhost_log_global_start(MemoryListener *listener)
if (r < 0) {
abort();
}
+ return true;
}
static void vhost_log_global_stop(MemoryListener *listener)
diff --git a/system/memory.c b/system/memory.c
index a229a79988..86d6c33180 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -2914,9 +2914,33 @@ static unsigned int postponed_stop_flags;
static VMChangeStateEntry *vmstate_change;
static void memory_global_dirty_log_stop_postponed_run(void);
+static bool memory_global_dirty_log_do_start(Error **errp)
+{
+ MemoryListener *listener;
+
+ QTAILQ_FOREACH(listener, &memory_listeners, link) {
+ if (listener->log_global_start) {
+ if (!listener->log_global_start(listener, errp)) {
+ goto err;
+ }
+ }
+ }
+ return true;
+
+err:
+ while ((listener = QTAILQ_PREV(listener, link)) != NULL) {
+ if (listener->log_global_stop) {
+ listener->log_global_stop(listener);
+ }
+ }
+
+ return false;
+}
+
void memory_global_dirty_log_start(unsigned int flags)
{
unsigned int old_flags;
+ Error *local_err = NULL;
assert(flags && !(flags & (~GLOBAL_DIRTY_MASK)));
@@ -2936,7 +2960,13 @@ void memory_global_dirty_log_start(unsigned int flags)
trace_global_dirty_changed(global_dirty_tracking);
if (!old_flags) {
- MEMORY_LISTENER_CALL_GLOBAL(log_global_start, Forward);
+ if (!memory_global_dirty_log_do_start(&local_err)) {
+ global_dirty_tracking &= ~flags;
+ trace_global_dirty_changed(global_dirty_tracking);
+ error_report_err(local_err);
+ return;
+ }
+
memory_region_transaction_begin();
memory_region_update_pending = true;
memory_region_transaction_commit();
@@ -3014,8 +3044,15 @@ static void listener_add_address_space(MemoryListener *listener,
listener->begin(listener);
}
if (global_dirty_tracking) {
+ /*
+ * Currently only VFIO can fail log_global_start(), and it's not
+ * yet allowed to hotplug any PCI device during migration. So this
+ * should never fail when invoked, guard it with error_abort. If
+ * it can start to fail in the future, we need to be able to fail
+ * the whole listener_add_address_space() and its callers.
+ */
if (listener->log_global_start) {
- listener->log_global_start(listener);
+ listener->log_global_start(listener, &error_abort);
}
}
--
2.44.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PULL 19/26] migration: Introduce ram_bitmaps_destroy()
2024-04-23 22:37 [PULL 00/26] Migration 20240423 patches Peter Xu
` (17 preceding siblings ...)
2024-04-23 22:38 ` [PULL 18/26] memory: Add Error** argument to .log_global_start() handler Peter Xu
@ 2024-04-23 22:38 ` Peter Xu
2024-04-23 22:38 ` [PULL 20/26] memory: Add Error** argument to the global_dirty_log routines Peter Xu
` (7 subsequent siblings)
26 siblings, 0 replies; 28+ messages in thread
From: Peter Xu @ 2024-04-23 22:38 UTC (permalink / raw)
To: qemu-devel
Cc: peterx, Prasad Pandit, Peter Maydell, Fabiano Rosas,
Cédric Le Goater
From: Cédric Le Goater <clg@redhat.com>
We will use it in ram_init_bitmaps() to clear the allocated bitmaps when
support for error reporting is added to memory_global_dirty_log_start().
Signed-off-by: Cédric Le Goater <clg@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20240320064911.545001-11-clg@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/ram.c | 22 +++++++++++++++-------
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index 4cd4f0158c..f0bd71438a 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2438,10 +2438,23 @@ static void xbzrle_cleanup(void)
XBZRLE_cache_unlock();
}
+static void ram_bitmaps_destroy(void)
+{
+ RAMBlock *block;
+
+ RAMBLOCK_FOREACH_NOT_IGNORED(block) {
+ g_free(block->clear_bmap);
+ block->clear_bmap = NULL;
+ g_free(block->bmap);
+ block->bmap = NULL;
+ g_free(block->file_bmap);
+ block->file_bmap = NULL;
+ }
+}
+
static void ram_save_cleanup(void *opaque)
{
RAMState **rsp = opaque;
- RAMBlock *block;
/* We don't use dirty log with background snapshots */
if (!migrate_background_snapshot()) {
@@ -2458,12 +2471,7 @@ static void ram_save_cleanup(void *opaque)
}
}
- RAMBLOCK_FOREACH_NOT_IGNORED(block) {
- g_free(block->clear_bmap);
- block->clear_bmap = NULL;
- g_free(block->bmap);
- block->bmap = NULL;
- }
+ ram_bitmaps_destroy();
xbzrle_cleanup();
compress_threads_save_cleanup();
--
2.44.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PULL 20/26] memory: Add Error** argument to the global_dirty_log routines
2024-04-23 22:37 [PULL 00/26] Migration 20240423 patches Peter Xu
` (18 preceding siblings ...)
2024-04-23 22:38 ` [PULL 19/26] migration: Introduce ram_bitmaps_destroy() Peter Xu
@ 2024-04-23 22:38 ` Peter Xu
2024-04-23 22:38 ` [PULL 21/26] migration: Add Error** argument to ram_state_init() Peter Xu
` (6 subsequent siblings)
26 siblings, 0 replies; 28+ messages in thread
From: Peter Xu @ 2024-04-23 22:38 UTC (permalink / raw)
To: qemu-devel
Cc: peterx, Prasad Pandit, Peter Maydell, Fabiano Rosas,
Cédric Le Goater, Stefano Stabellini, Anthony Perard,
Paul Durrant, Michael S. Tsirkin, Paolo Bonzini,
David Hildenbrand, Hyman Huang
From: Cédric Le Goater <clg@redhat.com>
Now that the log_global*() handlers take an Error** parameter and
return a bool, do the same for memory_global_dirty_log_start() and
memory_global_dirty_log_stop(). The error is reported in the callers
for now and it will be propagated in the call stack in the next
changes.
To be noted a functional change in ram_init_bitmaps(), if the dirty
pages logger fails to start, there is no need to synchronize the dirty
pages bitmaps. colo_incoming_start_dirty_log() could be modified in a
similar way.
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Paul Durrant <paul@xen.org>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: David Hildenbrand <david@redhat.com>
Cc: Hyman Huang <yong.huang@smartx.com>
Signed-off-by: Cédric Le Goater <clg@redhat.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Acked-by: Peter Xu <peterx@redhat.com>
Link: https://lore.kernel.org/r/20240320064911.545001-12-clg@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
include/exec/memory.h | 5 ++++-
hw/i386/xen/xen-hvm.c | 2 +-
migration/dirtyrate.c | 13 +++++++++++--
migration/ram.c | 23 +++++++++++++++++++++--
system/memory.c | 11 +++++------
5 files changed, 42 insertions(+), 12 deletions(-)
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 5555567bc4..c129ee6db7 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -2570,8 +2570,11 @@ void memory_listener_unregister(MemoryListener *listener);
* memory_global_dirty_log_start: begin dirty logging for all regions
*
* @flags: purpose of starting dirty log, migration or dirty rate
+ * @errp: pointer to Error*, to store an error if it happens.
+ *
+ * Return: true on success, else false setting @errp with error.
*/
-void memory_global_dirty_log_start(unsigned int flags);
+bool memory_global_dirty_log_start(unsigned int flags, Error **errp);
/**
* memory_global_dirty_log_stop: end dirty logging for all regions
diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index f6e9a1bc86..006d219ad5 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -669,7 +669,7 @@ void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length)
void qmp_xen_set_global_dirty_log(bool enable, Error **errp)
{
if (enable) {
- memory_global_dirty_log_start(GLOBAL_DIRTY_MIGRATION);
+ memory_global_dirty_log_start(GLOBAL_DIRTY_MIGRATION, errp);
} else {
memory_global_dirty_log_stop(GLOBAL_DIRTY_MIGRATION);
}
diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index 1d2e85746f..d02d70b7b4 100644
--- a/migration/dirtyrate.c
+++ b/migration/dirtyrate.c
@@ -90,9 +90,15 @@ static int64_t do_calculate_dirtyrate(DirtyPageRecord dirty_pages,
void global_dirty_log_change(unsigned int flag, bool start)
{
+ Error *local_err = NULL;
+ bool ret;
+
bql_lock();
if (start) {
- memory_global_dirty_log_start(flag);
+ ret = memory_global_dirty_log_start(flag, &local_err);
+ if (!ret) {
+ error_report_err(local_err);
+ }
} else {
memory_global_dirty_log_stop(flag);
}
@@ -608,9 +614,12 @@ static void calculate_dirtyrate_dirty_bitmap(struct DirtyRateConfig config)
{
int64_t start_time;
DirtyPageRecord dirty_pages;
+ Error *local_err = NULL;
bql_lock();
- memory_global_dirty_log_start(GLOBAL_DIRTY_DIRTY_RATE);
+ if (!memory_global_dirty_log_start(GLOBAL_DIRTY_DIRTY_RATE, &local_err)) {
+ error_report_err(local_err);
+ }
/*
* 1'round of log sync may return all 1 bits with
diff --git a/migration/ram.c b/migration/ram.c
index f0bd71438a..bade3e9281 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2862,18 +2862,32 @@ static void migration_bitmap_clear_discarded_pages(RAMState *rs)
static void ram_init_bitmaps(RAMState *rs)
{
+ Error *local_err = NULL;
+ bool ret = true;
+
qemu_mutex_lock_ramlist();
WITH_RCU_READ_LOCK_GUARD() {
ram_list_init_bitmaps();
/* We don't use dirty log with background snapshots */
if (!migrate_background_snapshot()) {
- memory_global_dirty_log_start(GLOBAL_DIRTY_MIGRATION);
+ ret = memory_global_dirty_log_start(GLOBAL_DIRTY_MIGRATION,
+ &local_err);
+ if (!ret) {
+ error_report_err(local_err);
+ goto out_unlock;
+ }
migration_bitmap_sync_precopy(rs, false);
}
}
+out_unlock:
qemu_mutex_unlock_ramlist();
+ if (!ret) {
+ ram_bitmaps_destroy();
+ return;
+ }
+
/*
* After an eventual first bitmap sync, fixup the initial bitmap
* containing all 1s to exclude any discarded pages from migration.
@@ -3665,6 +3679,8 @@ int colo_init_ram_cache(void)
void colo_incoming_start_dirty_log(void)
{
RAMBlock *block = NULL;
+ Error *local_err = NULL;
+
/* For memory_global_dirty_log_start below. */
bql_lock();
qemu_mutex_lock_ramlist();
@@ -3676,7 +3692,10 @@ void colo_incoming_start_dirty_log(void)
/* Discard this dirty bitmap record */
bitmap_zero(block->bmap, block->max_length >> TARGET_PAGE_BITS);
}
- memory_global_dirty_log_start(GLOBAL_DIRTY_MIGRATION);
+ if (!memory_global_dirty_log_start(GLOBAL_DIRTY_MIGRATION,
+ &local_err)) {
+ error_report_err(local_err);
+ }
}
ram_state->migration_dirty_pages = 0;
qemu_mutex_unlock_ramlist();
diff --git a/system/memory.c b/system/memory.c
index 86d6c33180..c02c1d4bed 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -2937,10 +2937,9 @@ err:
return false;
}
-void memory_global_dirty_log_start(unsigned int flags)
+bool memory_global_dirty_log_start(unsigned int flags, Error **errp)
{
unsigned int old_flags;
- Error *local_err = NULL;
assert(flags && !(flags & (~GLOBAL_DIRTY_MASK)));
@@ -2952,7 +2951,7 @@ void memory_global_dirty_log_start(unsigned int flags)
flags &= ~global_dirty_tracking;
if (!flags) {
- return;
+ return true;
}
old_flags = global_dirty_tracking;
@@ -2960,17 +2959,17 @@ void memory_global_dirty_log_start(unsigned int flags)
trace_global_dirty_changed(global_dirty_tracking);
if (!old_flags) {
- if (!memory_global_dirty_log_do_start(&local_err)) {
+ if (!memory_global_dirty_log_do_start(errp)) {
global_dirty_tracking &= ~flags;
trace_global_dirty_changed(global_dirty_tracking);
- error_report_err(local_err);
- return;
+ return false;
}
memory_region_transaction_begin();
memory_region_update_pending = true;
memory_region_transaction_commit();
}
+ return true;
}
static void memory_global_dirty_log_do_stop(unsigned int flags)
--
2.44.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PULL 21/26] migration: Add Error** argument to ram_state_init()
2024-04-23 22:37 [PULL 00/26] Migration 20240423 patches Peter Xu
` (19 preceding siblings ...)
2024-04-23 22:38 ` [PULL 20/26] memory: Add Error** argument to the global_dirty_log routines Peter Xu
@ 2024-04-23 22:38 ` Peter Xu
2024-04-23 22:38 ` [PULL 22/26] migration: Add Error** argument to xbzrle_init() Peter Xu
` (5 subsequent siblings)
26 siblings, 0 replies; 28+ messages in thread
From: Peter Xu @ 2024-04-23 22:38 UTC (permalink / raw)
To: qemu-devel
Cc: peterx, Prasad Pandit, Peter Maydell, Fabiano Rosas,
Cédric Le Goater
From: Cédric Le Goater <clg@redhat.com>
Since the return value not exploited, follow the recommendations of
qapi/error.h and change it to a bool
Signed-off-by: Cédric Le Goater <clg@redhat.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20240320064911.545001-13-clg@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/ram.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index bade3e9281..26ce11a337 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2780,13 +2780,13 @@ err_out:
return -ENOMEM;
}
-static int ram_state_init(RAMState **rsp)
+static bool ram_state_init(RAMState **rsp, Error **errp)
{
*rsp = g_try_new0(RAMState, 1);
if (!*rsp) {
- error_report("%s: Init ramstate fail", __func__);
- return -1;
+ error_setg(errp, "%s: Init ramstate fail", __func__);
+ return false;
}
qemu_mutex_init(&(*rsp)->bitmap_mutex);
@@ -2802,7 +2802,7 @@ static int ram_state_init(RAMState **rsp)
(*rsp)->migration_dirty_pages = (*rsp)->ram_bytes_total >> TARGET_PAGE_BITS;
ram_state_reset(*rsp);
- return 0;
+ return true;
}
static void ram_list_init_bitmaps(void)
@@ -2897,7 +2897,10 @@ out_unlock:
static int ram_init_all(RAMState **rsp)
{
- if (ram_state_init(rsp)) {
+ Error *local_err = NULL;
+
+ if (!ram_state_init(rsp, &local_err)) {
+ error_report_err(local_err);
return -1;
}
@@ -3624,7 +3627,11 @@ void ram_handle_zero(void *host, uint64_t size)
static void colo_init_ram_state(void)
{
- ram_state_init(&ram_state);
+ Error *local_err = NULL;
+
+ if (!ram_state_init(&ram_state, &local_err)) {
+ error_report_err(local_err);
+ }
}
/*
--
2.44.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PULL 22/26] migration: Add Error** argument to xbzrle_init()
2024-04-23 22:37 [PULL 00/26] Migration 20240423 patches Peter Xu
` (20 preceding siblings ...)
2024-04-23 22:38 ` [PULL 21/26] migration: Add Error** argument to ram_state_init() Peter Xu
@ 2024-04-23 22:38 ` Peter Xu
2024-04-23 22:38 ` [PULL 23/26] migration: Modify ram_init_bitmaps() to report dirty tracking errors Peter Xu
` (4 subsequent siblings)
26 siblings, 0 replies; 28+ messages in thread
From: Peter Xu @ 2024-04-23 22:38 UTC (permalink / raw)
To: qemu-devel
Cc: peterx, Prasad Pandit, Peter Maydell, Fabiano Rosas,
Cédric Le Goater
From: Cédric Le Goater <clg@redhat.com>
Since the return value (-ENOMEM) is not exploited, follow the
recommendations of qapi/error.h and change it to a bool
Signed-off-by: Cédric Le Goater <clg@redhat.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20240320064911.545001-14-clg@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/ram.c | 22 ++++++++++------------
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index 26ce11a337..70797ef5d8 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2727,44 +2727,41 @@ int ram_discard_range(const char *rbname, uint64_t start, size_t length)
* For every allocation, we will try not to crash the VM if the
* allocation failed.
*/
-static int xbzrle_init(void)
+static bool xbzrle_init(Error **errp)
{
- Error *local_err = NULL;
-
if (!migrate_xbzrle()) {
- return 0;
+ return true;
}
XBZRLE_cache_lock();
XBZRLE.zero_target_page = g_try_malloc0(TARGET_PAGE_SIZE);
if (!XBZRLE.zero_target_page) {
- error_report("%s: Error allocating zero page", __func__);
+ error_setg(errp, "%s: Error allocating zero page", __func__);
goto err_out;
}
XBZRLE.cache = cache_init(migrate_xbzrle_cache_size(),
- TARGET_PAGE_SIZE, &local_err);
+ TARGET_PAGE_SIZE, errp);
if (!XBZRLE.cache) {
- error_report_err(local_err);
goto free_zero_page;
}
XBZRLE.encoded_buf = g_try_malloc0(TARGET_PAGE_SIZE);
if (!XBZRLE.encoded_buf) {
- error_report("%s: Error allocating encoded_buf", __func__);
+ error_setg(errp, "%s: Error allocating encoded_buf", __func__);
goto free_cache;
}
XBZRLE.current_buf = g_try_malloc(TARGET_PAGE_SIZE);
if (!XBZRLE.current_buf) {
- error_report("%s: Error allocating current_buf", __func__);
+ error_setg(errp, "%s: Error allocating current_buf", __func__);
goto free_encoded_buf;
}
/* We are all good */
XBZRLE_cache_unlock();
- return 0;
+ return true;
free_encoded_buf:
g_free(XBZRLE.encoded_buf);
@@ -2777,7 +2774,7 @@ free_zero_page:
XBZRLE.zero_target_page = NULL;
err_out:
XBZRLE_cache_unlock();
- return -ENOMEM;
+ return false;
}
static bool ram_state_init(RAMState **rsp, Error **errp)
@@ -2904,7 +2901,8 @@ static int ram_init_all(RAMState **rsp)
return -1;
}
- if (xbzrle_init()) {
+ if (!xbzrle_init(&local_err)) {
+ error_report_err(local_err);
ram_state_cleanup(rsp);
return -1;
}
--
2.44.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PULL 23/26] migration: Modify ram_init_bitmaps() to report dirty tracking errors
2024-04-23 22:37 [PULL 00/26] Migration 20240423 patches Peter Xu
` (21 preceding siblings ...)
2024-04-23 22:38 ` [PULL 22/26] migration: Add Error** argument to xbzrle_init() Peter Xu
@ 2024-04-23 22:38 ` Peter Xu
2024-04-23 22:38 ` [PULL 24/26] migration: Add Error** argument to add_bitmaps_to_list() Peter Xu
` (3 subsequent siblings)
26 siblings, 0 replies; 28+ messages in thread
From: Peter Xu @ 2024-04-23 22:38 UTC (permalink / raw)
To: qemu-devel
Cc: peterx, Prasad Pandit, Peter Maydell, Fabiano Rosas,
Cédric Le Goater
From: Cédric Le Goater <clg@redhat.com>
The .save_setup() handler has now an Error** argument that we can use
to propagate errors reported by the .log_global_start() handler. Do
that for the RAM. The caller qemu_savevm_state_setup() will store the
error under the migration stream for later detection in the migration
sequence.
Signed-off-by: Cédric Le Goater <clg@redhat.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20240320064911.545001-15-clg@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/ram.c | 27 +++++++++++----------------
1 file changed, 11 insertions(+), 16 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index 70797ef5d8..daffcd82d4 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2857,9 +2857,8 @@ static void migration_bitmap_clear_discarded_pages(RAMState *rs)
}
}
-static void ram_init_bitmaps(RAMState *rs)
+static bool ram_init_bitmaps(RAMState *rs, Error **errp)
{
- Error *local_err = NULL;
bool ret = true;
qemu_mutex_lock_ramlist();
@@ -2868,10 +2867,8 @@ static void ram_init_bitmaps(RAMState *rs)
ram_list_init_bitmaps();
/* We don't use dirty log with background snapshots */
if (!migrate_background_snapshot()) {
- ret = memory_global_dirty_log_start(GLOBAL_DIRTY_MIGRATION,
- &local_err);
+ ret = memory_global_dirty_log_start(GLOBAL_DIRTY_MIGRATION, errp);
if (!ret) {
- error_report_err(local_err);
goto out_unlock;
}
migration_bitmap_sync_precopy(rs, false);
@@ -2882,7 +2879,7 @@ out_unlock:
if (!ret) {
ram_bitmaps_destroy();
- return;
+ return false;
}
/*
@@ -2890,24 +2887,23 @@ out_unlock:
* containing all 1s to exclude any discarded pages from migration.
*/
migration_bitmap_clear_discarded_pages(rs);
+ return true;
}
-static int ram_init_all(RAMState **rsp)
+static int ram_init_all(RAMState **rsp, Error **errp)
{
- Error *local_err = NULL;
-
- if (!ram_state_init(rsp, &local_err)) {
- error_report_err(local_err);
+ if (!ram_state_init(rsp, errp)) {
return -1;
}
- if (!xbzrle_init(&local_err)) {
- error_report_err(local_err);
+ if (!xbzrle_init(errp)) {
ram_state_cleanup(rsp);
return -1;
}
- ram_init_bitmaps(*rsp);
+ if (!ram_init_bitmaps(*rsp, errp)) {
+ return -1;
+ }
return 0;
}
@@ -3104,8 +3100,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque, Error **errp)
/* migration has already setup the bitmap, reuse it. */
if (!migration_in_colo_state()) {
- if (ram_init_all(rsp) != 0) {
- error_setg(errp, "%s: failed to setup RAM for migration", __func__);
+ if (ram_init_all(rsp, errp) != 0) {
compress_threads_save_cleanup();
return -1;
}
--
2.44.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PULL 24/26] migration: Add Error** argument to add_bitmaps_to_list()
2024-04-23 22:37 [PULL 00/26] Migration 20240423 patches Peter Xu
` (22 preceding siblings ...)
2024-04-23 22:38 ` [PULL 23/26] migration: Modify ram_init_bitmaps() to report dirty tracking errors Peter Xu
@ 2024-04-23 22:38 ` Peter Xu
2024-04-23 22:38 ` [PULL 25/26] migration/multifd: solve zero page causing multiple page faults Peter Xu
` (2 subsequent siblings)
26 siblings, 0 replies; 28+ messages in thread
From: Peter Xu @ 2024-04-23 22:38 UTC (permalink / raw)
To: qemu-devel
Cc: peterx, Prasad Pandit, Peter Maydell, Fabiano Rosas,
Cédric Le Goater, Vladimir Sementsov-Ogievskiy,
Philippe Mathieu-Daudé
From: Cédric Le Goater <clg@redhat.com>
This allows to report more precise errors in the migration handler
dirty_bitmap_save_setup().
Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Signed-off-by: Cédric Le Goater <clg@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Link: https://lore.kernel.org/r/20240329105627.311227-1-clg@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/block-dirty-bitmap.c | 34 ++++++++++++++++------------------
1 file changed, 16 insertions(+), 18 deletions(-)
diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
index 542a8c297b..a7d55048c2 100644
--- a/migration/block-dirty-bitmap.c
+++ b/migration/block-dirty-bitmap.c
@@ -481,13 +481,13 @@ static void dirty_bitmap_do_save_cleanup(DBMSaveState *s)
/* Called with the BQL taken. */
static int add_bitmaps_to_list(DBMSaveState *s, BlockDriverState *bs,
- const char *bs_name, GHashTable *alias_map)
+ const char *bs_name, GHashTable *alias_map,
+ Error **errp)
{
BdrvDirtyBitmap *bitmap;
SaveBitmapState *dbms;
GHashTable *bitmap_aliases;
const char *node_alias, *bitmap_name, *bitmap_alias;
- Error *local_err = NULL;
/* When an alias map is given, @bs_name must be @bs's node name */
assert(!alias_map || !strcmp(bs_name, bdrv_get_node_name(bs)));
@@ -504,8 +504,8 @@ static int add_bitmaps_to_list(DBMSaveState *s, BlockDriverState *bs,
bitmap_name = bdrv_dirty_bitmap_name(bitmap);
if (!bs_name || strcmp(bs_name, "") == 0) {
- error_report("Bitmap '%s' in unnamed node can't be migrated",
- bitmap_name);
+ error_setg(errp, "Bitmap '%s' in unnamed node can't be migrated",
+ bitmap_name);
return -1;
}
@@ -525,9 +525,9 @@ static int add_bitmaps_to_list(DBMSaveState *s, BlockDriverState *bs,
}
if (node_alias[0] == '#') {
- error_report("Bitmap '%s' in a node with auto-generated "
- "name '%s' can't be migrated",
- bitmap_name, node_alias);
+ error_setg(errp, "Bitmap '%s' in a node with auto-generated "
+ "name '%s' can't be migrated",
+ bitmap_name, node_alias);
return -1;
}
@@ -538,8 +538,7 @@ static int add_bitmaps_to_list(DBMSaveState *s, BlockDriverState *bs,
continue;
}
- if (bdrv_dirty_bitmap_check(bitmap, BDRV_BITMAP_DEFAULT, &local_err)) {
- error_report_err(local_err);
+ if (bdrv_dirty_bitmap_check(bitmap, BDRV_BITMAP_DEFAULT, errp)) {
return -1;
}
@@ -558,9 +557,9 @@ static int add_bitmaps_to_list(DBMSaveState *s, BlockDriverState *bs,
}
} else {
if (strlen(bitmap_name) > UINT8_MAX) {
- error_report("Cannot migrate bitmap '%s' on node '%s': "
- "Name is longer than %u bytes",
- bitmap_name, bs_name, UINT8_MAX);
+ error_setg(errp, "Cannot migrate bitmap '%s' on node '%s': "
+ "Name is longer than %u bytes",
+ bitmap_name, bs_name, UINT8_MAX);
return -1;
}
bitmap_alias = bitmap_name;
@@ -599,7 +598,7 @@ static int add_bitmaps_to_list(DBMSaveState *s, BlockDriverState *bs,
}
/* Called with the BQL taken. */
-static int init_dirty_bitmap_migration(DBMSaveState *s)
+static int init_dirty_bitmap_migration(DBMSaveState *s, Error **errp)
{
BlockDriverState *bs;
SaveBitmapState *dbms;
@@ -643,7 +642,7 @@ static int init_dirty_bitmap_migration(DBMSaveState *s)
}
if (bs && bs->drv && !bs->drv->is_filter) {
- if (add_bitmaps_to_list(s, bs, name, NULL)) {
+ if (add_bitmaps_to_list(s, bs, name, NULL, errp)) {
goto fail;
}
g_hash_table_add(handled_by_blk, bs);
@@ -656,7 +655,8 @@ static int init_dirty_bitmap_migration(DBMSaveState *s)
continue;
}
- if (add_bitmaps_to_list(s, bs, bdrv_get_node_name(bs), alias_map)) {
+ if (add_bitmaps_to_list(s, bs, bdrv_get_node_name(bs), alias_map,
+ errp)) {
goto fail;
}
}
@@ -1218,9 +1218,7 @@ static int dirty_bitmap_save_setup(QEMUFile *f, void *opaque, Error **errp)
DBMSaveState *s = &((DBMState *)opaque)->save;
SaveBitmapState *dbms = NULL;
- if (init_dirty_bitmap_migration(s) < 0) {
- error_setg(errp,
- "Failed to initialize dirty tracking bitmap for blocks");
+ if (init_dirty_bitmap_migration(s, errp) < 0) {
return -1;
}
--
2.44.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PULL 25/26] migration/multifd: solve zero page causing multiple page faults
2024-04-23 22:37 [PULL 00/26] Migration 20240423 patches Peter Xu
` (23 preceding siblings ...)
2024-04-23 22:38 ` [PULL 24/26] migration: Add Error** argument to add_bitmaps_to_list() Peter Xu
@ 2024-04-23 22:38 ` Peter Xu
2024-04-23 22:38 ` [PULL 26/26] migration/colo: Fix bdrv_graph_rdlock_main_loop: Assertion `!qemu_in_coroutine()' failed Peter Xu
2024-04-24 6:50 ` [PULL 00/26] Migration 20240423 patches Richard Henderson
26 siblings, 0 replies; 28+ messages in thread
From: Peter Xu @ 2024-04-23 22:38 UTC (permalink / raw)
To: qemu-devel; +Cc: peterx, Prasad Pandit, Peter Maydell, Fabiano Rosas, Yuan Liu
From: Yuan Liu <yuan1.liu@intel.com>
Implemented recvbitmap tracking of received pages in multifd.
If the zero page appears for the first time in the recvbitmap, this
page is not checked and set.
If the zero page has already appeared in the recvbitmap, there is no
need to check the data but directly set the data to 0, because it is
unlikely that the zero page will be migrated multiple times.
Signed-off-by: Yuan Liu <yuan1.liu@intel.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20240401154110.2028453-2-yuan1.liu@intel.com
[peterx: touch up the comment, as the bitmap is used outside postcopy now]
Signed-off-by: Peter Xu <peterx@redhat.com>
---
include/exec/ramblock.h | 2 +-
migration/ram.h | 1 +
migration/multifd-zero-page.c | 4 +++-
migration/multifd-zlib.c | 1 +
migration/multifd-zstd.c | 1 +
migration/multifd.c | 1 +
migration/ram.c | 4 ++++
7 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/include/exec/ramblock.h b/include/exec/ramblock.h
index 848915ea5b..7062da380b 100644
--- a/include/exec/ramblock.h
+++ b/include/exec/ramblock.h
@@ -57,7 +57,7 @@ struct RAMBlock {
off_t bitmap_offset;
uint64_t pages_offset;
- /* bitmap of already received pages in postcopy */
+ /* Bitmap of already received pages. Only used on destination side. */
unsigned long *receivedmap;
/*
diff --git a/migration/ram.h b/migration/ram.h
index 08feecaf51..bc0318b834 100644
--- a/migration/ram.h
+++ b/migration/ram.h
@@ -69,6 +69,7 @@ int ramblock_recv_bitmap_test(RAMBlock *rb, void *host_addr);
bool ramblock_recv_bitmap_test_byte_offset(RAMBlock *rb, uint64_t byte_offset);
void ramblock_recv_bitmap_set(RAMBlock *rb, void *host_addr);
void ramblock_recv_bitmap_set_range(RAMBlock *rb, void *host_addr, size_t nr);
+void ramblock_recv_bitmap_set_offset(RAMBlock *rb, uint64_t byte_offset);
int64_t ramblock_recv_bitmap_send(QEMUFile *file,
const char *block_name);
bool ram_dirty_bitmap_reload(MigrationState *s, RAMBlock *rb, Error **errp);
diff --git a/migration/multifd-zero-page.c b/migration/multifd-zero-page.c
index 1ba38be636..e1b8370f88 100644
--- a/migration/multifd-zero-page.c
+++ b/migration/multifd-zero-page.c
@@ -80,8 +80,10 @@ void multifd_recv_zero_page_process(MultiFDRecvParams *p)
{
for (int i = 0; i < p->zero_num; i++) {
void *page = p->host + p->zero[i];
- if (!buffer_is_zero(page, p->page_size)) {
+ if (ramblock_recv_bitmap_test_byte_offset(p->block, p->zero[i])) {
memset(page, 0, p->page_size);
+ } else {
+ ramblock_recv_bitmap_set_offset(p->block, p->zero[i]);
}
}
}
diff --git a/migration/multifd-zlib.c b/migration/multifd-zlib.c
index 99821cd4d5..737a9645d2 100644
--- a/migration/multifd-zlib.c
+++ b/migration/multifd-zlib.c
@@ -284,6 +284,7 @@ static int zlib_recv(MultiFDRecvParams *p, Error **errp)
int flush = Z_NO_FLUSH;
unsigned long start = zs->total_out;
+ ramblock_recv_bitmap_set_offset(p->block, p->normal[i]);
if (i == p->normal_num - 1) {
flush = Z_SYNC_FLUSH;
}
diff --git a/migration/multifd-zstd.c b/migration/multifd-zstd.c
index 02112255ad..256858df0a 100644
--- a/migration/multifd-zstd.c
+++ b/migration/multifd-zstd.c
@@ -278,6 +278,7 @@ static int zstd_recv(MultiFDRecvParams *p, Error **errp)
z->in.pos = 0;
for (i = 0; i < p->normal_num; i++) {
+ ramblock_recv_bitmap_set_offset(p->block, p->normal[i]);
z->out.dst = p->host + p->normal[i];
z->out.size = p->page_size;
z->out.pos = 0;
diff --git a/migration/multifd.c b/migration/multifd.c
index 2802afe79d..f317bff077 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -277,6 +277,7 @@ static int nocomp_recv(MultiFDRecvParams *p, Error **errp)
for (int i = 0; i < p->normal_num; i++) {
p->iov[i].iov_base = p->host + p->normal[i];
p->iov[i].iov_len = p->page_size;
+ ramblock_recv_bitmap_set_offset(p->block, p->normal[i]);
}
return qio_channel_readv_all(p->c, p->iov, p->normal_num, errp);
}
diff --git a/migration/ram.c b/migration/ram.c
index daffcd82d4..a975c5af16 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -275,6 +275,10 @@ void ramblock_recv_bitmap_set_range(RAMBlock *rb, void *host_addr,
nr);
}
+void ramblock_recv_bitmap_set_offset(RAMBlock *rb, uint64_t byte_offset)
+{
+ set_bit_atomic(byte_offset >> TARGET_PAGE_BITS, rb->receivedmap);
+}
#define RAMBLOCK_RECV_BITMAP_ENDING (0x0123456789abcdefULL)
/*
--
2.44.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PULL 26/26] migration/colo: Fix bdrv_graph_rdlock_main_loop: Assertion `!qemu_in_coroutine()' failed.
2024-04-23 22:37 [PULL 00/26] Migration 20240423 patches Peter Xu
` (24 preceding siblings ...)
2024-04-23 22:38 ` [PULL 25/26] migration/multifd: solve zero page causing multiple page faults Peter Xu
@ 2024-04-23 22:38 ` Peter Xu
2024-04-24 6:50 ` [PULL 00/26] Migration 20240423 patches Richard Henderson
26 siblings, 0 replies; 28+ messages in thread
From: Peter Xu @ 2024-04-23 22:38 UTC (permalink / raw)
To: qemu-devel
Cc: peterx, Prasad Pandit, Peter Maydell, Fabiano Rosas, Li Zhijian,
qemu-stable, Zhang Chen
From: Li Zhijian <lizhijian@fujitsu.com>
bdrv_activate_all() should not be called from the coroutine context, move
it to the QEMU thread colo_process_incoming_thread() with the bql_lock
protected.
The backtrace is as follows:
#4 0x0000561af7948362 in bdrv_graph_rdlock_main_loop () at ../block/graph-lock.c:260
#5 0x0000561af7907a68 in graph_lockable_auto_lock_mainloop (x=0x7fd29810be7b) at /patch/to/qemu/include/block/graph-lock.h:259
#6 0x0000561af79167d1 in bdrv_activate_all (errp=0x7fd29810bed0) at ../block.c:6906
#7 0x0000561af762b4af in colo_incoming_co () at ../migration/colo.c:935
#8 0x0000561af7607e57 in process_incoming_migration_co (opaque=0x0) at ../migration/migration.c:793
#9 0x0000561af7adbeeb in coroutine_trampoline (i0=-106876144, i1=22042) at ../util/coroutine-ucontext.c:175
#10 0x00007fd2a5cf21c0 in () at /lib64/libc.so.6
Cc: qemu-stable@nongnu.org
Cc: Fabiano Rosas <farosas@suse.de>
Closes: https://gitlab.com/qemu-project/qemu/-/issues/2277
Fixes: 2b3912f135 ("block: Mark bdrv_first_blk() and bdrv_is_root_node() GRAPH_RDLOCK")
Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
Reviewed-by: Zhang Chen <chen.zhang@intel.com>
Tested-by: Zhang Chen <chen.zhang@intel.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20240417025634.1014582-1-lizhijian@fujitsu.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/colo.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/migration/colo.c b/migration/colo.c
index 84632a603e..5600a43d78 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -835,6 +835,16 @@ static void *colo_process_incoming_thread(void *opaque)
return NULL;
}
+ /* Make sure all file formats throw away their mutable metadata */
+ bql_lock();
+ bdrv_activate_all(&local_err);
+ if (local_err) {
+ bql_unlock();
+ error_report_err(local_err);
+ return NULL;
+ }
+ bql_unlock();
+
failover_init_state();
mis->to_src_file = qemu_file_get_return_path(mis->from_src_file);
@@ -922,7 +932,6 @@ out:
int coroutine_fn colo_incoming_co(void)
{
MigrationIncomingState *mis = migration_incoming_get_current();
- Error *local_err = NULL;
QemuThread th;
assert(bql_locked());
@@ -931,13 +940,6 @@ int coroutine_fn colo_incoming_co(void)
return 0;
}
- /* Make sure all file formats throw away their mutable metadata */
- bdrv_activate_all(&local_err);
- if (local_err) {
- error_report_err(local_err);
- return -EINVAL;
- }
-
qemu_thread_create(&th, "COLO incoming", colo_process_incoming_thread,
mis, QEMU_THREAD_JOINABLE);
--
2.44.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PULL 00/26] Migration 20240423 patches
2024-04-23 22:37 [PULL 00/26] Migration 20240423 patches Peter Xu
` (25 preceding siblings ...)
2024-04-23 22:38 ` [PULL 26/26] migration/colo: Fix bdrv_graph_rdlock_main_loop: Assertion `!qemu_in_coroutine()' failed Peter Xu
@ 2024-04-24 6:50 ` Richard Henderson
26 siblings, 0 replies; 28+ messages in thread
From: Richard Henderson @ 2024-04-24 6:50 UTC (permalink / raw)
To: Peter Xu, qemu-devel; +Cc: Prasad Pandit, Peter Maydell, Fabiano Rosas
On 4/23/24 15:37, Peter Xu wrote:
> The following changes since commit c25df57ae8f9fe1c72eee2dab37d76d904ac382e:
>
> Update version for 9.0.0 release (2024-04-23 14:19:21 +0100)
>
> are available in the Git repository at:
>
> https://gitlab.com/peterx/qemu.git tags/migration-20240423-pull-request
>
> for you to fetch changes up to 2cc637f1ea08d2a1b19fc5b1a30bc609f948de93:
>
> migration/colo: Fix bdrv_graph_rdlock_main_loop: Assertion `!qemu_in_coroutine()' failed. (2024-04-23 18:36:01 -0400)
>
> ----------------------------------------------------------------
> Migration pull for 9.1
>
> - Het's new test cases for "channels"
> - Het's fix for a typo for vsock parsing
> - Cedric's VFIO error report series
> - Cedric's one more patch for dirty-bitmap error reports
> - Zhijian's rdma deprecation patch
> - Yuan's zeropage optimization to fix double faults on anon mem
> - Zhijian's COLO fix on a crash
Applied, thanks. Please update https://wiki.qemu.org/ChangeLog/9.1 as appropriate.
r~
^ permalink raw reply [flat|nested] 28+ messages in thread