Re: [PULL 00/30] Next patches
Richard Henderson wrote: > On 6/22/23 18:54, Juan Quintela wrote: >> The following changes since commit b455ce4c2f300c8ba47cba7232dd03261368a4cb: >>Merge tag 'q800-for-8.1-pull-request' >> ofhttps://github.com/vivier/qemu-m68k into staging (2023-06-22 >> 10:18:32 +0200) >> are available in the Git repository at: >>https://gitlab.com/juan.quintela/qemu.git tags/next-pull-request >> for you to fetch changes up to >> 23e4307eadc1497bd0a11ca91041768f15963b68: >>migration/rdma: Split qemu_fopen_rdma() into input/output >> functions (2023-06-22 18:11:58 +0200) >> >> Migration Pull request (20230621) take 2 >> In this pull request the only change is fixing 32 bits complitaion >> issue. >> Please apply. >> [take 1] >> - fix for multifd thread creation (fabiano) >> - dirtylimity (hyman) >>* migration-test will go on next PULL request, as it has failures. >> - Improve error description (tejus) >> - improve -incoming and set parameters before calling incoming (wei) >> - migration atomic counters reviewed patches (quintela) >> - migration-test refacttoring reviewed (quintela) > > New failure with check-cfi-x86_64: > > https://gitlab.com/qemu-project/qemu/-/jobs/4527202764#L188 First of all, is there a way to get to the test log? In particular, I am interested in knowing at least what test has failed (yes, migration-test don't tell you much more). After a bit more wrestling, I have been able to get things compiling with this command: $ /mnt/code/qemu/full/configure --enable-cfi --target-list=x86_64-softmmu --enable-cfi-debug --cc=clang --cxx=clang++ --disable-docs --enable-safe-stack --disable-slirp It should basically be the one that check-cfi-x86_64 is using if I understand the build recipes correctly (that is a BIG IF). And it passes for me with flying colors. Here I have Fedora38, builder has F37. > /builds/qemu-project/qemu/build/pyvenv/bin/meson test --no-rebuild -t > 0 --num-processes 1 --print-errorlogs > 1/350 qemu:qtest+qtest-x86_64 / qtest-x86_64/qom-test > OK 6.55s 8 subtests passed > ▶ 2/350 ERROR:../tests/qtest/migration-test.c:320:check_guests_ram: > assertion failed: (bad == 0) ERROR > 2/350 qemu:qtest+qtest-x86_64 / qtest-x86_64/migration-test > ERROR 151.99s killed by signal 6 SIGABRT > G_TEST_DBUS_DAEMON=/builds/qemu-project/qemu/tests/dbus-vmstate-daemon.sh > MALLOC_PERTURB_=3 QTEST_QEMU_IMG=./qemu-img > QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon > QTEST_QEMU_BINARY=./qemu-system-x86_64 > /builds/qemu-project/qemu/build/tests/qtest/migration-test --tap >-k > ― ✀ ― > stderr: > qemu-system-x86_64: Unable to read from socket: Connection reset by peer This is the interesting bit, why is the conection closed. > Memory content inconsistency at 4f65000 first_byte = 30 last_byte = 2f > current = 88 hit_edge = 1 > ** > ERROR:../tests/qtest/migration-test.c:320:check_guests_ram: assertion failed: > (bad == 0) > > (test program exited with status code -6) This makes zero sense, except if we haven't migrated all the guest state, that it is what it has happened. Is there a place on the web interface to see the full logs? Or that is the only thing that the CI system stores? Later, Juan.
[PATCH 3/5] throttle: support read-only and write-only
Only one direction is necessary in several scenarios:
- a read-only disk
- operations on a device are considered as *write* only. For example,
encrypt/decrypt/sign/verify operations on a cryptodev use a single
*write* timer(read timer callback is defined, but never invoked).
Allow a single direction in throttle, this reduces memory, and uplayer
does not need a dummy callback any more.
Signed-off-by: zhenwei pi
---
util/throttle.c | 24
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/util/throttle.c b/util/throttle.c
index c1cc24831c..01faee783c 100644
--- a/util/throttle.c
+++ b/util/throttle.c
@@ -199,12 +199,17 @@ static bool throttle_compute_timer(ThrottleState *ts,
void throttle_timers_attach_aio_context(ThrottleTimers *tt,
AioContext *new_context)
{
-tt->timers[THROTTLE_TIMER_READ] =
-aio_timer_new(new_context, tt->clock_type, SCALE_NS,
- tt->timer_cb[THROTTLE_TIMER_READ], tt->timer_opaque);
-tt->timers[THROTTLE_TIMER_WRITE] =
-aio_timer_new(new_context, tt->clock_type, SCALE_NS,
- tt->timer_cb[THROTTLE_TIMER_WRITE], tt->timer_opaque);
+if (tt->timer_cb[THROTTLE_TIMER_READ]) {
+tt->timers[THROTTLE_TIMER_READ] =
+aio_timer_new(new_context, tt->clock_type, SCALE_NS,
+ tt->timer_cb[THROTTLE_TIMER_READ], tt->timer_opaque);
+}
+
+if (tt->timer_cb[THROTTLE_TIMER_WRITE]) {
+tt->timers[THROTTLE_TIMER_WRITE] =
+aio_timer_new(new_context, tt->clock_type, SCALE_NS,
+ tt->timer_cb[THROTTLE_TIMER_WRITE],
tt->timer_opaque);
+}
}
/*
@@ -235,6 +240,7 @@ void throttle_timers_init(ThrottleTimers *tt,
QEMUTimerCB *write_timer_cb,
void *timer_opaque)
{
+assert(read_timer_cb || write_timer_cb);
memset(tt, 0, sizeof(ThrottleTimers));
tt->clock_type = clock_type;
@@ -247,7 +253,9 @@ void throttle_timers_init(ThrottleTimers *tt,
/* destroy a timer */
static void throttle_timer_destroy(QEMUTimer **timer)
{
-assert(*timer != NULL);
+if (*timer == NULL) {
+return;
+}
timer_free(*timer);
*timer = NULL;
@@ -272,7 +280,7 @@ void throttle_timers_destroy(ThrottleTimers *tt)
/* is any throttling timer configured */
bool throttle_timers_are_initialized(ThrottleTimers *tt)
{
-if (tt->timers[0]) {
+if (tt->timers[THROTTLE_TIMER_READ] || tt->timers[THROTTLE_TIMER_WRITE]) {
return true;
}
--
2.34.1
[PATCH 4/5] test-throttle: test read only and write only
Signed-off-by: zhenwei pi
---
tests/unit/test-throttle.c | 66 ++
1 file changed, 66 insertions(+)
diff --git a/tests/unit/test-throttle.c b/tests/unit/test-throttle.c
index 5fc2de4d47..d7973980d1 100644
--- a/tests/unit/test-throttle.c
+++ b/tests/unit/test-throttle.c
@@ -184,6 +184,70 @@ static void test_init(void)
throttle_timers_destroy(tt);
}
+static void test_init_readonly(void)
+{
+int i;
+
+tt = _timers;
+
+/* fill the structures with crap */
+memset(, 1, sizeof(ts));
+memset(tt, 1, sizeof(*tt));
+
+/* init structures */
+throttle_init();
+throttle_timers_init(tt, ctx, QEMU_CLOCK_VIRTUAL,
+ read_timer_cb, NULL, );
+
+/* check initialized fields */
+g_assert(tt->clock_type == QEMU_CLOCK_VIRTUAL);
+g_assert(tt->timers[THROTTLE_TIMER_READ]);
+g_assert(!tt->timers[THROTTLE_TIMER_WRITE]);
+
+/* check other fields where cleared */
+g_assert(!ts.previous_leak);
+g_assert(!ts.cfg.op_size);
+for (i = 0; i < BUCKETS_COUNT; i++) {
+g_assert(!ts.cfg.buckets[i].avg);
+g_assert(!ts.cfg.buckets[i].max);
+g_assert(!ts.cfg.buckets[i].level);
+}
+
+throttle_timers_destroy(tt);
+}
+
+static void test_init_writeonly(void)
+{
+int i;
+
+tt = _timers;
+
+/* fill the structures with crap */
+memset(, 1, sizeof(ts));
+memset(tt, 1, sizeof(*tt));
+
+/* init structures */
+throttle_init();
+throttle_timers_init(tt, ctx, QEMU_CLOCK_VIRTUAL,
+ NULL, write_timer_cb, );
+
+/* check initialized fields */
+g_assert(tt->clock_type == QEMU_CLOCK_VIRTUAL);
+g_assert(!tt->timers[THROTTLE_TIMER_READ]);
+g_assert(tt->timers[THROTTLE_TIMER_WRITE]);
+
+/* check other fields where cleared */
+g_assert(!ts.previous_leak);
+g_assert(!ts.cfg.op_size);
+for (i = 0; i < BUCKETS_COUNT; i++) {
+g_assert(!ts.cfg.buckets[i].avg);
+g_assert(!ts.cfg.buckets[i].max);
+g_assert(!ts.cfg.buckets[i].level);
+}
+
+throttle_timers_destroy(tt);
+}
+
static void test_destroy(void)
{
int i;
@@ -752,6 +816,8 @@ int main(int argc, char **argv)
g_test_add_func("/throttle/leak_bucket",test_leak_bucket);
g_test_add_func("/throttle/compute_wait", test_compute_wait);
g_test_add_func("/throttle/init", test_init);
+g_test_add_func("/throttle/init_readonly", test_init_readonly);
+g_test_add_func("/throttle/init_writeonly", test_init_writeonly);
g_test_add_func("/throttle/destroy",test_destroy);
g_test_add_func("/throttle/have_timer", test_have_timer);
g_test_add_func("/throttle/detach_attach", test_detach_attach);
--
2.34.1
[PATCH 1/5] throttle: introduce enum ThrottleTimerType
Use enum ThrottleTimerType instead of number index.
Signed-off-by: zhenwei pi
---
include/qemu/throttle.h | 12 +---
util/throttle.c | 16 +---
2 files changed, 18 insertions(+), 10 deletions(-)
diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h
index 05f6346137..0956094115 100644
--- a/include/qemu/throttle.h
+++ b/include/qemu/throttle.h
@@ -99,13 +99,19 @@ typedef struct ThrottleState {
int64_t previous_leak;/* timestamp of the last leak done */
} ThrottleState;
+typedef enum {
+THROTTLE_TIMER_READ = 0,
+THROTTLE_TIMER_WRITE,
+THROTTLE_TIMER_MAX
+} ThrottleTimerType;
+
typedef struct ThrottleTimers {
-QEMUTimer *timers[2]; /* timers used to do the throttling */
+/* timers used to do the throttling */
+QEMUTimer *timers[THROTTLE_TIMER_MAX];
QEMUClockType clock_type; /* the clock used */
/* Callbacks */
-QEMUTimerCB *read_timer_cb;
-QEMUTimerCB *write_timer_cb;
+QEMUTimerCB *timer_cb[THROTTLE_TIMER_MAX];
void *timer_opaque;
} ThrottleTimers;
diff --git a/util/throttle.c b/util/throttle.c
index 81f247a8d1..c1cc24831c 100644
--- a/util/throttle.c
+++ b/util/throttle.c
@@ -199,10 +199,12 @@ static bool throttle_compute_timer(ThrottleState *ts,
void throttle_timers_attach_aio_context(ThrottleTimers *tt,
AioContext *new_context)
{
-tt->timers[0] = aio_timer_new(new_context, tt->clock_type, SCALE_NS,
- tt->read_timer_cb, tt->timer_opaque);
-tt->timers[1] = aio_timer_new(new_context, tt->clock_type, SCALE_NS,
- tt->write_timer_cb, tt->timer_opaque);
+tt->timers[THROTTLE_TIMER_READ] =
+aio_timer_new(new_context, tt->clock_type, SCALE_NS,
+ tt->timer_cb[THROTTLE_TIMER_READ], tt->timer_opaque);
+tt->timers[THROTTLE_TIMER_WRITE] =
+aio_timer_new(new_context, tt->clock_type, SCALE_NS,
+ tt->timer_cb[THROTTLE_TIMER_WRITE], tt->timer_opaque);
}
/*
@@ -236,8 +238,8 @@ void throttle_timers_init(ThrottleTimers *tt,
memset(tt, 0, sizeof(ThrottleTimers));
tt->clock_type = clock_type;
-tt->read_timer_cb = read_timer_cb;
-tt->write_timer_cb = write_timer_cb;
+tt->timer_cb[THROTTLE_TIMER_READ] = read_timer_cb;
+tt->timer_cb[THROTTLE_TIMER_WRITE] = write_timer_cb;
tt->timer_opaque = timer_opaque;
throttle_timers_attach_aio_context(tt, aio_context);
}
@@ -256,7 +258,7 @@ void throttle_timers_detach_aio_context(ThrottleTimers *tt)
{
int i;
-for (i = 0; i < 2; i++) {
+for (i = 0; i < THROTTLE_TIMER_MAX; i++) {
throttle_timer_destroy(>timers[i]);
}
}
--
2.34.1
[PATCH 5/5] cryptodev: use NULL throttle timer cb for read direction
Operations on a crytpodev are considered as *write* only, the callback
of read direction is never invoked. Use NULL instead of an unreachable
path(cryptodev_backend_throttle_timer_cb on read direction).
Signed-off-by: zhenwei pi
---
backends/cryptodev.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/backends/cryptodev.c b/backends/cryptodev.c
index 94ca393cee..19c24ccfad 100644
--- a/backends/cryptodev.c
+++ b/backends/cryptodev.c
@@ -331,8 +331,7 @@ static void cryptodev_backend_set_throttle(CryptoDevBackend
*backend, int field,
if (!enabled) {
throttle_init(>ts);
throttle_timers_init(>tt, qemu_get_aio_context(),
- QEMU_CLOCK_REALTIME,
- cryptodev_backend_throttle_timer_cb, /* FIXME */
+ QEMU_CLOCK_REALTIME, NULL,
cryptodev_backend_throttle_timer_cb, backend);
}
--
2.34.1
[PATCH 2/5] test-throttle: use enum ThrottleTimerType
Use enum ThrottleTimerType instead in the throttle test codes.
Signed-off-by: zhenwei pi
---
tests/unit/test-throttle.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tests/unit/test-throttle.c b/tests/unit/test-throttle.c
index 7adb5e6652..5fc2de4d47 100644
--- a/tests/unit/test-throttle.c
+++ b/tests/unit/test-throttle.c
@@ -169,8 +169,8 @@ static void test_init(void)
/* check initialized fields */
g_assert(tt->clock_type == QEMU_CLOCK_VIRTUAL);
-g_assert(tt->timers[0]);
-g_assert(tt->timers[1]);
+g_assert(tt->timers[THROTTLE_TIMER_READ]);
+g_assert(tt->timers[THROTTLE_TIMER_WRITE]);
/* check other fields where cleared */
g_assert(!ts.previous_leak);
@@ -191,7 +191,7 @@ static void test_destroy(void)
throttle_timers_init(tt, ctx, QEMU_CLOCK_VIRTUAL,
read_timer_cb, write_timer_cb, );
throttle_timers_destroy(tt);
-for (i = 0; i < 2; i++) {
+for (i = 0; i < THROTTLE_TIMER_MAX; i++) {
g_assert(!tt->timers[i]);
}
}
--
2.34.1
[PATCH 0/5] Misc fixes for throttle
Hi, v1: - introduce enum ThrottleTimerType instead of timers[0], timer[1]... - support read-only and write-only for throttle - adapt related test codes - cryptodev uses a write-only throttle timer Zhenwei Pi (5): throttle: introduce enum ThrottleTimerType test-throttle: use enum ThrottleTimerType throttle: support read-only and write-only test-throttle: test read only and write only cryptodev: use NULL throttle timer cb for read direction backends/cryptodev.c | 3 +- include/qemu/throttle.h| 12 +-- tests/unit/test-throttle.c | 72 -- util/throttle.c| 28 ++- 4 files changed, 98 insertions(+), 17 deletions(-) -- 2.34.1
