Re: [RFC qemu/qemu-server/proxmox-backup-qemu 0/9] close #6169: backup: pbs: per-target dirty bitmaps

[Dominik Csapak <d.csapak@proxmox.com>]

On 9/2/26 3:56 PM, Fiona Ebner wrote:
> To decide whether an incremental backup can be done, the checksum of
> the latest backup on the target is compared against the checksum from
> the previous backup the QEMU client library did.
> 
> This was previously tracked per device as the PVE backup code in QEMU
> also only used a single bitmap per device. So backing up to target A,
> dirtying the disk and backing up to target B would mean that the
> checksum for target A is overwritten and a subsequent backup to target
> A needs to discard the dirty bitmap.
> 
> Allow for tracking multiple targets with different bitmaps, by saving
> the checksums per device+target combination, rather than just per
> device.
> 
> Whether the new behavior or the old behavior is used is controlled by
> QEMU, optionally passing along the target ID, and a flag for the
> export and import functions for migration. These are machine version
> guarded on the QEMU/qemu-server side to avoid a mismatch.
> 
> 
> This is an ABI change for the backup library, so a versioned breaks
> and a versioned depends in the other direction is needed.
> 

i guess it's not really necessary, but couldn't we add new methods
instead of modifying the existing ones? then the breaks pve-qemu-kvm
wouldn't be necessary? (i think?)

not that important since we must bump both anyway

> 
> QUESTION: with old qemu-server, target ID is not provided, but the new
> machine version might be in use. In this case, the dirty bitmap state
> is not migrated, in case of a new -> old -> new migration, which can
> lead to stale info! Would it be okay to add a Breaks for old
> qemu-server or should I try to resolve it differently somehow?

is such migration to an older qemu-server even supported?
IIUC the issue is that the
1. there is a bitmap with a target
2. old qemu-server makes a backup without a target creating
    a new dirty-bitmap (actually invalidating the old one, but
    since the code doesn't know about it doesn't do anything with
    it)
3. the new code uses the target name again, bitmap still existing

the checksum would be different though and it would not be an
incremental backup once, or?

question is, should we support both modes simultaniously at all?
we could require a target always, or if no target is given,
invalidate all named target bitmaps.

also, the same can happen with new versions only too?
backup to target storage X
on pve add identical storage Y -> backup to same storage as
different id
backup with X again -> stale bitmap?

> 
> QUESTION: How to best avoid 'orphaned' bitmaps? Auto-remove for
> storages no longer in configuration? Or have some knob/limit for how
> many to keep?
> 

i think it depends how big these are? I doubt many users
make many backups to storages they abandon later

also how could we differ between a user just recreating
a storage entry (for whatever reason) and a genuine
deletion? (if the backup timing is right)

> 
> The clippy fixes are independent.
> 
> proxmox-backup-qemu:
> 
> Fiona Ebner (6):
>    clippy: restore: fix needless borrows
>    clippy: commands: avoid manual implementation of ok()
>    cargo: add dependency for serde
>    close #6169: track checksums for incremental backups per target
>    update current-api.h
>    d/control: bump versioned breaks for pve-qemu-kvm
> 
>   Cargo.toml      |  1 +
>   current-api.h   |  9 +++--
>   debian/control  |  2 +-
>   src/backup.rs   | 27 ++++++++++----
>   src/commands.rs | 97 +++++++++++++++++++++++++++++++++++++++----------
>   src/lib.rs      | 25 +++++++++----
>   src/restore.rs  | 12 ++----
>   7 files changed, 126 insertions(+), 47 deletions(-)
> 
> 
> qemu:
> 
> Fiona Ebner (2):
>    PVE backup: properly track if snapshot access was set up
>    PVE backup: track per-target dirty bitmaps
> 
>   block/monitor/block-hmp-cmds.c |  1 +
>   hw/core/machine.c              |  2 ++
>   migration/pbs-state.c          | 42 ++++++++++++++++++++++++++++------
>   migration/pbs-state.h          | 32 ++++++++++++++++++++++++++
>   proxmox-backup-client.c        | 12 +++++++++-
>   proxmox-backup-client.h        |  1 +
>   pve-backup.c                   | 29 +++++++++++++++++------
>   qapi/block-core.json           |  7 +++++-
>   8 files changed, 110 insertions(+), 16 deletions(-)
>   create mode 100644 migration/pbs-state.h
> 
> 
> qemu-server:
> 
> Fiona Ebner (1):
>    close #6169: backup: pbs: specify target ID so QEMU keeps track of
>      per-target dirty bitmap
> 
>   src/PVE/VZDump/QemuServer.pm | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> 
> Summary over all repositories:
>    16 files changed, 242 insertions(+), 63 deletions(-)
>