Re: [RFC 0/4] mirror: implement incremental and bitmap modes

[Vladimir Sementsov-Ogievskiy]

On 01.03.24 17:52, Fiona Ebner wrote:
> Am 01.03.24 um 15:14 schrieb Vladimir Sementsov-Ogievskiy:
> > As we already understood, (block-)job-api needs some spring-cleaning.
> > Unfortunately I don't have much time on it, but still I decided to start
> > from finally depreacting block-job-* API and moving to job-*.. Probably
> > bitmap/bitmap-mode/sync APIs also need some optimization, keeping in
> > mind new block-dirty-bitmap-merge api.
> >
> > So, what I could advice in this situation for newc interfaces:
> >
> > 1. be minimalistic
> > 2. add `x-` prefix when unsure
> >
> > So, following these two rules, what about x-bitmap field, which may be
> > combined only with 'full' mode, and do what you need?
>
> AFAIU, it should rather be marked as @unstable in QAPI [0]? Then it
> doesn't need to be renamed if it becomes stable later.

Right, unstable feature is needed, using "x-" is optional.

Recent discussion about it was in my "vhost-user-blk: live resize additional 
APIs" series:

https://patchew.org/QEMU/20231006202045.1161543-1-vsementsov@yandex-team.ru/20231006202045.1161543-5-vsementsov@yandex-team.ru/

Following it, I think it's OK to not care anymore with "x-" prefixes, and rely 
on unstable feature.

> > About documentation: actually, I never liked that we use for backup job
> > "MirrorSyncMode". Now it looks more like "BackupSyncMode", having two
> > values supported only by backup.
> >
> > I'm also unsure how mode=full&bitmap=some_bitmap differs from
> > mode=bitmap&bitmap=some_bitmap..
>
> With the current patches, it was an error to specify @bitmap for other
> modes than 'incremental' and 'bitmap'.

Current documentation says:
  # @bitmap: The name of a dirty bitmap to use.  Must be present if sync
  #     is "bitmap" or "incremental". Can be present if sync is "full"
  #     or "top".  Must not be present otherwise.
  #     (Since 2.4 (drive-backup), 3.1 (blockdev-backup))


> > So, I'd suggest simply rename MirrorSyncMode to BackupSyncMode, and add
> > separate MirrorSyncMode with only "full", "top" and "none" values.
>
> Sounds good to me!
>
> [0]:
> https://gitlab.com/qemu-project/qemu/-/commit/a3c45b3e62962f99338716b1347cfb0d427cea44
>
> Best Regards,
> Fiona

--
Best regards,
Vladimir