Re: [Qemu-block] [Qemu-devel] [PATCH] tests: Set read-zeroes on for null-co driver

[Thomas Huth]

On 7/30/19 3:23 PM, Andrey Shinkevich wrote:
> 
> 
> On 30/07/2019 15:59, Thomas Huth wrote:
>> On 30/07/2019 14.52, Thomas Huth wrote:
>>> On 29/07/2019 14.46, Andrey Shinkevich wrote:
>>>> This patch is to reduce the number of Valgrind report messages about
>>>> using uninitialized memory with the null-co driver. It helps to filter
>>>> real memory issues and is the same work done for the iotests with the
>>>> commit ID a6862418fec4072.
>>>>
>>>> Suggested-by: Kevin Wolf <address@hidden>
>>>> Signed-off-by: Andrey Shinkevich <address@hidden>
>>>> ---
>>> [...]
>>>> diff --git a/tests/test-blockjob-txn.c b/tests/test-blockjob-txn.c
>>>> index 86606f9..7da9216 100644
>>>> --- a/tests/test-blockjob-txn.c
>>>> +++ b/tests/test-blockjob-txn.c
>>>> @@ -15,6 +15,7 @@
>>>>   #include "qemu/main-loop.h"
>>>>   #include "block/blockjob_int.h"
>>>>   #include "sysemu/block-backend.h"
>>>> +#include "qapi/qmp/qdict.h"
>>>>   
>>>>   typedef struct {
>>>>       BlockJob common;
>>>> @@ -96,7 +97,9 @@ static BlockJob *test_block_job_start(unsigned int 
>>>> iterations,
>>>>   
>>>>       data = g_new0(TestBlockJobCBData, 1);
>>>>   
>>>> -    bs = bdrv_open("null-co://", NULL, NULL, 0, &error_abort);
>>>> +    QDict *opt = qdict_new();
>>>> +    qdict_put_str(opt, "file.read-zeroes", "on");
>>>> +    bs = bdrv_open("null-co://", NULL, opt, 0, &error_abort);
>>>>       g_assert_nonnull(bs);
>>>
>>> Not sure, but don't you need to also qdict_destroy_obj(opt) at the end
>>> to avoid leaking memory? (Also in the other spots where you use
>>> qdict_new() ...)
>>
>> Never mind, seems like bdrv_open() is doing an qobject_unref() on the
>> options at the end, so I guess this should be fine...
>>
>> So in that case:
>>
>> Acked-by: Thomas Huth <address@hidden>
>>
>> (I assume this will be taken through the block tree? Let me know if you
>> prefer the qtest tree instead)
> 
> Thanks, Thomas!
> I have run the patched program under the Valgrind to double check for 
> memory issues - no leak reported.
> As for the branch, I would be happy with either.

Ok, FYI, I've queued this patch now in the qtest branch:

 https://gitlab.com/huth/qemu/commits/qtest-next

 Thomas