| Message ID | 20170925145526.32690-6-eblake@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | make dirty-bitmap byte-based | expand |
25.09.2017 17:55, Eric Blake wrote: > We've previously fixed several places where we failed to account > for possible errors from bdrv_nb_sectors(). Fix another one by > making bdrv_dirty_bitmap_truncate() take the new size from the > caller instead of querying itself; then adjust the sole caller > bdrv_truncate() to pass the size just determined by a successful > resize, or to skip the bitmap resize on failure, thus avoiding nothing is skipped in this version > sizing the bitmaps to -1. This also fixes a bug where not all > failure paths in bdrv_truncate() would set errp. > > Note that bdrv_truncate() is still a bit awkward. We may want > to revisit it later and clean up things to better guarantee that > a resize attempt either fails cleanly up front, or cannot fail > after guest-visible changes have been made (if temporary changes > are made, then they need to be cleanly rolled back). But that > is a task for another day; for now, the goal is the bare minimum > fix to ensure that just bdrv_dirty_bitmap_truncate() cannot fail. > > Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > > --- > v10: always resize bitmap as before [John], enhance commit message to > point out errp bugfix [Vladimir] > v9: skip only bdrv_dirty_bitmap_truncate on error [Fam] > v8: retitle and rework to avoid possibility of secondary failure [John] > v7: new patch [Kevin] > --- > include/block/dirty-bitmap.h | 2 +- > block.c | 16 +++++++++++----- > block/dirty-bitmap.c | 6 +++--- > 3 files changed, 15 insertions(+), 9 deletions(-) > > diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h > index 8fd842eac9..7a27590047 100644 > --- a/include/block/dirty-bitmap.h > +++ b/include/block/dirty-bitmap.h > @@ -83,7 +83,7 @@ int64_t bdrv_dirty_iter_next(BdrvDirtyBitmapIter *iter); > void bdrv_set_dirty_iter(BdrvDirtyBitmapIter *hbi, int64_t sector_num); > int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap); > int64_t bdrv_get_meta_dirty_count(BdrvDirtyBitmap *bitmap); > -void bdrv_dirty_bitmap_truncate(BlockDriverState *bs); > +void bdrv_dirty_bitmap_truncate(BlockDriverState *bs, int64_t bytes); > bool bdrv_dirty_bitmap_readonly(const BdrvDirtyBitmap *bitmap); > bool bdrv_has_readonly_bitmaps(BlockDriverState *bs); > bool bdrv_dirty_bitmap_get_autoload(const BdrvDirtyBitmap *bitmap); > diff --git a/block.c b/block.c > index 528cda7b2c..ef5af81f66 100644 > --- a/block.c > +++ b/block.c > @@ -3545,12 +3545,18 @@ int bdrv_truncate(BdrvChild *child, int64_t offset, PreallocMode prealloc, > assert(!(bs->open_flags & BDRV_O_INACTIVE)); > > ret = drv->bdrv_truncate(bs, offset, prealloc, errp); > - if (ret == 0) { > - ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS); > - bdrv_dirty_bitmap_truncate(bs); > - bdrv_parent_cb_resize(bs); > - atomic_inc(&bs->write_gen); > + if (ret < 0) { > + return ret; > } > + ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS); > + if (ret < 0) { > + error_setg_errno(errp, -ret, "Could not refresh total sector count"); > + } else { > + offset = bs->total_sectors * BDRV_SECTOR_SIZE; > + } > + bdrv_dirty_bitmap_truncate(bs, offset); > + bdrv_parent_cb_resize(bs); > + atomic_inc(&bs->write_gen); > return ret; > } > > diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c > index 42a55e4a4b..ee164fb518 100644 > --- a/block/dirty-bitmap.c > +++ b/block/dirty-bitmap.c > @@ -1,7 +1,7 @@ > /* > * Block Dirty Bitmap > * > - * Copyright (c) 2016 Red Hat. Inc > + * Copyright (c) 2016-2017 Red Hat. Inc > * > * Permission is hereby granted, free of charge, to any person obtaining a copy > * of this software and associated documentation files (the "Software"), to deal > @@ -302,10 +302,10 @@ BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs, > * Truncates _all_ bitmaps attached to a BDS. > * Called with BQL taken. > */ > -void bdrv_dirty_bitmap_truncate(BlockDriverState *bs) > +void bdrv_dirty_bitmap_truncate(BlockDriverState *bs, int64_t bytes) > { > BdrvDirtyBitmap *bitmap; > - uint64_t size = bdrv_nb_sectors(bs); > + int64_t size = DIV_ROUND_UP(bytes, BDRV_SECTOR_SIZE); > > bdrv_dirty_bitmaps_lock(bs); > QLIST_FOREACH(bitmap, &bs->dirty_bitmaps, list) {
On 09/25/2017 10:23 AM, Vladimir Sementsov-Ogievskiy wrote: > 25.09.2017 17:55, Eric Blake wrote: >> We've previously fixed several places where we failed to account >> for possible errors from bdrv_nb_sectors(). Fix another one by >> making bdrv_dirty_bitmap_truncate() take the new size from the >> caller instead of querying itself; then adjust the sole caller >> bdrv_truncate() to pass the size just determined by a successful >> resize, or to skip the bitmap resize on failure, thus avoiding > > nothing is skipped in this version So much for me squashing in a change. If the maintainer is willing (Kevin, is this going through your tree?), we can reword that to: "...pass the size just determined by a successful resize, or to reuse the size given to the original truncate operation when refresh_total_sectors() was not able to confirm the actual size (the two sizes can potentially differ according to rounding constraints), thus avoiding..." > >> sizing the bitmaps to -1. This also fixes a bug where not all >> failure paths in bdrv_truncate() would set errp. >> >> Note that bdrv_truncate() is still a bit awkward. We may want >> to revisit it later and clean up things to better guarantee that >> a resize attempt either fails cleanly up front, or cannot fail >> after guest-visible changes have been made (if temporary changes >> are made, then they need to be cleanly rolled back). But that >> is a task for another day; for now, the goal is the bare minimum >> fix to ensure that just bdrv_dirty_bitmap_truncate() cannot fail. >> >> Signed-off-by: Eric Blake <eblake@redhat.com> > > Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > > > >> >> --- >> v10: always resize bitmap as before [John], enhance commit message to >> point out errp bugfix [Vladimir] >> v9: skip only bdrv_dirty_bitmap_truncate on error [Fam] >> v8: retitle and rework to avoid possibility of secondary failure [John] >> v7: new patch [Kevin] >> --- >> include/block/dirty-bitmap.h | 2 +- >> block.c | 16 +++++++++++----- >> block/dirty-bitmap.c | 6 +++--- >> 3 files changed, 15 insertions(+), 9 deletions(-) >> >> diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h >> index 8fd842eac9..7a27590047 100644 >> --- a/include/block/dirty-bitmap.h >> +++ b/include/block/dirty-bitmap.h >> @@ -83,7 +83,7 @@ int64_t bdrv_dirty_iter_next(BdrvDirtyBitmapIter >> *iter); >> void bdrv_set_dirty_iter(BdrvDirtyBitmapIter *hbi, int64_t sector_num); >> int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap); >> int64_t bdrv_get_meta_dirty_count(BdrvDirtyBitmap *bitmap); >> -void bdrv_dirty_bitmap_truncate(BlockDriverState *bs); >> +void bdrv_dirty_bitmap_truncate(BlockDriverState *bs, int64_t bytes); >> bool bdrv_dirty_bitmap_readonly(const BdrvDirtyBitmap *bitmap); >> bool bdrv_has_readonly_bitmaps(BlockDriverState *bs); >> bool bdrv_dirty_bitmap_get_autoload(const BdrvDirtyBitmap *bitmap); >> diff --git a/block.c b/block.c >> index 528cda7b2c..ef5af81f66 100644 >> --- a/block.c >> +++ b/block.c >> @@ -3545,12 +3545,18 @@ int bdrv_truncate(BdrvChild *child, int64_t >> offset, PreallocMode prealloc, >> assert(!(bs->open_flags & BDRV_O_INACTIVE)); >> >> ret = drv->bdrv_truncate(bs, offset, prealloc, errp); >> - if (ret == 0) { >> - ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS); >> - bdrv_dirty_bitmap_truncate(bs); >> - bdrv_parent_cb_resize(bs); >> - atomic_inc(&bs->write_gen); >> + if (ret < 0) { >> + return ret; >> } >> + ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS); >> + if (ret < 0) { >> + error_setg_errno(errp, -ret, "Could not refresh total sector >> count"); >> + } else { >> + offset = bs->total_sectors * BDRV_SECTOR_SIZE; >> + } >> + bdrv_dirty_bitmap_truncate(bs, offset); >> + bdrv_parent_cb_resize(bs); >> + atomic_inc(&bs->write_gen); >> return ret; >> } >> >> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c >> index 42a55e4a4b..ee164fb518 100644 >> --- a/block/dirty-bitmap.c >> +++ b/block/dirty-bitmap.c >> @@ -1,7 +1,7 @@ >> /* >> * Block Dirty Bitmap >> * >> - * Copyright (c) 2016 Red Hat. Inc >> + * Copyright (c) 2016-2017 Red Hat. Inc >> * >> * Permission is hereby granted, free of charge, to any person >> obtaining a copy >> * of this software and associated documentation files (the >> "Software"), to deal >> @@ -302,10 +302,10 @@ BdrvDirtyBitmap >> *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs, >> * Truncates _all_ bitmaps attached to a BDS. >> * Called with BQL taken. >> */ >> -void bdrv_dirty_bitmap_truncate(BlockDriverState *bs) >> +void bdrv_dirty_bitmap_truncate(BlockDriverState *bs, int64_t bytes) >> { >> BdrvDirtyBitmap *bitmap; >> - uint64_t size = bdrv_nb_sectors(bs); >> + int64_t size = DIV_ROUND_UP(bytes, BDRV_SECTOR_SIZE); >> >> bdrv_dirty_bitmaps_lock(bs); >> QLIST_FOREACH(bitmap, &bs->dirty_bitmaps, list) { > >
On 09/25/2017 10:55 AM, Eric Blake wrote: > We've previously fixed several places where we failed to account > for possible errors from bdrv_nb_sectors(). Fix another one by > making bdrv_dirty_bitmap_truncate() take the new size from the > caller instead of querying itself; then adjust the sole caller > bdrv_truncate() to pass the size just determined by a successful > resize, or to skip the bitmap resize on failure, thus avoiding > sizing the bitmaps to -1. This also fixes a bug where not all > failure paths in bdrv_truncate() would set errp. > > Note that bdrv_truncate() is still a bit awkward. We may want > to revisit it later and clean up things to better guarantee that > a resize attempt either fails cleanly up front, or cannot fail > after guest-visible changes have been made (if temporary changes > are made, then they need to be cleanly rolled back). But that > is a task for another day; for now, the goal is the bare minimum > fix to ensure that just bdrv_dirty_bitmap_truncate() cannot fail. > > Signed-off-by: Eric Blake <eblake@redhat.com> > > --- > v10: always resize bitmap as before [John], enhance commit message to > point out errp bugfix [Vladimir] > v9: skip only bdrv_dirty_bitmap_truncate on error [Fam] > v8: retitle and rework to avoid possibility of secondary failure [John] > v7: new patch [Kevin] > --- > include/block/dirty-bitmap.h | 2 +- > block.c | 16 +++++++++++----- > block/dirty-bitmap.c | 6 +++--- > 3 files changed, 15 insertions(+), 9 deletions(-) > > diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h > index 8fd842eac9..7a27590047 100644 > --- a/include/block/dirty-bitmap.h > +++ b/include/block/dirty-bitmap.h > @@ -83,7 +83,7 @@ int64_t bdrv_dirty_iter_next(BdrvDirtyBitmapIter *iter); > void bdrv_set_dirty_iter(BdrvDirtyBitmapIter *hbi, int64_t sector_num); > int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap); > int64_t bdrv_get_meta_dirty_count(BdrvDirtyBitmap *bitmap); > -void bdrv_dirty_bitmap_truncate(BlockDriverState *bs); > +void bdrv_dirty_bitmap_truncate(BlockDriverState *bs, int64_t bytes); > bool bdrv_dirty_bitmap_readonly(const BdrvDirtyBitmap *bitmap); > bool bdrv_has_readonly_bitmaps(BlockDriverState *bs); > bool bdrv_dirty_bitmap_get_autoload(const BdrvDirtyBitmap *bitmap); > diff --git a/block.c b/block.c > index 528cda7b2c..ef5af81f66 100644 > --- a/block.c > +++ b/block.c > @@ -3545,12 +3545,18 @@ int bdrv_truncate(BdrvChild *child, int64_t offset, PreallocMode prealloc, > assert(!(bs->open_flags & BDRV_O_INACTIVE)); > > ret = drv->bdrv_truncate(bs, offset, prealloc, errp); > - if (ret == 0) { > - ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS); > - bdrv_dirty_bitmap_truncate(bs); > - bdrv_parent_cb_resize(bs); > - atomic_inc(&bs->write_gen); > + if (ret < 0) { > + return ret; > } > + ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS); > + if (ret < 0) { > + error_setg_errno(errp, -ret, "Could not refresh total sector count"); > + } else { > + offset = bs->total_sectors * BDRV_SECTOR_SIZE; > + } > + bdrv_dirty_bitmap_truncate(bs, offset); > + bdrv_parent_cb_resize(bs); > + atomic_inc(&bs->write_gen); > return ret; > } > > diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c > index 42a55e4a4b..ee164fb518 100644 > --- a/block/dirty-bitmap.c > +++ b/block/dirty-bitmap.c > @@ -1,7 +1,7 @@ > /* > * Block Dirty Bitmap > * > - * Copyright (c) 2016 Red Hat. Inc > + * Copyright (c) 2016-2017 Red Hat. Inc > * > * Permission is hereby granted, free of charge, to any person obtaining a copy > * of this software and associated documentation files (the "Software"), to deal > @@ -302,10 +302,10 @@ BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs, > * Truncates _all_ bitmaps attached to a BDS. > * Called with BQL taken. > */ > -void bdrv_dirty_bitmap_truncate(BlockDriverState *bs) > +void bdrv_dirty_bitmap_truncate(BlockDriverState *bs, int64_t bytes) > { > BdrvDirtyBitmap *bitmap; > - uint64_t size = bdrv_nb_sectors(bs); > + int64_t size = DIV_ROUND_UP(bytes, BDRV_SECTOR_SIZE); > > bdrv_dirty_bitmaps_lock(bs); > QLIST_FOREACH(bitmap, &bs->dirty_bitmaps, list) { > I *THINK* this is the most correct we can do for now... Reviewed-by: John Snow <jsnow@redhat.com>
On Mon, 09/25 09:55, Eric Blake wrote: > We've previously fixed several places where we failed to account > for possible errors from bdrv_nb_sectors(). Fix another one by > making bdrv_dirty_bitmap_truncate() take the new size from the > caller instead of querying itself; then adjust the sole caller > bdrv_truncate() to pass the size just determined by a successful > resize, or to skip the bitmap resize on failure, thus avoiding > sizing the bitmaps to -1. This also fixes a bug where not all > failure paths in bdrv_truncate() would set errp. > > Note that bdrv_truncate() is still a bit awkward. We may want > to revisit it later and clean up things to better guarantee that > a resize attempt either fails cleanly up front, or cannot fail > after guest-visible changes have been made (if temporary changes > are made, then they need to be cleanly rolled back). But that > is a task for another day; for now, the goal is the bare minimum > fix to ensure that just bdrv_dirty_bitmap_truncate() cannot fail. > > Signed-off-by: Eric Blake <eblake@redhat.com> > > --- > v10: always resize bitmap as before [John], enhance commit message to > point out errp bugfix [Vladimir] > v9: skip only bdrv_dirty_bitmap_truncate on error [Fam] > v8: retitle and rework to avoid possibility of secondary failure [John] > v7: new patch [Kevin] > --- > include/block/dirty-bitmap.h | 2 +- > block.c | 16 +++++++++++----- > block/dirty-bitmap.c | 6 +++--- > 3 files changed, 15 insertions(+), 9 deletions(-) > > diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h > index 8fd842eac9..7a27590047 100644 > --- a/include/block/dirty-bitmap.h > +++ b/include/block/dirty-bitmap.h > @@ -83,7 +83,7 @@ int64_t bdrv_dirty_iter_next(BdrvDirtyBitmapIter *iter); > void bdrv_set_dirty_iter(BdrvDirtyBitmapIter *hbi, int64_t sector_num); > int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap); > int64_t bdrv_get_meta_dirty_count(BdrvDirtyBitmap *bitmap); > -void bdrv_dirty_bitmap_truncate(BlockDriverState *bs); > +void bdrv_dirty_bitmap_truncate(BlockDriverState *bs, int64_t bytes); > bool bdrv_dirty_bitmap_readonly(const BdrvDirtyBitmap *bitmap); > bool bdrv_has_readonly_bitmaps(BlockDriverState *bs); > bool bdrv_dirty_bitmap_get_autoload(const BdrvDirtyBitmap *bitmap); > diff --git a/block.c b/block.c > index 528cda7b2c..ef5af81f66 100644 > --- a/block.c > +++ b/block.c > @@ -3545,12 +3545,18 @@ int bdrv_truncate(BdrvChild *child, int64_t offset, PreallocMode prealloc, > assert(!(bs->open_flags & BDRV_O_INACTIVE)); > > ret = drv->bdrv_truncate(bs, offset, prealloc, errp); > - if (ret == 0) { > - ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS); > - bdrv_dirty_bitmap_truncate(bs); > - bdrv_parent_cb_resize(bs); > - atomic_inc(&bs->write_gen); > + if (ret < 0) { > + return ret; > } > + ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS); > + if (ret < 0) { > + error_setg_errno(errp, -ret, "Could not refresh total sector count"); > + } else { > + offset = bs->total_sectors * BDRV_SECTOR_SIZE; > + } > + bdrv_dirty_bitmap_truncate(bs, offset); > + bdrv_parent_cb_resize(bs); > + atomic_inc(&bs->write_gen); > return ret; > } > > diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c > index 42a55e4a4b..ee164fb518 100644 > --- a/block/dirty-bitmap.c > +++ b/block/dirty-bitmap.c > @@ -1,7 +1,7 @@ > /* > * Block Dirty Bitmap > * > - * Copyright (c) 2016 Red Hat. Inc > + * Copyright (c) 2016-2017 Red Hat. Inc > * > * Permission is hereby granted, free of charge, to any person obtaining a copy > * of this software and associated documentation files (the "Software"), to deal > @@ -302,10 +302,10 @@ BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs, > * Truncates _all_ bitmaps attached to a BDS. > * Called with BQL taken. > */ > -void bdrv_dirty_bitmap_truncate(BlockDriverState *bs) > +void bdrv_dirty_bitmap_truncate(BlockDriverState *bs, int64_t bytes) > { > BdrvDirtyBitmap *bitmap; > - uint64_t size = bdrv_nb_sectors(bs); > + int64_t size = DIV_ROUND_UP(bytes, BDRV_SECTOR_SIZE); > > bdrv_dirty_bitmaps_lock(bs); > QLIST_FOREACH(bitmap, &bs->dirty_bitmaps, list) { > -- > 2.13.5 > Reviewed-by: Fam Zheng <famz@redhat.com>
Am 25.09.2017 um 17:46 hat Eric Blake geschrieben: > On 09/25/2017 10:23 AM, Vladimir Sementsov-Ogievskiy wrote: > > 25.09.2017 17:55, Eric Blake wrote: > >> We've previously fixed several places where we failed to account > >> for possible errors from bdrv_nb_sectors(). Fix another one by > >> making bdrv_dirty_bitmap_truncate() take the new size from the > >> caller instead of querying itself; then adjust the sole caller > >> bdrv_truncate() to pass the size just determined by a successful > >> resize, or to skip the bitmap resize on failure, thus avoiding > > > > nothing is skipped in this version > > So much for me squashing in a change. If the maintainer is willing > (Kevin, is this going through your tree?), we can reword that to: > > "...pass the size just determined by a successful resize, or to reuse > the size given to the original truncate operation when > refresh_total_sectors() was not able to confirm the actual size (the two > sizes can potentially differ according to rounding constraints), thus > avoiding..." I always thought sentences that long were only allowed in German. Anyway, I'm squashing it in. Kevin
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h index 8fd842eac9..7a27590047 100644 --- a/include/block/dirty-bitmap.h +++ b/include/block/dirty-bitmap.h @@ -83,7 +83,7 @@ int64_t bdrv_dirty_iter_next(BdrvDirtyBitmapIter *iter); void bdrv_set_dirty_iter(BdrvDirtyBitmapIter *hbi, int64_t sector_num); int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap); int64_t bdrv_get_meta_dirty_count(BdrvDirtyBitmap *bitmap); -void bdrv_dirty_bitmap_truncate(BlockDriverState *bs); +void bdrv_dirty_bitmap_truncate(BlockDriverState *bs, int64_t bytes); bool bdrv_dirty_bitmap_readonly(const BdrvDirtyBitmap *bitmap); bool bdrv_has_readonly_bitmaps(BlockDriverState *bs); bool bdrv_dirty_bitmap_get_autoload(const BdrvDirtyBitmap *bitmap); diff --git a/block.c b/block.c index 528cda7b2c..ef5af81f66 100644 --- a/block.c +++ b/block.c @@ -3545,12 +3545,18 @@ int bdrv_truncate(BdrvChild *child, int64_t offset, PreallocMode prealloc, assert(!(bs->open_flags & BDRV_O_INACTIVE)); ret = drv->bdrv_truncate(bs, offset, prealloc, errp); - if (ret == 0) { - ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS); - bdrv_dirty_bitmap_truncate(bs); - bdrv_parent_cb_resize(bs); - atomic_inc(&bs->write_gen); + if (ret < 0) { + return ret; } + ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS); + if (ret < 0) { + error_setg_errno(errp, -ret, "Could not refresh total sector count"); + } else { + offset = bs->total_sectors * BDRV_SECTOR_SIZE; + } + bdrv_dirty_bitmap_truncate(bs, offset); + bdrv_parent_cb_resize(bs); + atomic_inc(&bs->write_gen); return ret; } diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c index 42a55e4a4b..ee164fb518 100644 --- a/block/dirty-bitmap.c +++ b/block/dirty-bitmap.c @@ -1,7 +1,7 @@ /* * Block Dirty Bitmap * - * Copyright (c) 2016 Red Hat. Inc + * Copyright (c) 2016-2017 Red Hat. Inc * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to deal @@ -302,10 +302,10 @@ BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs, * Truncates _all_ bitmaps attached to a BDS. * Called with BQL taken. */ -void bdrv_dirty_bitmap_truncate(BlockDriverState *bs) +void bdrv_dirty_bitmap_truncate(BlockDriverState *bs, int64_t bytes) { BdrvDirtyBitmap *bitmap; - uint64_t size = bdrv_nb_sectors(bs); + int64_t size = DIV_ROUND_UP(bytes, BDRV_SECTOR_SIZE); bdrv_dirty_bitmaps_lock(bs); QLIST_FOREACH(bitmap, &bs->dirty_bitmaps, list) {
We've previously fixed several places where we failed to account for possible errors from bdrv_nb_sectors(). Fix another one by making bdrv_dirty_bitmap_truncate() take the new size from the caller instead of querying itself; then adjust the sole caller bdrv_truncate() to pass the size just determined by a successful resize, or to skip the bitmap resize on failure, thus avoiding sizing the bitmaps to -1. This also fixes a bug where not all failure paths in bdrv_truncate() would set errp. Note that bdrv_truncate() is still a bit awkward. We may want to revisit it later and clean up things to better guarantee that a resize attempt either fails cleanly up front, or cannot fail after guest-visible changes have been made (if temporary changes are made, then they need to be cleanly rolled back). But that is a task for another day; for now, the goal is the bare minimum fix to ensure that just bdrv_dirty_bitmap_truncate() cannot fail. Signed-off-by: Eric Blake <eblake@redhat.com> --- v10: always resize bitmap as before [John], enhance commit message to point out errp bugfix [Vladimir] v9: skip only bdrv_dirty_bitmap_truncate on error [Fam] v8: retitle and rework to avoid possibility of secondary failure [John] v7: new patch [Kevin] --- include/block/dirty-bitmap.h | 2 +- block.c | 16 +++++++++++----- block/dirty-bitmap.c | 6 +++--- 3 files changed, 15 insertions(+), 9 deletions(-)