| Message ID | 1602524605-481160-7-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 |
On 12.10.20 19:43, Andrey Shinkevich wrote: > Modify the comment for the flag BDRV_REQ_PREFETCH as we are going to > use it alone and pass it to the COR-filter driver for further > processing. > > Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> > --- > include/block/block.h | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/include/block/block.h b/include/block/block.h > index 981ab5b..2b7efd1 100644 > --- a/include/block/block.h > +++ b/include/block/block.h > @@ -71,9 +71,10 @@ typedef enum { > BDRV_REQ_NO_FALLBACK = 0x100, > > /* > - * BDRV_REQ_PREFETCH may be used only together with BDRV_REQ_COPY_ON_READ > - * on read request and means that caller doesn't really need data to be > - * written to qiov parameter which may be NULL. > + * BDRV_REQ_PREFETCH may be used together with the BDRV_REQ_COPY_ON_READ > + * flag or when the COR-filter applied to read operations and means that There’s some word missing here, but I’m not sure what it is... At least an “is” before “applied”. Perhaps something like ”or when a COR filter is involved (in read operations)” would be better. > + * caller doesn't really need data to be written to qiov parameter which And this “written to” confused me for a second, because we’re reading into qiov. Technically, that means writing into the buffer, but, you know. Could we rewrite the whole thing, perhaps? Something like “BDRV_REQ_PREFETCH makes sense only in the context of copy-on-read (i.e., together with the BDRV_REQ_COPY_ON_READ flag or when there is a COR filter), in which case it signals that the COR operation need not read the data into memory (qiov), but only ensure it is copied to the top layer (i.e., that COR is done).” I don’t know. Max > + * may be NULL. > */ > BDRV_REQ_PREFETCH = 0x200, > /* Mask of valid flags */ >
14.10.2020 15:22, Max Reitz wrote: > On 12.10.20 19:43, Andrey Shinkevich wrote: >> Modify the comment for the flag BDRV_REQ_PREFETCH as we are going to >> use it alone and pass it to the COR-filter driver for further >> processing. >> >> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> >> --- >> include/block/block.h | 7 ++++--- >> 1 file changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/include/block/block.h b/include/block/block.h >> index 981ab5b..2b7efd1 100644 >> --- a/include/block/block.h >> +++ b/include/block/block.h >> @@ -71,9 +71,10 @@ typedef enum { >> BDRV_REQ_NO_FALLBACK = 0x100, >> >> /* >> - * BDRV_REQ_PREFETCH may be used only together with BDRV_REQ_COPY_ON_READ >> - * on read request and means that caller doesn't really need data to be >> - * written to qiov parameter which may be NULL. >> + * BDRV_REQ_PREFETCH may be used together with the BDRV_REQ_COPY_ON_READ >> + * flag or when the COR-filter applied to read operations and means that > > There’s some word missing here, but I’m not sure what it is... At least > an “is” before “applied”. Perhaps something like ”or when a COR filter > is involved (in read operations)” would be better. > >> + * caller doesn't really need data to be written to qiov parameter which > > And this “written to” confused me for a second, because we’re reading > into qiov. Technically, that means writing into the buffer, but, you know. > > Could we rewrite the whole thing, perhaps? Something like > > “BDRV_REQ_PREFETCH makes sense only in the context of copy-on-read > (i.e., together with the BDRV_REQ_COPY_ON_READ flag or when there is a > COR filter), in which case it signals that the COR operation need not > read the data into memory (qiov), but only ensure it is copied to the > top layer (i.e., that COR is done).” > Sounds good > >> + * may be NULL. >> */ >> BDRV_REQ_PREFETCH = 0x200, >> /* Mask of valid flags */ >> > >
On 14.10.2020 15:22, Max Reitz wrote: > On 12.10.20 19:43, Andrey Shinkevich wrote: >> Modify the comment for the flag BDRV_REQ_PREFETCH as we are going to >> use it alone and pass it to the COR-filter driver for further >> processing. >> >> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> >> --- >> include/block/block.h | 7 ++++--- >> 1 file changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/include/block/block.h b/include/block/block.h >> index 981ab5b..2b7efd1 100644 >> --- a/include/block/block.h >> +++ b/include/block/block.h >> @@ -71,9 +71,10 @@ typedef enum { >> BDRV_REQ_NO_FALLBACK = 0x100, >> >> /* >> - * BDRV_REQ_PREFETCH may be used only together with BDRV_REQ_COPY_ON_READ >> - * on read request and means that caller doesn't really need data to be >> - * written to qiov parameter which may be NULL. >> + * BDRV_REQ_PREFETCH may be used together with the BDRV_REQ_COPY_ON_READ >> + * flag or when the COR-filter applied to read operations and means that > > There’s some word missing here, but I’m not sure what it is... At least > an “is” before “applied”. Perhaps something like ”or when a COR filter > is involved (in read operations)” would be better. > >> + * caller doesn't really need data to be written to qiov parameter which > > And this “written to” confused me for a second, because we’re reading > into qiov. Technically, that means writing into the buffer, but, you know. > > Could we rewrite the whole thing, perhaps? Something like > > “BDRV_REQ_PREFETCH makes sense only in the context of copy-on-read > (i.e., together with the BDRV_REQ_COPY_ON_READ flag or when there is a > COR filter), in which case it signals that the COR operation need not > read the data into memory (qiov), but only ensure it is copied to the > top layer (i.e., that COR is done).” > > I don’t know. > > Max > I would modify a little: “BDRV_REQ_PREFETCH makes sense only in the context of copy-on-read (i.e., together with the BDRV_REQ_COPY_ON_READ flag or when a COR filter is involved), in which case it signals that the COR operation need not read the data into memory (qiov) but only ensure they are copied to the top layer (i.e., that COR operation is done).” >> + * may be NULL. >> */ >> BDRV_REQ_PREFETCH = 0x200, >> /* Mask of valid flags */ >> > >
diff --git a/include/block/block.h b/include/block/block.h index 981ab5b..2b7efd1 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -71,9 +71,10 @@ typedef enum { BDRV_REQ_NO_FALLBACK = 0x100, /* - * BDRV_REQ_PREFETCH may be used only together with BDRV_REQ_COPY_ON_READ - * on read request and means that caller doesn't really need data to be - * written to qiov parameter which may be NULL. + * BDRV_REQ_PREFETCH may be used together with the BDRV_REQ_COPY_ON_READ + * flag or when the COR-filter applied to read operations and means that + * caller doesn't really need data to be written to qiov parameter which + * may be NULL. */ BDRV_REQ_PREFETCH = 0x200, /* Mask of valid flags */
Modify the comment for the flag BDRV_REQ_PREFETCH as we are going to use it alone and pass it to the COR-filter driver for further processing. Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> --- include/block/block.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)