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

[Vladimir Sementsov-Ogievskiy]

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.

--
Best regards,
Vladimir