Re: [Qemu-devel] [PATCH v22 00/30] qcow2: persistent dirty bitmaps
On 11/17/2017 10:17 AM, Vladimir Sementsov-Ogievskiy wrote: >> Nothing in this series touched qemu-img. At the very minimum, it would >> be nice if 'qemu-img info' were to display a summary of all bitmaps in >> the qcow2 file; the QMP type ImageInfoSpecificQCow2 should be updated to >> mention this information, and qemu-img updated to output it. Being able >> to create and remove persistent bitmaps from an at-rest qcow2 file >> without having to fire up qemu to issue QMP commands to do so would also >> be nice. >> >> It's a bit late for getting this request into 2.12, but had it been Make that "too late for 2.11" >> available, it would make my work at testing that 'qemu-img commit' >> doesn't corrupt persistent bitmaps. >> > > You are right, this is needed, but I can't promise something for the > nearest future.. And as we both agree it is work for 2.12 or later, it is not needed right away; so we have a couple of months. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v22 00/30] qcow2: persistent dirty bitmaps
17.11.2017 19:04, Eric Blake wrote: Revisiting an old series: On 06/28/2017 07:05 AM, Vladimir Sementsov-Ogievskiy wrote: Hi all! There is a new update of qcow2-bitmap series - v22. web: https://src.openvz.org/users/vsementsov/repos/qemu/browse?at=qcow2-bitmap-v22 git: https://src.openvz.org/scm/~vsementsov/qemu.git (tag qcow2-bitmap-v22) v22: Rebase on master, so changes, mostly related to new dirty bitmaps mutex: qcow2: add persistent dirty bitmaps support block.c | 65 +- block/Makefile.objs |2 +- block/dirty-bitmap.c | 154 - block/io.c |8 + block/qcow2-bitmap.c | 1481 ++ block/qcow2-refcount.c | 59 +- block/qcow2.c| 155 - block/qcow2.h| 43 ++ blockdev.c | 73 ++- docs/interop/qcow2.txt |8 +- include/block/block.h|3 + include/block/block_int.h| 14 + include/block/dirty-bitmap.h | 22 +- include/qemu/hbitmap.h | 49 +- qapi/block-core.json | 42 +- tests/Makefile.include |2 +- tests/qemu-iotests/165 | 105 +++ tests/qemu-iotests/165.out |5 + tests/qemu-iotests/group |1 + tests/test-hbitmap.c | 19 + util/hbitmap.c | 51 +- 21 files changed, 2280 insertions(+), 81 deletions(-) Nothing in this series touched qemu-img. At the very minimum, it would be nice if 'qemu-img info' were to display a summary of all bitmaps in the qcow2 file; the QMP type ImageInfoSpecificQCow2 should be updated to mention this information, and qemu-img updated to output it. Being able to create and remove persistent bitmaps from an at-rest qcow2 file without having to fire up qemu to issue QMP commands to do so would also be nice. It's a bit late for getting this request into 2.12, but had it been available, it would make my work at testing that 'qemu-img commit' doesn't corrupt persistent bitmaps. You are right, this is needed, but I can't promise something for the nearest future.. -- Best regards, Vladimir
Re: [Qemu-devel] [PATCH v22 00/30] qcow2: persistent dirty bitmaps
Revisiting an old series: On 06/28/2017 07:05 AM, Vladimir Sementsov-Ogievskiy wrote: > Hi all! > > There is a new update of qcow2-bitmap series - v22. > > web: > https://src.openvz.org/users/vsementsov/repos/qemu/browse?at=qcow2-bitmap-v22 > git: https://src.openvz.org/scm/~vsementsov/qemu.git (tag qcow2-bitmap-v22) > > v22: > > Rebase on master, so changes, mostly related to new dirty bitmaps mutex: > > qcow2: add persistent dirty bitmaps support > > block.c | 65 +- > block/Makefile.objs |2 +- > block/dirty-bitmap.c | 154 - > block/io.c |8 + > block/qcow2-bitmap.c | 1481 > ++ > block/qcow2-refcount.c | 59 +- > block/qcow2.c| 155 - > block/qcow2.h| 43 ++ > blockdev.c | 73 ++- > docs/interop/qcow2.txt |8 +- > include/block/block.h|3 + > include/block/block_int.h| 14 + > include/block/dirty-bitmap.h | 22 +- > include/qemu/hbitmap.h | 49 +- > qapi/block-core.json | 42 +- > tests/Makefile.include |2 +- > tests/qemu-iotests/165 | 105 +++ > tests/qemu-iotests/165.out |5 + > tests/qemu-iotests/group |1 + > tests/test-hbitmap.c | 19 + > util/hbitmap.c | 51 +- > 21 files changed, 2280 insertions(+), 81 deletions(-) Nothing in this series touched qemu-img. At the very minimum, it would be nice if 'qemu-img info' were to display a summary of all bitmaps in the qcow2 file; the QMP type ImageInfoSpecificQCow2 should be updated to mention this information, and qemu-img updated to output it. Being able to create and remove persistent bitmaps from an at-rest qcow2 file without having to fire up qemu to issue QMP commands to do so would also be nice. It's a bit late for getting this request into 2.12, but had it been available, it would make my work at testing that 'qemu-img commit' doesn't corrupt persistent bitmaps. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v22 00/30] qcow2: persistent dirty bitmaps
On 2017-06-28 14:05, Vladimir Sementsov-Ogievskiy wrote: > Hi all! > > There is a new update of qcow2-bitmap series - v22. Thank you for all the work. After resolving some conflicts with the qcow2 LUKS series (and after bisecting some unrelated iotest 055 breakage), I've applied this series to my block branch: https://github.com/XanClic/qemu/commits/block (I've also fixed the indentation issue Eric has found in patch 17.) Max signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v22 00/30] qcow2: persistent dirty bitmaps
On 06/28/2017 08:05 AM, Vladimir Sementsov-Ogievskiy wrote:
> Hi all!
>
> There is a new update of qcow2-bitmap series - v22.
>
> web:
> https://src.openvz.org/users/vsementsov/repos/qemu/browse?at=qcow2-bitmap-v22
> git: https://src.openvz.org/scm/~vsementsov/qemu.git (tag qcow2-bitmap-v22)
>
Minor rebase conflicts, I staged a branch here:
https://github.com/qemu/qemu/compare/master...jnsnow:review-vlad-22
As for the conflicts, they are:
08: Conflict when qed-gencb was removed; trivial fix.
https://github.com/jnsnow/qemu/commit/ffa326ca1f8ee972dbaf2fd5ec1bd38c0ab1b3fd#diff-ddc19015eeeab7a5c73e0a51a8933e8fL2
10: 'count' was changed to 'bytes' which disrupted patch context.
https://github.com/jnsnow/qemu/commit/19a6311ed09823ff2fb2b2bde4932cf53707c1c9#diff-02e65af103cfc4966c51c5aefe38da85L2272
Thanks,
--js
> v22:
>
> Rebase on master, so changes, mostly related to new dirty bitmaps mutex:
>
> 10: - asserts now in bdrv_{re,}set_dirty_bitmap_locked functions.
> - also add assert into bdrv_undo_clear_dirty_bitmap (the only change, not
> related to rebase)
> - add mutex lock into bdrv_dirty_bitmap_set_readonly (as it changes
> bitmap list,
> so the lock should be taken)
> - return instead of go-to in qmp_block_dirty_bitmap_clear
> - in dirty-bitmaps.h, move bdrv_dirty_bitmap_set_readonly before block
> "Functions that require manual locking", move
> bdrv_dirty_bitmap_readonly and bdrv_has_readonly_bitmaps into this block
> 15: - add mutex lock/unlock into bdrv_dirty_bitmap_set_autoload
> - in dirty-bitmaps.h, move bdrv_dirty_bitmap_set_autoload before block
> "Functions that require manual locking", move
> bdrv_dirty_bitmap_get_autoload into this block
> 17: - add mutex lock/unlock into bdrv_dirty_bitmap_set_persistance
> - in dirty-bitmaps.h, move bdrv_dirty_bitmap_set_persistance before block
> "Functions that require manual locking", move
> bdrv_dirty_bitmap_get_persistance and
> bdrv_has_changed_persistent_bitmaps into this block
> 18: in dirty-bitmaps.h, move bdrv_dirty_bitmap_next into block
> "Functions that require manual locking". (do not remove r-b, as it is
> just one empty line removed before function declaration)
> 23: return instead of go-to in qmp_block_dirty_bitmap_add
> 24: return instead of go-to in qmp_block_dirty_bitmap_add
> 25: - return instead of go-to
> - remove aio_context_acquire/release calls
> - no aio_context parameter for block_dirty_bitmap_lookup
> - in dirty-bitmaps.h, move bdrv_dirty_bitmap_sha256 into block
> "Functions that require manual locking".
> 29: - return instead of go-to in qmp_block_dirty_bitmap_remove
>
> r-b's are dropped from 10,15,17,25.
>
> v21:
>
> 09,10: improve comment, add r-b's by Max and John
> 10: improve comment,k
> 11,12: add r-b by John
> 13: prepend local_err with additional info (Max), add r-b by John
> 14: add r-b's by Max and John
> 20,30: add r-b by Max
>
>
> v20:
>
> handle reopening images ro and rw.
>
> On reopening ro: store bitmaps (storing sets 'IN_USE'=0 in the image)
> and mark them readonly (set readonly flag in BlockDirtyBitmap)
>
> After reopening rw: mark bitmaps IN_USE in the image
> and unset readonly flag in BlockDirtyBitmap
>
> 09: new
> 10: improve comment
> add parameter 'value' to bdrv_dirty_bitmap_set_readonly
> 11: use new parameter of bdrv_dirty_bitmap_set_readonly
> 12-14, 20: new
>
> v19:
>
> rebased on master
>
> 05: move 'sign-off' over 'reviewed-by's
> 08: error_report -> error_setg in qcow2_truncate (because of rebase)
> 09: return EPERM in bdrv_aligned_pwritev and bdrv_co_pdiscard if there
> are readonly bitmaps. EPERM is chosen because it is already used for
> readonly image in bdrv_co_pdiscard.
> Also handle readonly bitmap in block_dirty_bitmap_clear_prepare and
> qmp_block_dirty_bitmap_clear
> Max's r-b is not added
> 10: fix grammar in comment
> add Max's r-b
> 12, 13, 15, 21: add Max's r-b
> 24: fix grammar in comment
> 25: fix grammar and wording in comment
> also, I see contextual changes in inactiavate mechanism. Hope, they do not
> affect these series.
>
> v18:
>
> rebased on master (sorry for v17)
>
> 08: contextual: qcow2_do_open is changed instead of qcow2_open
> rename s/need_update_header/update_header/ in qcow2_do_open, to not do it
> in 10
> save r-b's by Max and John
> 09: new patch
> 10: load_bitmap_data: do not clear bitmap parameter - it should be already
> cleared
> (it actually created before single load_bitmap_data() call)
> if some bitmaps are loaded, but we can't write the image (it is readonly
> or inactive), so we can't mark bitmaps "in use" in the image, mark
> corresponding BdrvDirtyBitmap read-only.
> change error_setg to error_setg_errno for "Can't update bitmap directory"
> no needs to rename s/need_update_header/update_header/ here, as it done
> in 08
> 13: function
Re: [Qemu-devel] [PATCH v22 00/30] qcow2: persistent dirty bitmaps
28.06.2017 16:01, Paolo Bonzini wrote:
On 28/06/2017 14:05, Vladimir Sementsov-Ogievskiy wrote:
Rebase on master, so changes, mostly related to new dirty bitmaps mutex:
10: - asserts now in bdrv_{re,}set_dirty_bitmap_locked functions.
- also add assert into bdrv_undo_clear_dirty_bitmap (the only change, not
related to rebase)
- add mutex lock into bdrv_dirty_bitmap_set_readonly (as it changes bitmap
list,
so the lock should be taken)
- return instead of go-to in qmp_block_dirty_bitmap_clear
- in dirty-bitmaps.h, move bdrv_dirty_bitmap_set_readonly before block
"Functions that require manual locking", move
bdrv_dirty_bitmap_readonly and bdrv_has_readonly_bitmaps into this block
15: - add mutex lock/unlock into bdrv_dirty_bitmap_set_autoload
- in dirty-bitmaps.h, move bdrv_dirty_bitmap_set_autoload before block
"Functions that require manual locking", move
bdrv_dirty_bitmap_get_autoload into this block
17: - add mutex lock/unlock into bdrv_dirty_bitmap_set_persistance
- in dirty-bitmaps.h, move bdrv_dirty_bitmap_set_persistance before block
"Functions that require manual locking", move
bdrv_dirty_bitmap_get_persistance and
bdrv_has_changed_persistent_bitmaps into this block
18: in dirty-bitmaps.h, move bdrv_dirty_bitmap_next into block
"Functions that require manual locking". (do not remove r-b, as it is
just one empty line removed before function declaration)
23: return instead of go-to in qmp_block_dirty_bitmap_add
24: return instead of go-to in qmp_block_dirty_bitmap_add
25: - return instead of go-to
- remove aio_context_acquire/release calls
- no aio_context parameter for block_dirty_bitmap_lookup
- in dirty-bitmaps.h, move bdrv_dirty_bitmap_sha256 into block
"Functions that require manual locking".
29: - return instead of go-to in qmp_block_dirty_bitmap_remove
All looks good, thanks. I'll rebase my own fixes on top of these
patches, no need to have you respin them.
Paolo
Thank you! And for thunderbird-work-around too!
--
Best regards,
Vladimir
Re: [Qemu-devel] [PATCH v22 00/30] qcow2: persistent dirty bitmaps
On 28/06/2017 14:05, Vladimir Sementsov-Ogievskiy wrote:
> Rebase on master, so changes, mostly related to new dirty bitmaps mutex:
>
> 10: - asserts now in bdrv_{re,}set_dirty_bitmap_locked functions.
> - also add assert into bdrv_undo_clear_dirty_bitmap (the only change, not
> related to rebase)
> - add mutex lock into bdrv_dirty_bitmap_set_readonly (as it changes
> bitmap list,
> so the lock should be taken)
> - return instead of go-to in qmp_block_dirty_bitmap_clear
> - in dirty-bitmaps.h, move bdrv_dirty_bitmap_set_readonly before block
> "Functions that require manual locking", move
> bdrv_dirty_bitmap_readonly and bdrv_has_readonly_bitmaps into this block
> 15: - add mutex lock/unlock into bdrv_dirty_bitmap_set_autoload
> - in dirty-bitmaps.h, move bdrv_dirty_bitmap_set_autoload before block
> "Functions that require manual locking", move
> bdrv_dirty_bitmap_get_autoload into this block
> 17: - add mutex lock/unlock into bdrv_dirty_bitmap_set_persistance
> - in dirty-bitmaps.h, move bdrv_dirty_bitmap_set_persistance before block
> "Functions that require manual locking", move
> bdrv_dirty_bitmap_get_persistance and
> bdrv_has_changed_persistent_bitmaps into this block
> 18: in dirty-bitmaps.h, move bdrv_dirty_bitmap_next into block
> "Functions that require manual locking". (do not remove r-b, as it is
> just one empty line removed before function declaration)
> 23: return instead of go-to in qmp_block_dirty_bitmap_add
> 24: return instead of go-to in qmp_block_dirty_bitmap_add
> 25: - return instead of go-to
> - remove aio_context_acquire/release calls
> - no aio_context parameter for block_dirty_bitmap_lookup
> - in dirty-bitmaps.h, move bdrv_dirty_bitmap_sha256 into block
> "Functions that require manual locking".
> 29: - return instead of go-to in qmp_block_dirty_bitmap_remove
All looks good, thanks. I'll rebase my own fixes on top of these
patches, no need to have you respin them.
Paolo
[Qemu-devel] [PATCH v22 00/30] qcow2: persistent dirty bitmaps
Hi all!
There is a new update of qcow2-bitmap series - v22.
web:
https://src.openvz.org/users/vsementsov/repos/qemu/browse?at=qcow2-bitmap-v22
git: https://src.openvz.org/scm/~vsementsov/qemu.git (tag qcow2-bitmap-v22)
v22:
Rebase on master, so changes, mostly related to new dirty bitmaps mutex:
10: - asserts now in bdrv_{re,}set_dirty_bitmap_locked functions.
- also add assert into bdrv_undo_clear_dirty_bitmap (the only change, not
related to rebase)
- add mutex lock into bdrv_dirty_bitmap_set_readonly (as it changes bitmap
list,
so the lock should be taken)
- return instead of go-to in qmp_block_dirty_bitmap_clear
- in dirty-bitmaps.h, move bdrv_dirty_bitmap_set_readonly before block
"Functions that require manual locking", move
bdrv_dirty_bitmap_readonly and bdrv_has_readonly_bitmaps into this block
15: - add mutex lock/unlock into bdrv_dirty_bitmap_set_autoload
- in dirty-bitmaps.h, move bdrv_dirty_bitmap_set_autoload before block
"Functions that require manual locking", move
bdrv_dirty_bitmap_get_autoload into this block
17: - add mutex lock/unlock into bdrv_dirty_bitmap_set_persistance
- in dirty-bitmaps.h, move bdrv_dirty_bitmap_set_persistance before block
"Functions that require manual locking", move
bdrv_dirty_bitmap_get_persistance and
bdrv_has_changed_persistent_bitmaps into this block
18: in dirty-bitmaps.h, move bdrv_dirty_bitmap_next into block
"Functions that require manual locking". (do not remove r-b, as it is
just one empty line removed before function declaration)
23: return instead of go-to in qmp_block_dirty_bitmap_add
24: return instead of go-to in qmp_block_dirty_bitmap_add
25: - return instead of go-to
- remove aio_context_acquire/release calls
- no aio_context parameter for block_dirty_bitmap_lookup
- in dirty-bitmaps.h, move bdrv_dirty_bitmap_sha256 into block
"Functions that require manual locking".
29: - return instead of go-to in qmp_block_dirty_bitmap_remove
r-b's are dropped from 10,15,17,25.
v21:
09,10: improve comment, add r-b's by Max and John
10: improve comment,k
11,12: add r-b by John
13: prepend local_err with additional info (Max), add r-b by John
14: add r-b's by Max and John
20,30: add r-b by Max
v20:
handle reopening images ro and rw.
On reopening ro: store bitmaps (storing sets 'IN_USE'=0 in the image)
and mark them readonly (set readonly flag in BlockDirtyBitmap)
After reopening rw: mark bitmaps IN_USE in the image
and unset readonly flag in BlockDirtyBitmap
09: new
10: improve comment
add parameter 'value' to bdrv_dirty_bitmap_set_readonly
11: use new parameter of bdrv_dirty_bitmap_set_readonly
12-14, 20: new
v19:
rebased on master
05: move 'sign-off' over 'reviewed-by's
08: error_report -> error_setg in qcow2_truncate (because of rebase)
09: return EPERM in bdrv_aligned_pwritev and bdrv_co_pdiscard if there
are readonly bitmaps. EPERM is chosen because it is already used for
readonly image in bdrv_co_pdiscard.
Also handle readonly bitmap in block_dirty_bitmap_clear_prepare and
qmp_block_dirty_bitmap_clear
Max's r-b is not added
10: fix grammar in comment
add Max's r-b
12, 13, 15, 21: add Max's r-b
24: fix grammar in comment
25: fix grammar and wording in comment
also, I see contextual changes in inactiavate mechanism. Hope, they do not
affect these series.
v18:
rebased on master (sorry for v17)
08: contextual: qcow2_do_open is changed instead of qcow2_open
rename s/need_update_header/update_header/ in qcow2_do_open, to not do it
in 10
save r-b's by Max and John
09: new patch
10: load_bitmap_data: do not clear bitmap parameter - it should be already
cleared
(it actually created before single load_bitmap_data() call)
if some bitmaps are loaded, but we can't write the image (it is readonly
or inactive), so we can't mark bitmaps "in use" in the image, mark
corresponding BdrvDirtyBitmap read-only.
change error_setg to error_setg_errno for "Can't update bitmap directory"
no needs to rename s/need_update_header/update_header/ here, as it done in
08
13: function bdrv_has_persistent_bitmaps becomes
bdrv_has_changed_persistent_bitmaps,
to handle readonly field.
14: declaration moved to the bottom of .h, save r-b's
15: firstly check bdrv_has_changed_persistent_bitmaps and only then fail on
!can_write, and then QSIMPLEQ_INIT(_tables)
skip readonly bitmaps in saving loop
18: remove '#optional', 2.9 -> 2.10, save r-b's
19: remove '#optional', 2.9 -> 2.10, save r-b's
20: 2.9 -> 2.10, save r-b's
21: add check of read-only image open, drop r-b's
24: add comment to qapi/block-core.json, that block-dirty-bitmap-add removes
bitmap
from storage. r-b's by Max and John saved
v17:
08: add r-b's by Max and John
09: clear unknown autoclear features from BDRVQcow2State before calling
qcow2_load_autoloading_dirty_bitmaps(), and also do not extra update
header if it is
