[RFC PATCH 0/2] Add dirty page rate limit qtest

[RFC PATCH 0/2] Add dirty page rate limit qtest

[huangy81]

From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>

This patchset is dirtylimit qtest and plan to be a supplement
for patch series "support dirty restraint on vCPU".

Dirtylimit qtest use the existing bootsect in tests/migration/i386
to start test vm. The x86.bootsect repeatedly increments the
first byte of each page in a 100MB range, so it can be a workload
for dirtylimit test.

We limit dirtyrate at one-third of origin workload to check if
dirtylimit take effect, then query the dirtylimit info and cancel
it, so all the qmp interface can be covered.  

Since the dirtylimit has not been merged yet, this patchset is
posted for RFC.

Please review, thanks !

Yong.

Hyman Huang (2):
  migration-test: Export migration-test util funtions
  tests: Add dirty page rate limit test

 tests/qtest/dirtylimit-test.c   | 288 ++++++++++++++++++++++++++++++++++++++++
 tests/qtest/meson.build         |   2 +
 tests/qtest/migration-helpers.c |  95 +++++++++++++
 tests/qtest/migration-helpers.h |  15 +++
 tests/qtest/migration-test.c    | 102 --------------
 5 files changed, 400 insertions(+), 102 deletions(-)
 create mode 100644 tests/qtest/dirtylimit-test.c

-- 
1.8.3.1

[RFC PATCH 1/2] migration-test: Export migration-test util funtions

[huangy81]

From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>

Dirtylimit qtest can reuse the mechanisms that have been
implemented by migration-test to start a vm, so export the
relevant util functions.

Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
---
 tests/qtest/migration-helpers.c |  95 +++++++++++++++++++++++++++++++++++++
 tests/qtest/migration-helpers.h |  15 ++++++
 tests/qtest/migration-test.c    | 102 ----------------------------------------
 3 files changed, 110 insertions(+), 102 deletions(-)

diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
index 4ee2601..ffec54b 100644
--- a/tests/qtest/migration-helpers.c
+++ b/tests/qtest/migration-helpers.c
@@ -16,6 +16,7 @@
 #include "migration-helpers.h"
 
 bool got_stop;
+const char *tmpfs;
 
 static void check_stop_event(QTestState *who)
 {
@@ -188,3 +189,97 @@ void wait_for_migration_fail(QTestState *from, bool allow_active)
     g_assert(qdict_get_bool(rsp_return, "running"));
     qobject_unref(rsp_return);
 }
+
+void init_bootfile(const char *bootpath, void *content, size_t len)
+{
+    FILE *bootfile = fopen(bootpath, "wb");
+
+    g_assert_cmpint(fwrite(content, len, 1, bootfile), ==, 1);
+    fclose(bootfile);
+}
+
+/*
+ * Wait for some output in the serial output file,
+ * we get an 'A' followed by an endless string of 'B's
+ * but on the destination we won't have the A.
+ */
+void wait_for_serial(const char *side)
+{
+    g_autofree char *serialpath = g_strdup_printf("%s/%s", tmpfs, side);
+    FILE *serialfile = fopen(serialpath, "r");
+    const char *arch = qtest_get_arch();
+    int started = (strcmp(side, "src_serial") == 0 &&
+                   strcmp(arch, "ppc64") == 0) ? 0 : 1;
+
+    do {
+        int readvalue = fgetc(serialfile);
+
+        if (!started) {
+            /* SLOF prints its banner before starting test,
+             * to ignore it, mark the start of the test with '_',
+             * ignore all characters until this marker
+             */
+            switch (readvalue) {
+            case '_':
+                started = 1;
+                break;
+            case EOF:
+                fseek(serialfile, 0, SEEK_SET);
+                usleep(1000);
+                break;
+            }
+            continue;
+        }
+        switch (readvalue) {
+        case 'A':
+            /* Fine */
+            break;
+
+        case 'B':
+            /* It's alive! */
+            fclose(serialfile);
+            return;
+
+        case EOF:
+            started = (strcmp(side, "src_serial") == 0 &&
+                       strcmp(arch, "ppc64") == 0) ? 0 : 1;
+            fseek(serialfile, 0, SEEK_SET);
+            usleep(1000);
+            break;
+
+        default:
+            fprintf(stderr, "Unexpected %d on %s serial\n", readvalue, side);
+            g_assert_not_reached();
+        }
+    } while (true);
+}
+
+bool kvm_dirty_ring_supported(void)
+{
+#if defined(__linux__) && defined(HOST_X86_64)
+    int ret, kvm_fd = open("/dev/kvm", O_RDONLY);
+
+    if (kvm_fd < 0) {
+        return false;
+    }
+
+    ret = ioctl(kvm_fd, KVM_CHECK_EXTENSION, KVM_CAP_DIRTY_LOG_RING);
+    close(kvm_fd);
+
+    /* We test with 4096 slots */
+    if (ret < 4096) {
+        return false;
+    }
+
+    return true;
+#else
+    return false;
+#endif
+}
+
+void cleanup(const char *filename)
+{
+    g_autofree char *path = g_strdup_printf("%s/%s", tmpfs, filename);
+
+    unlink(path);
+}
diff --git a/tests/qtest/migration-helpers.h b/tests/qtest/migration-helpers.h
index d63bba9..d08551f 100644
--- a/tests/qtest/migration-helpers.h
+++ b/tests/qtest/migration-helpers.h
@@ -14,7 +14,14 @@
 
 #include "libqos/libqtest.h"
 
