Re: [PATCH RFC C0/2] support allocation-map for block-dirty-bitmap-merge

[Vladimir Sementsov-Ogievskiy]

27.04.2021 14:11, 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.
>
> 2. Transaction support is not here now. Will add in future version, if
> general approach is OK.

Ha actually, I think it should work, as block-dirty-bitmap-merge is already 
transactional, and we don't break it by the commit.

> 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.
>
> 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.
>
> 5. Probably we still need to create some BdrvChild to avoid node resize
> during the loop of block-status querying.
>
> 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.

8. Name "bitmaps" for list of sources in block-dirty-bitmap-merge becomes misleading. Probably we should add 
similar "sources" list as alternative to "bitmaps", and deprecate "bitmaps" field.


--
Best regards,
Vladimir