From da5e1de95bb235330d7724316e7a29239d1359d5 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Wed, 3 Jun 2015 10:15:33 +0100 Subject: Revert "iothread: release iothread around aio_poll" This reverts commit a0710f7995f914e3044e5899bd8ff6c43c62f916. In qemu-devel email message <556DBF87.2020908@de.ibm.com>, Christian Borntraeger writes: Having many guests all with a kernel/ramdisk (via -kernel) and several null block devices will result in hangs. All hanging guests are in partition detection code waiting for an I/O to return so very early maybe even the first I/O. Reverting that commit "fixes" the hangs. Reverting this commit for the 2.4 release. More time is needed to investigate and correct this patch. Reported-by: Christian Borntraeger Suggested-by: Paolo Bonzini Reviewed-by: Fam Zheng Signed-off-by: Stefan Hajnoczi --- tests/test-aio.c | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) (limited to 'tests') diff --git a/tests/test-aio.c b/tests/test-aio.c index 4b0cb45d31..a7cb5c9915 100644 --- a/tests/test-aio.c +++ b/tests/test-aio.c @@ -107,7 +107,6 @@ static void test_notify(void) typedef struct { QemuMutex start_lock; - EventNotifier notifier; bool thread_acquired; } AcquireTestData; @@ -119,8 +118,6 @@ static void *test_acquire_thread(void *opaque) qemu_mutex_lock(&data->start_lock); qemu_mutex_unlock(&data->start_lock); - g_usleep(500000); - event_notifier_set(&data->notifier); aio_context_acquire(ctx); aio_context_release(ctx); @@ -129,19 +126,20 @@ static void *test_acquire_thread(void *opaque) return NULL; } -static void dummy_notifier_read(EventNotifier *n) +static void dummy_notifier_read(EventNotifier *unused) { - event_notifier_test_and_clear(n); + g_assert(false); /* should never be invoked */ } static void test_acquire(void) { QemuThread thread; + EventNotifier notifier; AcquireTestData data; /* Dummy event notifier ensures aio_poll() will block */ - event_notifier_init(&data.notifier, false); - aio_set_event_notifier(ctx, &data.notifier, dummy_notifier_read); + event_notifier_init(¬ifier, false); + aio_set_event_notifier(ctx, ¬ifier, dummy_notifier_read); g_assert(!aio_poll(ctx, false)); /* consume aio_notify() */ qemu_mutex_init(&data.start_lock); @@ -155,13 +153,12 @@ static void test_acquire(void) /* Block in aio_poll(), let other thread kick us and acquire context */ aio_context_acquire(ctx); qemu_mutex_unlock(&data.start_lock); /* let the thread run */ - g_assert(aio_poll(ctx, true)); - g_assert(!data.thread_acquired); + g_assert(!aio_poll(ctx, true)); aio_context_release(ctx); qemu_thread_join(&thread); - aio_set_event_notifier(ctx, &data.notifier, NULL); - event_notifier_cleanup(&data.notifier); + aio_set_event_notifier(ctx, ¬ifier, NULL); + event_notifier_cleanup(¬ifier); g_assert(data.thread_acquired); } -- cgit v1.2.3-55-g7522 From 0e5b0a2d54f4dca2f6d1a676da8ec089dc143001 Mon Sep 17 00:00:00 2001 From: Benoît Canet Date: Mon, 8 Jun 2015 18:17:41 +0200 Subject: throttle: Extract timers from ThrottleState into a separate structure Group throttling will share ThrottleState between multiple bs. As a consequence the ThrottleState will be accessed by multiple aio context. Timers are tied to their aio context so they must go out of the ThrottleState structure. This commit paves the way for each bs of a common ThrottleState to have its own timer. Signed-off-by: Benoit Canet Signed-off-by: Alberto Garcia Reviewed-by: Stefan Hajnoczi Message-id: 6cf9ea96d8b32ae2f8769cead38f68a6a0c8c909.1433779731.git.berto@igalia.com Signed-off-by: Stefan Hajnoczi --- block.c | 11 ++++--- block/io.c | 24 ++++++++------ include/block/block_int.h | 1 + include/qemu/throttle.h | 38 ++++++++++++++-------- tests/test-throttle.c | 82 ++++++++++++++++++++++++++--------------------- util/throttle.c | 73 ++++++++++++++++++++++++----------------- 6 files changed, 135 insertions(+), 94 deletions(-) (limited to 'tests') diff --git a/block.c b/block.c index 2b9ceae02f..f9c6bb7404 100644 --- a/block.c +++ b/block.c @@ -1825,6 +1825,9 @@ static void bdrv_move_feature_fields(BlockDriverState *bs_dest, memcpy(&bs_dest->throttle_state, &bs_src->throttle_state, sizeof(ThrottleState)); + memcpy(&bs_dest->throttle_timers, + &bs_src->throttle_timers, + sizeof(ThrottleTimers)); bs_dest->throttled_reqs[0] = bs_src->throttled_reqs[0]; bs_dest->throttled_reqs[1] = bs_src->throttled_reqs[1]; bs_dest->io_limits_enabled = bs_src->io_limits_enabled; @@ -1886,7 +1889,7 @@ void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old) assert(QLIST_EMPTY(&bs_new->dirty_bitmaps)); assert(bs_new->job == NULL); assert(bs_new->io_limits_enabled == false); - assert(!throttle_have_timer(&bs_new->throttle_state)); + assert(!throttle_timers_are_initialized(&bs_new->throttle_timers)); tmp = *bs_new; *bs_new = *bs_old; @@ -1903,7 +1906,7 @@ void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old) /* Check a few fields that should remain attached to the device */ assert(bs_new->job == NULL); assert(bs_new->io_limits_enabled == false); - assert(!throttle_have_timer(&bs_new->throttle_state)); + assert(!throttle_timers_are_initialized(&bs_new->throttle_timers)); /* insert the nodes back into the graph node list if needed */ if (bs_new->node_name[0] != '\0') { @@ -3691,7 +3694,7 @@ void bdrv_detach_aio_context(BlockDriverState *bs) } if (bs->io_limits_enabled) { - throttle_detach_aio_context(&bs->throttle_state); + throttle_timers_detach_aio_context(&bs->throttle_timers); } if (bs->drv->bdrv_detach_aio_context) { bs->drv->bdrv_detach_aio_context(bs); @@ -3727,7 +3730,7 @@ void bdrv_attach_aio_context(BlockDriverState *bs, bs->drv->bdrv_attach_aio_context(bs, new_context); } if (bs->io_limits_enabled) { - throttle_attach_aio_context(&bs->throttle_state, new_context); + throttle_timers_attach_aio_context(&bs->throttle_timers, new_context); } QLIST_FOREACH(ban, &bs->aio_notifiers, list) { diff --git a/block/io.c b/block/io.c index e394d92626..61a9d1da28 100644 --- a/block/io.c +++ b/block/io.c @@ -65,7 +65,7 @@ void bdrv_set_io_limits(BlockDriverState *bs, { int i; - throttle_config(&bs->throttle_state, cfg); + throttle_config(&bs->throttle_state, &bs->throttle_timers, cfg); for (i = 0; i < 2; i++) { qemu_co_enter_next(&bs->throttled_reqs[i]); @@ -98,7 +98,7 @@ void bdrv_io_limits_disable(BlockDriverState *bs) bdrv_start_throttled_reqs(bs); - throttle_destroy(&bs->throttle_state); + throttle_timers_destroy(&bs->throttle_timers); } static void bdrv_throttle_read_timer_cb(void *opaque) @@ -123,12 +123,13 @@ void bdrv_io_limits_enable(BlockDriverState *bs) clock_type = QEMU_CLOCK_VIRTUAL; } assert(!bs->io_limits_enabled); - throttle_init(&bs->throttle_state, - bdrv_get_aio_context(bs), - clock_type, - bdrv_throttle_read_timer_cb, - bdrv_throttle_write_timer_cb, - bs); + throttle_init(&bs->throttle_state); + throttle_timers_init(&bs->throttle_timers, + bdrv_get_aio_context(bs), + clock_type, + bdrv_throttle_read_timer_cb, + bdrv_throttle_write_timer_cb, + bs); bs->io_limits_enabled = true; } @@ -142,7 +143,9 @@ static void bdrv_io_limits_intercept(BlockDriverState *bs, bool is_write) { /* does this io must wait */ - bool must_wait = throttle_schedule_timer(&bs->throttle_state, is_write); + bool must_wait = throttle_schedule_timer(&bs->throttle_state, + &bs->throttle_timers, + is_write); /* if must wait or any request of this type throttled queue the IO */ if (must_wait || @@ -155,7 +158,8 @@ static void bdrv_io_limits_intercept(BlockDriverState *bs, /* if the next request must wait -> do nothing */ - if (throttle_schedule_timer(&bs->throttle_state, is_write)) { + if (throttle_schedule_timer(&bs->throttle_state, &bs->throttle_timers, + is_write)) { return; } diff --git a/include/block/block_int.h b/include/block/block_int.h index f004378d58..02b312fea9 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -380,6 +380,7 @@ struct BlockDriverState { /* I/O throttling */ ThrottleState throttle_state; + ThrottleTimers throttle_timers; CoQueue throttled_reqs[2]; bool io_limits_enabled; diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h index b890613a9c..2c560db2f1 100644 --- a/include/qemu/throttle.h +++ b/include/qemu/throttle.h @@ -65,14 +65,17 @@ typedef struct ThrottleConfig { typedef struct ThrottleState { ThrottleConfig cfg; /* configuration */ int64_t previous_leak; /* timestamp of the last leak done */ - QEMUTimer * timers[2]; /* timers used to do the throttling */ +} ThrottleState; + +typedef struct ThrottleTimers { + QEMUTimer *timers[2]; /* timers used to do the throttling */ QEMUClockType clock_type; /* the clock used */ /* Callbacks */ QEMUTimerCB *read_timer_cb; QEMUTimerCB *write_timer_cb; void *timer_opaque; -} ThrottleState; +} ThrottleTimers; /* operations on single leaky buckets */ void throttle_leak_bucket(LeakyBucket *bkt, int64_t delta); @@ -86,20 +89,23 @@ bool throttle_compute_timer(ThrottleState *ts, int64_t *next_timestamp); /* init/destroy cycle */ -void throttle_init(ThrottleState *ts, - AioContext *aio_context, - QEMUClockType clock_type, - void (read_timer)(void *), - void (write_timer)(void *), - void *timer_opaque); +void throttle_init(ThrottleState *ts); + +void throttle_timers_init(ThrottleTimers *tt, + AioContext *aio_context, + QEMUClockType clock_type, + QEMUTimerCB *read_timer_cb, + QEMUTimerCB *write_timer_cb, + void *timer_opaque); -void throttle_destroy(ThrottleState *ts); +void throttle_timers_destroy(ThrottleTimers *tt); -void throttle_detach_aio_context(ThrottleState *ts); +void throttle_timers_detach_aio_context(ThrottleTimers *tt); -void throttle_attach_aio_context(ThrottleState *ts, AioContext *new_context); +void throttle_timers_attach_aio_context(ThrottleTimers *tt, + AioContext *new_context); -bool throttle_have_timer(ThrottleState *ts); +bool throttle_timers_are_initialized(ThrottleTimers *tt); /* configuration */ bool throttle_enabled(ThrottleConfig *cfg); @@ -108,12 +114,16 @@ bool throttle_conflicting(ThrottleConfig *cfg); bool throttle_is_valid(ThrottleConfig *cfg); -void throttle_config(ThrottleState *ts, ThrottleConfig *cfg); +void throttle_config(ThrottleState *ts, + ThrottleTimers *tt, + ThrottleConfig *cfg); void throttle_get_config(ThrottleState *ts, ThrottleConfig *cfg); /* usage */ -bool throttle_schedule_timer(ThrottleState *ts, bool is_write); +bool throttle_schedule_timer(ThrottleState *ts, + ThrottleTimers *tt, + bool is_write); void throttle_account(ThrottleState *ts, bool is_write, uint64_t size); diff --git a/tests/test-throttle.c b/tests/test-throttle.c index d8ba415e43..458f577603 100644 --- a/tests/test-throttle.c +++ b/tests/test-throttle.c @@ -20,6 +20,7 @@ static AioContext *ctx; static LeakyBucket bkt; static ThrottleConfig cfg; static ThrottleState ts; +static ThrottleTimers tt; /* useful function */ static bool double_cmp(double x, double y) @@ -103,17 +104,19 @@ static void test_init(void) { int i; - /* fill the structure with crap */ + /* fill the structures with crap */ memset(&ts, 1, sizeof(ts)); + memset(&tt, 1, sizeof(tt)); - /* init the structure */ - throttle_init(&ts, ctx, QEMU_CLOCK_VIRTUAL, - read_timer_cb, write_timer_cb, &ts); + /* init structures */ + throttle_init(&ts); + throttle_timers_init(&tt, ctx, QEMU_CLOCK_VIRTUAL, + read_timer_cb, write_timer_cb, &ts); /* check initialized fields */ - g_assert(ts.clock_type == QEMU_CLOCK_VIRTUAL); - g_assert(ts.timers[0]); - g_assert(ts.timers[1]); + g_assert(tt.clock_type == QEMU_CLOCK_VIRTUAL); + g_assert(tt.timers[0]); + g_assert(tt.timers[1]); /* check other fields where cleared */ g_assert(!ts.previous_leak); @@ -124,17 +127,18 @@ static void test_init(void) g_assert(!ts.cfg.buckets[i].level); } - throttle_destroy(&ts); + throttle_timers_destroy(&tt); } static void test_destroy(void) { int i; - throttle_init(&ts, ctx, QEMU_CLOCK_VIRTUAL, - read_timer_cb, write_timer_cb, &ts); - throttle_destroy(&ts); + throttle_init(&ts); + throttle_timers_init(&tt, ctx, QEMU_CLOCK_VIRTUAL, + read_timer_cb, write_timer_cb, &ts); + throttle_timers_destroy(&tt); for (i = 0; i < 2; i++) { - g_assert(!ts.timers[i]); + g_assert(!tt.timers[i]); } } @@ -170,11 +174,12 @@ static void test_config_functions(void) orig_cfg.op_size = 1; - throttle_init(&ts, ctx, QEMU_CLOCK_VIRTUAL, - read_timer_cb, write_timer_cb, &ts); + throttle_init(&ts); + throttle_timers_init(&tt, ctx, QEMU_CLOCK_VIRTUAL, + read_timer_cb, write_timer_cb, &ts); /* structure reset by throttle_init previous_leak should be null */ g_assert(!ts.previous_leak); - throttle_config(&ts, &orig_cfg); + throttle_config(&ts, &tt, &orig_cfg); /* has previous leak been initialized by throttle_config ? */ g_assert(ts.previous_leak); @@ -182,7 +187,7 @@ static void test_config_functions(void) /* get back the fixed configuration */ throttle_get_config(&ts, &final_cfg); - throttle_destroy(&ts); + throttle_timers_destroy(&tt); g_assert(final_cfg.buckets[THROTTLE_BPS_TOTAL].avg == 153); g_assert(final_cfg.buckets[THROTTLE_BPS_READ].avg == 56); @@ -323,43 +328,47 @@ static void test_is_valid(void) static void test_have_timer(void) { - /* zero the structure */ + /* zero structures */ memset(&ts, 0, sizeof(ts)); + memset(&tt, 0, sizeof(tt)); /* no timer set should return false */ - g_assert(!throttle_have_timer(&ts)); + g_assert(!throttle_timers_are_initialized(&tt)); - /* init the structure */ - throttle_init(&ts, ctx, QEMU_CLOCK_VIRTUAL, - read_timer_cb, write_timer_cb, &ts); + /* init structures */ + throttle_init(&ts); + throttle_timers_init(&tt, ctx, QEMU_CLOCK_VIRTUAL, + read_timer_cb, write_timer_cb, &ts); /* timer set by init should return true */ - g_assert(throttle_have_timer(&ts)); + g_assert(throttle_timers_are_initialized(&tt)); - throttle_destroy(&ts); + throttle_timers_destroy(&tt); } static void test_detach_attach(void) { - /* zero the structure */ + /* zero structures */ memset(&ts, 0, sizeof(ts)); + memset(&tt, 0, sizeof(tt)); /* init the structure */ - throttle_init(&ts, ctx, QEMU_CLOCK_VIRTUAL, - read_timer_cb, write_timer_cb, &ts); + throttle_init(&ts); + throttle_timers_init(&tt, ctx, QEMU_CLOCK_VIRTUAL, + read_timer_cb, write_timer_cb, &ts); /* timer set by init should return true */ - g_assert(throttle_have_timer(&ts)); + g_assert(throttle_timers_are_initialized(&tt)); /* timer should no longer exist after detaching */ - throttle_detach_aio_context(&ts); - g_assert(!throttle_have_timer(&ts)); + throttle_timers_detach_aio_context(&tt); + g_assert(!throttle_timers_are_initialized(&tt)); /* timer should exist again after attaching */ - throttle_attach_aio_context(&ts, ctx); - g_assert(throttle_have_timer(&ts)); + throttle_timers_attach_aio_context(&tt, ctx); + g_assert(throttle_timers_are_initialized(&tt)); - throttle_destroy(&ts); + throttle_timers_destroy(&tt); } static bool do_test_accounting(bool is_ops, /* are we testing bps or ops */ @@ -387,9 +396,10 @@ static bool do_test_accounting(bool is_ops, /* are we testing bps or ops */ cfg.op_size = op_size; - throttle_init(&ts, ctx, QEMU_CLOCK_VIRTUAL, - read_timer_cb, write_timer_cb, &ts); - throttle_config(&ts, &cfg); + throttle_init(&ts); + throttle_timers_init(&tt, ctx, QEMU_CLOCK_VIRTUAL, + read_timer_cb, write_timer_cb, &ts); + throttle_config(&ts, &tt, &cfg); /* account a read */ throttle_account(&ts, false, size); @@ -414,7 +424,7 @@ static bool do_test_accounting(bool is_ops, /* are we testing bps or ops */ return false; } - throttle_destroy(&ts); + throttle_timers_destroy(&tt); return true; } diff --git a/util/throttle.c b/util/throttle.c index f976ac7de5..d76a48e19b 100644 --- a/util/throttle.c +++ b/util/throttle.c @@ -159,29 +159,36 @@ bool throttle_compute_timer(ThrottleState *ts, } /* Add timers to event loop */ -void throttle_attach_aio_context(ThrottleState *ts, AioContext *new_context) +void throttle_timers_attach_aio_context(ThrottleTimers *tt, + AioContext *new_context) { - ts->timers[0] = aio_timer_new(new_context, ts->clock_type, SCALE_NS, - ts->read_timer_cb, ts->timer_opaque); - ts->timers[1] = aio_timer_new(new_context, ts->clock_type, SCALE_NS, - ts->write_timer_cb, ts->timer_opaque); + 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); } /* To be called first on the ThrottleState */ -void throttle_init(ThrottleState *ts, - AioContext *aio_context, - QEMUClockType clock_type, - QEMUTimerCB *read_timer_cb, - QEMUTimerCB *write_timer_cb, - void *timer_opaque) +void throttle_init(ThrottleState *ts) { memset(ts, 0, sizeof(ThrottleState)); +} + +/* To be called first on the ThrottleTimers */ +void throttle_timers_init(ThrottleTimers *tt, + AioContext *aio_context, + QEMUClockType clock_type, + QEMUTimerCB *read_timer_cb, + QEMUTimerCB *write_timer_cb, + void *timer_opaque) +{ + memset(tt, 0, sizeof(ThrottleTimers)); - ts->clock_type = clock_type; - ts->read_timer_cb = read_timer_cb; - ts->write_timer_cb = write_timer_cb; - ts->timer_opaque = timer_opaque; - throttle_attach_aio_context(ts, aio_context); + tt->clock_type = clock_type; + tt->read_timer_cb = read_timer_cb; + tt->write_timer_cb = write_timer_cb; + tt->timer_opaque = timer_opaque; + throttle_timers_attach_aio_context(tt, aio_context); } /* destroy a timer */ @@ -195,25 +202,25 @@ static void throttle_timer_destroy(QEMUTimer **timer) } /* Remove timers from event loop */ -void throttle_detach_aio_context(ThrottleState *ts) +void throttle_timers_detach_aio_context(ThrottleTimers *tt) { int i; for (i = 0; i < 2; i++) { - throttle_timer_destroy(&ts->timers[i]); + throttle_timer_destroy(&tt->timers[i]); } } -/* To be called last on the ThrottleState */ -void throttle_destroy(ThrottleState *ts) +/* To be called last on the ThrottleTimers */ +void throttle_timers_destroy(ThrottleTimers *tt) { - throttle_detach_aio_context(ts); + throttle_timers_detach_aio_context(tt); } /* is any throttling timer configured */ -bool throttle_have_timer(ThrottleState *ts) +bool throttle_timers_are_initialized(ThrottleTimers *tt) { - if (ts->timers[0]) { + if (tt->timers[0]) { return true; } @@ -324,9 +331,12 @@ static void throttle_cancel_timer(QEMUTimer *timer) /* Used to configure the throttle * * @ts: the throttle state we are working on + * @tt: the throttle timers we use in this aio context * @cfg: the config to set */ -void throttle_config(ThrottleState *ts, ThrottleConfig *cfg) +void throttle_config(ThrottleState *ts, + ThrottleTimers *tt, + ThrottleConfig *cfg) { int i; @@ -336,10 +346,10 @@ void throttle_config(ThrottleState *ts, ThrottleConfig *cfg) throttle_fix_bucket(&ts->cfg.buckets[i]); } - ts->previous_leak = qemu_clock_get_ns(ts->clock_type); + ts->previous_leak = qemu_clock_get_ns(tt->clock_type); for (i = 0; i < 2; i++) { - throttle_cancel_timer(ts->timers[i]); + throttle_cancel_timer(tt->timers[i]); } } @@ -358,12 +368,15 @@ void throttle_get_config(ThrottleState *ts, ThrottleConfig *cfg) * * NOTE: this function is not unit tested due to it's usage of timer_mod * + * @tt: the timers structure * @is_write: the type of operation (read/write) * @ret: true if the timer has been scheduled else false */ -bool throttle_schedule_timer(ThrottleState *ts, bool is_write) +bool throttle_schedule_timer(ThrottleState *ts, + ThrottleTimers *tt, + bool is_write) { - int64_t now = qemu_clock_get_ns(ts->clock_type); + int64_t now = qemu_clock_get_ns(tt->clock_type); int64_t next_timestamp; bool must_wait; @@ -378,12 +391,12 @@ bool throttle_schedule_timer(ThrottleState *ts, bool is_write) } /* request throttled and timer pending -> do nothing */ - if (timer_pending(ts->timers[is_write])) { + if (timer_pending(tt->timers[is_write])) { return true; } /* request throttled and timer not pending -> arm timer */ - timer_mod(ts->timers[is_write], next_timestamp); + timer_mod(tt->timers[is_write], next_timestamp); return true; } -- cgit v1.2.3-55-g7522 From 1fee955f9cc5903b3c7f79bbd90929aefad583a6 Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Mon, 8 Jun 2015 18:17:43 +0200 Subject: throttle: Add throttle group infrastructure tests Signed-off-by: Alberto Garcia Message-id: ba7b9dc7fca43efbb31d5f3aad91a8dbdbea635b.1433779731.git.berto@igalia.com Cc: Stefan Hajnoczi Signed-off-by: Stefan Hajnoczi --- tests/test-throttle.c | 81 ++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 71 insertions(+), 10 deletions(-) (limited to 'tests') diff --git a/tests/test-throttle.c b/tests/test-throttle.c index 458f577603..016844546a 100644 --- a/tests/test-throttle.c +++ b/tests/test-throttle.c @@ -1,10 +1,12 @@ /* * Throttle infrastructure tests * - * Copyright Nodalink, SARL. 2013 + * Copyright Nodalink, EURL. 2013-2014 + * Copyright Igalia, S.L. 2015 * * Authors: - * Benoît Canet + * Benoît Canet + * Alberto Garcia * * This work is licensed under the terms of the GNU LGPL, version 2 or later. * See the COPYING.LIB file in the top-level directory. @@ -15,6 +17,7 @@ #include "block/aio.h" #include "qemu/throttle.h" #include "qemu/error-report.h" +#include "block/throttle-groups.h" static AioContext *ctx; static LeakyBucket bkt; @@ -500,23 +503,80 @@ static void test_accounting(void) (64.0 / 13))); } +static void test_groups(void) +{ + ThrottleConfig cfg1, cfg2; + BlockDriverState *bdrv1, *bdrv2, *bdrv3; + + bdrv1 = bdrv_new(); + bdrv2 = bdrv_new(); + bdrv3 = bdrv_new(); + + g_assert(bdrv1->throttle_state == NULL); + g_assert(bdrv2->throttle_state == NULL); + g_assert(bdrv3->throttle_state == NULL); + + throttle_group_register_bs(bdrv1, "bar"); + throttle_group_register_bs(bdrv2, "foo"); + throttle_group_register_bs(bdrv3, "bar"); + + g_assert(bdrv1->throttle_state != NULL); + g_assert(bdrv2->throttle_state != NULL); + g_assert(bdrv3->throttle_state != NULL); + + g_assert(!strcmp(throttle_group_get_name(bdrv1), "bar")); + g_assert(!strcmp(throttle_group_get_name(bdrv2), "foo")); + g_assert(bdrv1->throttle_state == bdrv3->throttle_state); + + /* Setting the config of a group member affects the whole group */ + memset(&cfg1, 0, sizeof(cfg1)); + cfg1.buckets[THROTTLE_BPS_READ].avg = 500000; + cfg1.buckets[THROTTLE_BPS_WRITE].avg = 285000; + cfg1.buckets[THROTTLE_OPS_READ].avg = 20000; + cfg1.buckets[THROTTLE_OPS_WRITE].avg = 12000; + throttle_group_config(bdrv1, &cfg1); + + throttle_group_get_config(bdrv1, &cfg1); + throttle_group_get_config(bdrv3, &cfg2); + g_assert(!memcmp(&cfg1, &cfg2, sizeof(cfg1))); + + cfg2.buckets[THROTTLE_BPS_READ].avg = 4547; + cfg2.buckets[THROTTLE_BPS_WRITE].avg = 1349; + cfg2.buckets[THROTTLE_OPS_READ].avg = 123; + cfg2.buckets[THROTTLE_OPS_WRITE].avg = 86; + throttle_group_config(bdrv3, &cfg1); + + throttle_group_get_config(bdrv1, &cfg1); + throttle_group_get_config(bdrv3, &cfg2); + g_assert(!memcmp(&cfg1, &cfg2, sizeof(cfg1))); + + throttle_group_unregister_bs(bdrv1); + throttle_group_unregister_bs(bdrv2); + throttle_group_unregister_bs(bdrv3); + + g_assert(bdrv1->throttle_state == NULL); + g_assert(bdrv2->throttle_state == NULL); + g_assert(bdrv3->throttle_state == NULL); +} + int main(int argc, char **argv) { - GSource *src; Error *local_error = NULL; - init_clocks(); + qemu_init_main_loop(&local_error); + ctx = qemu_get_aio_context(); - ctx = aio_context_new(&local_error); if (!ctx) { error_report("Failed to create AIO Context: '%s'", - error_get_pretty(local_error)); - error_free(local_error); + local_error ? error_get_pretty(local_error) : + "Failed to initialize the QEMU main loop"); + if (local_error) { + error_free(local_error); + } exit(1); } - src = aio_get_g_source(ctx); - g_source_attach(src, NULL); - g_source_unref(src); + + bdrv_init(); do {} while (g_main_context_iteration(NULL, false)); @@ -533,6 +593,7 @@ int main(int argc, char **argv) g_test_add_func("/throttle/config/is_valid", test_is_valid); g_test_add_func("/throttle/config_functions", test_config_functions); g_test_add_func("/throttle/accounting", test_accounting); + g_test_add_func("/throttle/groups", test_groups); return g_test_run(); } -- cgit v1.2.3-55-g7522 From 2db33f88d2b340c049c576ad75d442e4b6ffe768 Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Mon, 8 Jun 2015 18:17:48 +0200 Subject: qemu-iotests: expand test 093 to support group throttling This patch improves the test by attaching a different number of drives to the VM and putting them in the same throttling group. The test verifies that the I/O is evenly distributed among all members of the group, and that the limits are enforced. By default the test is repeated 3 times with 1, 2 and 3 drives, but the maximum number of simultaneous drives is configurable. Signed-off-by: Alberto Garcia Reviewed-by: Stefan Hajnoczi Message-id: 513df1da5c658878191b579ebcddd985adcd4122.1433779731.git.berto@igalia.com Signed-off-by: Stefan Hajnoczi --- tests/qemu-iotests/093 | 93 +++++++++++++++++++++++++++++++++++--------------- 1 file changed, 65 insertions(+), 28 deletions(-) (limited to 'tests') diff --git a/tests/qemu-iotests/093 b/tests/qemu-iotests/093 index b9096a55d4..c0e9e2b0b5 100755 --- a/tests/qemu-iotests/093 +++ b/tests/qemu-iotests/093 @@ -3,6 +3,7 @@ # Tests for IO throttling # # Copyright (C) 2015 Red Hat, Inc. +# Copyright (C) 2015 Igalia, S.L. # # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -22,6 +23,7 @@ import iotests class ThrottleTestCase(iotests.QMPTestCase): test_img = "null-aio://" + max_drives = 3 def blockstats(self, device): result = self.vm.qmp("query-blockstats") @@ -32,26 +34,31 @@ class ThrottleTestCase(iotests.QMPTestCase): raise Exception("Device not found for blockstats: %s" % device) def setUp(self): - self.vm = iotests.VM().add_drive(self.test_img) + self.vm = iotests.VM() + for i in range(0, self.max_drives): + self.vm.add_drive(self.test_img) self.vm.launch() def tearDown(self): self.vm.shutdown() - def do_test_throttle(self, seconds, params): + def do_test_throttle(self, ndrives, seconds, params): def check_limit(limit, num): # IO throttling algorithm is discrete, allow 10% error so the test # is more robust return limit == 0 or \ - (num < seconds * limit * 1.1 - and num > seconds * limit * 0.9) + (num < seconds * limit * 1.1 / ndrives + and num > seconds * limit * 0.9 / ndrives) nsec_per_sec = 1000000000 - params['device'] = 'drive0' + params['group'] = 'test' - result = self.vm.qmp("block_set_io_throttle", conv_keys=False, **params) - self.assert_qmp(result, 'return', {}) + # Set the I/O throttling parameters to all drives + for i in range(0, ndrives): + params['device'] = 'drive%d' % i + result = self.vm.qmp("block_set_io_throttle", conv_keys=False, **params) + self.assert_qmp(result, 'return', {}) # Set vm clock to a known value ns = seconds * nsec_per_sec @@ -66,32 +73,60 @@ class ThrottleTestCase(iotests.QMPTestCase): params['iops'] / 2, params['iops_rd']) rd_nr *= seconds * 2 + rd_nr /= ndrives wr_nr = max(params['bps'] / rq_size / 2, params['bps_wr'] / rq_size, params['iops'] / 2, params['iops_wr']) wr_nr *= seconds * 2 + wr_nr /= ndrives + + # Send I/O requests to all drives for i in range(rd_nr): - self.vm.hmp_qemu_io("drive0", "aio_read %d %d" % (i * rq_size, rq_size)) - for i in range(wr_nr): - self.vm.hmp_qemu_io("drive0", "aio_write %d %d" % (i * rq_size, rq_size)) + for drive in range(0, ndrives): + self.vm.hmp_qemu_io("drive%d" % drive, "aio_read %d %d" % + (i * rq_size, rq_size)) - start_rd_bytes, start_rd_iops, start_wr_bytes, start_wr_iops = self.blockstats('drive0') + for i in range(wr_nr): + for drive in range(0, ndrives): + self.vm.hmp_qemu_io("drive%d" % drive, "aio_write %d %d" % + (i * rq_size, rq_size)) + + # We'll store the I/O stats for each drive in these arrays + start_rd_bytes = [0] * ndrives + start_rd_iops = [0] * ndrives + start_wr_bytes = [0] * ndrives + start_wr_iops = [0] * ndrives + end_rd_bytes = [0] * ndrives + end_rd_iops = [0] * ndrives + end_wr_bytes = [0] * ndrives + end_wr_iops = [0] * ndrives + + # Read the stats before advancing the clock + for i in range(0, ndrives): + start_rd_bytes[i], start_rd_iops[i], start_wr_bytes[i], \ + start_wr_iops[i] = self.blockstats('drive%d' % i) self.vm.qtest("clock_step %d" % ns) - end_rd_bytes, end_rd_iops, end_wr_bytes, end_wr_iops = self.blockstats('drive0') - - rd_bytes = end_rd_bytes - start_rd_bytes - rd_iops = end_rd_iops - start_rd_iops - wr_bytes = end_wr_bytes - start_wr_bytes - wr_iops = end_wr_iops - start_wr_iops - self.assertTrue(check_limit(params['bps'], rd_bytes + wr_bytes)) - self.assertTrue(check_limit(params['bps_rd'], rd_bytes)) - self.assertTrue(check_limit(params['bps_wr'], wr_bytes)) - self.assertTrue(check_limit(params['iops'], rd_iops + wr_iops)) - self.assertTrue(check_limit(params['iops_rd'], rd_iops)) - self.assertTrue(check_limit(params['iops_wr'], wr_iops)) + # Read the stats after advancing the clock + for i in range(0, ndrives): + end_rd_bytes[i], end_rd_iops[i], end_wr_bytes[i], \ + end_wr_iops[i] = self.blockstats('drive%d' % i) + + # Check that the I/O is within the limits and evenly distributed + for i in range(0, ndrives): + rd_bytes = end_rd_bytes[i] - start_rd_bytes[i] + rd_iops = end_rd_iops[i] - start_rd_iops[i] + wr_bytes = end_wr_bytes[i] - start_wr_bytes[i] + wr_iops = end_wr_iops[i] - start_wr_iops[i] + + self.assertTrue(check_limit(params['bps'], rd_bytes + wr_bytes)) + self.assertTrue(check_limit(params['bps_rd'], rd_bytes)) + self.assertTrue(check_limit(params['bps_wr'], wr_bytes)) + self.assertTrue(check_limit(params['iops'], rd_iops + wr_iops)) + self.assertTrue(check_limit(params['iops_rd'], rd_iops)) + self.assertTrue(check_limit(params['iops_wr'], wr_iops)) def test_all(self): params = {"bps": 4096, @@ -101,11 +136,13 @@ class ThrottleTestCase(iotests.QMPTestCase): "iops_rd": 10, "iops_wr": 10, } - # Pick each out of all possible params and test - for tk in params: - limits = dict([(k, 0) for k in params]) - limits[tk] = params[tk] - self.do_test_throttle(5, limits) + # Repeat the test with different numbers of drives + for ndrives in range(1, self.max_drives + 1): + # Pick each out of all possible params and test + for tk in params: + limits = dict([(k, 0) for k in params]) + limits[tk] = params[tk] * ndrives + self.do_test_throttle(ndrives, 5, limits) class ThrottleTestCoroutine(ThrottleTestCase): test_img = "null-co://" -- cgit v1.2.3-55-g7522