| Message ID | 1465924093-76875-3-git-send-email-vsementsov@virtuozzo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 06/14/2016 11:08 AM, Vladimir Sementsov-Ogievskiy wrote: > Fix the following bug: > > # virsh start test > Domain test started > > # virsh qemu-monitor-command test \ > '{"execute":"block-dirty-bitmap-add",\ > "arguments":{"node":"drive0","name":"ab"}}' > {"return":{},"id":"libvirt-36"}'}' > > # virsh snapshot-create test > error: Unable to read from monitor: Connection reset by peer > > Actually, assert "assert(pos < hb->size)" in hbitmap_iter_init fires, > because qcow2_save_vmstate just writes to bs (not to bs->file->bs) after > the end of the drive. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > block/dirty-bitmap.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c > index 4902ca5..d28b49c 100644 > --- a/block/dirty-bitmap.c > +++ b/block/dirty-bitmap.c > @@ -364,6 +364,20 @@ void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector, > int nr_sectors) > { > BdrvDirtyBitmap *bitmap; > + int64_t bitmap_size; > + > + if (QLIST_EMPTY(&bs->dirty_bitmaps)) { > + return; > + } > + > + bitmap_size = QLIST_FIRST(&bs->dirty_bitmaps)->size; > + > + if (cur_sector >= bitmap_size) { > + /* this may come from qcow2_save_vmstate */ > + return; > + } Do we still need this patch after Kevin's work to fix vmstate to no longer go through the block layer? https://lists.gnu.org/archive/html/qemu-devel/2016-06/msg02832.html
On 15.06.2016 00:33, Eric Blake wrote: > On 06/14/2016 11:08 AM, Vladimir Sementsov-Ogievskiy wrote: >> Fix the following bug: >> >> # virsh start test >> Domain test started >> >> # virsh qemu-monitor-command test \ >> '{"execute":"block-dirty-bitmap-add",\ >> "arguments":{"node":"drive0","name":"ab"}}' >> {"return":{},"id":"libvirt-36"}'}' >> >> # virsh snapshot-create test >> error: Unable to read from monitor: Connection reset by peer >> >> Actually, assert "assert(pos < hb->size)" in hbitmap_iter_init fires, >> because qcow2_save_vmstate just writes to bs (not to bs->file->bs) after >> the end of the drive. >> >> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> >> --- >> block/dirty-bitmap.c | 14 ++++++++++++++ >> 1 file changed, 14 insertions(+) >> >> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c >> index 4902ca5..d28b49c 100644 >> --- a/block/dirty-bitmap.c >> +++ b/block/dirty-bitmap.c >> @@ -364,6 +364,20 @@ void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector, >> int nr_sectors) >> { >> BdrvDirtyBitmap *bitmap; >> + int64_t bitmap_size; >> + >> + if (QLIST_EMPTY(&bs->dirty_bitmaps)) { >> + return; >> + } >> + >> + bitmap_size = QLIST_FIRST(&bs->dirty_bitmaps)->size; >> + >> + if (cur_sector >= bitmap_size) { >> + /* this may come from qcow2_save_vmstate */ >> + return; >> + } > Do we still need this patch after Kevin's work to fix vmstate to no > longer go through the block layer? I think not. If we are not going through block layer we are not touching dirty bitmaps. > > https://lists.gnu.org/archive/html/qemu-devel/2016-06/msg02832.html >
On 15.06.2016 15:08, Vladimir Sementsov-Ogievskiy wrote: > On 15.06.2016 00:33, Eric Blake wrote: >> On 06/14/2016 11:08 AM, Vladimir Sementsov-Ogievskiy wrote: >>> Fix the following bug: >>> >>> # virsh start test >>> Domain test started >>> >>> # virsh qemu-monitor-command test \ >>> '{"execute":"block-dirty-bitmap-add",\ >>> "arguments":{"node":"drive0","name":"ab"}}' >>> {"return":{},"id":"libvirt-36"}'}' >>> >>> # virsh snapshot-create test >>> error: Unable to read from monitor: Connection reset by peer >>> >>> Actually, assert "assert(pos < hb->size)" in hbitmap_iter_init fires, >>> because qcow2_save_vmstate just writes to bs (not to bs->file->bs) after >>> the end of the drive. >>> >>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> >>> --- >>> block/dirty-bitmap.c | 14 ++++++++++++++ >>> 1 file changed, 14 insertions(+) >>> >>> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c >>> index 4902ca5..d28b49c 100644 >>> --- a/block/dirty-bitmap.c >>> +++ b/block/dirty-bitmap.c >>> @@ -364,6 +364,20 @@ void bdrv_set_dirty(BlockDriverState *bs, >>> int64_t cur_sector, >>> int nr_sectors) >>> { >>> BdrvDirtyBitmap *bitmap; >>> + int64_t bitmap_size; >>> + >>> + if (QLIST_EMPTY(&bs->dirty_bitmaps)) { >>> + return; >>> + } >>> + >>> + bitmap_size = QLIST_FIRST(&bs->dirty_bitmaps)->size; >>> + >>> + if (cur_sector >= bitmap_size) { >>> + /* this may come from qcow2_save_vmstate */ >>> + return; >>> + } >> Do we still need this patch after Kevin's work to fix vmstate to no >> longer go through the block layer? > > I think not. If we are not going through block layer we are not touching > dirty bitmaps. OK, I'll drop this patch, then. I'll keep the first patch, though, because it seems useful anyway. Max
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c index 4902ca5..d28b49c 100644 --- a/block/dirty-bitmap.c +++ b/block/dirty-bitmap.c @@ -364,6 +364,20 @@ void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector, int nr_sectors) { BdrvDirtyBitmap *bitmap; + int64_t bitmap_size; + + if (QLIST_EMPTY(&bs->dirty_bitmaps)) { + return; + } + + bitmap_size = QLIST_FIRST(&bs->dirty_bitmaps)->size; + + if (cur_sector >= bitmap_size) { + /* this may come from qcow2_save_vmstate */ + return; + } + assert(cur_sector + nr_sectors <= bitmap_size); + QLIST_FOREACH(bitmap, &bs->dirty_bitmaps, list) { if (!bdrv_dirty_bitmap_enabled(bitmap)) { continue;
Fix the following bug: # virsh start test Domain test started # virsh qemu-monitor-command test \ '{"execute":"block-dirty-bitmap-add",\ "arguments":{"node":"drive0","name":"ab"}}' {"return":{},"id":"libvirt-36"}'}' # virsh snapshot-create test error: Unable to read from monitor: Connection reset by peer Actually, assert "assert(pos < hb->size)" in hbitmap_iter_init fires, because qcow2_save_vmstate just writes to bs (not to bs->file->bs) after the end of the drive. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> --- block/dirty-bitmap.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)