On 29.02.24 17:50, Fiona Ebner wrote:
Am 29.02.24 um 13:00 schrieb Vladimir Sementsov-Ogievskiy:
But anyway, this all could be simply achieved with
bitmap-copying/merging API, if we allow to pass user-given bitmap to the
mirror as working bitmap.
I see, I'll drop the 'bitmap-mode' in the next version if nobody
complains :)
Good. It's a golden rule: never make public interfaces which you don't
actually need for production. I myself sometimes violate it and spend
extra time on developing features, which we later have to just drop as
"not needed downstream, no sense in upstreaming".
Just wondering which new mode I should allow for the @MirrorSyncMode
then? The documentation states:
# @incremental: only copy data described by the dirty bitmap.
# (since: 2.4)
#
# @bitmap: only copy data described by the dirty bitmap. (since: 4.2)
# Behavior on completion is determined by the BitmapSyncMode.
For backup, do_backup_common() just maps @incremental to @bitmap +
@bitmap-mode == @on-success.
Using @bitmap for mirror would lead to being at odds with the
documentation, because it mentions the BitmapSyncMode, which mirror
won't have.
Using @incremental for mirror would be consistent with the
documentation, but behave a bit differently from backup.
Opinions?
Good question.
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?
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..
So, I'd suggest simply rename MirrorSyncMode to BackupSyncMode, and add separate MirrorSyncMode with only
"full", "top" and "none" values.
--
Best regards,
Vladimir