[PATCH 1/2] block/throttle-groups: use QEMU_CLOCK_REALTIME for qtest too

Vladimir Sementsov-Ogievskiy posted 2 patches 3 years, 10 months ago
Maintainers: Alberto Garcia <berto@igalia.com>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>
[PATCH 1/2] block/throttle-groups: use QEMU_CLOCK_REALTIME for qtest too
Posted by Vladimir Sementsov-Ogievskiy 3 years, 10 months ago
Virtual clock just doesn't tick for iotests, and throttling just not
work. Let's use realtime clock.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@openvz.org>
---
 block/throttle-groups.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/block/throttle-groups.c b/block/throttle-groups.c
index fb203c3ced..029158d797 100644
--- a/block/throttle-groups.c
+++ b/block/throttle-groups.c
@@ -753,10 +753,6 @@ static void throttle_group_obj_init(Object *obj)
     ThrottleGroup *tg = THROTTLE_GROUP(obj);
 
     tg->clock_type = QEMU_CLOCK_REALTIME;
-    if (qtest_enabled()) {
-        /* For testing block IO throttling only */
-        tg->clock_type = QEMU_CLOCK_VIRTUAL;
-    }
     tg->is_initialized = false;
     qemu_mutex_init(&tg->lock);
     throttle_init(&tg->ts);
-- 
2.35.1
Re: [PATCH 1/2] block/throttle-groups: use QEMU_CLOCK_REALTIME for qtest too
Posted by Hanna Reitz 3 years, 10 months ago
On 06.04.22 17:32, Vladimir Sementsov-Ogievskiy wrote:
> Virtual clock just doesn't tick for iotests, and throttling just not
> work. Let's use realtime clock.

It does tick when you make it take, specifically with the clock_step 
qtest command.  093 does this, and so with this patch, it fails, because 
it is no longer deterministic.

So far, if I needed realtime throttling, I simply switched the 
accelerator to tcg (e.g. in stream-error-on-reset).

I’m not really opposed to this, but it does break 093, and without 
looking too closely into it, I would guess that it’d be difficult to 
rewrite 093 in a deterministic way without it relying on throttling 
using the virtual clock.  (A runtime option for the throttle-group 
object to choose the clock type might be an option.)

> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@openvz.org>
> ---
>   block/throttle-groups.c | 4 ----
>   1 file changed, 4 deletions(-)
>
> diff --git a/block/throttle-groups.c b/block/throttle-groups.c
> index fb203c3ced..029158d797 100644
> --- a/block/throttle-groups.c
> +++ b/block/throttle-groups.c
> @@ -753,10 +753,6 @@ static void throttle_group_obj_init(Object *obj)
>       ThrottleGroup *tg = THROTTLE_GROUP(obj);
>   
>       tg->clock_type = QEMU_CLOCK_REALTIME;
> -    if (qtest_enabled()) {
> -        /* For testing block IO throttling only */
> -        tg->clock_type = QEMU_CLOCK_VIRTUAL;
> -    }
>       tg->is_initialized = false;
>       qemu_mutex_init(&tg->lock);
>       throttle_init(&tg->ts);


Re: [PATCH 1/2] block/throttle-groups: use QEMU_CLOCK_REALTIME for qtest too
Posted by Vladimir Sementsov-Ogievskiy 3 years, 10 months ago
Thanks for explanation!

07.04.2022 09:42, Hanna Reitz wrote:
> On 06.04.22 17:32, Vladimir Sementsov-Ogievskiy wrote:
>> Virtual clock just doesn't tick for iotests, and throttling just not
>> work. Let's use realtime clock.
> 
> It does tick when you make it take, specifically with the clock_step qtest command.  093 does this, and so with this patch, it fails, because it is no longer deterministic.
> 
> So far, if I needed realtime throttling, I simply switched the accelerator to tcg (e.g. in stream-error-on-reset).

Hm, I tried but it doesn't help (Add vm.add_args('-accel', 'tcg') before vm.launch() in the test), as " -accel qtest" is kept anyway, and therefore do_configure_accelerator is called for qtest and finally qtest_allowed is set to true.

But using QEMUMachine class instead of VM helps.

> 
> I’m not really opposed to this, but it does break 093, and without looking too closely into it, I would guess that it’d be difficult to rewrite 093 in a deterministic way without it relying on throttling using the virtual clock.  (A runtime option for the throttle-group object to choose the clock type might be an option.)

OK, I don't think we need these patches now.

> 
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@openvz.org>
>> ---
>>   block/throttle-groups.c | 4 ----
>>   1 file changed, 4 deletions(-)
>>
>> diff --git a/block/throttle-groups.c b/block/throttle-groups.c
>> index fb203c3ced..029158d797 100644
>> --- a/block/throttle-groups.c
>> +++ b/block/throttle-groups.c
>> @@ -753,10 +753,6 @@ static void throttle_group_obj_init(Object *obj)
>>       ThrottleGroup *tg = THROTTLE_GROUP(obj);
>>       tg->clock_type = QEMU_CLOCK_REALTIME;
>> -    if (qtest_enabled()) {
>> -        /* For testing block IO throttling only */
>> -        tg->clock_type = QEMU_CLOCK_VIRTUAL;
>> -    }
>>       tg->is_initialized = false;
>>       qemu_mutex_init(&tg->lock);
>>       throttle_init(&tg->ts);
> 


-- 
Best regards,
Vladimir