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

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

[Andrey Shinkevich]

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 <kwolf@redhat.com>
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
 tests/drive_del-test.c     | 3 ++-
 tests/libqos/virtio-scsi.c | 3 ++-
 tests/megasas-test.c       | 3 ++-
 tests/nvme-test.c          | 3 ++-
 tests/qmp-test.c           | 2 +-
 tests/test-blockjob-txn.c  | 5 ++++-
 tests/test-blockjob.c      | 5 ++++-
 tests/usb-hcd-uhci-test.c  | 3 ++-
 tests/usb-hcd-xhci-test.c  | 3 ++-
 tests/virtio-blk-test.c    | 6 ++++--
 tests/virtio-ccw-test.c    | 9 ++++++---
 tests/virtio-scsi-test.c   | 6 ++++--
 12 files changed, 35 insertions(+), 16 deletions(-)

diff --git a/tests/drive_del-test.c b/tests/drive_del-test.c
index b56b223..5f8839b 100644
--- a/tests/drive_del-test.c
+++ b/tests/drive_del-test.c
@@ -121,7 +121,8 @@ static void test_drive_del_device_del(void)
     QTestState *qts;
 
     /* Start with a drive used by a device that unplugs instantaneously */
-    qts = qtest_initf("-drive if=none,id=drive0,file=null-co://,format=raw"
+    qts = qtest_initf("-drive if=none,id=drive0,file=null-co://,"
+                      "file.read-zeroes=on,format=raw"
                       " -device virtio-scsi-%s"
                       " -device scsi-hd,drive=drive0,id=dev0",
                       qvirtio_get_dev_type());
diff --git a/tests/libqos/virtio-scsi.c b/tests/libqos/virtio-scsi.c
index 94842ec..de739be 100644
--- a/tests/libqos/virtio-scsi.c
+++ b/tests/libqos/virtio-scsi.c
@@ -95,7 +95,8 @@ static void virtio_scsi_register_nodes(void)
     };
 
     QOSGraphEdgeOptions opts = {
-        .before_cmd_line = "-drive id=drv0,if=none,file=null-co://,format=raw",
+        .before_cmd_line = "-drive id=drv0,if=none,file=null-co://,"
+                           "file.read-zeroes=on,format=raw",
         .after_cmd_line = "-device scsi-hd,bus=vs0.0,drive=drv0",
     };
 
diff --git a/tests/megasas-test.c b/tests/megasas-test.c
index c3e4ab6..d6796b9 100644
--- a/tests/megasas-test.c
+++ b/tests/megasas-test.c
@@ -75,7 +75,8 @@ static void megasas_register_nodes(void)
 {
     QOSGraphEdgeOptions opts = {
         .extra_device_opts = "addr=04.0,id=scsi0",
-        .before_cmd_line = "-drive id=drv0,if=none,file=null-co://,format=raw",
+        .before_cmd_line = "-drive id=drv0,if=none,file=null-co://,"
+                           "file.read-zeroes=on,format=raw",
         .after_cmd_line = "-device scsi-hd,bus=scsi0.0,drive=drv0",
     };
 
diff --git a/tests/nvme-test.c b/tests/nvme-test.c
index 5052993..ff04421 100644
--- a/tests/nvme-test.c
+++ b/tests/nvme-test.c
@@ -70,7 +70,8 @@ static void nvme_register_nodes(void)
 {
     QOSGraphEdgeOptions opts = {
         .extra_device_opts = "addr=04.0,drive=drv0,serial=foo",
-        .before_cmd_line = "-drive id=drv0,if=none,file=null-co://,format=raw",
+        .before_cmd_line = "-drive id=drv0,if=none,file=null-co://,"
+                           "file.read-zeroes=on,format=raw",
     };
 
     add_qpci_address(&opts, &(QPCIAddress) { .devfn = QPCI_DEVFN(4, 0) });
diff --git a/tests/qmp-test.c b/tests/qmp-test.c
index 48a4fa7..1b0eb69 100644
--- a/tests/qmp-test.c
+++ b/tests/qmp-test.c
@@ -187,7 +187,7 @@ static void send_cmd_that_blocks(QTestState *s, const char *id)
                    " 'arguments': {"
                    " 'driver': 'blkdebug', 'node-name': %s,"
                    " 'config': %s,"
-                   " 'image': { 'driver': 'null-co' } } }",
+                   " 'image': { 'driver': 'null-co', 'read-zeroes': true } } }",
                    id, id, fifo_name);
 }
 
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);
 
     snprintf(job_id, sizeof(job_id), "job%u", counter++);
