On 4/27/21 7:11 AM, Vladimir Sementsov-Ogievskiy wrote:
Hi all!
It's a simpler alternative for
"[PATCH v4 0/5] block: add block-dirty-bitmap-populate job"
<20200902181831.2570048-1-eblake@redhat.com>
https://lists.gnu.org/archive/html/qemu-devel/2020-09/msg00978.html
https://patchew.org/QEMU/20200902181831.2570048-1-eblake@redhat.com/
Since we have "coroutine: true" feature for qmp commands, I think,
maybe we can merge allocation status to bitmap without bothering with
new block-job?
It's an RFC:
1. Main question: is it OK as a simple blocking command, even in a
coroutine mode. It's a lot simpler, and it can be simply used in a
transaction with other bitmap commands.
Hm, possibly... I did not follow the discussion of coroutine QMP commands
closely to know what the qualifying criteria to use them are.
(Any wisdom for me here, Markus?)
2. Transaction support is not here now. Will add in future version, if
general approach is OK.
That should be alright, I think. It means that the operation needs to succeed
before the transaction returns success, though.
Depending on what else is in the transaction, do we run the risk of a deadlock
if we need to wait for a coroutine to finish?
3. I just do bdrv_co_enter() / bdrv_co_leave() like it is done in the
only coroutine qmp command - block_resize(). I'm not sure how much is it
correct.
See above concern!
4. I don't do any "drain". I think it's not needed, as intended usage
is to merge block-status to _active_ bitmap. So all concurrent
operations will just increase dirtyness of the bitmap and it is OK.
That sounds fine for individual usage, but I can't convince myself it's safe
for transactions.
5. Probably we still need to create some BdrvChild to avoid node resize
during the loop of block-status querying.
I'm less sure that it's OK to cause temporary graph changes during the course
of a blocking QMP function... but maybe that's OK?
Peter Krempa is the expert to consult on that one.
6. Test is mostly copied from parallels-read-bitmap, I'll refactor it in
next version to avoid copy-paste.
7. Probably patch 01 is better be split into 2-3 patches.
Vladimir Sementsov-Ogievskiy (2):
qapi: block-dirty-bitmap-merge: support allocation maps
iotests: add allocation-map-to-bitmap
qapi/block-core.json | 31 ++++++++-
include/block/block_int.h | 4 ++
block/dirty-bitmap.c | 42 ++++++++++++
block/monitor/bitmap-qmp-cmds.c | 55 +++++++++++++---
.../tests/allocation-map-to-bitmap | 64 +++++++++++++++++++
.../tests/allocation-map-to-bitmap.out | 9 +++
6 files changed, 195 insertions(+), 10 deletions(-)
create mode 100755 tests/qemu-iotests/tests/allocation-map-to-bitmap
create mode 100644 tests/qemu-iotests/tests/allocation-map-to-bitmap.out