Re: [libvirt] [PATCH 01/21] API: Introduce field for reporting temporary disk space usage of a domain job

[Eric Blake]

On 11/27/19 9:28 AM, Eric Blake wrote:
> [adding some qemu folks for a side note]
>
> On 11/26/19 3:39 PM, Peter Krempa wrote:
> > A pull mode backup job uses temporary disk images to hold the changed
> > parts of the disk while the client is copying the changes. Since usage
> > of the temporary space can be monitored but doesn't really fit any of
> > the existing stats fields introduce new fields for reporting this data.
> >
> > Signed-off-by: Peter Krempa <address@hidden>
> > ---
> >   include/libvirt/libvirt-domain.h | 14 ++++++++++++++
> >   1 file changed, 14 insertions(+)
>
> Nice addition.
>
> Note that in an incremental backup, the temporary file should never grow 
> larger in size than the bitmap that is feeding it (as only the clusters 
> represented in the bitmap have to do a copy-on-write to the temporary file)

Arrgh; I hit send too soon.

As currently implemented in qemu, the temporary file CAN grow larger in 
size than the bitmap feeding it.  Even though the user only cares about 
the clusters in the dirty bitmap, qemu is performing copy-on-write for 
ALL clusters.

> I'd still thinking about a possible tweak to qemu to make it possible to 
> guarantee that the temporary file during an incremental pull mode cannot 
> occupy more space than the size covered by the bitmap (right now, if the 
> backup lasts long enough, a guest could proceed to overwrite the entire 
> disk, and the temporary file would grow to the guest-visible size; but 
> if I add a mode where the NBD export returns EIO for attempts to read a 
> portion of the disk not covered by the bitmap, then only the portions 
> covered by the bitmap need to worry about copy-on-write to the temporary 
> file).

It is this future change to qemu that would allow us to limit the size 
of the temporary file to the bitmap - but it comes at the expense of 
forcing the NBD client to be unable to read the non-dirty portions of 
the file (if they are truly doing an incremental backup, they weren't 
going to read those portions anyway).  So even if we add such a knob in 
qemu, we'll still need a way for libvirt to opt in or out of using the 
mode, to control whether the NBD client can read the entire disk from 
the point of the backup or only the portions of the disk covered by the 
bitmap.

> > diff --git a/include/libvirt/libvirt-domain.h 
> > b/include/libvirt/libvirt-domain.h
> > index 0e490254fd..b52f58aa8f 100644
> > --- a/include/libvirt/libvirt-domain.h
> > +++ b/include/libvirt/libvirt-domain.h
> > @@ -3586,6 +3586,20 @@ typedef enum {
> >    */
> >   # define VIR_DOMAIN_JOB_SUCCESS "success"
> >
> > +/**
> > + * VIR_DOMAIN_JOB_DISK_TEMP_USED:
> > + * virDomainGetJobStats field: current usage of temporary disk space 
> > for the
> > + * job in bytes as VIR_TYPED_PARAM_ULLONG.
> > + */
> > +# define VIR_DOMAIN_JOB_DISK_TEMP_USED "disk_temp_used"
> > +
> > +/**
> > + * VIR_DOMAIN_JOB_DISK_TEMP_TOTAL:
> > + * virDomainGetJobStats field: possible total temporary disk space 
> > for the
> > + * job in bytes as VIR_TYPED_PARAM_ULLONG.
> > + */
> > +# define VIR_DOMAIN_JOB_DISK_TEMP_TOTAL "disk_temp_total"
>
> If I'm running an incremental backup of a 1G disk, and the bitmap says 
> only 512M was dirty, will disk_temp_total report 512M (even though 
> disk_temp_used can grow up to 1G, without my proposed patch to make qemu 
> enforce the use of the temp file only to portions covered by the 
> bitmap), or will disk_temp_total report 1G?

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org