diff --git a/tests/test-blockjob.c b/tests/test-blockjob.c
index b33f899..68a0819 100644
--- a/tests/test-blockjob.c
+++ b/tests/test-blockjob.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"
 
 static const BlockJobDriver test_block_job_driver = {
     .job_driver = {
@@ -71,7 +72,9 @@ static BlockBackend *create_blk(const char *name)
     BlockBackend *blk = blk_new(qemu_get_aio_context(), 0, BLK_PERM_ALL);
     BlockDriverState *bs;
 
-    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);
 
     blk_insert_bs(blk, bs, &error_abort);
diff --git a/tests/usb-hcd-uhci-test.c b/tests/usb-hcd-uhci-test.c
index a119d6d..6d355c2 100644
--- a/tests/usb-hcd-uhci-test.c
+++ b/tests/usb-hcd-uhci-test.c
@@ -57,7 +57,8 @@ int main(int argc, char **argv)
 {
     const char *arch = qtest_get_arch();
     const char *cmd = "-device piix3-usb-uhci,id=uhci,addr=1d.0"
-                      " -drive id=drive0,if=none,file=null-co://,format=raw"
+                      " -drive id=drive0,if=none,file=null-co://,"
+                      "file.read-zeroes=on,format=raw"
                       " -device usb-tablet,bus=uhci.0,port=1";
     int ret;
 
diff --git a/tests/usb-hcd-xhci-test.c b/tests/usb-hcd-xhci-test.c
index 9eb24b0..b15a511 100644
--- a/tests/usb-hcd-xhci-test.c
+++ b/tests/usb-hcd-xhci-test.c
@@ -56,7 +56,8 @@ int main(int argc, char **argv)
     qtest_add_func("/xhci/pci/hotplug/usb-ccid", test_usb_ccid_hotplug);
 
     qtest_start("-device nec-usb-xhci,id=xhci"
-                " -drive id=drive0,if=none,file=null-co://,format=raw");
+                " -drive id=drive0,if=none,file=null-co://,"
+                "file.read-zeroes=on,format=raw");
     ret = g_test_run();
     qtest_end();
 
diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index 1b02714..1589ab0 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -753,8 +753,10 @@ static void *virtio_blk_test_setup(GString *cmd_line, void *arg)
     char *tmp_path = drive_create();
 
     g_string_append_printf(cmd_line,
-                           " -drive if=none,id=drive0,file=%s,format=raw,auto-read-only=off "
-                           "-drive if=none,id=drive1,file=null-co://,format=raw ",
+                           " -drive if=none,id=drive0,file=%s,"
+                           "format=raw,auto-read-only=off "
+                           "-drive if=none,id=drive1,file=null-co://,"
+                           "file.read-zeroes=on,format=raw ",
                            tmp_path);
 
     return arg;
diff --git a/tests/virtio-ccw-test.c b/tests/virtio-ccw-test.c
index 48c714d..6be4e6a 100644
--- a/tests/virtio-ccw-test.c
+++ b/tests/virtio-ccw-test.c
@@ -53,7 +53,8 @@ static void virtio_serial_hotplug(void)
 
 static void virtio_blk_nop(void)
 {
-    global_qtest = qtest_initf("-drive if=none,id=drv0,file=null-co://,format=raw "
+    global_qtest = qtest_initf("-drive if=none,id=drv0,file=null-co://,"
+                               "file.read-zeroes=on,format=raw "
                                 "-device virtio-blk-ccw,drive=drv0");
     qtest_end();
 }
@@ -78,8 +79,10 @@ static void virtio_scsi_nop(void)
 
 static void virtio_scsi_hotplug(void)
 {
-    global_qtest = qtest_initf("-drive if=none,id=drv0,file=null-co://,format=raw "
-                                "-drive if=none,id=drv1,file=null-co://,format=raw "
+    global_qtest = qtest_initf("-drive if=none,id=drv0,file=null-co://,"
+                               "file.read-zeroes=on,format=raw "
+                                "-drive if=none,id=drv1,file=null-co://,"
+                                "file.read-zeroes=on,format=raw "
                                 "-device virtio-scsi-ccw "
                                 "-device scsi-hd,drive=drv0");
     qtest_qmp_device_add("scsi-hd", "scsihd", "{'drive': 'drv1'}");
diff --git a/tests/virtio-scsi-test.c b/tests/virtio-scsi-test.c
index 1ad9f75..ae6698d 100644
--- a/tests/virtio-scsi-test.c
+++ b/tests/virtio-scsi-test.c
@@ -239,7 +239,8 @@ fail:
 static void *virtio_scsi_hotplug_setup(GString *cmd_line, void *arg)
 {
     g_string_append(cmd_line,
-                    " -drive id=drv1,if=none,file=null-co://,format=raw");
+                    " -drive id=drv1,if=none,file=null-co://,"
+                    "file.read-zeroes=on,format=raw");
     return arg;
 }
 
@@ -247,6 +248,7 @@ static void *virtio_scsi_setup(GString *cmd_line, void *arg)
 {
     g_string_append(cmd_line,
                     " -drive file=blkdebug::null-co://,"
+                    "file.image.read-zeroes=on,"
                     "if=none,id=dr1,format=raw,file.align=4k "
                     "-device scsi-hd,drive=dr1,lun=0,scsi-id=1");
     return arg;
@@ -256,7 +258,7 @@ static void *virtio_scsi_setup_iothread(GString *cmd_line, void *arg)
 {
     g_string_append(cmd_line,
                     " -object iothread,id=thread0"
-                    " -blockdev driver=null-co,node-name=null0"
+                    " -blockdev driver=null-co,read-zeroes=on,node-name=null0"
                     " -device scsi-hd,drive=null0");
     return arg;
 }
-- 
1.8.3.1

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

[Thomas Huth]

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 <kwolf@redhat.com>
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---
[...]
> 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() ...)

 Thomas

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

[Thomas Huth]

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 <kwolf@redhat.com>
>> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
>> ---
> [...]
>> 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 <thuth@redhat.com>

(I assume this will be taken through the block tree? Let me know if you
prefer the qtest tree instead)

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

[Andrey Shinkevich]

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 <kwolf@redhat.com>
>>> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
>>> ---
>> [...]
>>> 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 <thuth@redhat.com>
> 
> (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.

Andey
-- 
With the best regards,
Andrey Shinkevich

Re: [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 <kwolf@redhat.com>
>>>> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
>>>> ---
>>> [...]
>>>> 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 <thuth@redhat.com>
>>
>> (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

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

[Andrey Shinkevich]

On 13/08/2019 11:46, Thomas Huth wrote:
> 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 <kwolf@redhat.com>
>>>>> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
>>>>> ---
>>>> [...]
>>>>> 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 <thuth@redhat.com>
>>>
>>> (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
> 

Thank you Thomas,
I appreciate.

Andrey
-- 
With the best regards,
Andrey Shinkevich