Re: [Qemu-block] [PATCH v4 3/5] block/qcow2: refactor qcow2_co_preadv_part

[Max Reitz]

On 13.09.19 12:53, Vladimir Sementsov-Ogievskiy wrote:
> 13.09.2019 13:01, Kevin Wolf wrote:
>> Am 16.08.2019 um 17:30 hat Vladimir Sementsov-Ogievskiy geschrieben:
>>> Further patch will run partial requests of iterations of
>>> qcow2_co_preadv in parallel for performance reasons. To prepare for
>>> this, separate part which may be parallelized into separate function
>>> (qcow2_co_preadv_task).
>>>
>>> While being here, also separate encrypted clusters reading to own
>>> function, like it is done for compressed reading.
>>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
>>> Reviewed-by: Max Reitz <address@hidden>
>>> ---
>>>   qapi/block-core.json |   2 +-
>>>   block/qcow2.c        | 205 +++++++++++++++++++++++--------------------
>>>   2 files changed, 111 insertions(+), 96 deletions(-)
>>>
>>> diff --git a/qapi/block-core.json b/qapi/block-core.json
>>> index 0d43d4f37c..dd80aa11db 100644
>>> --- a/qapi/block-core.json
>>> +++ b/qapi/block-core.json
>>> @@ -3266,7 +3266,7 @@
>>>               'pwritev_rmw_tail', 'pwritev_rmw_after_tail', 'pwritev',
>>>               'pwritev_zero', 'pwritev_done', 'empty_image_prepare',
>>>               'l1_shrink_write_table', 'l1_shrink_free_l2_clusters',
>>> -            'cor_write', 'cluster_alloc_space', 'none'] }
>>> +            'cor_write', 'cluster_alloc_space', 'none', 'read_encrypted'] }
>>
>> What's the point of this new blkdebug event?
>>
>> Obviously, read_aio for an encrypted image must mean a read of encrypted
>> data. The same image can never trigger both read_aio and
>> read_encrypted, so why do we need to distinguish them as two different
>> events?
>>
> 
> Seems I just done it looking at qcow2_co_preadv_compressed..
> 
> Anyway, I think you are right, so, I don't mind if Max drops this new event
> and use read_aio in his branch, or I can resend the series or send a 
> follow-up,
> whichever you prefer.

Should I squash this in?

diff --git a/qapi/block-core.json b/qapi/block-core.json
index d9ae73a43c..e6edd641f1 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3264,7 +3264,7 @@
             'pwritev_rmw_tail', 'pwritev_rmw_after_tail', 'pwritev',
             'pwritev_zero', 'pwritev_done', 'empty_image_prepare',
             'l1_shrink_write_table', 'l1_shrink_free_l2_clusters',
-            'cor_write', 'cluster_alloc_space', 'none', 'read_encrypted'] }
+            'cor_write', 'cluster_alloc_space', 'none'] }

 ##
 # @BlkdebugIOType:
diff --git a/block/qcow2.c b/block/qcow2.c
index b5fe014b20..c07ce84d54 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2001,7 +2001,7 @@ qcow2_co_preadv_encrypted(BlockDriverState *bs,
         return -ENOMEM;
     }

-    BLKDBG_EVENT(bs->file, BLKDBG_READ_ENCRYPTED);
+    BLKDBG_EVENT(bs->file, BLKDBG_READ_AIO);
     ret = bdrv_co_pread(s->data_file,
                         file_cluster_offset + offset_into_cluster(s,
offset),
                         bytes, buf, 0);

signature.asc
Description: OpenPGP digital signature