diff mbox series

[PULL,08/30] tests: Add dirty page rate limit test

Message ID 20220720111926.107055-9-dgilbert@redhat.com (mailing list archive)
State New, archived
Headers show
Series [PULL,01/30] accel/kvm/kvm-all: Refactor per-vcpu dirty ring reaping | expand

Commit Message

Dr. David Alan Gilbert July 20, 2022, 11:19 a.m. UTC
From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>

Add dirty page rate limit test if kernel support dirty ring,

The following qmp commands are covered by this test case:
"calc-dirty-rate", "query-dirty-rate", "set-vcpu-dirty-limit",
"cancel-vcpu-dirty-limit" and "query-vcpu-dirty-limit".

Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
Acked-by: Peter Xu <peterx@redhat.com>
Message-Id: <eed5b847a6ef0a9c02a36383dbdd7db367dd1e7e.1656177590.git.huangy81@chinatelecom.cn>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 tests/qtest/migration-helpers.c |  22 +++
 tests/qtest/migration-helpers.h |   2 +
 tests/qtest/migration-test.c    | 256 ++++++++++++++++++++++++++++++++
 3 files changed, 280 insertions(+)

Comments

Thomas Huth Aug. 1, 2022, 10:41 a.m. UTC | #1
On 20/07/2022 13.19, Dr. David Alan Gilbert (git) wrote:
> From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
> 
> Add dirty page rate limit test if kernel support dirty ring,
> 
> The following qmp commands are covered by this test case:
> "calc-dirty-rate", "query-dirty-rate", "set-vcpu-dirty-limit",
> "cancel-vcpu-dirty-limit" and "query-vcpu-dirty-limit".
> 
> Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
> Acked-by: Peter Xu <peterx@redhat.com>
> Message-Id: <eed5b847a6ef0a9c02a36383dbdd7db367dd1e7e.1656177590.git.huangy81@chinatelecom.cn>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>   tests/qtest/migration-helpers.c |  22 +++
>   tests/qtest/migration-helpers.h |   2 +
>   tests/qtest/migration-test.c    | 256 ++++++++++++++++++++++++++++++++
>   3 files changed, 280 insertions(+)
[...]
> +static QTestState *dirtylimit_start_vm(void)
> +{
> +    QTestState *vm = NULL;
> +    g_autofree gchar *cmd = NULL;
> +    const char *arch = qtest_get_arch();
> +    g_autofree char *bootpath = NULL;
> +
> +    assert((strcmp(arch, "x86_64") == 0));

  Hi!

This assert triggers on my x86 laptop when I run "make check" there. More 
precisely, it triggers when qemu-system-aarch64 is getting tested:

$ QTEST_QEMU_BINARY=./qemu-system-aarch64 tests/qtest/migration-test
/aarch64/migration/bad_dest: OK
/aarch64/migration/fd_proto: OK
/aarch64/migration/validate_uuid: OK
/aarch64/migration/validate_uuid_error: OK
/aarch64/migration/validate_uuid_src_not_set: OK
/aarch64/migration/validate_uuid_dst_not_set: OK
/aarch64/migration/auto_converge: OK
/aarch64/migration/dirty_ring: OK
/aarch64/migration/vcpu_dirty_limit: migration-test: 
../../devel/qemu/tests/qtest/migration-test.c:2304: dirtylimit_start_vm: 
Assertion `(strcmp(arch, "x86_64") == 0)' failed.
Aborted (core dumped)

>   static bool kvm_dirty_ring_supported(void)
>   {
>   #if defined(__linux__) && defined(HOST_X86_64)
> @@ -2204,6 +2458,8 @@ int main(int argc, char **argv)
>       if (kvm_dirty_ring_supported()) {
>           qtest_add_func("/migration/dirty_ring",
>                          test_precopy_unix_dirty_ring);
> +        qtest_add_func("/migration/vcpu_dirty_limit",
> +                       test_vcpu_dirty_limit);
>       }

kvm_dirty_ring_supported() returns "true" if the KVM of the *host* is x86 
and has the dirty ring support, but it does *not* check the target 
architecture that is currently tested, so you also get here "true" for 
qemu-system-aarch64 being tested on a x86 host... Thus I guess 
kvm_dirty_ring_supported() needs also a check for the right target architecture?

  Thomas
Peter Maydell Aug. 1, 2022, 11:20 a.m. UTC | #2
On Mon, 1 Aug 2022 at 11:41, Thomas Huth <thuth@redhat.com> wrote:
>
> On 20/07/2022 13.19, Dr. David Alan Gilbert (git) wrote:
> > From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
> >
> > Add dirty page rate limit test if kernel support dirty ring,
> >
> > The following qmp commands are covered by this test case:
> > "calc-dirty-rate", "query-dirty-rate", "set-vcpu-dirty-limit",
> > "cancel-vcpu-dirty-limit" and "query-vcpu-dirty-limit".

> This assert triggers on my x86 laptop when I run "make check" there. More
> precisely, it triggers when qemu-system-aarch64 is getting tested:
>
> $ QTEST_QEMU_BINARY=./qemu-system-aarch64 tests/qtest/migration-test
> /aarch64/migration/bad_dest: OK
> /aarch64/migration/fd_proto: OK
> /aarch64/migration/validate_uuid: OK
> /aarch64/migration/validate_uuid_error: OK
> /aarch64/migration/validate_uuid_src_not_set: OK
> /aarch64/migration/validate_uuid_dst_not_set: OK
> /aarch64/migration/auto_converge: OK
> /aarch64/migration/dirty_ring: OK
> /aarch64/migration/vcpu_dirty_limit: migration-test:
> ../../devel/qemu/tests/qtest/migration-test.c:2304: dirtylimit_start_vm:
> Assertion `(strcmp(arch, "x86_64") == 0)' failed.
> Aborted (core dumped)
>
> >   static bool kvm_dirty_ring_supported(void)
> >   {
> >   #if defined(__linux__) && defined(HOST_X86_64)
> > @@ -2204,6 +2458,8 @@ int main(int argc, char **argv)
> >       if (kvm_dirty_ring_supported()) {
> >           qtest_add_func("/migration/dirty_ring",
> >                          test_precopy_unix_dirty_ring);
> > +        qtest_add_func("/migration/vcpu_dirty_limit",
> > +                       test_vcpu_dirty_limit);
> >       }
>
> kvm_dirty_ring_supported() returns "true" if the KVM of the *host* is x86
> and has the dirty ring support, but it does *not* check the target
> architecture that is currently tested, so you also get here "true" for
> qemu-system-aarch64 being tested on a x86 host... Thus I guess
> kvm_dirty_ring_supported() needs also a check for the right target architecture?

Presumably it should be checking qtest_has_accel("kvm") -- even
if you're on an x86 host and with an x86 guest, if you're using
TCG you're not going to get KVM dirty ring support...

thanks
-- PMM
diff mbox series

Patch

diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
index e81e831c85..c6fbeb3974 100644
--- a/tests/qtest/migration-helpers.c
+++ b/tests/qtest/migration-helpers.c
@@ -83,6 +83,28 @@  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 78587c2b82..59561898d0 100644
--- a/tests/qtest/migration-helpers.h
+++ b/tests/qtest/migration-helpers.h
@@ -23,6 +23,8 @@  QDict *wait_command_fd(QTestState *who, int fd, const char *command, ...);
 G_GNUC_PRINTF(2, 3)
 QDict *wait_command(QTestState *who, const char *command, ...);
 
+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 9e64125f02..db4dcc5b31 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -24,6 +24,7 @@ 
 #include "qapi/qobject-input-visitor.h"
 #include "qapi/qobject-output-visitor.h"
 #include "crypto/tlscredspsk.h"
+#include "qapi/qmp/qlist.h"
 
 #include "migration-helpers.h"
 #include "tests/migration/migration-test.h"
@@ -46,6 +47,12 @@  unsigned start_address;
 unsigned end_address;
 static bool uffd_feature_thread_id;
 
+/*
+ * Dirtylimit stop working if dirty page rate error
+ * value less than DIRTYLIMIT_TOLERANCE_RANGE
+ */
+#define DIRTYLIMIT_TOLERANCE_RANGE  25  /* MB/s */
+
 #if defined(__linux__)
 #include <sys/syscall.h>
 #include <sys/vfs.h>
@@ -2059,6 +2066,253 @@  static void test_multifd_tcp_cancel(void)
     test_migrate_end(from, to2, true);
 }
 
+static void calc_dirty_rate(QTestState *who, uint64_t calc_time)
+{
+    qobject_unref(qmp_command(who,
+                  "{ 'execute': 'calc-dirty-rate',"
+                  "'arguments': { "
+                  "'calc-time': %ld,"
+                  "'mode': 'dirty-ring' }}",
+                  calc_time));
+}
+
+static QDict *query_dirty_rate(QTestState *who)
+{
+    return qmp_command(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': %ld } }",
+                  dirtyrate));
+}
+
+static void cancel_vcpu_dirty_limit(QTestState *who)
+{
+    qobject_unref(qmp_command(who,
+                  "{ 'execute': 'cancel-vcpu-dirty-limit' }"));
+}
+
+static QDict *query_vcpu_dirty_limit(QTestState *who)
+{
+    QDict *rsp;
+
+    rsp = qtest_qmp(who, "{ 'execute': 'query-vcpu-dirty-limit' }");
+    g_assert(!qdict_haskey(rsp, "error"));
+    g_assert(qdict_haskey(rsp, "return"));
+
+    return rsp;
+}
+
+static bool calc_dirtyrate_ready(QTestState *who)
+{
+    QDict *rsp_return;
+    gchar *status;
+
+    rsp_return = query_dirty_rate(who);
+    g_assert(rsp_return);
+
+    status = g_strdup(qdict_get_str(rsp_return, "status"));
+    g_assert(status);
+
+    return g_strcmp0(status, "measuring");
+}
+
+static void wait_for_calc_dirtyrate_complete(QTestState *who,
+                                             int64_t time_s)
+{
+    int max_try_count = 10000;
+    usleep(time_s * 1000000);
+
+    while (!calc_dirtyrate_ready(who) && max_try_count--) {
+        usleep(1000);
+    }
+
+    /*
+     * Set the timeout with 10 s(max_try_count * 1000us),
+     * if dirtyrate measurement not complete, fail test.
+     */
+    g_assert_cmpint(max_try_count, !=, 0);
+}
+
+static int64_t get_dirty_rate(QTestState *who)
+{
+    QDict *rsp_return;
+    gchar *status;
+    QList *rates;
+    const QListEntry *entry;
+    QDict *rate;
+    int64_t dirtyrate;
+
+    rsp_return = query_dirty_rate(who);
+    g_assert(rsp_return);
+
+    status = g_strdup(qdict_get_str(rsp_return, "status"));
+    g_assert(status);
+    g_assert_cmpstr(status, ==, "measured");
+
+    rates = qdict_get_qlist(rsp_return, "vcpu-dirty-rate");
+    g_assert(rates && !qlist_empty(rates));
+
+    entry = qlist_first(rates);
+    g_assert(entry);
+
+    rate = qobject_to(QDict, qlist_entry_obj(entry));
+    g_assert(rate);
+
+    dirtyrate = qdict_get_try_int(rate, "dirty-rate", -1);
+
+    qobject_unref(rsp_return);
+    return dirtyrate;
+}
+
+static int64_t get_limit_rate(QTestState *who)
+{
+    QDict *rsp_return;
+    QList *rates;
+    const QListEntry *entry;
+    QDict *rate;
+    int64_t dirtyrate;
+
+    rsp_return = query_vcpu_dirty_limit(who);
+    g_assert(rsp_return);
+
+    rates = qdict_get_qlist(rsp_return, "return");
+    g_assert(rates && !qlist_empty(rates));
+
+    entry = qlist_first(rates);
+    g_assert(entry);
+
+    rate = qobject_to(QDict, qlist_entry_obj(entry));
+    g_assert(rate);
+
+    dirtyrate = qdict_get_try_int(rate, "limit-rate", -1);
+
+    qobject_unref(rsp_return);
+    return dirtyrate;
+}
+
+static QTestState *dirtylimit_start_vm(void)
+{
+    QTestState *vm = NULL;
+    g_autofree gchar *cmd = NULL;
+    const char *arch = qtest_get_arch();
+    g_autofree char *bootpath = NULL;
+
+    assert((strcmp(arch, "x86_64") == 0));
+    bootpath = g_strdup_printf("%s/bootsect", tmpfs);
+    assert(sizeof(x86_bootsect) == 512);
+    init_bootfile(bootpath, x86_bootsect, sizeof(x86_bootsect));
+
+    cmd = g_strdup_printf("-accel kvm,dirty-ring-size=4096 "
+                          "-name dirtylimit-test,debug-threads=on "
+                          "-m 150M -smp 1 "
+                          "-serial file:%s/vm_serial "
+                          "-drive file=%s,format=raw ",
+                          tmpfs, bootpath);
+
+    vm = qtest_init(cmd);
+    return vm;
+}
+
+static void dirtylimit_stop_vm(QTestState *vm)
+{
+    qtest_quit(vm);
+    cleanup("bootsect");
+    cleanup("vm_serial");
+}
+
+static void test_vcpu_dirty_limit(void)
+{
+    QTestState *vm;
+    int64_t origin_rate;
+    int64_t quota_rate;
+    int64_t rate ;
+    int max_try_count = 20;
+    int hit = 0;
+
+    /* Start vm for vcpu dirtylimit test */
+    vm = dirtylimit_start_vm();
+
+    /* Wait for the first serial output from the vm*/
+    wait_for_serial("vm_serial");
+
+    /* Do dirtyrate measurement with calc time equals 1s */
+    calc_dirty_rate(vm, 1);
+
+    /* Sleep calc time and wait for calc dirtyrate complete */
+    wait_for_calc_dirtyrate_complete(vm, 1);
+
+    /* Query original dirty page rate */
+    origin_rate = get_dirty_rate(vm);
+
+    /* VM booted from bootsect should dirty memory steadily */
+    assert(origin_rate != 0);
+
+    /* Setup quota dirty page rate at half of origin */
+    quota_rate = origin_rate / 2;
+
+    /* Set dirtylimit */
+    dirtylimit_set_all(vm, quota_rate);
+
+    /*
+     * Check if set-vcpu-dirty-limit and query-vcpu-dirty-limit
+     * works literally
+     */
+    g_assert_cmpint(quota_rate, ==, get_limit_rate(vm));
+
+    /* Sleep a bit to check if it take effect */
+    usleep(2000000);
+
+    /*
+     * Check if dirtylimit take effect realistically, set the
+     * timeout with 20 s(max_try_count * 1s), if dirtylimit
+     * doesn't take effect, fail test.
+     */
+    while (--max_try_count) {
+        calc_dirty_rate(vm, 1);
+        wait_for_calc_dirtyrate_complete(vm, 1);
+        rate = get_dirty_rate(vm);
+
+        /*
+         * Assume hitting if current rate is less
+         * than quota rate (within accepting error)
+         */
+        if (rate < (quota_rate + DIRTYLIMIT_TOLERANCE_RANGE)) {
+            hit = 1;
+            break;
+        }
+    }
+
+    g_assert_cmpint(hit, ==, 1);
+
+    hit = 0;
+    max_try_count = 20;
+
+    /* Check if dirtylimit cancellation take effect */
+    cancel_vcpu_dirty_limit(vm);
+    while (--max_try_count) {
+        calc_dirty_rate(vm, 1);
+        wait_for_calc_dirtyrate_complete(vm, 1);
+        rate = get_dirty_rate(vm);
+
+        /*
+         * Assume dirtylimit be canceled if current rate is
+         * greater than quota rate (within accepting error)
+         */
+        if (rate > (quota_rate + DIRTYLIMIT_TOLERANCE_RANGE)) {
+            hit = 1;
+            break;
+        }
+    }
+
+    g_assert_cmpint(hit, ==, 1);
+    dirtylimit_stop_vm(vm);
+}
+
 static bool kvm_dirty_ring_supported(void)
 {
 #if defined(__linux__) && defined(HOST_X86_64)
@@ -2204,6 +2458,8 @@  int main(int argc, char **argv)
     if (kvm_dirty_ring_supported()) {
         qtest_add_func("/migration/dirty_ring",
                        test_precopy_unix_dirty_ring);
+        qtest_add_func("/migration/vcpu_dirty_limit",
+                       test_vcpu_dirty_limit);
     }
 
     ret = g_test_run();