* [PATCH v4 01/10] tests/qtest: add various qtest_qmp_assert_success() variants
2023-06-01 16:13 [PATCH v4 00/10] tests/qtest: make migration-test massively faster Daniel P. Berrangé
@ 2023-06-01 16:13 ` Daniel P. Berrangé
2023-06-01 22:44 ` Juan Quintela
2023-06-01 16:13 ` [PATCH v4 02/10] tests/qtest: add support for callback to receive QMP events Daniel P. Berrangé
` (10 subsequent siblings)
11 siblings, 1 reply; 22+ messages in thread
From: Daniel P. Berrangé @ 2023-06-01 16:13 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Juan Quintela, Peter Xu, Laurent Vivier,
Paolo Bonzini, Leonardo Bras, Daniel P. Berrangé
Add several counterparts of qtest_qmp_assert_success() that can
* Use va_list instead of ...
* Accept a list of FDs to send
* Return the response data
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/qtest/libqtest.c | 97 +++++++++++++++++++++++++++++++---
tests/qtest/libqtest.h | 115 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 205 insertions(+), 7 deletions(-)
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index c3a0ef5bb4..7ae28fb6ac 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -1229,24 +1229,107 @@ void qtest_memset(QTestState *s, uint64_t addr, uint8_t pattern, size_t size)
qtest_rsp(s);
}
-void qtest_qmp_assert_success(QTestState *qts, const char *fmt, ...)
+QDict *qtest_vqmp_assert_success_ref(QTestState *qts,
+ const char *fmt, va_list args)
{
- va_list ap;
QDict *response;
+ QDict *ret;
- va_start(ap, fmt);
- response = qtest_vqmp(qts, fmt, ap);
- va_end(ap);
+ response = qtest_vqmp(qts, fmt, args);
+
+ g_assert(response);
+ if (!qdict_haskey(response, "return")) {
+ g_autoptr(GString) s = qobject_to_json_pretty(QOBJECT(response), true);
+ g_test_message("%s", s->str);
+ }
+ g_assert(qdict_haskey(response, "return"));
+ ret = qdict_get_qdict(response, "return");
+ qobject_ref(ret);
+ qobject_unref(response);
+
+ return ret;
+}
+
+void qtest_vqmp_assert_success(QTestState *qts,
+ const char *fmt, va_list args)
+{
+ QDict *response;
+
+ response = qtest_vqmp_assert_success_ref(qts, fmt, args);
+
+ qobject_unref(response);
+}
+
+#ifndef _WIN32
+QDict *qtest_vqmp_fds_assert_success_ref(QTestState *qts, int *fds, size_t nfds,
+ const char *fmt, va_list args)
+{
+ QDict *response;
+ QDict *ret;
+
+ response = qtest_vqmp_fds(qts, fds, nfds, fmt, args);
g_assert(response);
if (!qdict_haskey(response, "return")) {
- GString *s = qobject_to_json_pretty(QOBJECT(response), true);
+ g_autoptr(GString) s = qobject_to_json_pretty(QOBJECT(response), true);
g_test_message("%s", s->str);
- g_string_free(s, true);
}
g_assert(qdict_haskey(response, "return"));
+ ret = qdict_get_qdict(response, "return");
+ qobject_ref(ret);
qobject_unref(response);
+
+ return ret;
+}
+
+void qtest_vqmp_fds_assert_success(QTestState *qts, int *fds, size_t nfds,
+ const char *fmt, va_list args)
+{
+ QDict *response;
+ response = qtest_vqmp_fds_assert_success_ref(qts, fds, nfds, fmt, args);
+ qobject_unref(response);
+}
+#endif /* !_WIN32 */
+
+QDict *qtest_qmp_assert_success_ref(QTestState *qts, const char *fmt, ...)
+{
+ QDict *response;
+ va_list ap;
+ va_start(ap, fmt);
+ response = qtest_vqmp_assert_success_ref(qts, fmt, ap);
+ va_end(ap);
+ return response;
+}
+
+void qtest_qmp_assert_success(QTestState *qts, const char *fmt, ...)
+{
+ va_list ap;
+ va_start(ap, fmt);
+ qtest_vqmp_assert_success(qts, fmt, ap);
+ va_end(ap);
+}
+
+#ifndef _WIN32
+QDict *qtest_qmp_fds_assert_success_ref(QTestState *qts, int *fds, size_t nfds,
+ const char *fmt, ...)
+{
+ QDict *response;
+ va_list ap;
+ va_start(ap, fmt);
+ response = qtest_vqmp_fds_assert_success_ref(qts, fds, nfds, fmt, ap);
+ va_end(ap);
+ return response;
+}
+
+void qtest_qmp_fds_assert_success(QTestState *qts, int *fds, size_t nfds,
+ const char *fmt, ...)
+{
+ va_list ap;
+ va_start(ap, fmt);
+ qtest_vqmp_fds_assert_success(qts, fds, nfds, fmt, ap);
+ va_end(ap);
}
+#endif /* !_WIN32 */
bool qtest_big_endian(QTestState *s)
{
diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
index 8d7d450963..d310eba7fb 100644
--- a/tests/qtest/libqtest.h
+++ b/tests/qtest/libqtest.h
@@ -693,6 +693,86 @@ void qtest_add_abrt_handler(GHookFunc fn, const void *data);
*/
void qtest_remove_abrt_handler(void *data);
+/**
+ * qtest_vqmp_assert_success_ref:
+ * @qts: QTestState instance to operate on
+ * @fmt: QMP message to send to qemu, formatted like
+ * qobject_from_jsonf_nofail(). See parse_interpolation() for what's
+ * supported after '%'.
+ * @args: variable arguments for @fmt
+ *
+ * Sends a QMP message to QEMU, asserts that a 'return' key is present in
+ * the response, and returns the response.
+ */
+QDict *qtest_vqmp_assert_success_ref(QTestState *qts,
+ const char *fmt, va_list args)
+ G_GNUC_PRINTF(2, 0);
+
+/**
+ * qtest_vqmp_assert_success:
+ * @qts: QTestState instance to operate on
+ * @fmt: QMP message to send to qemu, formatted like
+ * qobject_from_jsonf_nofail(). See parse_interpolation() for what's
+ * supported after '%'.
+ * @args: variable arguments for @fmt
+ *
+ * Sends a QMP message to QEMU and asserts that a 'return' key is present in
+ * the response.
+ */
+void qtest_vqmp_assert_success(QTestState *qts,
+ const char *fmt, va_list args)
+ G_GNUC_PRINTF(2, 0);
+
+#ifndef _WIN32
+/**
+ * qtest_vqmp_fds_assert_success_ref:
+ * @qts: QTestState instance to operate on
+ * @fds: the file descriptors to send
+ * @nfds: number of @fds to send
+ * @fmt: QMP message to send to qemu, formatted like
+ * qobject_from_jsonf_nofail(). See parse_interpolation() for what's
+ * supported after '%'.
+ * @args: variable arguments for @fmt
+ *
+ * Sends a QMP message with file descriptors to QEMU,
+ * asserts that a 'return' key is present in the response,
+ * and returns the response.
+ */
+QDict *qtest_vqmp_fds_assert_success_ref(QTestState *qts, int *fds, size_t nfds,
+ const char *fmt, va_list args)
+ G_GNUC_PRINTF(4, 0);
+
+/**
+ * qtest_vqmp_fds_assert_success:
+ * @qts: QTestState instance to operate on
+ * @fds: the file descriptors to send
+ * @nfds: number of @fds to send
+ * @fmt: QMP message to send to qemu, formatted like
+ * qobject_from_jsonf_nofail(). See parse_interpolation() for what's
+ * supported after '%'.
+ * @args: variable arguments for @fmt
+ *
+ * Sends a QMP message with file descriptors to QEMU and
+ * asserts that a 'return' key is present in the response.
+ */
+void qtest_vqmp_fds_assert_success(QTestState *qts, int *fds, size_t nfds,
+ const char *fmt, va_list args)
+ G_GNUC_PRINTF(4, 0);
+#endif /* !_WIN32 */
+
+/**
+ * qtest_qmp_assert_success_ref:
+ * @qts: QTestState instance to operate on
+ * @fmt: QMP message to send to qemu, formatted like
+ * qobject_from_jsonf_nofail(). See parse_interpolation() for what's
+ * supported after '%'.
+ *
+ * Sends a QMP message to QEMU, asserts that a 'return' key is present in
+ * the response, and returns the response.
+ */
+QDict *qtest_qmp_assert_success_ref(QTestState *qts, const char *fmt, ...)
+ G_GNUC_PRINTF(2, 3);
+
/**
* qtest_qmp_assert_success:
* @qts: QTestState instance to operate on
@@ -706,6 +786,41 @@ void qtest_remove_abrt_handler(void *data);
void qtest_qmp_assert_success(QTestState *qts, const char *fmt, ...)
G_GNUC_PRINTF(2, 3);
+#ifndef _WIN32
+/**
+ * qtest_qmp_fd_assert_success_ref:
+ * @qts: QTestState instance to operate on
+ * @fds: the file descriptors to send
+ * @nfds: number of @fds to send
+ * @fmt: QMP message to send to qemu, formatted like
+ * qobject_from_jsonf_nofail(). See parse_interpolation() for what's
+ * supported after '%'.
+ *
+ * Sends a QMP message with file descriptors to QEMU,
+ * asserts that a 'return' key is present in the response,
+ * and returns the response.
+ */
+QDict *qtest_qmp_fds_assert_success_ref(QTestState *qts, int *fds, size_t nfds,
+ const char *fmt, ...)
+ G_GNUC_PRINTF(4, 5);
+
+/**
+ * qtest_qmp_fd_assert_success:
+ * @qts: QTestState instance to operate on
+ * @fds: the file descriptors to send
+ * @nfds: number of @fds to send
+ * @fmt: QMP message to send to qemu, formatted like
+ * qobject_from_jsonf_nofail(). See parse_interpolation() for what's
+ * supported after '%'.
+ *
+ * Sends a QMP message with file descriptors to QEMU and
+ * asserts that a 'return' key is present in the response.
+ */
+void qtest_qmp_fds_assert_success(QTestState *qts, int *fds, size_t nfds,
+ const char *fmt, ...)
+ G_GNUC_PRINTF(4, 5);
+#endif /* !_WIN32 */
+
/**
* qtest_cb_for_every_machine:
* @cb: Pointer to the callback function
--
2.40.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v4 02/10] tests/qtest: add support for callback to receive QMP events
2023-06-01 16:13 [PATCH v4 00/10] tests/qtest: make migration-test massively faster Daniel P. Berrangé
2023-06-01 16:13 ` [PATCH v4 01/10] tests/qtest: add various qtest_qmp_assert_success() variants Daniel P. Berrangé
@ 2023-06-01 16:13 ` Daniel P. Berrangé
2023-06-01 22:45 ` Juan Quintela
2023-06-02 7:51 ` Thomas Huth
2023-06-01 16:13 ` [PATCH v4 03/10] tests/qtest: get rid of 'qmp_command' helper in migration test Daniel P. Berrangé
` (9 subsequent siblings)
11 siblings, 2 replies; 22+ messages in thread
From: Daniel P. Berrangé @ 2023-06-01 16:13 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Juan Quintela, Peter Xu, Laurent Vivier,
Paolo Bonzini, Leonardo Bras, Daniel P. Berrangé
Currently code must call one of the qtest_qmp_event* functions to
fetch events. These are only usable if the immediate caller knows
the particular event they want to capture, and are only interested
in one specific event type. Adding ability to register an event
callback lets the caller capture a range of events over any period
of time.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/qtest/libqtest.c | 18 ++++++++++++++++--
tests/qtest/libqtest.h | 43 ++++++++++++++++++++++++++++++++++++++++--
2 files changed, 57 insertions(+), 4 deletions(-)
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 7ae28fb6ac..77de16227f 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -82,6 +82,8 @@ struct QTestState
GString *rx;
QTestTransportOps ops;
GList *pending_events;
+ QTestQMPEventCallback eventCB;
+ void *eventData;
};
static GHookList abrt_hooks;
@@ -703,8 +705,13 @@ QDict *qtest_qmp_receive(QTestState *s)
if (!qdict_get_try_str(response, "event")) {
return response;
}
- /* Stash the event for a later consumption */
- s->pending_events = g_list_append(s->pending_events, response);
+
+ if (!s->eventCB ||
+ !s->eventCB(s, qdict_get_str(response, "event"),
+ response, s->eventData)) {
+ /* Stash the event for a later consumption */
+ s->pending_events = g_list_append(s->pending_events, response);
+ }
}
}
@@ -808,6 +815,13 @@ void qtest_qmp_send_raw(QTestState *s, const char *fmt, ...)
va_end(ap);
}
+void qtest_qmp_set_event_callback(QTestState *s,
+ QTestQMPEventCallback cb, void *opaque)
+{
+ s->eventCB = cb;
+ s->eventData = opaque;
+}
+
QDict *qtest_qmp_event_ref(QTestState *s, const char *event)
{
while (s->pending_events) {
diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
index d310eba7fb..a12acf7fa9 100644
--- a/tests/qtest/libqtest.h
+++ b/tests/qtest/libqtest.h
@@ -238,17 +238,52 @@ QDict *qtest_qmp_receive_dict(QTestState *s);
* @s: #QTestState instance to operate on.
*
* Reads a QMP message from QEMU and returns the response.
- * Buffers all the events received meanwhile, until a
- * call to qtest_qmp_eventwait
+ *
+ * If a callback is registered with qtest_qmp_set_event_callback,
+ * it will be invoked for every event seen, otherwise events
+ * will be buffered until a call to one of the qtest_qmp_eventwait
+ * family of functions.
*/
QDict *qtest_qmp_receive(QTestState *s);
+/*
+ * QTestQMPEventCallback:
+ * @s: #QTestState instance event was received on
+ * @name: name of the event type
+ * @event: #QDict for the event details
+ * @opaque: opaque data from time of callback registration
+ *
+ * This callback will be invoked whenever an event is received.
+ * If the callback returns true the event will be consumed,
+ * otherwise it will be put on the list of pending events.
+ * Pending events can be later handled by calling either
+ * qtest_qmp_eventwait or qtest_qmp_eventwait_ref.
+ *
+ * Return: true to consume the event, false to let it be queued
+ */
+typedef bool (*QTestQMPEventCallback)(QTestState *s, const char *name,
+ QDict *event, void *opaque);
+
+/**
+ * qtest_qmp_set_event_callback:
+ * @s: #QTestSTate instance to operate on
+ * @cb: callback to invoke for events
+ * @opaque: data to pass to @cb
+ *
+ * Register a callback to be invoked whenever an event arrives
+ */
+void qtest_qmp_set_event_callback(QTestState *s,
+ QTestQMPEventCallback cb, void *opaque);
+
/**
* qtest_qmp_eventwait:
* @s: #QTestState instance to operate on.
* @event: event to wait for.
*
* Continuously polls for QMP responses until it receives the desired event.
+ *
+ * Any callback registered with qtest_qmp_set_event_callback will
+ * be invoked for every event seen.
*/
void qtest_qmp_eventwait(QTestState *s, const char *event);
@@ -258,6 +293,10 @@ void qtest_qmp_eventwait(QTestState *s, const char *event);
* @event: event to wait for.
*
* Continuously polls for QMP responses until it receives the desired event.
+ *
+ * Any callback registered with qtest_qmp_set_event_callback will
+ * be invoked for every event seen.
+ *
* Returns a copy of the event for further investigation.
*/
QDict *qtest_qmp_eventwait_ref(QTestState *s, const char *event);
--
2.40.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v4 02/10] tests/qtest: add support for callback to receive QMP events
2023-06-01 16:13 ` [PATCH v4 02/10] tests/qtest: add support for callback to receive QMP events Daniel P. Berrangé
@ 2023-06-01 22:45 ` Juan Quintela
2023-06-02 7:51 ` Thomas Huth
1 sibling, 0 replies; 22+ messages in thread
From: Juan Quintela @ 2023-06-01 22:45 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, Thomas Huth, Peter Xu, Laurent Vivier, Paolo Bonzini,
Leonardo Bras
Daniel P. Berrangé <berrange@redhat.com> wrote:
> Currently code must call one of the qtest_qmp_event* functions to
> fetch events. These are only usable if the immediate caller knows
> the particular event they want to capture, and are only interested
> in one specific event type. Adding ability to register an event
> callback lets the caller capture a range of events over any period
> of time.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 02/10] tests/qtest: add support for callback to receive QMP events
2023-06-01 16:13 ` [PATCH v4 02/10] tests/qtest: add support for callback to receive QMP events Daniel P. Berrangé
2023-06-01 22:45 ` Juan Quintela
@ 2023-06-02 7:51 ` Thomas Huth
1 sibling, 0 replies; 22+ messages in thread
From: Thomas Huth @ 2023-06-02 7:51 UTC (permalink / raw)
To: Daniel P. Berrangé, qemu-devel
Cc: Juan Quintela, Peter Xu, Laurent Vivier, Paolo Bonzini,
Leonardo Bras
On 01/06/2023 18.13, Daniel P. Berrangé wrote:
> Currently code must call one of the qtest_qmp_event* functions to
> fetch events. These are only usable if the immediate caller knows
> the particular event they want to capture, and are only interested
> in one specific event type. Adding ability to register an event
> callback lets the caller capture a range of events over any period
> of time.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> tests/qtest/libqtest.c | 18 ++++++++++++++++--
> tests/qtest/libqtest.h | 43 ++++++++++++++++++++++++++++++++++++++++--
> 2 files changed, 57 insertions(+), 4 deletions(-)
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v4 03/10] tests/qtest: get rid of 'qmp_command' helper in migration test
2023-06-01 16:13 [PATCH v4 00/10] tests/qtest: make migration-test massively faster Daniel P. Berrangé
2023-06-01 16:13 ` [PATCH v4 01/10] tests/qtest: add various qtest_qmp_assert_success() variants Daniel P. Berrangé
2023-06-01 16:13 ` [PATCH v4 02/10] tests/qtest: add support for callback to receive QMP events Daniel P. Berrangé
@ 2023-06-01 16:13 ` Daniel P. Berrangé
2023-06-02 7:52 ` Thomas Huth
2023-06-01 16:13 ` [PATCH v4 04/10] tests/qtest: get rid of some 'qtest_qmp' usage " Daniel P. Berrangé
` (8 subsequent siblings)
11 siblings, 1 reply; 22+ messages in thread
From: Daniel P. Berrangé @ 2023-06-01 16:13 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Juan Quintela, Peter Xu, Laurent Vivier,
Paolo Bonzini, Leonardo Bras, Daniel P. Berrangé
This function duplicates logic of qtest_qmp_assert_success_ref.
The qtest_qmp_assert_success_ref method has better diagnostics
on failure because it prints the entire QMP response, instead
of just asserting on existance of the 'error' key.
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/qtest/migration-helpers.c | 22 ----------------------
tests/qtest/migration-helpers.h | 3 ---
tests/qtest/migration-test.c | 29 +++++++++++++++--------------
3 files changed, 15 insertions(+), 39 deletions(-)
diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
index f6f3c6680f..bddf3f8d4d 100644
--- a/tests/qtest/migration-helpers.c
+++ b/tests/qtest/migration-helpers.c
@@ -85,28 +85,6 @@ QDict *wait_command(QTestState *who, const char *command, ...)
return ret;
}
-/*
- * Execute the qmp command only
- */
-QDict *qmp_command(QTestState *who, const char *command, ...)
-{
- va_list ap;
- QDict *resp, *ret;
-
- va_start(ap, command);
- resp = qtest_vqmp(who, command, ap);
- va_end(ap);
-
- g_assert(!qdict_haskey(resp, "error"));
- g_assert(qdict_haskey(resp, "return"));
-
- ret = qdict_get_qdict(resp, "return");
- qobject_ref(ret);
- qobject_unref(resp);
-
- return ret;
-}
-
/*
* Send QMP command "migrate".
* Arguments are built from @fmt... (formatted like
diff --git a/tests/qtest/migration-helpers.h b/tests/qtest/migration-helpers.h
index a188b62787..2e51a6e195 100644
--- a/tests/qtest/migration-helpers.h
+++ b/tests/qtest/migration-helpers.h
@@ -25,9 +25,6 @@ QDict *wait_command_fd(QTestState *who, int fd, const char *command, ...);
G_GNUC_PRINTF(2, 3)
QDict *wait_command(QTestState *who, const char *command, ...);
-G_GNUC_PRINTF(2, 3)
-QDict *qmp_command(QTestState *who, const char *command, ...);
-
G_GNUC_PRINTF(3, 4)
void migrate_qmp(QTestState *who, const char *uri, const char *fmt, ...);
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index b99b49a314..9ce27f89ec 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -2322,32 +2322,33 @@ static void test_multifd_tcp_cancel(void)
static void calc_dirty_rate(QTestState *who, uint64_t calc_time)
{
- qobject_unref(qmp_command(who,
- "{ 'execute': 'calc-dirty-rate',"
- "'arguments': { "
- "'calc-time': %" PRIu64 ","
- "'mode': 'dirty-ring' }}",
- calc_time));
+ qtest_qmp_assert_success(who,
+ "{ 'execute': 'calc-dirty-rate',"
+ "'arguments': { "
+ "'calc-time': %" PRIu64 ","
+ "'mode': 'dirty-ring' }}",
+ calc_time);
}
static QDict *query_dirty_rate(QTestState *who)
{
- return qmp_command(who, "{ 'execute': 'query-dirty-rate' }");
+ return qtest_qmp_assert_success_ref(who,
+ "{ 'execute': 'query-dirty-rate' }");
}
static void dirtylimit_set_all(QTestState *who, uint64_t dirtyrate)
{
- qobject_unref(qmp_command(who,
- "{ 'execute': 'set-vcpu-dirty-limit',"
- "'arguments': { "
- "'dirty-rate': %" PRIu64 " } }",
- dirtyrate));
+ qtest_qmp_assert_success(who,
+ "{ 'execute': 'set-vcpu-dirty-limit',"
+ "'arguments': { "
+ "'dirty-rate': %" PRIu64 " } }",
+ dirtyrate);
}
static void cancel_vcpu_dirty_limit(QTestState *who)
{
- qobject_unref(qmp_command(who,
- "{ 'execute': 'cancel-vcpu-dirty-limit' }"));
+ qtest_qmp_assert_success(who,
+ "{ 'execute': 'cancel-vcpu-dirty-limit' }");
}
static QDict *query_vcpu_dirty_limit(QTestState *who)
--
2.40.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v4 03/10] tests/qtest: get rid of 'qmp_command' helper in migration test
2023-06-01 16:13 ` [PATCH v4 03/10] tests/qtest: get rid of 'qmp_command' helper in migration test Daniel P. Berrangé
@ 2023-06-02 7:52 ` Thomas Huth
0 siblings, 0 replies; 22+ messages in thread
From: Thomas Huth @ 2023-06-02 7:52 UTC (permalink / raw)
To: Daniel P. Berrangé, qemu-devel
Cc: Juan Quintela, Peter Xu, Laurent Vivier, Paolo Bonzini,
Leonardo Bras
On 01/06/2023 18.13, Daniel P. Berrangé wrote:
> This function duplicates logic of qtest_qmp_assert_success_ref.
> The qtest_qmp_assert_success_ref method has better diagnostics
> on failure because it prints the entire QMP response, instead
> of just asserting on existance of the 'error' key.
>
> Reviewed-by: Juan Quintela <quintela@redhat.com>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> tests/qtest/migration-helpers.c | 22 ----------------------
> tests/qtest/migration-helpers.h | 3 ---
> tests/qtest/migration-test.c | 29 +++++++++++++++--------------
> 3 files changed, 15 insertions(+), 39 deletions(-)
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v4 04/10] tests/qtest: get rid of some 'qtest_qmp' usage in migration test
2023-06-01 16:13 [PATCH v4 00/10] tests/qtest: make migration-test massively faster Daniel P. Berrangé
` (2 preceding siblings ...)
2023-06-01 16:13 ` [PATCH v4 03/10] tests/qtest: get rid of 'qmp_command' helper in migration test Daniel P. Berrangé
@ 2023-06-01 16:13 ` Daniel P. Berrangé
2023-06-01 16:13 ` [PATCH v4 05/10] tests/qtest: switch to using event callbacks for STOP event Daniel P. Berrangé
` (7 subsequent siblings)
11 siblings, 0 replies; 22+ messages in thread
From: Daniel P. Berrangé @ 2023-06-01 16:13 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Juan Quintela, Peter Xu, Laurent Vivier,
Paolo Bonzini, Leonardo Bras, Daniel P. Berrangé
Some of the usage is just a verbose way of re-inventing the
qtest_qmp_assert_success(_ref) methods.
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/qtest/migration-helpers.c | 8 ++---
tests/qtest/migration-test.c | 52 ++++++++++++---------------------
2 files changed, 21 insertions(+), 39 deletions(-)
diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
index bddf3f8d4d..e26fdcb132 100644
--- a/tests/qtest/migration-helpers.c
+++ b/tests/qtest/migration-helpers.c
@@ -93,7 +93,7 @@ QDict *wait_command(QTestState *who, const char *command, ...)
void migrate_qmp(QTestState *who, const char *uri, const char *fmt, ...)
{
va_list ap;
- QDict *args, *rsp;
+ QDict *args;
va_start(ap, fmt);
args = qdict_from_vjsonf_nofail(fmt, ap);
@@ -102,10 +102,8 @@ void migrate_qmp(QTestState *who, const char *uri, const char *fmt, ...)
g_assert(!qdict_haskey(args, "uri"));
qdict_put_str(args, "uri", uri);
- rsp = qtest_qmp(who, "{ 'execute': 'migrate', 'arguments': %p}", args);
-
- g_assert(qdict_haskey(rsp, "return"));
- qobject_unref(rsp);
+ 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 9ce27f89ec..822516286d 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -359,14 +359,10 @@ static void migrate_check_parameter_int(QTestState *who, const char *parameter,
static void migrate_set_parameter_int(QTestState *who, const char *parameter,
long long value)
{
- QDict *rsp;
-
- rsp = qtest_qmp(who,
- "{ 'execute': 'migrate-set-parameters',"
- "'arguments': { %s: %lld } }",
- parameter, value);
- g_assert(qdict_haskey(rsp, "return"));
- qobject_unref(rsp);
+ qtest_qmp_assert_success(who,
+ "{ 'execute': 'migrate-set-parameters',"
+ "'arguments': { %s: %lld } }",
+ parameter, value);
migrate_check_parameter_int(who, parameter, value);
}
@@ -392,14 +388,10 @@ static void migrate_check_parameter_str(QTestState *who, const char *parameter,
static void migrate_set_parameter_str(QTestState *who, const char *parameter,
const char *value)
{
- QDict *rsp;
-
- rsp = qtest_qmp(who,
- "{ 'execute': 'migrate-set-parameters',"
- "'arguments': { %s: %s } }",
- parameter, value);
- g_assert(qdict_haskey(rsp, "return"));
- qobject_unref(rsp);
+ qtest_qmp_assert_success(who,
+ "{ 'execute': 'migrate-set-parameters',"
+ "'arguments': { %s: %s } }",
+ parameter, value);
migrate_check_parameter_str(who, parameter, value);
}
@@ -427,14 +419,10 @@ static void migrate_check_parameter_bool(QTestState *who, const char *parameter,
static void migrate_set_parameter_bool(QTestState *who, const char *parameter,
int value)
{
- QDict *rsp;
-
- rsp = qtest_qmp(who,
- "{ 'execute': 'migrate-set-parameters',"
- "'arguments': { %s: %i } }",
- parameter, value);
- g_assert(qdict_haskey(rsp, "return"));
- qobject_unref(rsp);
+ qtest_qmp_assert_success(who,
+ "{ 'execute': 'migrate-set-parameters',"
+ "'arguments': { %s: %i } }",
+ parameter, value);
migrate_check_parameter_bool(who, parameter, value);
}
@@ -494,16 +482,12 @@ static void migrate_cancel(QTestState *who)
static void migrate_set_capability(QTestState *who, const char *capability,
bool value)
{
- QDict *rsp;
-
- rsp = qtest_qmp(who,
- "{ 'execute': 'migrate-set-capabilities',"
- "'arguments': { "
- "'capabilities': [ { "
- "'capability': %s, 'state': %i } ] } }",
- capability, value);
- g_assert(qdict_haskey(rsp, "return"));
- qobject_unref(rsp);
+ qtest_qmp_assert_success(who,
+ "{ 'execute': 'migrate-set-capabilities',"
+ "'arguments': { "
+ "'capabilities': [ { "
+ "'capability': %s, 'state': %i } ] } }",
+ capability, value);
}
static void migrate_postcopy_start(QTestState *from, QTestState *to)
--
2.40.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v4 05/10] tests/qtest: switch to using event callbacks for STOP event
2023-06-01 16:13 [PATCH v4 00/10] tests/qtest: make migration-test massively faster Daniel P. Berrangé
` (3 preceding siblings ...)
2023-06-01 16:13 ` [PATCH v4 04/10] tests/qtest: get rid of some 'qtest_qmp' usage " Daniel P. Berrangé
@ 2023-06-01 16:13 ` Daniel P. Berrangé
2023-06-01 16:13 ` [PATCH v4 06/10] tests/qtest: replace wait_command() with qtest_qmp_assert_success Daniel P. Berrangé
` (6 subsequent siblings)
11 siblings, 0 replies; 22+ messages in thread
From: Daniel P. Berrangé @ 2023-06-01 16:13 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Juan Quintela, Peter Xu, Laurent Vivier,
Paolo Bonzini, Leonardo Bras, Daniel P. Berrangé
Change the migration test to use the new qtest event callback to watch
for the stop event. This ensures that we only watch for the STOP event
on the source QEMU. The previous code would set the single 'got_stop'
flag when either source or dest QEMU got the STOP event.
Reviewed-by: Juan Quintela <quintela@redhat.com>
Acked-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/qtest/migration-helpers.c | 19 +++++++++----------
tests/qtest/migration-helpers.h | 3 ++-
tests/qtest/migration-test.c | 4 ++++
3 files changed, 15 insertions(+), 11 deletions(-)
diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
index e26fdcb132..7ceadecf84 100644
--- a/tests/qtest/migration-helpers.c
+++ b/tests/qtest/migration-helpers.c
@@ -23,15 +23,17 @@
*/
#define MIGRATION_STATUS_WAIT_TIMEOUT 120
-bool got_stop;
-
-static void check_stop_event(QTestState *who)
+bool migrate_watch_for_stop(QTestState *who, const char *name,
+ QDict *event, void *opaque)
{
- QDict *event = qtest_qmp_event_ref(who, "STOP");
- if (event) {
- got_stop = true;
- qobject_unref(event);
+ bool *seen = opaque;
+
+ if (g_str_equal(name, "STOP")) {
+ *seen = true;
+ return true;
}
+
+ return false;
}
#ifndef _WIN32
@@ -48,7 +50,6 @@ QDict *wait_command_fd(QTestState *who, int fd, const char *command, ...)
va_end(ap);
resp = qtest_qmp_receive(who);
- check_stop_event(who);
g_assert(!qdict_haskey(resp, "error"));
g_assert(qdict_haskey(resp, "return"));
@@ -73,8 +74,6 @@ QDict *wait_command(QTestState *who, const char *command, ...)
resp = qtest_vqmp(who, command, ap);
va_end(ap);
- check_stop_event(who);
-
g_assert(!qdict_haskey(resp, "error"));
g_assert(qdict_haskey(resp, "return"));
diff --git a/tests/qtest/migration-helpers.h b/tests/qtest/migration-helpers.h
index 2e51a6e195..fa69d1780a 100644
--- a/tests/qtest/migration-helpers.h
+++ b/tests/qtest/migration-helpers.h
@@ -15,7 +15,8 @@
#include "libqtest.h"
-extern bool got_stop;
+bool migrate_watch_for_stop(QTestState *who, const char *name,
+ QDict *event, void *opaque);
#ifndef _WIN32
G_GNUC_PRINTF(3, 4)
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 822516286d..0af72c37c2 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -43,6 +43,7 @@
unsigned start_address;
unsigned end_address;
static bool uffd_feature_thread_id;
+static bool got_stop;
/*
* Dirtylimit stop working if dirty page rate error
@@ -703,6 +704,9 @@ static int test_migrate_start(QTestState **from, QTestState **to,
ignore_stderr);
if (!args->only_target) {
*from = qtest_init(cmd_source);
+ qtest_qmp_set_event_callback(*from,
+ migrate_watch_for_stop,
+ &got_stop);
}
cmd_target = g_strdup_printf("-accel kvm%s -accel tcg%s%s "
--
2.40.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v4 06/10] tests/qtest: replace wait_command() with qtest_qmp_assert_success
2023-06-01 16:13 [PATCH v4 00/10] tests/qtest: make migration-test massively faster Daniel P. Berrangé
` (4 preceding siblings ...)
2023-06-01 16:13 ` [PATCH v4 05/10] tests/qtest: switch to using event callbacks for STOP event Daniel P. Berrangé
@ 2023-06-01 16:13 ` Daniel P. Berrangé
2023-06-01 16:13 ` [PATCH v4 07/10] tests/qtest: capture RESUME events during migration Daniel P. Berrangé
` (5 subsequent siblings)
11 siblings, 0 replies; 22+ messages in thread
From: Daniel P. Berrangé @ 2023-06-01 16:13 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Juan Quintela, Peter Xu, Laurent Vivier,
Paolo Bonzini, Leonardo Bras, Daniel P. Berrangé
Most usage of wait_command() is followed by qobject_unref(), which
is just a verbose re-implementation of qtest_qmp_assert_success().
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/qtest/migration-helpers.c | 53 +---------
tests/qtest/migration-helpers.h | 8 --
tests/qtest/migration-test.c | 170 +++++++++++++-------------------
3 files changed, 74 insertions(+), 157 deletions(-)
diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
index 7ceadecf84..73e506a5f8 100644
--- a/tests/qtest/migration-helpers.c
+++ b/tests/qtest/migration-helpers.c
@@ -36,54 +36,6 @@ bool migrate_watch_for_stop(QTestState *who, const char *name,
return false;
}
-#ifndef _WIN32
-/*
- * Events can get in the way of responses we are actually waiting for.
- */
-QDict *wait_command_fd(QTestState *who, int fd, const char *command, ...)
-{
- va_list ap;
- QDict *resp, *ret;
-
- va_start(ap, command);
- qtest_qmp_vsend_fds(who, &fd, 1, command, ap);
- va_end(ap);
-
- resp = qtest_qmp_receive(who);
-
- g_assert(!qdict_haskey(resp, "error"));
- g_assert(qdict_haskey(resp, "return"));
-
- ret = qdict_get_qdict(resp, "return");
- qobject_ref(ret);
- qobject_unref(resp);
-
- return ret;
-}
-#endif
-
-/*
- * Events can get in the way of responses we are actually waiting for.
- */
-QDict *wait_command(QTestState *who, const char *command, ...)
-{
- va_list ap;
- QDict *resp, *ret;
-
- va_start(ap, command);
- resp = qtest_vqmp(who, command, ap);
- va_end(ap);
-
- g_assert(!qdict_haskey(resp, "error"));
- g_assert(qdict_haskey(resp, "return"));
-
- ret = qdict_get_qdict(resp, "return");
- qobject_ref(ret);
- qobject_unref(resp);
-
- return ret;
-}
-
/*
* Send QMP command "migrate".
* Arguments are built from @fmt... (formatted like
@@ -111,7 +63,7 @@ void migrate_qmp(QTestState *who, const char *uri, const char *fmt, ...)
*/
QDict *migrate_query(QTestState *who)
{
- return wait_command(who, "{ 'execute': 'query-migrate' }");
+ return qtest_qmp_assert_success_ref(who, "{ 'execute': 'query-migrate' }");
}
QDict *migrate_query_not_failed(QTestState *who)
@@ -209,7 +161,8 @@ void wait_for_migration_fail(QTestState *from, bool allow_active)
} while (!failed);
/* Is the machine currently running? */
- rsp_return = wait_command(from, "{ 'execute': 'query-status' }");
+ rsp_return = qtest_qmp_assert_success_ref(from,
+ "{ 'execute': 'query-status' }");
g_assert(qdict_haskey(rsp_return, "running"));
g_assert(qdict_get_bool(rsp_return, "running"));
qobject_unref(rsp_return);
diff --git a/tests/qtest/migration-helpers.h b/tests/qtest/migration-helpers.h
index fa69d1780a..aab0745cfe 100644
--- a/tests/qtest/migration-helpers.h
+++ b/tests/qtest/migration-helpers.h
@@ -18,14 +18,6 @@
bool migrate_watch_for_stop(QTestState *who, const char *name,
QDict *event, void *opaque);
-#ifndef _WIN32
-G_GNUC_PRINTF(3, 4)
-QDict *wait_command_fd(QTestState *who, int fd, const char *command, ...);
-#endif
-
-G_GNUC_PRINTF(2, 3)
-QDict *wait_command(QTestState *who, const char *command, ...);
-
G_GNUC_PRINTF(3, 4)
void migrate_qmp(QTestState *who, const char *uri, const char *fmt, ...);
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 0af72c37c2..822cf13536 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -342,7 +342,8 @@ static long long migrate_get_parameter_int(QTestState *who,
QDict *rsp;
long long result;
- rsp = wait_command(who, "{ 'execute': 'query-migrate-parameters' }");
+ rsp = qtest_qmp_assert_success_ref(
+ who, "{ 'execute': 'query-migrate-parameters' }");
result = qdict_get_int(rsp, parameter);
qobject_unref(rsp);
return result;
@@ -373,7 +374,8 @@ static char *migrate_get_parameter_str(QTestState *who,
QDict *rsp;
char *result;
- rsp = wait_command(who, "{ 'execute': 'query-migrate-parameters' }");
+ rsp = qtest_qmp_assert_success_ref(
+ who, "{ 'execute': 'query-migrate-parameters' }");
result = g_strdup(qdict_get_str(rsp, parameter));
qobject_unref(rsp);
return result;
@@ -402,7 +404,8 @@ static long long migrate_get_parameter_bool(QTestState *who,
QDict *rsp;
int result;
- rsp = wait_command(who, "{ 'execute': 'query-migrate-parameters' }");
+ rsp = qtest_qmp_assert_success_ref(
+ who, "{ 'execute': 'query-migrate-parameters' }");
result = qdict_get_bool(rsp, parameter);
qobject_unref(rsp);
return !!result;
@@ -443,41 +446,29 @@ static void migrate_ensure_converge(QTestState *who)
static void migrate_pause(QTestState *who)
{
- QDict *rsp;
-
- rsp = wait_command(who, "{ 'execute': 'migrate-pause' }");
- qobject_unref(rsp);
+ qtest_qmp_assert_success(who, "{ 'execute': 'migrate-pause' }");
}
static void migrate_continue(QTestState *who, const char *state)
{
- QDict *rsp;
-
- rsp = wait_command(who,
- "{ 'execute': 'migrate-continue',"
- " 'arguments': { 'state': %s } }",
- state);
- qobject_unref(rsp);
+ qtest_qmp_assert_success(who,
+ "{ 'execute': 'migrate-continue',"
+ " 'arguments': { 'state': %s } }",
+ state);
}
static void migrate_recover(QTestState *who, const char *uri)
{
- QDict *rsp;
-
- rsp = wait_command(who,
- "{ 'execute': 'migrate-recover', "
- " 'id': 'recover-cmd', "
- " 'arguments': { 'uri': %s } }",
- uri);
- qobject_unref(rsp);
+ qtest_qmp_assert_success(who,
+ "{ 'execute': 'migrate-recover', "
+ " 'id': 'recover-cmd', "
+ " 'arguments': { 'uri': %s } }",
+ uri);
}
static void migrate_cancel(QTestState *who)
{
- QDict *rsp;
-
- rsp = wait_command(who, "{ 'execute': 'migrate_cancel' }");
- qobject_unref(rsp);
+ qtest_qmp_assert_success(who, "{ 'execute': 'migrate_cancel' }");
}
static void migrate_set_capability(QTestState *who, const char *capability,
@@ -493,10 +484,7 @@ static void migrate_set_capability(QTestState *who, const char *capability,
static void migrate_postcopy_start(QTestState *from, QTestState *to)
{
- QDict *rsp;
-
- rsp = wait_command(from, "{ 'execute': 'migrate-start-postcopy' }");
- qobject_unref(rsp);
+ qtest_qmp_assert_success(from, "{ 'execute': 'migrate-start-postcopy' }");
if (!got_stop) {
qtest_qmp_eventwait(from, "STOP");
@@ -785,7 +773,6 @@ test_migrate_tls_psk_start_common(QTestState *from,
{
struct TestMigrateTLSPSKData *data =
g_new0(struct TestMigrateTLSPSKData, 1);
- QDict *rsp;
data->workdir = g_strdup_printf("%s/tlscredspsk0", tmpfs);
data->pskfile = g_strdup_printf("%s/%s", data->workdir,
@@ -801,24 +788,22 @@ test_migrate_tls_psk_start_common(QTestState *from,
test_tls_psk_init_alt(data->pskfilealt);
}
- rsp = wait_command(from,
- "{ 'execute': 'object-add',"
- " 'arguments': { 'qom-type': 'tls-creds-psk',"
- " 'id': 'tlscredspsk0',"
- " 'endpoint': 'client',"
- " 'dir': %s,"
- " 'username': 'qemu'} }",
- data->workdir);
- qobject_unref(rsp);
+ qtest_qmp_assert_success(from,
+ "{ 'execute': 'object-add',"
+ " 'arguments': { 'qom-type': 'tls-creds-psk',"
+ " 'id': 'tlscredspsk0',"
+ " 'endpoint': 'client',"
+ " 'dir': %s,"
+ " 'username': 'qemu'} }",
+ data->workdir);
- rsp = wait_command(to,
- "{ 'execute': 'object-add',"
- " 'arguments': { 'qom-type': 'tls-creds-psk',"
- " 'id': 'tlscredspsk0',"
- " 'endpoint': 'server',"
- " 'dir': %s } }",
- mismatch ? data->workdiralt : data->workdir);
- qobject_unref(rsp);
+ qtest_qmp_assert_success(to,
+ "{ 'execute': 'object-add',"
+ " 'arguments': { 'qom-type': 'tls-creds-psk',"
+ " 'id': 'tlscredspsk0',"
+ " 'endpoint': 'server',"
+ " 'dir': %s } }",
+ mismatch ? data->workdiralt : data->workdir);
migrate_set_parameter_str(from, "tls-creds", "tlscredspsk0");
migrate_set_parameter_str(to, "tls-creds", "tlscredspsk0");
@@ -889,7 +874,6 @@ test_migrate_tls_x509_start_common(QTestState *from,
TestMigrateTLSX509 *args)
{
TestMigrateTLSX509Data *data = g_new0(TestMigrateTLSX509Data, 1);
- QDict *rsp;
data->workdir = g_strdup_printf("%s/tlscredsx5090", tmpfs);
data->keyfile = g_strdup_printf("%s/key.pem", data->workdir);
@@ -932,40 +916,38 @@ test_migrate_tls_x509_start_common(QTestState *from,
args->certhostname,
args->certipaddr);
- rsp = wait_command(from,
- "{ 'execute': 'object-add',"
- " 'arguments': { 'qom-type': 'tls-creds-x509',"
- " 'id': 'tlscredsx509client0',"
- " 'endpoint': 'client',"
- " 'dir': %s,"
- " 'sanity-check': true,"
- " 'verify-peer': true} }",
- data->workdir);
- qobject_unref(rsp);
+ qtest_qmp_assert_success(from,
+ "{ 'execute': 'object-add',"
+ " 'arguments': { 'qom-type': 'tls-creds-x509',"
+ " 'id': 'tlscredsx509client0',"
+ " 'endpoint': 'client',"
+ " 'dir': %s,"
+ " 'sanity-check': true,"
+ " 'verify-peer': true} }",
+ data->workdir);
migrate_set_parameter_str(from, "tls-creds", "tlscredsx509client0");
if (args->certhostname) {
migrate_set_parameter_str(from, "tls-hostname", args->certhostname);
}
- rsp = wait_command(to,
- "{ 'execute': 'object-add',"
- " 'arguments': { 'qom-type': 'tls-creds-x509',"
- " 'id': 'tlscredsx509server0',"
- " 'endpoint': 'server',"
- " 'dir': %s,"
- " 'sanity-check': true,"
- " 'verify-peer': %i} }",
- data->workdir, args->verifyclient);
- qobject_unref(rsp);
+ qtest_qmp_assert_success(to,
+ "{ 'execute': 'object-add',"
+ " 'arguments': { 'qom-type': 'tls-creds-x509',"
+ " 'id': 'tlscredsx509server0',"
+ " 'endpoint': 'server',"
+ " 'dir': %s,"
+ " 'sanity-check': true,"
+ " 'verify-peer': %i} }",
+ data->workdir, args->verifyclient);
migrate_set_parameter_str(to, "tls-creds", "tlscredsx509server0");
if (args->authzclient) {
- rsp = wait_command(to,
- "{ 'execute': 'object-add',"
- " 'arguments': { 'qom-type': 'authz-simple',"
- " 'id': 'tlsauthz0',"
- " 'identity': %s} }",
- "CN=" QCRYPTO_TLS_TEST_CLIENT_NAME);
+ qtest_qmp_assert_success(to,
+ "{ 'execute': 'object-add',"
+ " 'arguments': { 'qom-type': 'authz-simple',"
+ " 'id': 'tlsauthz0',"
+ " 'identity': %s} }",
+ "CN=" QCRYPTO_TLS_TEST_CLIENT_NAME);
migrate_set_parameter_str(to, "tls-authz", "tlsauthz0");
}
@@ -1759,7 +1741,6 @@ static void test_precopy_tcp_tls_x509_reject_anon_client(void)
static void *test_migrate_fd_start_hook(QTestState *from,
QTestState *to)
{
- QDict *rsp;
int ret;
int pair[2];
@@ -1768,22 +1749,19 @@ static void *test_migrate_fd_start_hook(QTestState *from,
g_assert_cmpint(ret, ==, 0);
/* Send the 1st socket to the target */
- rsp = wait_command_fd(to, pair[0],
- "{ 'execute': 'getfd',"
- " 'arguments': { 'fdname': 'fd-mig' }}");
- qobject_unref(rsp);
+ qtest_qmp_fds_assert_success(to, &pair[0], 1,
+ "{ 'execute': 'getfd',"
+ " 'arguments': { 'fdname': 'fd-mig' }}");
close(pair[0]);
/* Start incoming migration from the 1st socket */
- rsp = wait_command(to, "{ 'execute': 'migrate-incoming',"
- " 'arguments': { 'uri': 'fd:fd-mig' }}");
- qobject_unref(rsp);
+ qtest_qmp_assert_success(to, "{ 'execute': 'migrate-incoming',"
+ " 'arguments': { 'uri': 'fd:fd-mig' }}");
/* Send the 2nd socket to the target */
- rsp = wait_command_fd(from, pair[1],
- "{ 'execute': 'getfd',"
- " 'arguments': { 'fdname': 'fd-mig' }}");
- qobject_unref(rsp);
+ qtest_qmp_fds_assert_success(from, &pair[1], 1,
+ "{ 'execute': 'getfd',"
+ " 'arguments': { 'fdname': 'fd-mig' }}");
close(pair[1]);
return NULL;
@@ -1990,8 +1968,6 @@ test_migrate_precopy_tcp_multifd_start_common(QTestState *from,
QTestState *to,
const char *method)
{
- QDict *rsp;
-
migrate_set_parameter_int(from, "multifd-channels", 16);
migrate_set_parameter_int(to, "multifd-channels", 16);
@@ -2002,9 +1978,8 @@ test_migrate_precopy_tcp_multifd_start_common(QTestState *from,
migrate_set_capability(to, "multifd", true);
/* Start incoming migration from the 1st socket */
- rsp = wait_command(to, "{ 'execute': 'migrate-incoming',"
- " 'arguments': { 'uri': 'tcp:127.0.0.1:0' }}");
- qobject_unref(rsp);
+ qtest_qmp_assert_success(to, "{ 'execute': 'migrate-incoming',"
+ " 'arguments': { 'uri': 'tcp:127.0.0.1:0' }}");
return NULL;
}
@@ -2235,7 +2210,6 @@ static void test_multifd_tcp_cancel(void)
.hide_stderr = true,
};
QTestState *from, *to, *to2;
- QDict *rsp;
g_autofree char *uri = NULL;
if (test_migrate_start(&from, &to, "defer", &args)) {
@@ -2251,9 +2225,8 @@ static void test_multifd_tcp_cancel(void)
migrate_set_capability(to, "multifd", true);
/* Start incoming migration from the 1st socket */
- rsp = wait_command(to, "{ 'execute': 'migrate-incoming',"
- " 'arguments': { 'uri': 'tcp:127.0.0.1:0' }}");
- qobject_unref(rsp);
+ qtest_qmp_assert_success(to, "{ 'execute': 'migrate-incoming',"
+ " 'arguments': { 'uri': 'tcp:127.0.0.1:0' }}");
/* Wait for the first serial output from the source */
wait_for_serial("src_serial");
@@ -2283,9 +2256,8 @@ static void test_multifd_tcp_cancel(void)
migrate_set_capability(to2, "multifd", true);
/* Start incoming migration from the 1st socket */
- rsp = wait_command(to2, "{ 'execute': 'migrate-incoming',"
- " 'arguments': { 'uri': 'tcp:127.0.0.1:0' }}");
- qobject_unref(rsp);
+ qtest_qmp_assert_success(to2, "{ 'execute': 'migrate-incoming',"
+ " 'arguments': { 'uri': 'tcp:127.0.0.1:0' }}");
g_free(uri);
uri = migrate_get_socket_address(to2, "socket-address");
--
2.40.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v4 07/10] tests/qtest: capture RESUME events during migration
2023-06-01 16:13 [PATCH v4 00/10] tests/qtest: make migration-test massively faster Daniel P. Berrangé
` (5 preceding siblings ...)
2023-06-01 16:13 ` [PATCH v4 06/10] tests/qtest: replace wait_command() with qtest_qmp_assert_success Daniel P. Berrangé
@ 2023-06-01 16:13 ` Daniel P. Berrangé
2023-06-02 7:45 ` Juan Quintela
2023-06-01 16:13 ` [PATCH v4 08/10] tests/qtest: distinguish src/dst migration VM stop/resume events Daniel P. Berrangé
` (4 subsequent siblings)
11 siblings, 1 reply; 22+ messages in thread
From: Daniel P. Berrangé @ 2023-06-01 16:13 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Juan Quintela, Peter Xu, Laurent Vivier,
Paolo Bonzini, Leonardo Bras, Daniel P. Berrangé
When running migration tests we monitor for a STOP event so we can skip
redundant waits. This will be needed for the RESUME event too shortly.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/qtest/migration-helpers.c | 13 +++++++++++++
tests/qtest/migration-helpers.h | 2 ++
tests/qtest/migration-test.c | 5 +++++
3 files changed, 20 insertions(+)
diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
index 73e506a5f8..be00c52d00 100644
--- a/tests/qtest/migration-helpers.c
+++ b/tests/qtest/migration-helpers.c
@@ -36,6 +36,19 @@ bool migrate_watch_for_stop(QTestState *who, const char *name,
return false;
}
+bool migrate_watch_for_resume(QTestState *who, const char *name,
+ QDict *event, void *opaque)
+{
+ bool *seen = opaque;
+
+ if (g_str_equal(name, "RESUME")) {
+ *seen = true;
+ return true;
+ }
+
+ return false;
+}
+
/*
* Send QMP command "migrate".
* Arguments are built from @fmt... (formatted like
diff --git a/tests/qtest/migration-helpers.h b/tests/qtest/migration-helpers.h
index aab0745cfe..009e250e90 100644
--- a/tests/qtest/migration-helpers.h
+++ b/tests/qtest/migration-helpers.h
@@ -17,6 +17,8 @@
bool migrate_watch_for_stop(QTestState *who, const char *name,
QDict *event, void *opaque);
+bool migrate_watch_for_resume(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, ...);
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 822cf13536..0948d13e14 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -44,6 +44,7 @@ unsigned start_address;
unsigned end_address;
static bool uffd_feature_thread_id;
static bool got_stop;
+static bool got_resume;
/*
* Dirtylimit stop working if dirty page rate error
@@ -607,6 +608,7 @@ static int test_migrate_start(QTestState **from, QTestState **to,
}
got_stop = false;
+ got_resume = false;
bootpath = g_strdup_printf("%s/bootsect", tmpfs);
if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
/* the assembled x86 boot sector should be exactly one sector large */
@@ -712,6 +714,9 @@ static int test_migrate_start(QTestState **from, QTestState **to,
args->opts_target ? args->opts_target : "",
ignore_stderr);
*to = qtest_init(cmd_target);
+ qtest_qmp_set_event_callback(*to,
+ migrate_watch_for_resume,
+ &got_resume);
/*
* Remove shmem file immediately to avoid memory leak in test failed case.
--
2.40.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v4 08/10] tests/qtest: distinguish src/dst migration VM stop/resume events
2023-06-01 16:13 [PATCH v4 00/10] tests/qtest: make migration-test massively faster Daniel P. Berrangé
` (6 preceding siblings ...)
2023-06-01 16:13 ` [PATCH v4 07/10] tests/qtest: capture RESUME events during migration Daniel P. Berrangé
@ 2023-06-01 16:13 ` Daniel P. Berrangé
2023-06-01 22:47 ` Juan Quintela
2023-06-01 16:13 ` [PATCH v4 09/10] tests/qtest: make more migration pre-copy scenarios run non-live Daniel P. Berrangé
` (3 subsequent siblings)
11 siblings, 1 reply; 22+ messages in thread
From: Daniel P. Berrangé @ 2023-06-01 16:13 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Juan Quintela, Peter Xu, Laurent Vivier,
Paolo Bonzini, Leonardo Bras, Daniel P. Berrangé
The 'got_stop' and 'got_resume' global variables apply to the src and
dst migration VM respectively. Change their names to make this explicit
to developers.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/qtest/migration-test.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 0948d13e14..23fb61506c 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -43,8 +43,8 @@
unsigned start_address;
unsigned end_address;
static bool uffd_feature_thread_id;
-static bool got_stop;
-static bool got_resume;
+static bool got_src_stop;
+static bool got_dst_resume;
/*
* Dirtylimit stop working if dirty page rate error
@@ -227,7 +227,7 @@ static void wait_for_migration_pass(QTestState *who)
uint64_t pass;
/* Wait for the 1st sync */
- while (!got_stop && !initial_pass) {
+ while (!got_src_stop && !initial_pass) {
usleep(1000);
initial_pass = get_migration_pass(who);
}
@@ -235,7 +235,7 @@ static void wait_for_migration_pass(QTestState *who)
do {
usleep(1000);
pass = get_migration_pass(who);
- } while (pass == initial_pass && !got_stop);
+ } while (pass == initial_pass && !got_src_stop);
}
static void check_guests_ram(QTestState *who)
@@ -487,7 +487,7 @@ static void migrate_postcopy_start(QTestState *from, QTestState *to)
{
qtest_qmp_assert_success(from, "{ 'execute': 'migrate-start-postcopy' }");
- if (!got_stop) {
+ if (!got_src_stop) {
qtest_qmp_eventwait(from, "STOP");
}
@@ -607,8 +607,8 @@ static int test_migrate_start(QTestState **from, QTestState **to,
}
}
- got_stop = false;
- got_resume = false;
+ got_src_stop = false;
+ got_dst_resume = false;
bootpath = g_strdup_printf("%s/bootsect", tmpfs);
if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
/* the assembled x86 boot sector should be exactly one sector large */
@@ -696,7 +696,7 @@ static int test_migrate_start(QTestState **from, QTestState **to,
*from = qtest_init(cmd_source);
qtest_qmp_set_event_callback(*from,
migrate_watch_for_stop,
- &got_stop);
+ &got_src_stop);
}
cmd_target = g_strdup_printf("-accel kvm%s -accel tcg%s%s "
@@ -716,7 +716,7 @@ static int test_migrate_start(QTestState **from, QTestState **to,
*to = qtest_init(cmd_target);
qtest_qmp_set_event_callback(*to,
migrate_watch_for_resume,
- &got_resume);
+ &got_dst_resume);
/*
* Remove shmem file immediately to avoid memory leak in test failed case.
@@ -1427,7 +1427,7 @@ static void test_precopy_common(MigrateCommon *args)
* hanging forever if migration didn't converge */
wait_for_migration_complete(from);
- if (!got_stop) {
+ if (!got_src_stop) {
qtest_qmp_eventwait(from, "STOP");
}
@@ -1537,7 +1537,7 @@ static void test_ignore_shared(void)
wait_for_migration_pass(from);
- if (!got_stop) {
+ if (!got_src_stop) {
qtest_qmp_eventwait(from, "STOP");
}
@@ -1942,7 +1942,7 @@ static void test_migrate_auto_converge(void)
break;
}
usleep(20);
- g_assert_false(got_stop);
+ g_assert_false(got_src_stop);
} while (true);
/* The first percentage of throttling should be at least init_pct */
g_assert_cmpint(percentage, >=, init_pct);
@@ -2275,7 +2275,7 @@ static void test_multifd_tcp_cancel(void)
wait_for_migration_pass(from);
- if (!got_stop) {
+ if (!got_src_stop) {
qtest_qmp_eventwait(from, "STOP");
}
qtest_qmp_eventwait(to2, "RESUME");
--
2.40.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v4 09/10] tests/qtest: make more migration pre-copy scenarios run non-live
2023-06-01 16:13 [PATCH v4 00/10] tests/qtest: make migration-test massively faster Daniel P. Berrangé
` (7 preceding siblings ...)
2023-06-01 16:13 ` [PATCH v4 08/10] tests/qtest: distinguish src/dst migration VM stop/resume events Daniel P. Berrangé
@ 2023-06-01 16:13 ` Daniel P. Berrangé
2023-06-01 22:50 ` Juan Quintela
2023-06-01 16:13 ` [PATCH v4 10/10] tests/qtest: massively speed up migration-test Daniel P. Berrangé
` (2 subsequent siblings)
11 siblings, 1 reply; 22+ messages in thread
From: Daniel P. Berrangé @ 2023-06-01 16:13 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Juan Quintela, Peter Xu, Laurent Vivier,
Paolo Bonzini, Leonardo Bras, Daniel P. Berrangé
There are 27 pre-copy live migration scenarios being tested. In all of
these we force non-convergence and run for one iteration, then let it
converge and wait for completion during the second (or following)
iterations. At 3 mbps bandwidth limit the first iteration takes a very
long time (~30 seconds).
While it is important to test the migration passes and convergence
logic, it is overkill to do this for all 27 pre-copy scenarios. The
TLS migration scenarios in particular are merely exercising different
code paths during connection establishment.
To optimize time taken, switch most of the test scenarios to run
non-live (ie guest CPUs paused) with no bandwidth limits. This gives
a massive speed up for most of the test scenarios.
For test coverage the following scenarios are unchanged
* Precopy with UNIX sockets
* Precopy with UNIX sockets and dirty ring tracking
* Precopy with XBZRLE
* Precopy with UNIX compress
* Precopy with UNIX compress (nowait)
* Precopy with multifd
On a test machine this reduces execution time from 13 minutes to
8 minutes.
Tested-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/qtest/migration-test.c | 81 +++++++++++++++++++++++++++++-------
1 file changed, 66 insertions(+), 15 deletions(-)
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 23fb61506c..0b9d045152 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -577,9 +577,12 @@ typedef struct {
MIG_TEST_FAIL_DEST_QUIT_ERR,
} result;
- /* Optional: set number of migration passes to wait for */
+ /* Optional: set number of migration passes to wait for, if live==true */
unsigned int iterations;
+ /* Optional: whether the guest CPUs should be running during migration */
+ bool live;
+
/* Postcopy specific fields */
void *postcopy_data;
bool postcopy_preempt;
@@ -1385,8 +1388,6 @@ static void test_precopy_common(MigrateCommon *args)
return;
}
- migrate_ensure_non_converge(from);
-
if (args->start_hook) {
data_hook = args->start_hook(from, to);
}
@@ -1396,6 +1397,31 @@ static void test_precopy_common(MigrateCommon *args)
wait_for_serial("src_serial");
}
+ if (args->live) {
+ /*
+ * Testing live migration, we want to ensure that some
+ * memory is re-dirtied after being transferred, so that
+ * we exercise logic for dirty page handling. We achieve
+ * this with a ridiculosly low bandwidth that guarantees
+ * non-convergance.
+ */
+ migrate_ensure_non_converge(from);
+ } else {
+ /*
+ * Testing non-live migration, we allow it to run at
+ * full speed to ensure short test case duration.
+ * For tests expected to fail, we don't need to
+ * change anything.
+ */
+ if (args->result == MIG_TEST_SUCCEED) {
+ qtest_qmp_assert_success(from, "{ 'execute' : 'stop'}");
+ if (!got_src_stop) {
+ qtest_qmp_eventwait(from, "STOP");
+ }
+ migrate_ensure_converge(from);
+ }
+ }
+
if (!args->connect_uri) {
g_autofree char *local_connect_uri =
migrate_get_socket_address(to, "socket-address");
@@ -1413,25 +1439,41 @@ static void test_precopy_common(MigrateCommon *args)
qtest_set_expected_status(to, EXIT_FAILURE);
}
} else {
- if (args->iterations) {
- while (args->iterations--) {
+ if (args->live) {
+ if (args->iterations) {
+ while (args->iterations--) {
+ wait_for_migration_pass(from);
+ }
+ } else {
wait_for_migration_pass(from);
}
- } else {
- wait_for_migration_pass(from);
- }
- migrate_ensure_converge(from);
+ migrate_ensure_converge(from);
- /* We do this first, as it has a timeout to stop us
- * hanging forever if migration didn't converge */
- wait_for_migration_complete(from);
+ /*
+ * We do this first, as it has a timeout to stop us
+ * hanging forever if migration didn't converge
+ */
+ wait_for_migration_complete(from);
- if (!got_src_stop) {
- qtest_qmp_eventwait(from, "STOP");
+ if (!got_src_stop) {
+ qtest_qmp_eventwait(from, "STOP");
+ }
+ } else {
+ wait_for_migration_complete(from);
+ /*
+ * Must wait for dst to finish reading all incoming
+ * data on the socket before issuing 'cont' otherwise
+ * it'll be ignored
+ */
+ wait_for_migration_complete(to);
+
+ qtest_qmp_assert_success(to, "{ 'execute' : 'cont'}");
}
- qtest_qmp_eventwait(to, "RESUME");
+ if (!got_dst_resume) {
+ qtest_qmp_eventwait(to, "RESUME");
+ }
wait_for_serial("dest_serial");
}
@@ -1449,6 +1491,8 @@ static void test_precopy_unix_plain(void)
MigrateCommon args = {
.listen_uri = uri,
.connect_uri = uri,
+
+ .live = true,
};
test_precopy_common(&args);
@@ -1464,6 +1508,8 @@ static void test_precopy_unix_dirty_ring(void)
},
.listen_uri = uri,
.connect_uri = uri,
+
+ .live = true,
};
test_precopy_common(&args);
@@ -1575,6 +1621,7 @@ static void test_precopy_unix_xbzrle(void)
.start_hook = test_migrate_xbzrle_start,
.iterations = 2,
+ .live = true,
};
test_precopy_common(&args);
@@ -1592,6 +1639,7 @@ static void test_precopy_unix_compress(void)
* the previous iteration.
*/
.iterations = 2,
+ .live = true,
};
test_precopy_common(&args);
@@ -1609,6 +1657,7 @@ static void test_precopy_unix_compress_nowait(void)
* the previous iteration.
*/
.iterations = 2,
+ .live = true,
};
test_precopy_common(&args);
@@ -2017,6 +2066,8 @@ static void test_multifd_tcp_none(void)
MigrateCommon args = {
.listen_uri = "defer",
.start_hook = test_migrate_precopy_tcp_multifd_start,
+
+ .live = true,
};
test_precopy_common(&args);
}
--
2.40.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v4 09/10] tests/qtest: make more migration pre-copy scenarios run non-live
2023-06-01 16:13 ` [PATCH v4 09/10] tests/qtest: make more migration pre-copy scenarios run non-live Daniel P. Berrangé
@ 2023-06-01 22:50 ` Juan Quintela
0 siblings, 0 replies; 22+ messages in thread
From: Juan Quintela @ 2023-06-01 22:50 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: qemu-devel, Thomas Huth, Peter Xu, Laurent Vivier, Paolo Bonzini,
Leonardo Bras
Daniel P. Berrangé <berrange@redhat.com> wrote:
> There are 27 pre-copy live migration scenarios being tested. In all of
> these we force non-convergence and run for one iteration, then let it
> converge and wait for completion during the second (or following)
> iterations. At 3 mbps bandwidth limit the first iteration takes a very
> long time (~30 seconds).
>
> While it is important to test the migration passes and convergence
> logic, it is overkill to do this for all 27 pre-copy scenarios. The
> TLS migration scenarios in particular are merely exercising different
> code paths during connection establishment.
>
> To optimize time taken, switch most of the test scenarios to run
> non-live (ie guest CPUs paused) with no bandwidth limits. This gives
> a massive speed up for most of the test scenarios.
>
> For test coverage the following scenarios are unchanged
>
> * Precopy with UNIX sockets
> * Precopy with UNIX sockets and dirty ring tracking
> * Precopy with XBZRLE
> * Precopy with UNIX compress
> * Precopy with UNIX compress (nowait)
> * Precopy with multifd
>
> On a test machine this reduces execution time from 13 minutes to
> 8 minutes.
>
> Tested-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v4 10/10] tests/qtest: massively speed up migration-test
2023-06-01 16:13 [PATCH v4 00/10] tests/qtest: make migration-test massively faster Daniel P. Berrangé
` (8 preceding siblings ...)
2023-06-01 16:13 ` [PATCH v4 09/10] tests/qtest: make more migration pre-copy scenarios run non-live Daniel P. Berrangé
@ 2023-06-01 16:13 ` Daniel P. Berrangé
2023-06-01 17:29 ` [PATCH] qtest/migration: Document live=true cases Peter Xu
2023-07-03 16:37 ` [PATCH v4 00/10] tests/qtest: make migration-test massively faster Thomas Huth
11 siblings, 0 replies; 22+ messages in thread
From: Daniel P. Berrangé @ 2023-06-01 16:13 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Juan Quintela, Peter Xu, Laurent Vivier,
Paolo Bonzini, Leonardo Bras, Daniel P. Berrangé
The migration test cases that actually exercise live migration want to
ensure there is a minimum of two iterations of pre-copy, in order to
exercise the dirty tracking code.
Historically we've queried the migration status, looking for the
'dirty-sync-count' value to increment to track iterations. This was
not entirely reliable because often all the data would get transferred
quickly enough that the migration would finish before we wanted it
to. So we massively dropped the bandwidth and max downtime to
guarantee non-convergance. This had the unfortunate side effect
that every migration took at least 30 seconds to run (100 MB of
dirty pages / 3 MB/sec).
This optimization takes a different approach to ensuring that a
mimimum of two iterations. Rather than waiting for dirty-sync-count
to increment, directly look for an indication that the source VM
has dirtied RAM that has already been transferred.
On the source VM a magic marker is written just after the 3 MB
offset. The destination VM is now montiored to detect when the
magic marker is transferred. This gives a guarantee that the
first 3 MB of memory have been transferred. Now the source VM
memory is monitored at exactly the 3MB offset until we observe
a flip in its value. This gives us a guaranteed that the guest
workload has dirtied a byte that has already been transferred.
Since we're looking at a place that is only 3 MB from the start
of memory, with the 3 MB/sec bandwidth, this test should complete
in 1 second, instead of 30 seconds.
Once we've proved there is some dirty memory, migration can be
set back to full speed for the remainder of the 1st iteration,
and the entire of the second iteration at which point migration
should be complete.
On a test machine this further reduces the migration test time
from 8 minutes to 1 minute 40.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/qtest/migration-test.c | 143 ++++++++++++++++++++++++++++++-----
1 file changed, 125 insertions(+), 18 deletions(-)
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 0b9d045152..298291f01c 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -46,6 +46,20 @@ static bool uffd_feature_thread_id;
static bool got_src_stop;
static bool got_dst_resume;
+/*
+ * An initial 3 MB offset is used as that corresponds
+ * to ~1 sec of data transfer with our bandwidth setting.
+ */
+#define MAGIC_OFFSET_BASE (3 * 1024 * 1024)
+/*
+ * A further 1k is added to ensure we're not a multiple
+ * of TEST_MEM_PAGE_SIZE, thus avoid clash with writes
+ * from the migration guest workload.
+ */
+#define MAGIC_OFFSET_SHUFFLE 1024
+#define MAGIC_OFFSET (MAGIC_OFFSET_BASE + MAGIC_OFFSET_SHUFFLE)
+#define MAGIC_MARKER 0xFEED12345678CAFEULL
+
/*
* Dirtylimit stop working if dirty page rate error
* value less than DIRTYLIMIT_TOLERANCE_RANGE
@@ -445,6 +459,91 @@ static void migrate_ensure_converge(QTestState *who)
migrate_set_parameter_int(who, "downtime-limit", 30 * 1000);
}
+/*
+ * Our goal is to ensure that we run a single full migration
+ * iteration, and also dirty memory, ensuring that at least
+ * one further iteration is required.
+ *
+ * We can't directly synchronize with the start of a migration
+ * so we have to apply some tricks monitoring memory that is
+ * transferred.
+ *
+ * Initially we set the migration bandwidth to an insanely
+ * low value, with tiny max downtime too. This basically
+ * guarantees migration will never complete.
+ *
+ * This will result in a test that is unacceptably slow though,
+ * so we can't let the entire migration pass run at this speed.
+ * Our intent is to let it run just long enough that we can
+ * prove data prior to the marker has been transferred *AND*
+ * also prove this transferred data is dirty again.
+ *
+ * Before migration starts, we write a 64-bit magic marker
+ * into a fixed location in the src VM RAM.
+ *
+ * Then watch dst memory until the marker appears. This is
+ * proof that start_address -> MAGIC_OFFSET_BASE has been
+ * transferred.
+ *
+ * Finally we go back to the source and read a byte just
+ * before the marker untill we see it flip in value. This
+ * is proof that start_address -> MAGIC_OFFSET_BASE
+ * is now dirty again.
+ *
+ * IOW, we're guaranteed at least a 2nd migration pass
+ * at this point.
+ *
+ * We can now let migration run at full speed to finish
+ * the test
+ */
+static void migrate_prepare_for_dirty_mem(QTestState *from)
+{
+ /*
+ * The guest workflow iterates from start_address to
+ * end_address, writing 1 byte every TEST_MEM_PAGE_SIZE
+ * bytes.
+ *
+ * IOW, if we write to mem at a point which is NOT
+ * a multiple of TEST_MEM_PAGE_SIZE, our write won't
+ * conflict with the migration workflow.
+ *
+ * We put in a marker here, that we'll use to determine
+ * when the data has been transferred to the dst.
+ */
+ qtest_writeq(from, start_address + MAGIC_OFFSET, MAGIC_MARKER);
+}
+
+static void migrate_wait_for_dirty_mem(QTestState *from,
+ QTestState *to)
+{
+ uint64_t watch_address = start_address + MAGIC_OFFSET_BASE;
+ uint64_t marker_address = start_address + MAGIC_OFFSET;
+ uint8_t watch_byte;
+
+ /*
+ * Wait for the MAGIC_MARKER to get transferred, as an
+ * indicator that a migration pass has made some known
+ * amount of progress.
+ */
+ do {
+ usleep(1000 * 10);
+ } while (qtest_readq(to, marker_address) != MAGIC_MARKER);
+
+ /*
+ * Now ensure that already transferred bytes are
+ * dirty again from the guest workload. Note the
+ * guest byte value will wrap around and by chance
+ * match the original watch_byte. This is harmless
+ * as we'll eventually see a different value if we
+ * keep watching
+ */
+ watch_byte = qtest_readb(from, watch_address);
+ do {
+ usleep(1000 * 10);
+ } while (qtest_readb(from, watch_address) == watch_byte);
+}
+
+
static void migrate_pause(QTestState *who)
{
qtest_qmp_assert_success(who, "{ 'execute': 'migrate-pause' }");
@@ -577,7 +676,10 @@ typedef struct {
MIG_TEST_FAIL_DEST_QUIT_ERR,
} result;
- /* Optional: set number of migration passes to wait for, if live==true */
+ /*
+ * Optional: set number of migration passes to wait for, if live==true.
+ * If zero, then merely wait for a few MB of dirty data
+ */
unsigned int iterations;
/* Optional: whether the guest CPUs should be running during migration */
@@ -1158,12 +1260,14 @@ static int migrate_postcopy_prepare(QTestState **from_ptr,
migrate_ensure_non_converge(from);
+ migrate_prepare_for_dirty_mem(from);
+
/* Wait for the first serial output from the source */
wait_for_serial("src_serial");
migrate_qmp(from, uri, "{}");
- wait_for_migration_pass(from);
+ migrate_wait_for_dirty_mem(from, to);
*from_ptr = from;
*to_ptr = to;
@@ -1398,14 +1502,8 @@ static void test_precopy_common(MigrateCommon *args)
}
if (args->live) {
- /*
- * Testing live migration, we want to ensure that some
- * memory is re-dirtied after being transferred, so that
- * we exercise logic for dirty page handling. We achieve
- * this with a ridiculosly low bandwidth that guarantees
- * non-convergance.
- */
migrate_ensure_non_converge(from);
+ migrate_prepare_for_dirty_mem(from);
} else {
/*
* Testing non-live migration, we allow it to run at
@@ -1440,13 +1538,16 @@ static void test_precopy_common(MigrateCommon *args)
}
} else {
if (args->live) {
- if (args->iterations) {
- while (args->iterations--) {
- wait_for_migration_pass(from);
- }
- } else {
+ /*
+ * For initial iteration(s) we must do a full pass,
+ * but for the final iteration, we need only wait
+ * for some dirty mem before switching to converge
+ */
+ while (args->iterations > 1) {
wait_for_migration_pass(from);
+ args->iterations--;
}
+ migrate_wait_for_dirty_mem(from, to);
migrate_ensure_converge(from);
@@ -1573,6 +1674,9 @@ static void test_ignore_shared(void)
return;
}
+ migrate_ensure_non_converge(from);
+ migrate_prepare_for_dirty_mem(from);
+
migrate_set_capability(from, "x-ignore-shared", true);
migrate_set_capability(to, "x-ignore-shared", true);
@@ -1581,7 +1685,7 @@ static void test_ignore_shared(void)
migrate_qmp(from, uri, "{}");
- wait_for_migration_pass(from);
+ migrate_wait_for_dirty_mem(from, to);
if (!got_src_stop) {
qtest_qmp_eventwait(from, "STOP");
@@ -2273,6 +2377,7 @@ static void test_multifd_tcp_cancel(void)
}
migrate_ensure_non_converge(from);
+ migrate_prepare_for_dirty_mem(from);
migrate_set_parameter_int(from, "multifd-channels", 16);
migrate_set_parameter_int(to, "multifd-channels", 16);
@@ -2291,7 +2396,7 @@ static void test_multifd_tcp_cancel(void)
migrate_qmp(from, uri, "{}");
- wait_for_migration_pass(from);
+ migrate_wait_for_dirty_mem(from, to);
migrate_cancel(from);
@@ -2320,11 +2425,13 @@ static void test_multifd_tcp_cancel(void)
wait_for_migration_status(from, "cancelled", NULL);
- migrate_ensure_converge(from);
+ migrate_ensure_non_converge(from);
migrate_qmp(from, uri, "{}");
- wait_for_migration_pass(from);
+ migrate_wait_for_dirty_mem(from, to);
+
+ migrate_ensure_converge(from);
if (!got_src_stop) {
qtest_qmp_eventwait(from, "STOP");
--
2.40.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH] qtest/migration: Document live=true cases
2023-06-01 16:13 [PATCH v4 00/10] tests/qtest: make migration-test massively faster Daniel P. Berrangé
` (9 preceding siblings ...)
2023-06-01 16:13 ` [PATCH v4 10/10] tests/qtest: massively speed up migration-test Daniel P. Berrangé
@ 2023-06-01 17:29 ` Peter Xu
2023-06-01 18:22 ` Daniel P. Berrangé
2023-06-01 22:52 ` Juan Quintela
2023-07-03 16:37 ` [PATCH v4 00/10] tests/qtest: make migration-test massively faster Thomas Huth
11 siblings, 2 replies; 22+ messages in thread
From: Peter Xu @ 2023-06-01 17:29 UTC (permalink / raw)
To: qemu-devel, Thomas Huth, Juan Quintela, Peter Xu, Laurent Vivier,
Paolo Bonzini, Leonardo Bras, Daniel P. Berrangé
Document every single live=true use cases on why it should be done in the
live manner. Also document on the parameter so new precopy cases should
always use live=off unless with explicit reasonings.
Cc: Thomas Huth <thuth@redhat.com>
Cc: Juan Quintela <quintela@redhat.com>
Cc: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
---
tests/qtest/migration-test.c | 37 ++++++++++++++++++++++++++++++------
1 file changed, 31 insertions(+), 6 deletions(-)
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 298291f01c..d2cd71e6cf 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -682,7 +682,14 @@ typedef struct {
*/
unsigned int iterations;
- /* Optional: whether the guest CPUs should be running during migration */
+ /*
+ * Optional: whether the guest CPUs should be running during a precopy
+ * migration test. We used to always run with live but it took much
+ * longer so we reduced live tests to only the ones that have solid
+ * reason to be tested live-only. For each of the new test cases for
+ * precopy please provide justifications to use live explicitly (please
+ * refer to existing ones with live=true), or use live=off by default.
+ */
bool live;
/* Postcopy specific fields */
@@ -1592,7 +1599,10 @@ static void test_precopy_unix_plain(void)
MigrateCommon args = {
.listen_uri = uri,
.connect_uri = uri,
-
+ /*
+ * The simplest use case of precopy, covering smoke tests of
+ * get-dirty-log dirty tracking.
+ */
.live = true,
};
@@ -1609,7 +1619,10 @@ static void test_precopy_unix_dirty_ring(void)
},
.listen_uri = uri,
.connect_uri = uri,
-
+ /*
+ * Besides the precopy/unix basic test, cover dirty ring interface
+ * rather than get-dirty-log.
+ */
.live = true,
};
@@ -1721,10 +1734,12 @@ static void test_precopy_unix_xbzrle(void)
MigrateCommon args = {
.connect_uri = uri,
.listen_uri = uri,
-
.start_hook = test_migrate_xbzrle_start,
-
.iterations = 2,
+ /*
+ * XBZRLE needs pages to be modified when doing the 2nd+ round
+ * iteration to have real data pushed to the stream.
+ */
.live = true,
};
@@ -1743,6 +1758,11 @@ static void test_precopy_unix_compress(void)
* the previous iteration.
*/
.iterations = 2,
+ /*
+ * We make sure the compressor can always work well even if guest
+ * memory is changing. See commit 34ab9e9743 where we used to fix
+ * a bug when only trigger-able with guest memory changing.
+ */
.live = true,
};
@@ -1761,6 +1781,7 @@ static void test_precopy_unix_compress_nowait(void)
* the previous iteration.
*/
.iterations = 2,
+ /* Same reason for the wait version of precopy compress test */
.live = true,
};
@@ -2170,7 +2191,11 @@ static void test_multifd_tcp_none(void)
MigrateCommon args = {
.listen_uri = "defer",
.start_hook = test_migrate_precopy_tcp_multifd_start,
-
+ /*
+ * Multifd is more complicated than most of the features, it
+ * directly takes guest page buffers when sending, make sure
+ * everything will work alright even if guest page is changing.
+ */
.live = true,
};
test_precopy_common(&args);
--
2.40.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH] qtest/migration: Document live=true cases
2023-06-01 17:29 ` [PATCH] qtest/migration: Document live=true cases Peter Xu
@ 2023-06-01 18:22 ` Daniel P. Berrangé
2023-06-01 22:52 ` Juan Quintela
1 sibling, 0 replies; 22+ messages in thread
From: Daniel P. Berrangé @ 2023-06-01 18:22 UTC (permalink / raw)
To: Peter Xu
Cc: qemu-devel, Thomas Huth, Juan Quintela, Laurent Vivier,
Paolo Bonzini, Leonardo Bras
On Thu, Jun 01, 2023 at 01:29:35PM -0400, Peter Xu wrote:
> Document every single live=true use cases on why it should be done in the
> live manner. Also document on the parameter so new precopy cases should
> always use live=off unless with explicit reasonings.
>
> Cc: Thomas Huth <thuth@redhat.com>
> Cc: Juan Quintela <quintela@redhat.com>
> Cc: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
> tests/qtest/migration-test.c | 37 ++++++++++++++++++++++++++++++------
> 1 file changed, 31 insertions(+), 6 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] qtest/migration: Document live=true cases
2023-06-01 17:29 ` [PATCH] qtest/migration: Document live=true cases Peter Xu
2023-06-01 18:22 ` Daniel P. Berrangé
@ 2023-06-01 22:52 ` Juan Quintela
1 sibling, 0 replies; 22+ messages in thread
From: Juan Quintela @ 2023-06-01 22:52 UTC (permalink / raw)
To: Peter Xu
Cc: qemu-devel, Thomas Huth, Laurent Vivier, Paolo Bonzini,
Leonardo Bras, Daniel P. Berrangé
Peter Xu <peterx@redhat.com> wrote:
> Document every single live=true use cases on why it should be done in the
> live manner. Also document on the parameter so new precopy cases should
> always use live=off unless with explicit reasonings.
>
> Cc: Thomas Huth <thuth@redhat.com>
> Cc: Juan Quintela <quintela@redhat.com>
> Cc: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 00/10] tests/qtest: make migration-test massively faster
2023-06-01 16:13 [PATCH v4 00/10] tests/qtest: make migration-test massively faster Daniel P. Berrangé
` (10 preceding siblings ...)
2023-06-01 17:29 ` [PATCH] qtest/migration: Document live=true cases Peter Xu
@ 2023-07-03 16:37 ` Thomas Huth
11 siblings, 0 replies; 22+ messages in thread
From: Thomas Huth @ 2023-07-03 16:37 UTC (permalink / raw)
To: Daniel P. Berrangé, qemu-devel, Peter Xu, Juan Quintela
Cc: Laurent Vivier, Paolo Bonzini, Leonardo Bras
On 01/06/2023 18.13, Daniel P. Berrangé wrote:
> This makes migration-test faster by observing that most of the pre-copy
> tests don't need to be doing a live migration. They get sufficient code
> coverage with the guest CPUs paused.
>
> On my machine this cuts the overall execution time of migration-test
> from 13 minutes, down to 8 minutes, without sacrificing any noticeable
> code coverage.
>
> Of the tests which do still run in live mode, some need to guarantee
> a certain number of iterions. This is achieved by running the 1
> iteration with an incredibly small bandwidth and max downtime to
> prevent convergance, and watching query-migrate for the reported
> iteration to increment. This guarantees that all the tests take at
> least 30 seconds to run per iteration required.
>
> Watching for the iteration counter to flip is inefficient and not
> actually needed, except on the final iteration before starting
> convergance. On this final iteration we merely need to prove that
> some amount of already transferred data has been made dirty again.
> This in turn will guarantee that a further iteration is required
> beyond the current one. This proof is easy to achieve by monitoring
> the values at two distinct addresses in guest RAM, and can cut the
> 30 second duration down to 1 second for one of the iterations.
>
> After this this second optimization the runtime is reduced from
> 8 minutes, down to 1 minute 40 seconds, which is pretty decent given
> the amount of coverage we're getting.
It's now ~1 week until the soft freeze, and the migration test still run for
~8 minutes. This is still quite annoying. Could we please get one of the
solutions merged before the soft freeze, either Daniel's or Peter's ?
Thanks,
Thomas
^ permalink raw reply [flat|nested] 22+ messages in thread