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

2020-10-27 Thread 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 
---
  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



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

2020-10-27 Thread Vladimir Sementsov-Ogievskiy

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 
---
  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).

--
Best regards,
Vladimir



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

2020-10-22 Thread Andrey Shinkevich via
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 
---
 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);
 
-- 
1.8.3.1