[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-block] [PATCH v15 00/25] qcow2: persistent dirty bitmaps

From: Vladimir Sementsov-Ogievskiy
Subject: [Qemu-block] [PATCH v15 00/25] qcow2: persistent dirty bitmaps
Date: Wed, 15 Feb 2017 13:10:05 +0300

Hi all!

There is a new update of qcow2-bitmap series - v15.

git: https://src.openvz.org/scm/~vsementsov/qemu.git (tag qcow2-bitmap-v15)

13,14: add John's r-b
15: qcow2_can_store_new_dirty_bitmap:
      - check max dirty bitmaps and bitmap directory overhead
      - switch to error_prepend
    rm Max's r-b
    not add John's r-b
17-24: add John's r-b
25: changed because 15 changed,
    not add John's r-b


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) {
               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 () {/

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
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
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 
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 


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).


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
      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 
        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
    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


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)


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 
- 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 

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 
IN_USE, autoclear, check-constraints things are merged into other patches.

14-15 are mew patches, to early check possibility of creating persistent bitmap 
  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.

- 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


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

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 
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         | 1373 ++++++++++++++++++++++++++++++++++++++++++
 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, 2060 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


reply via email to

[Prev in Thread] Current Thread [Next in Thread]