[PATCH] qemu: backup: Install bitmap for incremental backup to appropriate node only

[Peter Krempa]

On Fri, Nov 13, 2020 at 09:07:24 -0600, Eric Blake wrote:
> On 11/13/20 8:55 AM, Peter Krempa wrote:
> > Libvirt's backup code has two modes:
> > 
> > 1) push - where qemu actively writes the difference since the checkpoint
> >           into the output file
> > 
> > 2) pull - where we instruct qemu to expose a frozen disk state along
> >           with a bitmap of blocks which changed since the checkpoint
> > 
> > For push mode qemu needs the temporary bitmap we use where we calculate
> > the actual changes to be present on the block node backing the disk.
> > 
> > For pull mode where we expose the bitmap via NBD qemu actually wants the
> > bitmap to be present for the exported block node which is the scratch
> > file.
> > 
> > Until now we've calculated the bitmap twice and installed it both to the
> > scratch file and to the disk node, but we don't need to since we know
> > when it's needed.
> > 
> > Pass in the 'pull' flag and decide where to install the bitmap according
> > to it and also when to register the bitmap name with the blockjob.
> > 
> > Signed-off-by: Peter Krempa <pkrempa at redhat.com>
> > ---
> >  src/qemu/qemu_backup.c | 42 ++++++++++++++++++++++++++----------------
> >  1 file changed, 26 insertions(+), 16 deletions(-)
> 
> Reviewed-by: Eric Blake <eblake at redhat.com>
> 
> 
> > +    /* For pull-mode backup, we need the bitmap to be present in the scratch
> > +     * file as that will be exported. For push-mode backup the bitmap is
> > +     * actually required on top of the image backing the disk */
> > 
> > -    if (qemuBackupDiskPrepareOneBitmapsChain(dd->domdisk->src,
> > -                                             dd->store,
> > -                                             dd->incrementalBitmap,
> > -                                             dd->backupdisk->incremental,
> > -                                             actions,
> > -                                             blockNamedNodeData) < 0)
> > -        return -1;
> > +    if (pull) {
> > +        if (qemuBackupDiskPrepareOneBitmapsChain(dd->domdisk->src,
> > +                                                 dd->store,
> > +                                                 dd->incrementalBitmap,
> > +                                                 dd->backupdisk->incremental,
> > +                                                 actions,
> > +                                                 blockNamedNodeData) < 0)
> 
> Technically, with both qemu 5.0 and qemu 5.2, the bitmap can live in the
> backing store for BOTH modes, (in 5.2 and beyond, because qemu will
> chase through the backing chain to find the named bitmap if it is not
> present in the temporary store; in 5.0 because the temporary store is
> not a filter node).  But since qemu 5.1 used a filter node for the
> temporary store but was unable to chase through filter nodes, always
> placing the bitmap in the temporary node for pull mode is fine.

I can consider just putting it in one place. Since incremental backup in
libvirt requires 'blockdev-reopen' and that was not made stable yet in
qemu, libvirt will officially support incremental backups only with
qemu-6.0 at soonest.