Re: [libvirt] [PATCH v2 21/25] qemu: backup: Implement stats gathering while the job is running
On Thu, Dec 05, 2019 at 13:54:33 -0600, Eric Blake wrote:
On 12/3/19 11:17 AM, Peter Krempa wrote:
> We can use the output of 'query-jobs' to figure out some useful
> information about a backup job. That is progress in case of a push job
> and scratch file use in case of a pull job.
[I still need to finish my review on 20, but this one's easier to knock out
in the short term]
>
> Add a worker which will total up the data and call it from
> qemuDomainGetJobStatsInternal.
>
> Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
> ---
> +static void
> +qemuBackupGetJobInfoStatsUpdateOne(virDomainObjPtr vm,
> +
> + if (push) {
> + stats->total += monblockjob->progressTotal;
> + stats->transferred += monblockjob->progressCurrent;
> + } else {
> + stats->tmp_used += monblockjob->progressCurrent;
> + stats->tmp_total += monblockjob->progressTotal;
> + }
I don't know what qemu reports for the job used/total on the temp image (I
guess the total is the guest-visible disk size?) but this reporting is
reasonable enough for now.
Yes it's the guest visible size, which is basically the maximum amount
that can be written into the scratch image in the current impl. In case
you add the flag you suggested earlier which will make the blocks not
covered by backup inaccessible and thus optimize the size of the scratch
image the job stats reported by qemu should reflect that.
This patch looks fine, so:
Reviewed-by: Eric Blake <eblake(a)redhat.com>
Still, seeing it made me wonder - when I first wrote the bitmap code, I
added flag VIR_DOMAIN_CHECKPOINT_XML_SIZE for grabbing the current size of a
bitmap, but disabled the qemu implementation of it at the last minute when
committing the Checkpoint API because the monitor actions I used to get at
it before -blockdev was not sane. But I don't see it supported anywhere in
this series. The progress stats of a backup job are similar, but at some
point, we do need to get a followup patch that gets size estimation from a
bitmap prior to starting the backup back to viability.
I'm holding that patch until the integration with snapshots is done as
it requires merging of the bitmaps to figure out the size of the backup.