|
From: | Vladimir Sementsov-Ogievskiy |
Subject: | Re: [PATCH v2 3/9] test-write-threshold: rewrite test_threshold_(not_)trigger tests |
Date: | Wed, 5 May 2021 18:20:20 +0300 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.0 |
05.05.2021 17:28, Max Reitz wrote:
On 04.05.21 10:25, Vladimir Sementsov-Ogievskiy wrote:These tests use bdrv_write_threshold_exceeded() API, which is used only for test.Well, now. That used to be different before patch 1.Better is testing real API, which is used in block.c as well. So, let's call bdrv_write_threshold_check_write(), and check is bs->write_threshold_offset cleared or not (it's cleared iff threshold triggered). Also we get rid of BdrvTrackedRequest use here. Tracked requests are unrelated to write-threshold since we get rid of write notifiers.The purpose behind the BdrvTrackedRequest was clearly because this is the object bdrv_write_threshold_exceeded() expected. This reads like there was some other purpose (i.e. actually tracked requests), but there wasn’t. The question that comes to my mind is why we had the bdrv_check_request() calls there, and why this patch removes them. They seem unrelated to the write threshold, but someone must have thought something when adding them.
I wanted to add a note for it but forget. Something like: "Such small requests are obviously good, no reason to check them" :)
Looking into git blame, that someone is you :) (8b1170012b1)
Oops:) When I read your "but someone must have thought something", I really doubt in it :) But that was me, and I really thought something. Respect for your punctuality!
Looks like you added those calls because BdrvTrackedRequest is a block layer structure, so getting rid of it means the reason for bdrv_check_request() disappears. OK.
Yes, I was worried about the fact that BdrvTrackedRequest is a public structure and is a potential backdoor for not-checked offset/bytes things. At some moment we'd better close it (hide structure and add some interfaces).
Reviewed-by: Max Reitz <mreitz@redhat.com>Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> --- tests/unit/test-write-threshold.c | 22 ++++------------------ 1 file changed, 4 insertions(+), 18 deletions(-) diff --git a/tests/unit/test-write-threshold.c b/tests/unit/test-write-threshold.c index fc1c45a2eb..fd40a815b8 100644 --- a/tests/unit/test-write-threshold.c +++ b/tests/unit/test-write-threshold.c @@ -55,41 +55,27 @@ static void test_threshold_multi_set_get(void) static void test_threshold_not_trigger(void) { - uint64_t amount = 0; uint64_t threshold = 4 * 1024 * 1024; BlockDriverState bs; - BdrvTrackedRequest req; memset(&bs, 0, sizeof(bs)); - memset(&req, 0, sizeof(req)); - req.offset = 1024; - req.bytes = 1024; - - bdrv_check_request(req.offset, req.bytes, &error_abort); bdrv_write_threshold_set(&bs, threshold); - amount = bdrv_write_threshold_exceeded(&bs, &req); - g_assert_cmpuint(amount, ==, 0); + bdrv_write_threshold_check_write(&bs, 1024, 1024); + g_assert_cmpuint(bdrv_write_threshold_get(&bs), ==, threshold); } static void test_threshold_trigger(void) { - uint64_t amount = 0; uint64_t threshold = 4 * 1024 * 1024; BlockDriverState bs; - BdrvTrackedRequest req; memset(&bs, 0, sizeof(bs)); - memset(&req, 0, sizeof(req)); - req.offset = (4 * 1024 * 1024) - 1024; - req.bytes = 2 * 1024; - - bdrv_check_request(req.offset, req.bytes, &error_abort); bdrv_write_threshold_set(&bs, threshold); - amount = bdrv_write_threshold_exceeded(&bs, &req); - g_assert_cmpuint(amount, >=, 1024); + bdrv_write_threshold_check_write(&bs, threshold - 1024, 2 * 1024); + g_assert_cmpuint(bdrv_write_threshold_get(&bs), ==, 0); } typedef struct TestStruct {
-- Best regards, Vladimir
[Prev in Thread] | Current Thread | [Next in Thread] |