On 6/15/20 12:10 PM, Peter Krempa wrote:
Chaining bitmaps for checkpoints (disabling the active one and creating a new) severely overcomplicated all operations in regards to bitmaps.
Specifically it requires us re-matching the on-disk state to the internal metadata and in case of merging during block jobs it makes it almost impossible to cover all corner cases.
Since the checkpoints and incremental backups were not yet enabled, let's change the design to keep one bitmap per checkpoint. In case of layered snapshots this will be filled in by using dirty-bitmap-populate.
Finally the main reason for this unnecessary complexity was the fear that qemu's performance could degrade. In the end I think that addressing the performance issue will be better done in qemu (e.g by keeping an internal bitmap updated with changes and merging it periodically back to the real bitmaps. QEMU writes out changes to disk at shutdown so consistency is not a problem).
Removing the relationships between bitmaps frees us from complex handling and also makes all the surrounding code more robust as one broken bitmap doesn't necessarily invalidate whole chains of backups.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_checkpoint.c | 21 +-------------------- 1 file changed, 1 insertion(+), 20 deletions(-)
Definitely looks simpler. In graphics, pre-patch we had: time --C1-----C2-----C3-----now :b1....: :b2....: :b3.... where the differential from C1 is b1+b2+b3, the differential from C2 is b2+b3, the incremental from C3 is b3. post-patch, we have: time --C1-----C2-----C3-----now :b1................... :b2............ :b3.... where the differential from C1 is b1, the differential from C2 is b2, the incremental from C3 is b3. Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org