diff mbox series

[v12,11/14] copy-on-read: add support for read flags to COR-filter

Message ID 1603390423-980205-12-git-send-email-andrey.shinkevich@virtuozzo.com (mailing list archive)
State New, archived
Headers show
Series Apply COR-filter to the block-stream permanently | expand

Commit Message

Andrey Shinkevich Oct. 22, 2020, 6:13 p.m. UTC
Add the BDRV_REQ_COPY_ON_READ and BDRV_REQ_PREFETCH flags to the
supported_read_flags of the COR-filter.

Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
 block/copy-on-read.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Vladimir Sementsov-Ogievskiy Oct. 27, 2020, 2:46 p.m. UTC | #1
22.10.2020 21:13, Andrey Shinkevich wrote:
> Add the BDRV_REQ_COPY_ON_READ and BDRV_REQ_PREFETCH flags to the
> supported_read_flags of the COR-filter.
> 
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---
>   block/copy-on-read.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/block/copy-on-read.c b/block/copy-on-read.c
> index 8178a91..a2b180a 100644
> --- a/block/copy-on-read.c
> +++ b/block/copy-on-read.c
> @@ -50,6 +50,8 @@ static int cor_open(BlockDriverState *bs, QDict *options, int flags,
>           return -EINVAL;
>       }
>   
> +    bs->supported_read_flags = BDRV_REQ_COPY_ON_READ | BDRV_REQ_PREFETCH;
> +
>       bs->supported_write_flags = BDRV_REQ_WRITE_UNCHANGED |
>           (BDRV_REQ_FUA & bs->file->bs->supported_write_flags);
>   
> 

This should be merged with the following patch, otherwise it doesn't make sense. You mark filter as supporting PREFETCH, but actually it just ignores it (and may crash on trying to read into qiov=NULL).
Vladimir Sementsov-Ogievskiy Oct. 27, 2020, 2:56 p.m. UTC | #2
27.10.2020 17:46, Vladimir Sementsov-Ogievskiy wrote:
> 22.10.2020 21:13, Andrey Shinkevich wrote:
>> Add the BDRV_REQ_COPY_ON_READ and BDRV_REQ_PREFETCH flags to the
>> supported_read_flags of the COR-filter.
>>
>> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
>> ---
>>   block/copy-on-read.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/block/copy-on-read.c b/block/copy-on-read.c
>> index 8178a91..a2b180a 100644
>> --- a/block/copy-on-read.c
>> +++ b/block/copy-on-read.c
>> @@ -50,6 +50,8 @@ static int cor_open(BlockDriverState *bs, QDict *options, int flags,
>>           return -EINVAL;
>>       }
>> +    bs->supported_read_flags = BDRV_REQ_COPY_ON_READ | BDRV_REQ_PREFETCH;
>> +
>>       bs->supported_write_flags = BDRV_REQ_WRITE_UNCHANGED |
>>           (BDRV_REQ_FUA & bs->file->bs->supported_write_flags);
>>
> 
> This should be merged with the following patch, otherwise it doesn't make sense. You mark filter as supporting PREFETCH, but actually it just ignores it (and may crash on trying to read into qiov=NULL).
> 

Ah, no, problem is not in qiov=NULL, but in that we will just pass PREFETCH to bs->file, which may not support it and crash in block.io in the new abort() from patch 10.


Also, any reason to add support for BDRV_REQ_COPY_ON_READ ? What it means for cor filter? I don't know. It make sense only for generic layer and handled in generic layer. It never passed to driver, so let's not declare support for it.
diff mbox series

Patch

diff --git a/block/copy-on-read.c b/block/copy-on-read.c
index 8178a91..a2b180a 100644
--- a/block/copy-on-read.c
+++ b/block/copy-on-read.c
@@ -50,6 +50,8 @@  static int cor_open(BlockDriverState *bs, QDict *options, int flags,
         return -EINVAL;
     }
 
+    bs->supported_read_flags = BDRV_REQ_COPY_ON_READ | BDRV_REQ_PREFETCH;
+
     bs->supported_write_flags = BDRV_REQ_WRITE_UNCHANGED |
         (BDRV_REQ_FUA & bs->file->bs->supported_write_flags);