On 02/14/2017 11:59 AM, Vladimir Sementsov-Ogievskiy wrote: > Hi all! >
Hi! :) > There is a new update of qcow2-bitmap series - v14. > Having the cover letter be 00/24 but including 25 patches confuses the patch scraping tool a good deal. Also, can you include the "v14" in the patch emails themselves, too? (Don't respin just for this, thanks!) > web: > https://src.openvz.org/users/vsementsov/repos/qemu/browse?at=qcow2-bitmap-v14 > git: https://src.openvz.org/scm/~vsementsov/qemu.git (tag qcow2-bitmap-v14) > > v14: > > 07: use '|=' to update need_update_header > add John's r-b > add Max's r-b > 09: remove unused bitmap_table_to_cpu() > left Max's r-b, hope it's ok > add John's r-b > 10: remove extra new line > add John's r-b > 11: add John's r-b > 12: add John's r-b > 13: small fixes by John's review: > - remove weird g_free of NULL pointer from > if (tb == NULL) { > g_free(tb); > return -EINVAL; > } > - remove extra comment "/* errp is already set */" > - s/"Too large bitmap directory"/"Bitmap directory is too large"/ > left Max's r-b, hope you don't mind > 22: add Max's r-b > 23: add Max's r-b > 24: add Max's r-b > 25: new patch to improve error message on check_constraints_on_bitmap fail > > > v13: Just a fix for style checker. > 13: line over 80 > 14: line over 80 > 22: s/if () \n{/if () {/ > > v12: > 07: do not update header in qcow2_read_extensions, instead do it in the > end of qcow2_open, where it is updated also to clear unknown > autoclear features. > Wrong bdrv_is_root_node used instead of bdrv_is_read_only is fixed > automatically. > 08: add Max's r-b > 09: contextual: s/raw_bsd/raw-format/ > qcow2-bitmap.c: copyright s/2016/2017/ > fix compilation: move update_header_sync() to this patch > add Max's r-b > 13: update_header_sync() is moved to 09 > add Max's r-b > 15: add Max's r-b > 16: As qmp-commands.txt is removed from master, remove it here too. > Sentence "For now only Qcow2 disks support persistent bitmaps. > Default is false." moved to qapi/block-core.json. > Hope that is OK, Max's r-b is not dropped. > 17: qmp-commands.txt deleted, r-b is not dropped too. > 18: s/is failed/has failed/, add Max's r-b > 19: copyright: s/2016/2017/ > 21: drop "res->corruptions > 0 => ret = -EINVAL", add Max's r-b > 22: actually, patch is replaced with a new one, however some parts are the > same. > instead of changing bdrv_release_dirty_bitmap behaviour, create new > bdrv_remove_persistent_dirty_bitmap > 23: improve comment, about not-exists is not an error > 24: new patch > > Old 24 (qcow2-bitmap: cache bitmap list in BDRVQcow2State) will be sent > separately, to be applied after these series. > > Also: about bdrv_dirty_bitmap_set_persistance: I've decided not change its > behaviour > for now and just call bdrv_remove_persistent_dirty_bitmap from > qmp_block_dirty_bitmap_remove. Reasons: > 1. Do not change reviewed part. > 2. I'm not sure, that this complication of BdrvDirtyBitmap.persistent > semantics > is good (current semantics are just: .persistent means that bitmap should > be > saved on disk close). .persistent actually is not very related to "is > there > stored version of the bitmap in the image". > 3. It may be done later. > 4. It is not actually needed for now, as the only usage is dropping bitmap in > bdrv_remove_persistent_dirty_bitmap. May be in future it will be needed, > when we will have qmp interfaces for finer control of persistent dirty > bitmaps. > > > v11: > > Fix automatic build test fail. > > 18: wording - "ASCII representation of SHA256 ..." > 24: fix redeclaration error. (This is still RFC, may be not needed patch, > see description in v10 below). > > v10: > > 07: rm John's r-b > not add Max's r-b > fix subject s/dirty bitmaps/bitmaps > improve WARNING message > remove header ext if autoclear bit is unset, through > qcow2_updata_header() - r-b blocking change > 08: move bdrv_load_autoloading_dirty_bitmaps under asserts block (Max) > fix typo in commit message > move bdrv_load_autoloading_dirty_bitmaps after asserts > John's r-b > 09: rewrite check_dir_entry > remove extra feature cluster_size=0 from check_table_entry > which clears autoclear bit before trying to update bitmap directory > bitmap_list_store - do not free old clusters if in_place=true > changes in comments, fix indents > add functions > bitmap_list_count > update_ext_header_and_dir_in_place (unset autoclear bit before trying to > modify bitmap directory) > (for usage in qcow2_load_autoloading_dirty_bitmaps) > 11: add node name to error report > Max's r-b > 13: add type Qcow2BitmapTable > move from bm.table_* to bm.table.* > rewrite check_constraints_on_bitmap > flush(bs->file) instead of flush(bs) in update_ext_header_and_dir > assert(DIV_ROUND_UP(bm_size, dsc) == tb_size); > and assert(write_size <= s->cluster_size); in store_bitmap_data > fix: s/tb_size/tb_size * sizeof(tb[0]) in store_bitmap > in qcow2_store_persistent_dirty_bitmaps: > fail if !can_write > fix counting of new_nb_bitmaps and new_dir_size > free old clusters _after_ successful directory update (aggregate old > bitmap > tables into drop_tables) > 2 14: rename to can_store_new_dirty_bitmap > Max's r-b > 15: rename to can_store_new_dirty_bitmap > rm extra !! > 16: Max's r-b > rename to can_store_new_dirty_bitmap > indenting > Since s/2.8/2.9 > 17: Max's r-b > Since s/2.8/2.9 > 18: hashing can fail in comment for qapi > Since s/2.8/2.9 > remove dead comment > 19: indentation > Max's r-b > 20: Max's r-b > 21: fix error handling > return 0 if nb_bitmaps == 0 > new patches: > 22-23: remove bitmap from file on bdrv_release_dirty_bitmap > 24: experimental, RFC: cache bitmap list to bs, to not reload it from file. > I'm not sure that is needed now, but if you want I can merge it to earlier > patches. > > v9: > > rebase on master! > > 01-04,06,07: John's r-b > 03: rewording in comment ("The concurrent resetting ..."), John's r-b > 07: reword error messages > commit message: dirty bitmap -> bitmap > 09: fix uninitialized ret in bitmap_list_store() (fix compilation warning) > 18: fix compilation of tests/test-hbitmap > > also, change constants names for qcow2: DIRTY_BITMAP -> BITMAP (as it is not > dirty bitmaps extension, but just bitmaps extension), QCOW -> QCOW2 (bitmaps > are only for qcow2) > > v8: > > Patches 01-06 are mostly unchanged, except for spelling and wording. > 02 - add Eric's r-b > 03,04 - add Max's r-b > > 07-09 is refactored "bitmap-read" part of the feature. Main changes: > - introduce bitmap list - normal list, which represents bitmap directory. > Function bitmap_list_load loads bitmap directory, checks everything and > creates normal list. > - introduce .bdrv_load_autoloading_dirty_bitmaps : instead of loading bitmaps > in qcow2_open, lets load them only in bdrv_open, to avoid reloading in > bdrv_snapshot_goto > - do not delete bitmaps after read, but mark them IN_USE > > 10 has contextual changes and rewording of comment. I've added Max's r-b, > hope it's ok. > > 11: add error_report("Persistent bitmaps are lost") in case of failed bitmap > store > > 12: add Max's r-b > > 13 is refactored "bitmap-store" part of the feature. see 07-09 description > - for now I just free old clusters and allocate new. This will be improved > with a separate patch. > > patch about "delete bitmaps on truncate" is removed. This case will be > handled later. > IN_USE, autoclear, check-constraints things are merged into other patches. > > 14-15 are mew patches, to early check possibility of creating persistent > bitmap with > specified name and granularity in specified BDS > > 16-17: spelling, rewording, indenting, tiny code simplifying, check can_store > (by 14-15), > s/if (autoload && !persistent)/if (has_autoload && !persistent)/, > rebase (deleted qmp-commands.hx) > > 18: alternative to md5 in query-block: > - separated qmp command -x-debug-block-dirty-bitmap-sha256 > - as adviced by Daniel P. Berrange in my parallel thread: > - sha256 instead of md5 > - use qcrypto_hash_* instead of GChecksum > - fix bug =) (size was wrong in hbitmap_md5) > > 19: s/3999/3fff/, use x-debug-block-dirty-bitmap-sha256 > > 20: new patch to rename and publish inc_refcounts > > 21: some fixes and refactoring mostyly by Max's comments. > > Max, Eric, great tanks for your review! > I hope, I've covered most of your comments by this update. > > TODO: > - handle reopening image RO->RW and incoming migration, set IN_USE for > existing loaded bitmaps > in these cases. > - reuse old, already allocated data clusters for bitmaps storing > - truncate bitmaps in the image on truncate > > > v7: > > https://src.openvz.org/users/vsementsov/repos/qemu/browse?at=refs%2Ftags%2Fqcow2-bitmap-v7 > based on block-next (https://github.com/XanClic/qemu/commits/block-next) > > - a lot of refactoring and reordering of patches. > - dead code removed (bdrv_dirty_bitmap_load, etc.) > - do not maintain extra data for now > - do not store dirty bitmap directory in memory > (as we use it seldom, we can just reread if needed) > > By Kevin's review: > 01 - commit message changed: fix->improvement (as it was not a bug) > 03 - r-b > 04 - r-b > 05 - add 21 patch to fix spec, also, removed all (I hope) mentions of > "Bitmap Header", switch to one unified name for it - "Bitmap > Directory Entry", to avoid misunderstanding with Qcow2 header. > (also, add patch 22, to fix it in spec) > v6.06 - improve for_each_dir_entry loop, reorder patches, other small fixes > v6.07 ~> v7.09 - dead code removed, I've moved to one function > .bdrv_store_persistent_bitmaps and have wrapper and callback in one > patch (with also some other staff. If it not ok, I can split them) > v6.08 - about keeping bitmap directory instead of bitmap list: no I don't keep > it at all. > v6.16 - old bdrv_ bitmap-storing related functions are removed. The new one is > bdrv_store_persistent_bitmaps. > > > v6: > https://src.openvz.org/users/vsementsov/repos/qemu/browse?at=refs%2Ftags%2Fqcow2-bitmap-v6 > based on block-next (https://github.com/XanClic/qemu/commits/block-next) > > There are a lot of changes, reorderings and additions in comparement with v5. > One principal thing: now bitmaps are removed from image after loading instead > of marking them in_use. It is simpler and we do not need to store superfluous > data. > Also, we are no more interested in command line interface to dirty bitmaps. > So it is dropped. If someone needs it I can add it later. > > Vladimir Sementsov-Ogievskiy (25): > specs/qcow2: fix bitmap granularity qemu-specific note > specs/qcow2: do not use wording 'bitmap header' > hbitmap: improve dirty iter > tests: add hbitmap iter test > block: fix bdrv_dirty_bitmap_granularity signature > block/dirty-bitmap: add deserialize_ones func > qcow2: add bitmaps extension > block: introduce auto-loading bitmaps > qcow2: add .bdrv_load_autoloading_dirty_bitmaps > block/dirty-bitmap: add autoload field to BdrvDirtyBitmap > block: introduce persistent dirty bitmaps > block/dirty-bitmap: add bdrv_dirty_bitmap_next() > qcow2: add .bdrv_store_persistent_dirty_bitmaps() > block: add bdrv_can_store_new_dirty_bitmap > qcow2: add .bdrv_can_store_new_dirty_bitmap > qmp: add persistent flag to block-dirty-bitmap-add > qmp: add autoload parameter to block-dirty-bitmap-add > qmp: add x-debug-block-dirty-bitmap-sha256 > iotests: test qcow2 persistent dirty bitmap > qcow2-refcount: rename inc_refcounts() and make it public > qcow2-bitmap: refcounts > block/dirty-bitmap: add bdrv_remove_persistent_dirty_bitmap > qcow2: add .bdrv_remove_persistent_dirty_bitmap > qmp: block-dirty-bitmap-remove: remove persistent > qcow2-bitmap: improve check_constraints_on_bitmap > > block.c | 68 +++ > block/Makefile.objs | 2 +- > block/dirty-bitmap.c | 80 ++- > block/qcow2-bitmap.c | 1360 > ++++++++++++++++++++++++++++++++++++++++++ > block/qcow2-refcount.c | 59 +- > block/qcow2.c | 128 +++- > block/qcow2.h | 42 ++ > blockdev.c | 71 ++- > docs/specs/qcow2.txt | 8 +- > include/block/block.h | 5 + > include/block/block_int.h | 12 + > include/block/dirty-bitmap.h | 21 +- > include/qemu/hbitmap.h | 49 +- > qapi/block-core.json | 39 +- > tests/Makefile.include | 2 +- > tests/qemu-iotests/165 | 89 +++ > tests/qemu-iotests/165.out | 5 + > tests/qemu-iotests/group | 1 + > tests/test-hbitmap.c | 19 + > util/hbitmap.c | 51 +- > 20 files changed, 2047 insertions(+), 64 deletions(-) > create mode 100644 block/qcow2-bitmap.c > create mode 100755 tests/qemu-iotests/165 > create mode 100644 tests/qemu-iotests/165.out >
