| Message ID | 1475232808-4852-12-git-send-email-vsementsov@virtuozzo.com |
|---|---|
| State | New |
| Headers | show |
On 30.09.2016 12:53, Vladimir Sementsov-Ogievskiy wrote: > Realize block bitmap stroing interface, to allow qcow2 images store > persistent bitmaps. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > block/qcow2-bitmap.c | 241 +++++++++++++++++++++++++++++++++++++++++++++++++++ > block/qcow2.c | 2 + > block/qcow2.h | 2 + > 3 files changed, 245 insertions(+) > > diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c > index 81520cd..a5be25a 100644 > --- a/block/qcow2-bitmap.c > +++ b/block/qcow2-bitmap.c > @@ -27,6 +27,7 @@ > > #include "qemu/osdep.h" > #include "qapi/error.h" > +#include "qemu/cutils.h" > > #include "block/block_int.h" > #include "block/qcow2.h" > @@ -96,6 +97,15 @@ static inline void bitmap_table_to_cpu(uint64_t *bitmap_table, size_t size) > } > } > > +static inline void bitmap_table_to_be(uint64_t *bitmap_table, size_t size) > +{ > + size_t i; > + > + for (i = 0; i < size; ++i) { > + cpu_to_be64s(&bitmap_table[i]); > + } > +} > + > static inline int calc_dir_entry_size(size_t name_size, size_t extra_data_size) > { > return align_offset(sizeof(Qcow2BitmapDirEntry) + > @@ -564,3 +574,234 @@ out: > > return ret; > } > + > +/* store_bitmap_data() > + * Store bitmap to image, filling bitamp table accordingly. s/bitamp/bitmap/ > + */ > +static int store_bitmap_data(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, > + uint64_t *bitmap_table, uint32_t bitmap_table_size) > +{ > + int ret; > + BDRVQcow2State *s = bs->opaque; > + uint64_t sector, dsc; > + uint64_t bm_size = bdrv_dirty_bitmap_size(bitmap); > + int cl_size = s->cluster_size; Once more, I don't think this variable is necessary, and I feel like it makes reading the code more difficult for no gain. > + uint8_t *buf = NULL; > + uint32_t tb_size = > + size_to_clusters(s, > + bdrv_dirty_bitmap_serialization_size(bitmap, 0, bm_size)); Should be uint64_t or you might accidentally get a truncation here. I'm not sure how you would get such huge bitmaps but it's trivial to use uint64_t. > + > + BdrvDirtyBitmapIter *dbi; > + > + if (tb_size != bitmap_table_size) { > + return -EINVAL; > + } > + > + memset(bitmap_table, 0, bitmap_table_size * sizeof(bitmap_table[0])); On 32 bit machines, this multiplication can overflow. There should at least be an assertion to prevent this. Other than that, of course, there needs to be some place which limits bitmap_table_size to some sane value and emits a real error if it exceeds that value. > + > + dbi = bdrv_dirty_iter_new(bitmap, 0); > + buf = g_malloc(cl_size); > + dsc = dirty_sectors_in_cluster(s, bitmap); > + > + while ((sector = bdrv_dirty_iter_next(dbi)) != -1) { sector should be int64_t instead of uint64_t, then. > + uint64_t cluster = sector / dsc; > + sector = cluster * dsc; Our coding style does not allow interleaving declarations and non-declarations. > + uint64_t end = MIN(bm_size, sector + dsc); > + uint64_t write_size = > + bdrv_dirty_bitmap_serialization_size(bitmap, sector, end - sector); > + > + int64_t off = qcow2_alloc_clusters(bs, cl_size); > + if (off < 0) { > + ret = off; > + goto finish; > + } > + bitmap_table[cluster] = off; > + > + bdrv_dirty_bitmap_serialize_part(bitmap, buf, sector, end); s/end/end - sector/? > + if (write_size < cl_size) { > + memset(buf + write_size, 0, cl_size - write_size); > + } > + I guess there should be a metadata overlap check here. > + ret = bdrv_pwrite(bs->file, off, buf, cl_size); > + if (ret < 0) { > + goto finish; > + } > + > + if (end >= bm_size) { > + break; > + } > + > + bdrv_set_dirty_iter(dbi, end); > + } > + ret = 0; /* writes */ What is that comment supposed to mean? > + > +finish: > + if (ret < 0) { > + clear_bitmap_table(bs, bitmap_table, bitmap_table_size); > + } > + g_free(buf); > + bdrv_dirty_iter_free(dbi); > + > + return ret; In case you decide to keep BME_MAX_PHYS_SIZE, this function should check somewhere that the physical size of the bitmap does not exceed that value. > +} > + > +/* store_bitmap() > + * Store bitmap to qcow2 and set bitmap_table. bitmap_table itself is not > + * stored to qcow2. First of all, there is no parameter called "bitmap_table", and second, yes, the bitmap table is written to the qcow2 file. > + */ > +static int store_bitmap(BlockDriverState *bs, > + BdrvDirtyBitmap *bitmap, > + Qcow2BitmapDirEntry *entry) > +{ > + int ret; > + BDRVQcow2State *s = bs->opaque; > + uint64_t bm_size = bdrv_dirty_bitmap_size(bitmap); > + const char *bm_name = bdrv_dirty_bitmap_name(bitmap); > + > + uint64_t *tb; > + int64_t tb_offset; > + uint32_t tb_size = > + size_to_clusters(s, > + bdrv_dirty_bitmap_serialization_size(bitmap, 0, bm_size)); As above, this variable should be of type uint64_t. Also, you have to check that it does not exceed BME_MAX_TABLE_SIZE. > + > + tb = g_try_new(uint64_t, tb_size); > + if (tb == NULL) { > + return -ENOMEM; > + } > + > + ret = store_bitmap_data(bs, bitmap, tb, tb_size); > + if (ret < 0) { > + g_free(tb); > + return ret; > + } > + > + tb_offset = qcow2_alloc_clusters(bs, tb_size * sizeof(tb[0])); If you don't limit tb_size, then this multiplication can overflow on 32 bit machines. > + if (tb_offset < 0) { > + ret = tb_offset; > + goto fail; > + } > + There should be a metadata overlap check here. > + bitmap_table_to_be(tb, tb_size); > + ret = bdrv_pwrite(bs->file, tb_offset, tb, tb_size * sizeof(tb[0])); > + if (ret < 0) { > + goto fail; > + } > + > + g_free(tb); > + > + entry->bitmap_table_offset = tb_offset; > + entry->bitmap_table_size = tb_size; > + entry->flags = bdrv_dirty_bitmap_granularity(bitmap) ? BME_FLAG_AUTO : 0; s/granularity/get_autoload/ > + entry->type = BT_DIRTY_TRACKING_BITMAP; > + entry->granularity_bits = ctz32(bdrv_dirty_bitmap_granularity(bitmap)); You should probably check somewhere that the resulting value for entry->granularity_bits is in the BME_{MIN,MAX}_GRANULARITY_BITS range. > + entry->name_size = strlen(bm_name); And that this length does not exceed BME_MAX_NAME_SIZE. > + entry->extra_data_size = 0; > + memcpy(entry + 1, bm_name, entry->name_size); > + > + return 0; > + > +fail: > + clear_bitmap_table(bs, tb, tb_size); > + > + if (tb_offset > 0) { > + qcow2_free_clusters(bs, tb_offset, tb_size, QCOW2_DISCARD_ALWAYS); As before, I'd vote for QCOW2_DISCARD_OTHER. > + } > + > + g_free(tb); > + > + return ret; > +} > + > +static Qcow2BitmapDirEntry *find_bitmap_by_name(uint8_t *bitmap_directory, > + size_t size, const char *name) > +{ > + Qcow2BitmapDirEntry *e; > + > + for_each_bitmap_dir_entry(e, bitmap_directory, size) { > + if (strncmp((char *)(e + 1), name, e->name_size) == 0) { > + return e; > + } > + } > + > + return NULL; > +} > + > +void qcow2_store_persistent_bitmaps(BlockDriverState *bs, Error **errp) > +{ > + BdrvDirtyBitmap *bm; > + BDRVQcow2State *s = bs->opaque; > + uint32_t new_nb_bitmaps = s->nb_bitmaps; > + uint64_t new_dir_size = s->bitmap_directory_size; > + uint8_t *dir = NULL, *new_dir = NULL; > + int ret; > + Qcow2BitmapDirEntry *new_pos; > + > + if (s->nb_bitmaps > 0) { > + dir = directory_read(bs, s->bitmap_directory_offset, > + s->bitmap_directory_size, errp); > + if (dir == NULL) { > + goto out; > + } > + } > + > + for (bm = bdrv_dirty_bitmap_next(bs, NULL); bm != NULL; > + bm = bdrv_dirty_bitmap_next(bs, bm)) { > + const char *name = bdrv_dirty_bitmap_name(bm); > + > + if (!bdrv_dirty_bitmap_get_persistance(bm)) { > + continue; > + } > + > + if (s->nb_bitmaps > 0 && > + find_bitmap_by_name(dir, s->bitmap_directory_size, name)) { > + error_setg(errp, > + "Can't store bitmap '%s' to '%s', as it already exists", > + name, bdrv_get_device_or_node_name(bs)); > + goto out; > + } > + > + new_nb_bitmaps++; > + new_dir_size += calc_dir_entry_size(strlen(name), 0); > + } > + > + if (s->nb_bitmaps == new_nb_bitmaps) { > + /* No new bitmaps - nothing to do */ > + goto out; > + } > + > + new_dir = g_try_malloc0(new_dir_size); > + if (new_dir == NULL) { > + error_setg(errp, "Can't allocate space for bitmap directory."); > + goto out; > + } > + > + memcpy(new_dir, dir, s->bitmap_directory_size); > + new_pos = (Qcow2BitmapDirEntry *)(new_dir + s->bitmap_directory_size); > + > + for (bm = bdrv_dirty_bitmap_next(bs, NULL); bm != NULL; > + bm = bdrv_dirty_bitmap_next(bs, bm)) { > + if (!bdrv_dirty_bitmap_get_persistance(bm)) { > + continue; > + } > + > + ret = store_bitmap(bs, bm, new_pos); > + if (ret < 0) { > + error_setg_errno(errp, -ret, "Can't store bitmap '%s' to '%s'", > + bdrv_dirty_bitmap_name(bm), > + bdrv_get_device_or_node_name(bs)); > + goto out; > + } > + new_pos = next_dir_entry(new_pos); > + } > + > + ret = directory_update(bs, new_dir, new_dir_size, new_nb_bitmaps); > + if (ret < 0) { > + error_setg_errno(errp, -ret, "Can't update bitmap directory in '%s'", > + bdrv_get_device_or_node_name(bs)); > + goto out; > + } > + > +out: > + g_free(new_dir); > + g_free(dir); This error path leaks all the bitmaps that have been written successfully (if any). I guess this is more or less fine if directory_update() failed (because you can't really tell the state of the image header after directory_update(), so better be safe) but it's not so fine if just some store_bitmap() failed. Max > +} > diff --git a/block/qcow2.c b/block/qcow2.c > index 02ec224..8238205 100644 > --- a/block/qcow2.c > +++ b/block/qcow2.c > @@ -3493,6 +3493,8 @@ BlockDriver bdrv_qcow2 = { > > .bdrv_detach_aio_context = qcow2_detach_aio_context, > .bdrv_attach_aio_context = qcow2_attach_aio_context, > + > + .bdrv_store_persistent_bitmaps = qcow2_store_persistent_bitmaps, > }; > > static void bdrv_qcow2_init(void) > diff --git a/block/qcow2.h b/block/qcow2.h > index 482a29f..dfcf4c6 100644 > --- a/block/qcow2.h > +++ b/block/qcow2.h > @@ -627,4 +627,6 @@ int qcow2_cache_get_empty(BlockDriverState *bs, Qcow2Cache *c, uint64_t offset, > void **table); > void qcow2_cache_put(BlockDriverState *bs, Qcow2Cache *c, void **table); > > +void qcow2_store_persistent_bitmaps(BlockDriverState *bs, Error **errp); > + > #endif >
On 07.10.2016 22:24, Max Reitz wrote: > On 30.09.2016 12:53, Vladimir Sementsov-Ogievskiy wrote: >> Realize block bitmap stroing interface, to allow qcow2 images store [snip] >> + uint64_t end = MIN(bm_size, sector + dsc); >> + uint64_t write_size = >> + bdrv_dirty_bitmap_serialization_size(bitmap, sector, end - sector); >> + >> + int64_t off = qcow2_alloc_clusters(bs, cl_size); >> + if (off < 0) { >> + ret = off; >> + goto finish; >> + } >> + bitmap_table[cluster] = off; >> + >> + bdrv_dirty_bitmap_serialize_part(bitmap, buf, sector, end); > s/end/end - sector/? o_0 terrible mistake, thank you. > >> + if (write_size < cl_size) { >> + memset(buf + write_size, 0, cl_size - write_size); >> + } >> + > I guess there should be a metadata overlap check here. What is the general rule of checking it? Should I check it before all my extension related writes?
On 13.10.2016 18:48, Vladimir Sementsov-Ogievskiy wrote: > On 07.10.2016 22:24, Max Reitz wrote: >> On 30.09.2016 12:53, Vladimir Sementsov-Ogievskiy wrote: >>> Realize block bitmap stroing interface, to allow qcow2 images store > > [snip] > >>> + uint64_t end = MIN(bm_size, sector + dsc); >>> + uint64_t write_size = >>> + bdrv_dirty_bitmap_serialization_size(bitmap, sector, end >>> - sector); >>> + >>> + int64_t off = qcow2_alloc_clusters(bs, cl_size); >>> + if (off < 0) { >>> + ret = off; >>> + goto finish; >>> + } >>> + bitmap_table[cluster] = off; >>> + >>> + bdrv_dirty_bitmap_serialize_part(bitmap, buf, sector, end); >> s/end/end - sector/? > > o_0 terrible mistake, thank you. > >> >>> + if (write_size < cl_size) { >>> + memset(buf + write_size, 0, cl_size - write_size); >>> + } >>> + >> I guess there should be a metadata overlap check here. > > What is the general rule of checking it? Should I check it before all my > extension related writes? The general rule is supposed to be "One check before every write to bs->file". Max
On 07.10.2016 22:24, Max Reitz wrote: > On 30.09.2016 12:53, Vladimir Sementsov-Ogievskiy wrote: >> Realize block bitmap stroing interface, to allow qcow2 images store >> persistent bitmaps. >> >> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> >> --- >> block/qcow2-bitmap.c | 241 +++++++++++++++++++++++++++++++++++++++++++++++++++ [...] >> >> + ret = bdrv_pwrite(bs->file, off, buf, cl_size); >> + if (ret < 0) { >> + goto finish; >> + } >> + >> + if (end >= bm_size) { >> + break; >> + } >> + >> + bdrv_set_dirty_iter(dbi, end); >> + } >> + ret = 0; /* writes */ > What is that comment supposed to mean? > > Now I think I can drop this assignment, as bdrv_aligned_preadv have 'return ret < 0 ? ret : 0;' in the end... Am I right? Can bdrv_pwrite and friends return positive value on success?
On 30.09.2016 12:53, Vladimir Sementsov-Ogievskiy wrote: > Realize block bitmap stroing interface, to allow qcow2 images store > persistent bitmaps. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > block/qcow2-bitmap.c | 241 +++++++++++++++++++++++++++++++++++++++++++++++++++ > block/qcow2.c | 2 + > block/qcow2.h | 2 + > 3 files changed, 245 insertions(+) > > diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c > index 81520cd..a5be25a 100644 > --- a/block/qcow2-bitmap.c > +++ b/block/qcow2-bitmap.c > @@ -27,6 +27,7 @@ > > #include "qemu/osdep.h" > #include "qapi/error.h" > +#include "qemu/cutils.h" > > #include "block/block_int.h" > #include "block/qcow2.h" > @@ -96,6 +97,15 @@ static inline void bitmap_table_to_cpu(uint64_t *bitmap_table, size_t size) > } > } > > +static inline void bitmap_table_to_be(uint64_t *bitmap_table, size_t size) > +{ > + size_t i; > + > + for (i = 0; i < size; ++i) { > + cpu_to_be64s(&bitmap_table[i]); > + } > +} > + > static inline int calc_dir_entry_size(size_t name_size, size_t extra_data_size) > { > return align_offset(sizeof(Qcow2BitmapDirEntry) + > @@ -564,3 +574,234 @@ out: > > return ret; > } > + > +/* store_bitmap_data() > + * Store bitmap to image, filling bitamp table accordingly. > + */ > +static int store_bitmap_data(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, > + uint64_t *bitmap_table, uint32_t bitmap_table_size) > +{ > + int ret; > + BDRVQcow2State *s = bs->opaque; > + uint64_t sector, dsc; > + uint64_t bm_size = bdrv_dirty_bitmap_size(bitmap); > + int cl_size = s->cluster_size; > + uint8_t *buf = NULL; > + uint32_t tb_size = > + size_to_clusters(s, > + bdrv_dirty_bitmap_serialization_size(bitmap, 0, bm_size)); > + > + BdrvDirtyBitmapIter *dbi; > + > + if (tb_size != bitmap_table_size) { > + return -EINVAL; > + } > + > + memset(bitmap_table, 0, bitmap_table_size * sizeof(bitmap_table[0])); > + > + dbi = bdrv_dirty_iter_new(bitmap, 0); > + buf = g_malloc(cl_size); > + dsc = dirty_sectors_in_cluster(s, bitmap); > + > + while ((sector = bdrv_dirty_iter_next(dbi)) != -1) { > + uint64_t cluster = sector / dsc; > + sector = cluster * dsc; > + uint64_t end = MIN(bm_size, sector + dsc); > + uint64_t write_size = > + bdrv_dirty_bitmap_serialization_size(bitmap, sector, end - sector); > + > + int64_t off = qcow2_alloc_clusters(bs, cl_size); > + if (off < 0) { > + ret = off; > + goto finish; > + } > + bitmap_table[cluster] = off; > + > + bdrv_dirty_bitmap_serialize_part(bitmap, buf, sector, end); > + if (write_size < cl_size) { > + memset(buf + write_size, 0, cl_size - write_size); > + } > + > + ret = bdrv_pwrite(bs->file, off, buf, cl_size); > + if (ret < 0) { > + goto finish; > + } > + > + if (end >= bm_size) { > + break; > + } > + > + bdrv_set_dirty_iter(dbi, end); > + } > + ret = 0; /* writes */ What is that comment supposed to mean? > + > +finish: > + if (ret < 0) { > + clear_bitmap_table(bs, bitmap_table, bitmap_table_size); > + } > + g_free(buf); > + bdrv_dirty_iter_free(dbi); > + > + return ret; In case you decide to keep BME_MAX_PHYS_SIZE, this function should check somewhere that the physical size of the bitmap does not exceed that value. > +} > + > +/* store_bitmap() > + * Store bitmap to qcow2 and set bitmap_table. bitmap_table itself is not > + * stored to qcow2. First of all, there is no parameter called "bitmap_table", and second, yes, the bitmap table is written to the qcow2 file. > + */ > +static int store_bitmap(BlockDriverState *bs, > + BdrvDirtyBitmap *bitmap, > + Qcow2BitmapDirEntry *entry) > +{ > + int ret; > + BDRVQcow2State *s = bs->opaque; > + uint64_t bm_size = bdrv_dirty_bitmap_size(bitmap); > + const char *bm_name = bdrv_dirty_bitmap_name(bitmap); > + > + uint64_t *tb; > + int64_t tb_offset; > + uint32_t tb_size = > + size_to_clusters(s, > + bdrv_dirty_bitmap_serialization_size(bitmap, 0, bm_size)); As above, this variable should be of type uint64_t. Also, you have to check that it does not exceed BME_MAX_TABLE_SIZE. > + > + tb = g_try_new(uint64_t, tb_size); > + if (tb == NULL) { > + return -ENOMEM; > + } > + > + ret = store_bitmap_data(bs, bitmap, tb, tb_size); > + if (ret < 0) { > + g_free(tb); > + return ret; > + } > + > + tb_offset = qcow2_alloc_clusters(bs, tb_size * sizeof(tb[0])); If you don't limit tb_size, then this multiplication can overflow on 32 bit machines. > + if (tb_offset < 0) { > + ret = tb_offset; > + goto fail; > + } > + There should be a metadata overlap check here. > + bitmap_table_to_be(tb, tb_size); > + ret = bdrv_pwrite(bs->file, tb_offset, tb, tb_size * sizeof(tb[0])); > + if (ret < 0) { > + goto fail; > + } > + > + g_free(tb); > + > + entry->bitmap_table_offset = tb_offset; > + entry->bitmap_table_size = tb_size; > + entry->flags = bdrv_dirty_bitmap_granularity(bitmap) ? BME_FLAG_AUTO : 0; s/granularity/get_autoload/ > + entry->type = BT_DIRTY_TRACKING_BITMAP; > + entry->granularity_bits = ctz32(bdrv_dirty_bitmap_granularity(bitmap)); You should probably check somewhere that the resulting value for entry->granularity_bits is in the BME_{MIN,MAX}_GRANULARITY_BITS range. > + entry->name_size = strlen(bm_name); And that this length does not exceed BME_MAX_NAME_SIZE. > + entry->extra_data_size = 0; > + memcpy(entry + 1, bm_name, entry->name_size); > + > + return 0; > + > +fail: > + clear_bitmap_table(bs, tb, tb_size); > + > + if (tb_offset > 0) { > + qcow2_free_clusters(bs, tb_offset, tb_size, QCOW2_DISCARD_ALWAYS); As before, I'd vote for QCOW2_DISCARD_OTHER. > + } > + > + g_free(tb); > + > + return ret; > +} > + > +static Qcow2BitmapDirEntry *find_bitmap_by_name(uint8_t *bitmap_directory, > + size_t size, const char *name) > +{ > + Qcow2BitmapDirEntry *e; > + > + for_each_bitmap_dir_entry(e, bitmap_directory, size) { > + if (strncmp((char *)(e + 1), name, e->name_size) == 0) { > + return e; > + } > + } > + > + return NULL; > +} > + > +void qcow2_store_persistent_bitmaps(BlockDriverState *bs, Error **errp) > +{ > + BdrvDirtyBitmap *bm; > + BDRVQcow2State *s = bs->opaque; > + uint32_t new_nb_bitmaps = s->nb_bitmaps; > + uint64_t new_dir_size = s->bitmap_directory_size; > + uint8_t *dir = NULL, *new_dir = NULL; > + int ret; > + Qcow2BitmapDirEntry *new_pos; > + > + if (s->nb_bitmaps > 0) { > + dir = directory_read(bs, s->bitmap_directory_offset, > + s->bitmap_directory_size, errp); > + if (dir == NULL) { > + goto out; > + } > + } > + > + for (bm = bdrv_dirty_bitmap_next(bs, NULL); bm != NULL; > + bm = bdrv_dirty_bitmap_next(bs, bm)) { > + const char *name = bdrv_dirty_bitmap_name(bm); > + > + if (!bdrv_dirty_bitmap_get_persistance(bm)) { > + continue; > + } > + > + if (s->nb_bitmaps > 0 && > + find_bitmap_by_name(dir, s->bitmap_directory_size, name)) { > + error_setg(errp, > + "Can't store bitmap '%s' to '%s', as it already exists", > + name, bdrv_get_device_or_node_name(bs)); > + goto out; > + } > + > + new_nb_bitmaps++; > + new_dir_size += calc_dir_entry_size(strlen(name), 0); > + } > + > + if (s->nb_bitmaps == new_nb_bitmaps) { > + /* No new bitmaps - nothing to do */ > + goto out; > + } > + > + new_dir = g_try_malloc0(new_dir_size); > + if (new_dir == NULL) { > + error_setg(errp, "Can't allocate space for bitmap directory."); > + goto out; > + } > + > + memcpy(new_dir, dir, s->bitmap_directory_size); > + new_pos = (Qcow2BitmapDirEntry *)(new_dir + s->bitmap_directory_size); > + > + for (bm = bdrv_dirty_bitmap_next(bs, NULL); bm != NULL; > + bm = bdrv_dirty_bitmap_next(bs, bm)) { > + if (!bdrv_dirty_bitmap_get_persistance(bm)) { > + continue; > + } > + > + ret = store_bitmap(bs, bm, new_pos); > + if (ret < 0) { > + error_setg_errno(errp, -ret, "Can't store bitmap '%s' to '%s'", > + bdrv_dirty_bitmap_name(bm), > + bdrv_get_device_or_node_name(bs)); > + goto out; > + } > + new_pos = next_dir_entry(new_pos); > + } > + > + ret = directory_update(bs, new_dir, new_dir_size, new_nb_bitmaps); > + if (ret < 0) { > + error_setg_errno(errp, -ret, "Can't update bitmap directory in '%s'", > + bdrv_get_device_or_node_name(bs)); > + goto out; > + } > + > +out: > + g_free(new_dir); > + g_free(dir); This error path leaks all the bitmaps that have been written successfully (if any). I guess this is more or less fine if directory_update() failed (because you can't really tell the state of the image header after directory_update(), so better be safe) but it's not so fine if just some store_bitmap() failed. Max > +} > diff --git a/block/qcow2.c b/block/qcow2.c > index 02ec224..8238205 100644 > --- a/block/qcow2.c > +++ b/block/qcow2.c > @@ -3493,6 +3493,8 @@ BlockDriver bdrv_qcow2 = { > > .bdrv_detach_aio_context = qcow2_detach_aio_context, > .bdrv_attach_aio_context = qcow2_attach_aio_context, > + > + .bdrv_store_persistent_bitmaps = qcow2_store_persistent_bitmaps, > }; > > static void bdrv_qcow2_init(void) > diff --git a/block/qcow2.h b/block/qcow2.h > index 482a29f..dfcf4c6 100644 > --- a/block/qcow2.h > +++ b/block/qcow2.h > @@ -627,4 +627,6 @@ int qcow2_cache_get_empty(BlockDriverState *bs, Qcow2Cache *c, uint64_t offset, > void **table); > void qcow2_cache_put(BlockDriverState *bs, Qcow2Cache *c, void **table); > > +void qcow2_store_persistent_bitmaps(BlockDriverState *bs, Error **errp); > + > #endif >
Sorry, this was an accidental reply. On 17.10.2016 20:57, Vladimir Sementsov-Ogievskiy wrote: > On 30.09.2016 12:53, Vladimir Sementsov-Ogievskiy wrote: >> Realize block bitmap stroing interface, to allow qcow2 images store >> persistent bitmaps. >> >> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> >> --- >> block/qcow2-bitmap.c | 241 >> +++++++++++++++++++++++++++++++++++++++++++++++++++ >> block/qcow2.c | 2 + >> block/qcow2.h | 2 + >> 3 files changed, 245 insertions(+) >> >> diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c >> index 81520cd..a5be25a 100644 >> --- a/block/qcow2-bitmap.c >> +++ b/block/qcow2-bitmap.c >> @@ -27,6 +27,7 @@ >> >> #include "qemu/osdep.h" >> #include "qapi/error.h" >> +#include "qemu/cutils.h" >> >> #include "block/block_int.h" >> #include "block/qcow2.h" >> @@ -96,6 +97,15 @@ static inline void bitmap_table_to_cpu(uint64_t >> *bitmap_table, size_t size) >> } >> } >> >> +static inline void bitmap_table_to_be(uint64_t *bitmap_table, size_t >> size) >> +{ >> + size_t i; >> + >> + for (i = 0; i < size; ++i) { >> + cpu_to_be64s(&bitmap_table[i]); >> + } >> +} >> + >> static inline int calc_dir_entry_size(size_t name_size, size_t >> extra_data_size) >> { >> return align_offset(sizeof(Qcow2BitmapDirEntry) + >> @@ -564,3 +574,234 @@ out: >> >> return ret; >> } >> + >> +/* store_bitmap_data() >> + * Store bitmap to image, filling bitamp table accordingly. > > >> + */ >> +static int store_bitmap_data(BlockDriverState *bs, BdrvDirtyBitmap >> *bitmap, >> + uint64_t *bitmap_table, uint32_t >> bitmap_table_size) >> +{ >> + int ret; >> + BDRVQcow2State *s = bs->opaque; >> + uint64_t sector, dsc; >> + uint64_t bm_size = bdrv_dirty_bitmap_size(bitmap); >> + int cl_size = s->cluster_size; > > > >> + uint8_t *buf = NULL; >> + uint32_t tb_size = >> + size_to_clusters(s, >> + bdrv_dirty_bitmap_serialization_size(bitmap, 0, >> bm_size)); > > >> + >> + BdrvDirtyBitmapIter *dbi; >> + >> + if (tb_size != bitmap_table_size) { >> + return -EINVAL; >> + } >> + >> + memset(bitmap_table, 0, bitmap_table_size * >> sizeof(bitmap_table[0])); > > >> + >> + dbi = bdrv_dirty_iter_new(bitmap, 0); >> + buf = g_malloc(cl_size); >> + dsc = dirty_sectors_in_cluster(s, bitmap); >> + >> + while ((sector = bdrv_dirty_iter_next(dbi)) != -1) { > > >> + uint64_t cluster = sector / dsc; >> + sector = cluster * dsc; > > > >> + uint64_t end = MIN(bm_size, sector + dsc); >> + uint64_t write_size = >> + bdrv_dirty_bitmap_serialization_size(bitmap, sector, end >> - sector); >> + >> + int64_t off = qcow2_alloc_clusters(bs, cl_size); >> + if (off < 0) { >> + ret = off; >> + goto finish; >> + } >> + bitmap_table[cluster] = off; >> + >> + bdrv_dirty_bitmap_serialize_part(bitmap, buf, sector, end); > > > >> + if (write_size < cl_size) { >> + memset(buf + write_size, 0, cl_size - write_size); >> + } >> + > > >> + ret = bdrv_pwrite(bs->file, off, buf, cl_size); >> + if (ret < 0) { >> + goto finish; >> + } >> + >> + if (end >= bm_size) { >> + break; >> + } >> + >> + bdrv_set_dirty_iter(dbi, end); >> + } >> + ret = 0; /* writes */ > > What is that comment supposed to mean? > >> + >> +finish: >> + if (ret < 0) { >> + clear_bitmap_table(bs, bitmap_table, bitmap_table_size); >> + } >> + g_free(buf); >> + bdrv_dirty_iter_free(dbi); >> + >> + return ret; > > In case you decide to keep BME_MAX_PHYS_SIZE, this function should check > somewhere that the physical size of the bitmap does not exceed that > value. > >> +} >> + >> +/* store_bitmap() >> + * Store bitmap to qcow2 and set bitmap_table. bitmap_table itself >> is not >> + * stored to qcow2. > > First of all, there is no parameter called "bitmap_table", and second, > yes, the bitmap table is written to the qcow2 file. > >> + */ >> +static int store_bitmap(BlockDriverState *bs, >> + BdrvDirtyBitmap *bitmap, >> + Qcow2BitmapDirEntry *entry) >> +{ >> + int ret; >> + BDRVQcow2State *s = bs->opaque; >> + uint64_t bm_size = bdrv_dirty_bitmap_size(bitmap); >> + const char *bm_name = bdrv_dirty_bitmap_name(bitmap); >> + >> + uint64_t *tb; >> + int64_t tb_offset; >> + uint32_t tb_size = >> + size_to_clusters(s, >> + bdrv_dirty_bitmap_serialization_size(bitmap, 0, >> bm_size)); > > As above, this variable should be of type uint64_t. > > Also, you have to check that it does not exceed BME_MAX_TABLE_SIZE. > >> + >> + tb = g_try_new(uint64_t, tb_size); >> + if (tb == NULL) { >> + return -ENOMEM; >> + } >> + >> + ret = store_bitmap_data(bs, bitmap, tb, tb_size); >> + if (ret < 0) { >> + g_free(tb); >> + return ret; >> + } >> + >> + tb_offset = qcow2_alloc_clusters(bs, tb_size * sizeof(tb[0])); > > If you don't limit tb_size, then this multiplication can overflow on 32 > bit machines. > >> + if (tb_offset < 0) { >> + ret = tb_offset; >> + goto fail; >> + } >> + > > There should be a metadata overlap check here. > >> + bitmap_table_to_be(tb, tb_size); >> + ret = bdrv_pwrite(bs->file, tb_offset, tb, tb_size * >> sizeof(tb[0])); >> + if (ret < 0) { >> + goto fail; >> + } >> + >> + g_free(tb); >> + >> + entry->bitmap_table_offset = tb_offset; >> + entry->bitmap_table_size = tb_size; >> + entry->flags = bdrv_dirty_bitmap_granularity(bitmap) ? >> BME_FLAG_AUTO : 0; > > s/granularity/get_autoload/ > >> + entry->type = BT_DIRTY_TRACKING_BITMAP; >> + entry->granularity_bits = >> ctz32(bdrv_dirty_bitmap_granularity(bitmap)); > > You should probably check somewhere that the resulting value for > entry->granularity_bits is in the BME_{MIN,MAX}_GRANULARITY_BITS range. > >> + entry->name_size = strlen(bm_name); > > And that this length does not exceed BME_MAX_NAME_SIZE. > >> + entry->extra_data_size = 0; >> + memcpy(entry + 1, bm_name, entry->name_size); >> + >> + return 0; >> + >> +fail: >> + clear_bitmap_table(bs, tb, tb_size); >> + >> + if (tb_offset > 0) { >> + qcow2_free_clusters(bs, tb_offset, tb_size, >> QCOW2_DISCARD_ALWAYS); > > As before, I'd vote for QCOW2_DISCARD_OTHER. > >> + } >> + >> + g_free(tb); >> + >> + return ret; >> +} >> + >> +static Qcow2BitmapDirEntry *find_bitmap_by_name(uint8_t >> *bitmap_directory, >> + size_t size, const >> char *name) >> +{ >> + Qcow2BitmapDirEntry *e; >> + >> + for_each_bitmap_dir_entry(e, bitmap_directory, size) { >> + if (strncmp((char *)(e + 1), name, e->name_size) == 0) { >> + return e; >> + } >> + } >> + >> + return NULL; >> +} >> + >> +void qcow2_store_persistent_bitmaps(BlockDriverState *bs, Error **errp) >> +{ >> + BdrvDirtyBitmap *bm; >> + BDRVQcow2State *s = bs->opaque; >> + uint32_t new_nb_bitmaps = s->nb_bitmaps; >> + uint64_t new_dir_size = s->bitmap_directory_size; >> + uint8_t *dir = NULL, *new_dir = NULL; >> + int ret; >> + Qcow2BitmapDirEntry *new_pos; >> + >> + if (s->nb_bitmaps > 0) { >> + dir = directory_read(bs, s->bitmap_directory_offset, >> + s->bitmap_directory_size, errp); >> + if (dir == NULL) { >> + goto out; >> + } >> + } >> + >> + for (bm = bdrv_dirty_bitmap_next(bs, NULL); bm != NULL; >> + bm = bdrv_dirty_bitmap_next(bs, bm)) { >> + const char *name = bdrv_dirty_bitmap_name(bm); >> + >> + if (!bdrv_dirty_bitmap_get_persistance(bm)) { >> + continue; >> + } >> + >> + if (s->nb_bitmaps > 0 && >> + find_bitmap_by_name(dir, s->bitmap_directory_size, >> name)) { >> + error_setg(errp, >> + "Can't store bitmap '%s' to '%s', as it >> already exists", >> + name, bdrv_get_device_or_node_name(bs)); >> + goto out; >> + } >> + >> + new_nb_bitmaps++; >> + new_dir_size += calc_dir_entry_size(strlen(name), 0); >> + } >> + >> + if (s->nb_bitmaps == new_nb_bitmaps) { >> + /* No new bitmaps - nothing to do */ >> + goto out; >> + } >> + >> + new_dir = g_try_malloc0(new_dir_size); >> + if (new_dir == NULL) { >> + error_setg(errp, "Can't allocate space for bitmap directory."); >> + goto out; >> + } >> + >> + memcpy(new_dir, dir, s->bitmap_directory_size); >> + new_pos = (Qcow2BitmapDirEntry *)(new_dir + >> s->bitmap_directory_size); >> + >> + for (bm = bdrv_dirty_bitmap_next(bs, NULL); bm != NULL; >> + bm = bdrv_dirty_bitmap_next(bs, bm)) { >> + if (!bdrv_dirty_bitmap_get_persistance(bm)) { >> + continue; >> + } >> + >> + ret = store_bitmap(bs, bm, new_pos); >> + if (ret < 0) { >> + error_setg_errno(errp, -ret, "Can't store bitmap '%s' to >> '%s'", >> + bdrv_dirty_bitmap_name(bm), >> + bdrv_get_device_or_node_name(bs)); >> + goto out; >> + } >> + new_pos = next_dir_entry(new_pos); >> + } >> + >> + ret = directory_update(bs, new_dir, new_dir_size, new_nb_bitmaps); >> + if (ret < 0) { >> + error_setg_errno(errp, -ret, "Can't update bitmap directory >> in '%s'", >> + bdrv_get_device_or_node_name(bs)); >> + goto out; >> + } >> + >> +out: >> + g_free(new_dir); >> + g_free(dir); > > This error path leaks all the bitmaps that have been written > successfully (if any). I guess this is more or less fine if > directory_update() failed (because you can't really tell the state of > the image header after directory_update(), so better be safe) but it's > not so fine if just some store_bitmap() failed. > > Max > >> +} >> diff --git a/block/qcow2.c b/block/qcow2.c >> index 02ec224..8238205 100644 >> --- a/block/qcow2.c >> +++ b/block/qcow2.c >> @@ -3493,6 +3493,8 @@ BlockDriver bdrv_qcow2 = { >> >> .bdrv_detach_aio_context = qcow2_detach_aio_context, >> .bdrv_attach_aio_context = qcow2_attach_aio_context, >> + >> + .bdrv_store_persistent_bitmaps = qcow2_store_persistent_bitmaps, >> }; >> >> static void bdrv_qcow2_init(void) >> diff --git a/block/qcow2.h b/block/qcow2.h >> index 482a29f..dfcf4c6 100644 >> --- a/block/qcow2.h >> +++ b/block/qcow2.h >> @@ -627,4 +627,6 @@ int qcow2_cache_get_empty(BlockDriverState *bs, >> Qcow2Cache *c, uint64_t offset, >> void **table); >> void qcow2_cache_put(BlockDriverState *bs, Qcow2Cache *c, void >> **table); >> >> +void qcow2_store_persistent_bitmaps(BlockDriverState *bs, Error >> **errp); >> + >> #endif >> > > >
On 17.10.2016 19:19, Vladimir Sementsov-Ogievskiy wrote: > On 07.10.2016 22:24, Max Reitz wrote: >> On 30.09.2016 12:53, Vladimir Sementsov-Ogievskiy wrote: >>> Realize block bitmap stroing interface, to allow qcow2 images store >>> persistent bitmaps. >>> >>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> >>> --- >>> block/qcow2-bitmap.c | 241 >>> +++++++++++++++++++++++++++++++++++++++++++++++++++ > > [...] > >>> >>> + ret = bdrv_pwrite(bs->file, off, buf, cl_size); >>> + if (ret < 0) { >>> + goto finish; >>> + } >>> + >>> + if (end >= bm_size) { >>> + break; >>> + } >>> + >>> + bdrv_set_dirty_iter(dbi, end); >>> + } >>> + ret = 0; /* writes */ >> What is that comment supposed to mean? >> >> > > Now I think I can drop this assignment, as bdrv_aligned_preadv have > 'return ret < 0 ? ret : 0;' in the end... Am I right? Can bdrv_pwrite > and friends return positive value on success? I think so, but if they do return positive value they're supposed to be equal to the number of bytes written. In practice, though, any non-negative return value is treated the same (as success). I was just wondering about the comment, though, not about the assignment itself. Max
On 10/21/2016 02:44 PM, Max Reitz wrote: >> Now I think I can drop this assignment, as bdrv_aligned_preadv have >> 'return ret < 0 ? ret : 0;' in the end... Am I right? Can bdrv_pwrite >> and friends return positive value on success? > > I think so, but if they do return positive value they're supposed to be > equal to the number of bytes written. In practice, though, any > non-negative return value is treated the same (as success). At one point in my conversion to byte access, I ran into test failures when trying to return number of bytes written; so commit 1a62d0a hard-codes a return of 0 on success instead. It will be a long and painful audit to find which callers expect which semantics, which I did not want to do. > > I was just wondering about the comment, though, not about the assignment > itself. > > Max >
diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c index 81520cd..a5be25a 100644 --- a/block/qcow2-bitmap.c +++ b/block/qcow2-bitmap.c @@ -27,6 +27,7 @@ #include "qemu/osdep.h" #include "qapi/error.h" +#include "qemu/cutils.h" #include "block/block_int.h" #include "block/qcow2.h" @@ -96,6 +97,15 @@ static inline void bitmap_table_to_cpu(uint64_t *bitmap_table, size_t size) } } +static inline void bitmap_table_to_be(uint64_t *bitmap_table, size_t size) +{ + size_t i; + + for (i = 0; i < size; ++i) { + cpu_to_be64s(&bitmap_table[i]); + } +} + static inline int calc_dir_entry_size(size_t name_size, size_t extra_data_size) { return align_offset(sizeof(Qcow2BitmapDirEntry) + @@ -564,3 +574,234 @@ out: return ret; } + +/* store_bitmap_data() + * Store bitmap to image, filling bitamp table accordingly. + */ +static int store_bitmap_data(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, + uint64_t *bitmap_table, uint32_t bitmap_table_size) +{ + int ret; + BDRVQcow2State *s = bs->opaque; + uint64_t sector, dsc; + uint64_t bm_size = bdrv_dirty_bitmap_size(bitmap); + int cl_size = s->cluster_size; + uint8_t *buf = NULL; + uint32_t tb_size = + size_to_clusters(s, + bdrv_dirty_bitmap_serialization_size(bitmap, 0, bm_size)); + + BdrvDirtyBitmapIter *dbi; + + if (tb_size != bitmap_table_size) { + return -EINVAL; + } + + memset(bitmap_table, 0, bitmap_table_size * sizeof(bitmap_table[0])); + + dbi = bdrv_dirty_iter_new(bitmap, 0); + buf = g_malloc(cl_size); + dsc = dirty_sectors_in_cluster(s, bitmap); + + while ((sector = bdrv_dirty_iter_next(dbi)) != -1) { + uint64_t cluster = sector / dsc; + sector = cluster * dsc; + uint64_t end = MIN(bm_size, sector + dsc); + uint64_t write_size = + bdrv_dirty_bitmap_serialization_size(bitmap, sector, end - sector); + + int64_t off = qcow2_alloc_clusters(bs, cl_size); + if (off < 0) { + ret = off; + goto finish; + } + bitmap_table[cluster] = off; + + bdrv_dirty_bitmap_serialize_part(bitmap, buf, sector, end); + if (write_size < cl_size) { + memset(buf + write_size, 0, cl_size - write_size); + } + + ret = bdrv_pwrite(bs->file, off, buf, cl_size); + if (ret < 0) { + goto finish; + } + + if (end >= bm_size) { + break; + } + + bdrv_set_dirty_iter(dbi, end); + } + ret = 0; /* writes */ + +finish: + if (ret < 0) { + clear_bitmap_table(bs, bitmap_table, bitmap_table_size); + } + g_free(buf); + bdrv_dirty_iter_free(dbi); + + return ret; +} + +/* store_bitmap() + * Store bitmap to qcow2 and set bitmap_table. bitmap_table itself is not + * stored to qcow2. + */ +static int store_bitmap(BlockDriverState *bs, + BdrvDirtyBitmap *bitmap, + Qcow2BitmapDirEntry *entry) +{ + int ret; + BDRVQcow2State *s = bs->opaque; + uint64_t bm_size = bdrv_dirty_bitmap_size(bitmap); + const char *bm_name = bdrv_dirty_bitmap_name(bitmap); + + uint64_t *tb; + int64_t tb_offset; + uint32_t tb_size = + size_to_clusters(s, + bdrv_dirty_bitmap_serialization_size(bitmap, 0, bm_size)); + + tb = g_try_new(uint64_t, tb_size); + if (tb == NULL) { + return -ENOMEM; + } + + ret = store_bitmap_data(bs, bitmap, tb, tb_size); + if (ret < 0) { + g_free(tb); + return ret; + } + + tb_offset = qcow2_alloc_clusters(bs, tb_size * sizeof(tb[0])); + if (tb_offset < 0) { + ret = tb_offset; + goto fail; + } + + bitmap_table_to_be(tb, tb_size); + ret = bdrv_pwrite(bs->file, tb_offset, tb, tb_size * sizeof(tb[0])); + if (ret < 0) { + goto fail; + } + + g_free(tb); + + entry->bitmap_table_offset = tb_offset; + entry->bitmap_table_size = tb_size; + entry->flags = bdrv_dirty_bitmap_granularity(bitmap) ? BME_FLAG_AUTO : 0; + entry->type = BT_DIRTY_TRACKING_BITMAP; + entry->granularity_bits = ctz32(bdrv_dirty_bitmap_granularity(bitmap)); + entry->name_size = strlen(bm_name); + entry->extra_data_size = 0; + memcpy(entry + 1, bm_name, entry->name_size); + + return 0; + +fail: + clear_bitmap_table(bs, tb, tb_size); + + if (tb_offset > 0) { + qcow2_free_clusters(bs, tb_offset, tb_size, QCOW2_DISCARD_ALWAYS); + } + + g_free(tb); + + return ret; +} + +static Qcow2BitmapDirEntry *find_bitmap_by_name(uint8_t *bitmap_directory, + size_t size, const char *name) +{ + Qcow2BitmapDirEntry *e; + + for_each_bitmap_dir_entry(e, bitmap_directory, size) { + if (strncmp((char *)(e + 1), name, e->name_size) == 0) { + return e; + } + } + + return NULL; +} + +void qcow2_store_persistent_bitmaps(BlockDriverState *bs, Error **errp) +{ + BdrvDirtyBitmap *bm; + BDRVQcow2State *s = bs->opaque; + uint32_t new_nb_bitmaps = s->nb_bitmaps; + uint64_t new_dir_size = s->bitmap_directory_size; + uint8_t *dir = NULL, *new_dir = NULL; + int ret; + Qcow2BitmapDirEntry *new_pos; + + if (s->nb_bitmaps > 0) { + dir = directory_read(bs, s->bitmap_directory_offset, + s->bitmap_directory_size, errp); + if (dir == NULL) { + goto out; + } + } + + for (bm = bdrv_dirty_bitmap_next(bs, NULL); bm != NULL; + bm = bdrv_dirty_bitmap_next(bs, bm)) { + const char *name = bdrv_dirty_bitmap_name(bm); + + if (!bdrv_dirty_bitmap_get_persistance(bm)) { + continue; + } + + if (s->nb_bitmaps > 0 && + find_bitmap_by_name(dir, s->bitmap_directory_size, name)) { + error_setg(errp, + "Can't store bitmap '%s' to '%s', as it already exists", + name, bdrv_get_device_or_node_name(bs)); + goto out; + } + + new_nb_bitmaps++; + new_dir_size += calc_dir_entry_size(strlen(name), 0); + } + + if (s->nb_bitmaps == new_nb_bitmaps) { + /* No new bitmaps - nothing to do */ + goto out; + } + + new_dir = g_try_malloc0(new_dir_size); + if (new_dir == NULL) { + error_setg(errp, "Can't allocate space for bitmap directory."); + goto out; + } + + memcpy(new_dir, dir, s->bitmap_directory_size); + new_pos = (Qcow2BitmapDirEntry *)(new_dir + s->bitmap_directory_size); + + for (bm = bdrv_dirty_bitmap_next(bs, NULL); bm != NULL; + bm = bdrv_dirty_bitmap_next(bs, bm)) { + if (!bdrv_dirty_bitmap_get_persistance(bm)) { + continue; + } + + ret = store_bitmap(bs, bm, new_pos); + if (ret < 0) { + error_setg_errno(errp, -ret, "Can't store bitmap '%s' to '%s'", + bdrv_dirty_bitmap_name(bm), + bdrv_get_device_or_node_name(bs)); + goto out; + } + new_pos = next_dir_entry(new_pos); + } + + ret = directory_update(bs, new_dir, new_dir_size, new_nb_bitmaps); + if (ret < 0) { + error_setg_errno(errp, -ret, "Can't update bitmap directory in '%s'", + bdrv_get_device_or_node_name(bs)); + goto out; + } + +out: + g_free(new_dir); + g_free(dir); +} diff --git a/block/qcow2.c b/block/qcow2.c index 02ec224..8238205 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -3493,6 +3493,8 @@ BlockDriver bdrv_qcow2 = { .bdrv_detach_aio_context = qcow2_detach_aio_context, .bdrv_attach_aio_context = qcow2_attach_aio_context, + + .bdrv_store_persistent_bitmaps = qcow2_store_persistent_bitmaps, }; static void bdrv_qcow2_init(void) diff --git a/block/qcow2.h b/block/qcow2.h index 482a29f..dfcf4c6 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -627,4 +627,6 @@ int qcow2_cache_get_empty(BlockDriverState *bs, Qcow2Cache *c, uint64_t offset, void **table); void qcow2_cache_put(BlockDriverState *bs, Qcow2Cache *c, void **table); +void qcow2_store_persistent_bitmaps(BlockDriverState *bs, Error **errp); + #endif
Realize block bitmap stroing interface, to allow qcow2 images store persistent bitmaps. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> --- block/qcow2-bitmap.c | 241 +++++++++++++++++++++++++++++++++++++++++++++++++++ block/qcow2.c | 2 + block/qcow2.h | 2 + 3 files changed, 245 insertions(+)