+/* For dirty ring test; so far only x86_64 is supported */
+#if defined(__linux__) && defined(HOST_X86_64)
+#include "linux/kvm.h"
+#endif
+#include <sys/ioctl.h>
+
 extern bool got_stop;
+extern const char *tmpfs;
 
 GCC_FMT_ATTR(3, 4)
 QDict *wait_command_fd(QTestState *who, int fd, const char *command, ...);
@@ -34,4 +41,12 @@ void wait_for_migration_complete(QTestState *who);
 
 void wait_for_migration_fail(QTestState *from, bool allow_active);
 
+void init_bootfile(const char *bootpath, void *content, size_t len);
+
+void wait_for_serial(const char *side);
+
+bool kvm_dirty_ring_supported(void);
+
+void cleanup(const char *filename);
+
 #endif /* MIGRATION_HELPERS_H_ */
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 7b42f6f..4316a66 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -27,11 +27,6 @@
 #include "migration-helpers.h"
 #include "tests/migration/migration-test.h"
 
-/* For dirty ring test; so far only x86_64 is supported */
-#if defined(__linux__) && defined(HOST_X86_64)
-#include "linux/kvm.h"
-#endif
-
 /* TODO actually test the results and get rid of this */
 #define qtest_qmp_discard_response(...) qobject_unref(qtest_qmp(__VA_ARGS__))
 
@@ -49,7 +44,6 @@ static bool uffd_feature_thread_id;
 
 #if defined(__linux__) && defined(__NR_userfaultfd) && defined(CONFIG_EVENTFD)
 #include <sys/eventfd.h>
-#include <sys/ioctl.h>
 #include <linux/userfaultfd.h>
 
 static bool ufd_version_check(void)
@@ -91,8 +85,6 @@ static bool ufd_version_check(void)
 
 #endif
 
-static const char *tmpfs;
-
 /* The boot file modifies memory area in [start_address, end_address)
  * repeatedly. It outputs a 'B' at a fixed rate while it's still running.
  */
@@ -100,70 +92,6 @@ static const char *tmpfs;
 #include "tests/migration/aarch64/a-b-kernel.h"
 #include "tests/migration/s390x/a-b-bios.h"
 
