mirror: correct buf_size

PATCH Superseded Wen Congyang 3 participants 4 messages May 14, 2015 Block layer

lore

Share & tools

lore

Wen Congyang

author

2015-05-14 10:25 UTC #0 lore raw

PATCH v2 1/1 mirror: correct buf_size

If buf_size % granularity is not 0, mirror_free_init() will do dangerous things.

Signed-off-by Wen Congyang <[email protected]>

---
 block/mirror.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

block/mirror.c +1 -1

index 58f391a..9521212 100644
@@ -684,7 +684,7 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
     s->is_none_mode = is_none_mode;
     s->base = base;
     s->granularity = granularity;
-    s->buf_size = MAX(buf_size, granularity);
+    s->buf_size = ROUND_UP(buf_size, granularity);
 
     s->dirty_bitmap = bdrv_create_dirty_bitmap(bs, granularity, NULL, errp);
     if (!s->dirty_bitmap) {

Signature · 1 line

2.1.0

Paolo Bonzini

Reviewed-by on v2 1/1

2015-05-14 10:38 UTC #1 lore raw

Quoted 15 lines · Wen Congyang

If buf_size % granularity is not 0, mirror_free_init() will do dangerous things.
Signed-off-by Wen Congyang <[email protected]>
---
 block/mirror.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
block/mirror.c +0 -0
index 58f391a..9521212 100644

block/mirror.c static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target, · line 690

Quoted 8 lines

block/mirror.c +1 -1

@@ -684,7 +684,7 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
     s->is_none_mode = is_none_mode;
     s->base = base;
     s->granularity = granularity;
-    s->buf_size = MAX(buf_size, granularity);
+    s->buf_size = ROUND_UP(buf_size, granularity);
 
     s->dirty_bitmap = bdrv_create_dirty_bitmap(bs, granularity, NULL, errp);
     if (!s->dirty_bitmap) {

Reviewed-by Paolo Bonzini <[email protected]>

Fam Zheng

2015-05-14 14:06 UTC #2 lore raw

Quoted 23 lines · Paolo Bonzini

Quoted 20 lines · Wen Congyang

If buf_size % granularity is not 0, mirror_free_init() will do dangerous things.

Signed-off-by Wen Congyang <[email protected]>

---
 block/mirror.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

block/mirror.c +1 -1

index 58f391a..9521212 100644
@@ -684,7 +684,7 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
     s->is_none_mode = is_none_mode;
     s->base = base;
     s->granularity = granularity;
-    s->buf_size = MAX(buf_size, granularity);
+    s->buf_size = ROUND_UP(buf_size, granularity);

Quoted 5 lines

block/mirror.c static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target, · line 690
Quoted 3 lines
block/mirror.c +0 -0
     s->dirty_bitmap = bdrv_create_dirty_bitmap(bs, granularity, NULL, errp);
     if (!s->dirty_bitmap) {
Reviewed-by Paolo Bonzini <[email protected]>

What if buf_size is negative?

Fam

Paolo Bonzini

2015-05-14 14:09 UTC #3 lore raw

Quoted 24 lines · Fam Zheng

Quoted 23 lines · Paolo Bonzini
Quoted 20 lines · Wen Congyang
If buf_size % granularity is not 0, mirror_free_init() will do dangerous things.
Signed-off-by Wen Congyang <[email protected]>
---
 block/mirror.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
block/mirror.c +1 -1
index 58f391a..9521212 100644
@@ -684,7 +684,7 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
     s->is_none_mode = is_none_mode;
     s->base = base;
     s->granularity = granularity;
-    s->buf_size = MAX(buf_size, granularity);
+    s->buf_size = ROUND_UP(buf_size, granularity);
 

Quoted 7 lines

Quoted 5 lines
block/mirror.c static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target, · line 690
Quoted 3 lines
block/mirror.c +0 -0
     s->dirty_bitmap = bdrv_create_dirty_bitmap(bs, granularity, NULL, errp);
     if (!s->dirty_bitmap) {
Reviewed-by Paolo Bonzini <[email protected]>
What if buf_size is negative?

This will fail:

    s->buf = qemu_try_blockalign(bs, s->buf_size);

O:-)

but really that should be checked in mirror_start_job, so that the command fails.

Paolo