diff mbox

[v2,8/9] block: Fix bdrv_co_truncate overlap check

Message ID 20180705073701.10558-9-famz@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Fam Zheng July 5, 2018, 7:37 a.m. UTC
If we are growing the image and potentially using preallocation for the
new area, we need to make sure that no write requests are made to the
"preallocated" area which [@old_size, @offset), not [@offset, offset * 2
- @old_size).

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/io.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Eric Blake July 6, 2018, 10:09 p.m. UTC | #1
On 07/05/2018 02:37 AM, Fam Zheng wrote:
> If we are growing the image and potentially using preallocation for the
> new area, we need to make sure that no write requests are made to the
> "preallocated" area which [@old_size, @offset), not [@offset, offset * 2

s/which/which is/

> - @old_size).
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>   block/io.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

> 
> diff --git a/block/io.c b/block/io.c
> index d07849fa96..ed18eb0ca3 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -3070,7 +3070,8 @@ int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset,
>       }
>   
>       bdrv_inc_in_flight(bs);
> -    tracked_request_begin(&req, bs, offset, new_bytes, BDRV_TRACKED_TRUNCATE);
> +    tracked_request_begin(&req, bs, offset - new_bytes, new_bytes,
> +                          BDRV_TRACKED_TRUNCATE);

Is it any more legible to do s/offset - new_bytes/old_size/, since those 
are equivalent?

>   
>       /* If we are growing the image and potentially using preallocation for the
>        * new area, we need to make sure that no write requests are made to it
>
Fam Zheng July 10, 2018, 5:24 a.m. UTC | #2
On Fri, 07/06 17:09, Eric Blake wrote:
> On 07/05/2018 02:37 AM, Fam Zheng wrote:
> > If we are growing the image and potentially using preallocation for the
> > new area, we need to make sure that no write requests are made to the
> > "preallocated" area which [@old_size, @offset), not [@offset, offset * 2
> 
> s/which/which is/
> 
> > - @old_size).
> > 
> > Signed-off-by: Fam Zheng <famz@redhat.com>
> > ---
> >   block/io.c | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>
> 
> > 
> > diff --git a/block/io.c b/block/io.c
> > index d07849fa96..ed18eb0ca3 100644
> > --- a/block/io.c
> > +++ b/block/io.c
> > @@ -3070,7 +3070,8 @@ int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset,
> >       }
> >       bdrv_inc_in_flight(bs);
> > -    tracked_request_begin(&req, bs, offset, new_bytes, BDRV_TRACKED_TRUNCATE);
> > +    tracked_request_begin(&req, bs, offset - new_bytes, new_bytes,
> > +                          BDRV_TRACKED_TRUNCATE);
> 
> Is it any more legible to do s/offset - new_bytes/old_size/, since those are
> equivalent?

No they are not. offset - new_bytes is either old_size (if expanding), or
smaller than old_size (if shrinking).

Fam

> 
> >       /* If we are growing the image and potentially using preallocation for the
> >        * new area, we need to make sure that no write requests are made to it
> > 
> 
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org
diff mbox

Patch

diff --git a/block/io.c b/block/io.c
index d07849fa96..ed18eb0ca3 100644
--- a/block/io.c
+++ b/block/io.c
@@ -3070,7 +3070,8 @@  int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset,
     }
 
     bdrv_inc_in_flight(bs);
-    tracked_request_begin(&req, bs, offset, new_bytes, BDRV_TRACKED_TRUNCATE);
+    tracked_request_begin(&req, bs, offset - new_bytes, new_bytes,
+                          BDRV_TRACKED_TRUNCATE);
 
     /* If we are growing the image and potentially using preallocation for the
      * new area, we need to make sure that no write requests are made to it