-static void init_bootfile(const char *bootpath, void *content, size_t len)
-{
-    FILE *bootfile = fopen(bootpath, "wb");
-
-    g_assert_cmpint(fwrite(content, len, 1, bootfile), ==, 1);
-    fclose(bootfile);
-}
-
-/*
- * Wait for some output in the serial output file,
- * we get an 'A' followed by an endless string of 'B's
- * but on the destination we won't have the A.
- */
-static void wait_for_serial(const char *side)
-{
-    g_autofree char *serialpath = g_strdup_printf("%s/%s", tmpfs, side);
-    FILE *serialfile = fopen(serialpath, "r");
-    const char *arch = qtest_get_arch();
-    int started = (strcmp(side, "src_serial") == 0 &&
-                   strcmp(arch, "ppc64") == 0) ? 0 : 1;
-
-    do {
-        int readvalue = fgetc(serialfile);
-
-        if (!started) {
-            /* SLOF prints its banner before starting test,
-             * to ignore it, mark the start of the test with '_',
-             * ignore all characters until this marker
-             */
-            switch (readvalue) {
-            case '_':
-                started = 1;
-                break;
-            case EOF:
-                fseek(serialfile, 0, SEEK_SET);
-                usleep(1000);
-                break;
-            }
-            continue;
-        }
-        switch (readvalue) {
-        case 'A':
-            /* Fine */
-            break;
-
-        case 'B':
-            /* It's alive! */
-            fclose(serialfile);
-            return;
-
-        case EOF:
-            started = (strcmp(side, "src_serial") == 0 &&
-                       strcmp(arch, "ppc64") == 0) ? 0 : 1;
-            fseek(serialfile, 0, SEEK_SET);
-            usleep(1000);
-            break;
-
-        default:
-            fprintf(stderr, "Unexpected %d on %s serial\n", readvalue, side);
-            g_assert_not_reached();
-        }
-    } while (true);
-}
-
 /*
  * It's tricky to use qemu's migration event capability with qtest,
  * events suddenly appearing confuse the qmp()/hmp() responses.
@@ -276,13 +204,6 @@ static void check_guests_ram(QTestState *who)
     g_assert(bad == 0);
 }
 
-static void cleanup(const char *filename)
-{
-    g_autofree char *path = g_strdup_printf("%s/%s", tmpfs, filename);
-
-    unlink(path);
-}
-
 static char *SocketAddress_to_str(SocketAddress *addr)
 {
     switch (addr->type) {
@@ -1394,29 +1315,6 @@ static void test_multifd_tcp_cancel(void)
     test_migrate_end(from, to2, true);
 }
 
-static bool kvm_dirty_ring_supported(void)
-{
-#if defined(__linux__) && defined(HOST_X86_64)
-    int ret, kvm_fd = open("/dev/kvm", O_RDONLY);
-
-    if (kvm_fd < 0) {
-        return false;
-    }
-
-    ret = ioctl(kvm_fd, KVM_CHECK_EXTENSION, KVM_CAP_DIRTY_LOG_RING);
-    close(kvm_fd);
-
-    /* We test with 4096 slots */
-    if (ret < 4096) {
-        return false;
-    }
-
-    return true;
-#else
-    return false;
-#endif
-}
-
 int main(int argc, char **argv)
 {
     char template[] = "/tmp/migration-test-XXXXXX";
-- 
1.8.3.1

[RFC PATCH 2/2] tests: Add dirty page rate limit test

[huangy81]

From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>

Add dirty page rate limit test if kernel support dirty ring,
create a standalone file to implement the test case.

Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
---
 tests/qtest/dirtylimit-test.c | 288 ++++++++++++++++++++++++++++++++++++++++++
 tests/qtest/meson.build       |   2 +
 2 files changed, 290 insertions(+)
 create mode 100644 tests/qtest/dirtylimit-test.c

diff --git a/tests/qtest/dirtylimit-test.c b/tests/qtest/dirtylimit-test.c
new file mode 100644
index 0000000..07eac2c
--- /dev/null
+++ b/tests/qtest/dirtylimit-test.c
@@ -0,0 +1,288 @@
+/*
+ * QTest testcase for Dirty Page Rate Limit
+ *
+ * Copyright (c) 2022 CHINA TELECOM CO.,LTD.
+ *
+ * Authors:
+ *  Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "libqos/libqtest.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/qmp/qlist.h"
+#include "qapi/qobject-input-visitor.h"
+#include "qapi/qobject-output-visitor.h"
+
+#include "migration-helpers.h"
+#include "tests/migration/i386/a-b-bootblock.h"
+
+/*
+ * Dirtylimit stop working if dirty page rate error
+ * value less than DIRTYLIMIT_TOLERANCE_RANGE
+ */
+#define DIRTYLIMIT_TOLERANCE_RANGE  25  /* MB/s */
+
+static 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;
+}
+
+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 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 *start_vm(void)
+{
+    QTestState *vm = NULL;
+    g_autofree gchar *cmd = NULL;
+    const char *arch = qtest_get_arch();
+    g_autofree char *bootpath = NULL;
+
+    if (strcmp(arch, "i386") != 0 && strcmp(arch, "x86_64") != 0) {
+        return NULL;
+    }
+
+    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 -accel tcg "
+                          "-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 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 = 5;
+    int hit = 0;
+
+    if (!(vm = start_vm())) {
+        return;
+    }
+
+    /* 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 a little bit longer than calc time,
+     * and ensure dirtyrate measurement has been done
+     */
+    usleep(1200000);
+
+    /* Query original dirty page rate */
+    origin_rate = get_dirty_rate(vm);
+
+    /* VM booted from bootsect should dirty memory */
+    assert(origin_rate != 0);
+
+    /* Setup quota dirty page rate at one-third of origin */
+    quota_rate = origin_rate / 3;
+
+    /* Set dirtylimit and wait a bit to check if it take effect */
+    dirtylimit_set_all(vm, quota_rate);
+    usleep(2000000);
+
+    /* Check if set-vcpu-dirty-limit and query-vcpu-dirty-limit
+     * works literally */
+    g_assert_cmpint(quota_rate, ==, get_limit_rate(vm));
+
+    /* Check if dirtylimit take effect realistically */
+    while (--max_try_count) {
+        calc_dirty_rate(vm, 1);
+        usleep(1200000);
+        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 = 5;
+
+    /* Check if dirtylimit cancellation take effect */
+    cancel_vcpu_dirty_limit(vm);
+    while (--max_try_count) {
+        calc_dirty_rate(vm, 1);
+        usleep(1200000);
+        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);
+    stop_vm(vm);
+}
+
+int main(int argc, char **argv)
+{
+    char template[] = "/tmp/dirtylimit-test-XXXXXX";
+    int ret;
+
+    tmpfs = mkdtemp(template);
+    if (!tmpfs) {
+        g_test_message("mkdtemp on path (%s): %s", template, strerror(errno));
+    }
+    g_assert(tmpfs);
+
+    if (!kvm_dirty_ring_supported()) {
+        return 0;
+    }
+
+    g_test_init(&argc, &argv, NULL);
+    qtest_add_func("/dirtylimit/test", test_vcpu_dirty_limit);
+    ret = g_test_run();
+
+    g_assert_cmpint(ret, ==, 0);
+
+    ret = rmdir(tmpfs);
+    if (ret != 0) {
+        g_test_message("unable to rmdir: path (%s): %s",
+                       tmpfs, strerror(errno));
+    }
+
+    return ret;
+}
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index f33d84d..3b4debf 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -32,6 +32,7 @@ qtests_generic = \
   'qom-test',
   'test-hmp',
   'qos-test',
+  'dirtylimit-test',
 ]
 if config_host.has_key('CONFIG_MODULES')
   qtests_generic += [ 'modules-test' ]
