Message ID | 20180512012537.22478-5-jsnow@redhat.com |
---|---|
State | New |
Headers | show |
Series | qemu-img: add bitmap queries | expand |
12.05.2018 04:25, John Snow wrote: > For the purposes of qemu-img manipulation and querying of bitmaps, load > bitmaps that are "in use" -- if the image is read only. This will allow > us to diagnose problems with this flag using the qemu-img tool. It looks unsafe.. We can load them, then reopen rw and then store them on vm close, the result would be unpredictable. And they become available for qmp commands.. If we want to load them, we may need some additional state for such dirty bitmaps, something like "invalid", to be sure, that they will not be used like normal bitmap. And we may need some additional flag to load or not load them, and this behavior will be unrelated to can_write(). > > Signed-off-by: John Snow <jsnow@redhat.com> > --- > block/qcow2-bitmap.c | 32 +++++++++++++++----------------- > 1 file changed, 15 insertions(+), 17 deletions(-) > > diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c > index fc8d52fc3e..b556dbdccd 100644 > --- a/block/qcow2-bitmap.c > +++ b/block/qcow2-bitmap.c > @@ -346,7 +346,7 @@ static BdrvDirtyBitmap *load_bitmap(BlockDriverState *bs, > uint32_t granularity; > BdrvDirtyBitmap *bitmap = NULL; > > - if (bm->flags & BME_FLAG_IN_USE) { > + if (can_write(bs) && (bm->flags & BME_FLAG_IN_USE)) { > error_setg(errp, "Bitmap '%s' is in use", bm->name); > goto fail; > } > @@ -974,23 +974,21 @@ bool qcow2_load_dirty_bitmaps(BlockDriverState *bs, Error **errp) > } > > QSIMPLEQ_FOREACH(bm, bm_list, entry) { > - if (!(bm->flags & BME_FLAG_IN_USE)) { > - BdrvDirtyBitmap *bitmap = load_bitmap(bs, bm, errp); > - if (bitmap == NULL) { > - goto fail; > - } > - bm->dirty_bitmap = bitmap; > + BdrvDirtyBitmap *bitmap = load_bitmap(bs, bm, errp); > + if (bitmap == NULL) { > + goto fail; > + } > + bm->dirty_bitmap = bitmap; > > - if (!(bm->flags & BME_FLAG_AUTO)) { > - bdrv_disable_dirty_bitmap(bitmap); > - } > - bdrv_dirty_bitmap_set_persistance(bitmap, true); > - if (can_write(bs)) { > - bm->flags |= BME_FLAG_IN_USE; > - needs_update = true; > - } else { > - bdrv_dirty_bitmap_set_readonly(bitmap, true); > - } > + if (!(bm->flags & BME_FLAG_AUTO)) { > + bdrv_disable_dirty_bitmap(bitmap); > + } > + bdrv_dirty_bitmap_set_persistance(bitmap, true); > + if (can_write(bs)) { > + bm->flags |= BME_FLAG_IN_USE; > + needs_update = true; > + } else { > + bdrv_dirty_bitmap_set_readonly(bitmap, true); > } > } >
On 05/14/2018 08:55 AM, Vladimir Sementsov-Ogievskiy wrote: > 12.05.2018 04:25, John Snow wrote: >> For the purposes of qemu-img manipulation and querying of bitmaps, load >> bitmaps that are "in use" -- if the image is read only. This will allow >> us to diagnose problems with this flag using the qemu-img tool. > > It looks unsafe.. We can load them, then reopen rw and then store them > on vm close, the result would be unpredictable. And they become > available for qmp commands.. > > If we want to load them, we may need some additional state for such > dirty bitmaps, something like "invalid", to be sure, that they will not > be used like normal bitmap. And we may > need some additional flag to load or not load them, and this behavior > will be unrelated to can_write(). > Yeah, you're right -- reopening is scary. I'll see what I can do. The goal is just to get them in memory so that qemu-img can use the standard BdrvDirtyBitmap API to print various info about them for the user. Since 'info' and 'dump' don't need RW access, it'd be nicest for these commands to work in RO mode, hence the patch.
diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c index fc8d52fc3e..b556dbdccd 100644 --- a/block/qcow2-bitmap.c +++ b/block/qcow2-bitmap.c @@ -346,7 +346,7 @@ static BdrvDirtyBitmap *load_bitmap(BlockDriverState *bs, uint32_t granularity; BdrvDirtyBitmap *bitmap = NULL; - if (bm->flags & BME_FLAG_IN_USE) { + if (can_write(bs) && (bm->flags & BME_FLAG_IN_USE)) { error_setg(errp, "Bitmap '%s' is in use", bm->name); goto fail; } @@ -974,23 +974,21 @@ bool qcow2_load_dirty_bitmaps(BlockDriverState *bs, Error **errp) } QSIMPLEQ_FOREACH(bm, bm_list, entry) { - if (!(bm->flags & BME_FLAG_IN_USE)) { - BdrvDirtyBitmap *bitmap = load_bitmap(bs, bm, errp); - if (bitmap == NULL) { - goto fail; - } - bm->dirty_bitmap = bitmap; + BdrvDirtyBitmap *bitmap = load_bitmap(bs, bm, errp); + if (bitmap == NULL) { + goto fail; + } + bm->dirty_bitmap = bitmap; - if (!(bm->flags & BME_FLAG_AUTO)) { - bdrv_disable_dirty_bitmap(bitmap); - } - bdrv_dirty_bitmap_set_persistance(bitmap, true); - if (can_write(bs)) { - bm->flags |= BME_FLAG_IN_USE; - needs_update = true; - } else { - bdrv_dirty_bitmap_set_readonly(bitmap, true); - } + if (!(bm->flags & BME_FLAG_AUTO)) { + bdrv_disable_dirty_bitmap(bitmap); + } + bdrv_dirty_bitmap_set_persistance(bitmap, true); + if (can_write(bs)) { + bm->flags |= BME_FLAG_IN_USE; + needs_update = true; + } else { + bdrv_dirty_bitmap_set_readonly(bitmap, true); } }
For the purposes of qemu-img manipulation and querying of bitmaps, load bitmaps that are "in use" -- if the image is read only. This will allow us to diagnose problems with this flag using the qemu-img tool. Signed-off-by: John Snow <jsnow@redhat.com> --- block/qcow2-bitmap.c | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-)