@@ -292,6 +293,7 @@ qtests = {
   'tpm-tis-device-swtpm-test': [io, tpmemu_files, 'tpm-tis-util.c'],
   'tpm-tis-device-test': [io, tpmemu_files, 'tpm-tis-util.c'],
   'vmgenid-test': files('boot-sector.c', 'acpi-utils.c'),
+  'dirtylimit-test': files('migration-helpers.c'),
 }
 
 if dbus_display
-- 
1.8.3.1

Re: [RFC PATCH 2/2] tests: Add dirty page rate limit test

[Peter Xu]

On Wed, Mar 09, 2022 at 11:58:01PM +0800, huangy81@chinatelecom.cn wrote:
> From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
> 
> Add dirty page rate limit test if kernel support dirty ring,
> create a standalone file to implement the test case.

Thanks for writting this test case.

> 
> Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
> ---
>  tests/qtest/dirtylimit-test.c | 288 ++++++++++++++++++++++++++++++++++++++++++
>  tests/qtest/meson.build       |   2 +
>  2 files changed, 290 insertions(+)
>  create mode 100644 tests/qtest/dirtylimit-test.c
> 
> diff --git a/tests/qtest/dirtylimit-test.c b/tests/qtest/dirtylimit-test.c
> new file mode 100644
> index 0000000..07eac2c
> --- /dev/null
> +++ b/tests/qtest/dirtylimit-test.c
> @@ -0,0 +1,288 @@
> +/*
> + * QTest testcase for Dirty Page Rate Limit
> + *
> + * Copyright (c) 2022 CHINA TELECOM CO.,LTD.
> + *
> + * Authors:
> + *  Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "libqos/libqtest.h"
> +#include "qapi/qmp/qdict.h"
> +#include "qapi/qmp/qlist.h"
> +#include "qapi/qobject-input-visitor.h"
> +#include "qapi/qobject-output-visitor.h"
> +
> +#include "migration-helpers.h"
> +#include "tests/migration/i386/a-b-bootblock.h"
> +
> +/*
> + * Dirtylimit stop working if dirty page rate error
> + * value less than DIRTYLIMIT_TOLERANCE_RANGE
> + */
> +#define DIRTYLIMIT_TOLERANCE_RANGE  25  /* MB/s */
> +
> +static 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;
> +}
> +
> +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 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 *start_vm(void)
> +{
> +    QTestState *vm = NULL;
> +    g_autofree gchar *cmd = NULL;
> +    const char *arch = qtest_get_arch();
> +    g_autofree char *bootpath = NULL;
> +
> +    if (strcmp(arch, "i386") != 0 && strcmp(arch, "x86_64") != 0) {
> +        return NULL;
> +    }

Perhaps assert() on x86_64?  Dirty ring support is checked earlier, so I
don't see how it'll fail... Very possible i386 will not ever support the
ring anyway.

> +
> +    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 -accel tcg "
> +                          "-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 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 = 5;
> +    int hit = 0;
> +
> +    if (!(vm = start_vm())) {
> +        return;

If you could change above into assert, then no "if" here.

> +    }
> +
> +    /* 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 a little bit longer than calc time,
> +     * and ensure dirtyrate measurement has been done
> +     */
> +    usleep(1200000);

Can we avoid using exact sleep numbers?  We need to guarantee the test
doesn't fail even if the scheduler decides to do weird things..

How about loop until fetching a result, but with a timeout?

> +
> +    /* Query original dirty page rate */
> +    origin_rate = get_dirty_rate(vm);
> +
> +    /* VM booted from bootsect should dirty memory */
> +    assert(origin_rate != 0);
> +
> +    /* Setup quota dirty page rate at one-third of origin */
> +    quota_rate = origin_rate / 3;
> +
> +    /* Set dirtylimit and wait a bit to check if it take effect */
> +    dirtylimit_set_all(vm, quota_rate);
> +    usleep(2000000);
> +
> +    /* Check if set-vcpu-dirty-limit and query-vcpu-dirty-limit
> +     * works literally */
> +    g_assert_cmpint(quota_rate, ==, get_limit_rate(vm));
> +
> +    /* Check if dirtylimit take effect realistically */
> +    while (--max_try_count) {
> +        calc_dirty_rate(vm, 1);
> +        usleep(1200000);

Same here.

> +        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 = 5;
> +
> +    /* Check if dirtylimit cancellation take effect */
> +    cancel_vcpu_dirty_limit(vm);
> +    while (--max_try_count) {
> +        calc_dirty_rate(vm, 1);
> +        usleep(1200000);

Same here.

> +        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);
> +    stop_vm(vm);
> +}

Thanks,

-- 
Peter Xu

Re: [RFC PATCH 2/2] tests: Add dirty page rate limit test

[Hyman]

在 2022/3/10 16:29, Peter Xu 写道:
> On Wed, Mar 09, 2022 at 11:58:01PM +0800, huangy81@chinatelecom.cn wrote:
>> From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
>>
>> Add dirty page rate limit test if kernel support dirty ring,
>> create a standalone file to implement the test case.
> 
> Thanks for writting this test case.
> 
>>
>> Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
>> ---
>>   tests/qtest/dirtylimit-test.c | 288 ++++++++++++++++++++++++++++++++++++++++++
>>   tests/qtest/meson.build       |   2 +
>>   2 files changed, 290 insertions(+)
>>   create mode 100644 tests/qtest/dirtylimit-test.c
>>
>> diff --git a/tests/qtest/dirtylimit-test.c b/tests/qtest/dirtylimit-test.c
>> new file mode 100644
>> index 0000000..07eac2c
>> --- /dev/null
>> +++ b/tests/qtest/dirtylimit-test.c
>> @@ -0,0 +1,288 @@
>> +/*
>> + * QTest testcase for Dirty Page Rate Limit
>> + *
>> + * Copyright (c) 2022 CHINA TELECOM CO.,LTD.
>> + *
>> + * Authors:
>> + *  Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> + * See the COPYING file in the top-level directory.
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "libqos/libqtest.h"
>> +#include "qapi/qmp/qdict.h"
>> +#include "qapi/qmp/qlist.h"
>> +#include "qapi/qobject-input-visitor.h"
>> +#include "qapi/qobject-output-visitor.h"
>> +
>> +#include "migration-helpers.h"
>> +#include "tests/migration/i386/a-b-bootblock.h"
>> +
>> +/*
>> + * Dirtylimit stop working if dirty page rate error
>> + * value less than DIRTYLIMIT_TOLERANCE_RANGE
>> + */
>> +#define DIRTYLIMIT_TOLERANCE_RANGE  25  /* MB/s */
>> +
>> +static 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;
>> +}
>> +
>> +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 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 *start_vm(void)
>> +{
>> +    QTestState *vm = NULL;
>> +    g_autofree gchar *cmd = NULL;
>> +    const char *arch = qtest_get_arch();
>> +    g_autofree char *bootpath = NULL;
>> +
>> +    if (strcmp(arch, "i386") != 0 && strcmp(arch, "x86_64") != 0) {
>> +        return NULL;
>> +    }
> 
> Perhaps assert() on x86_64?  Dirty ring support is checked earlier, so I
> don't see how it'll fail... Very possible i386 will not ever support the
> ring anyway.
Ok.That make code more clean.
> 
>> +
>> +    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 -accel tcg "
>> +                          "-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 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 = 5;
>> +    int hit = 0;
>> +
>> +    if (!(vm = start_vm())) {
>> +        return;
> 
> If you could change above into assert, then no "if" here.
> 
>> +    }
>> +
>> +    /* 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 a little bit longer than calc time,
>> +     * and ensure dirtyrate measurement has been done
>> +     */
>> +    usleep(1200000);
> 
> Can we avoid using exact sleep numbers?  We need to guarantee the test
> doesn't fail even if the scheduler decides to do weird things..
> 
> How about loop until fetching a result, but with a timeout?
Ok.
> 
>> +
>> +    /* Query original dirty page rate */
>> +    origin_rate = get_dirty_rate(vm);
>> +
>> +    /* VM booted from bootsect should dirty memory */
>> +    assert(origin_rate != 0);
>> +
>> +    /* Setup quota dirty page rate at one-third of origin */
>> +    quota_rate = origin_rate / 3;
>> +
>> +    /* Set dirtylimit and wait a bit to check if it take effect */
>> +    dirtylimit_set_all(vm, quota_rate);
>> +    usleep(2000000);
>> +
>> +    /* Check if set-vcpu-dirty-limit and query-vcpu-dirty-limit
>> +     * works literally */
>> +    g_assert_cmpint(quota_rate, ==, get_limit_rate(vm));
>> +
>> +    /* Check if dirtylimit take effect realistically */
>> +    while (--max_try_count) {
>> +        calc_dirty_rate(vm, 1);
>> +        usleep(1200000);
> 
> Same here.
> 
>> +        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 = 5;
>> +
>> +    /* Check if dirtylimit cancellation take effect */
>> +    cancel_vcpu_dirty_limit(vm);
>> +    while (--max_try_count) {
>> +        calc_dirty_rate(vm, 1);
>> +        usleep(1200000);
> 
> Same here.
> 
>> +        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);
>> +    stop_vm(vm);
>> +}
> 
> Thanks,
> 
Ok, thanks Peter for commenting. I'll post the next version after 
patchset "support dirty restraint on vCPU" get merged, and hope more 
comments about the patch be given during the time.