Re: [PATCH v3 7/9] qcow2: Expose bitmaps' size during measure

[Eric Blake]

On 5/12/20 3:35 PM, Nir Soffer wrote:

> > First, a usage question: would you rather that 'qemu-img convert
> > --bitmaps' silently succeeds even when the image being converted has no
> > bitmaps, or would you rather that it fails loudly when there are no
> > bitmaps to be copied over?
>
> I think the meaning of --bitmaps should be "copy also bitmaps".
>
> This request makes sense only for qcow2 images, since other images do
> not have bitmaps, so failing loudly when the source format does not support
> bitmaps seems right.
>
> Same for the target format does not support bitmaps, this is invalid request
> and it should fail loudly.
>
> If the source and target are qcow2, and there are no bitmaps in source, I don't
> see any reason to fail. We don't want to check if an image has bitmaps before
> we copy the image, it does not help us.
>
> > As implemented in this patch series, patch 8
> > currently silently succeeds.
>
> Sounds good for qcow2 format.

So, to make sure I'm clear, you'll be happy with something like:

- no source
$ qemu-img measure --bitmaps -O qcow2 1G
error: --bitmaps requires source file

- src cannot support bitmaps
$ qemu-img measure --bitmaps -f raw -O qcow2 file.raw
error: raw source does not support bitmaps
$ qemu-img measure -f raw -O qcow2 file.raw
success, totals are unchanged (since there are no bitmaps)
$ qemu-img convert --bitmaps -f raw -O qcow2 file.raw file.qcow2
error: raw source does not support bitmaps

- dest cannot support bitmaps
$ qemu-img measure --bitmaps -f qcow2 -O raw file.qcow2
error: raw destination does not support bitmaps
$ qemu-img measure -f qcow2 -O raw file.qcow2
success, totals are unchanged (since there are no bitmaps)
$ qemu-img convert --bitmaps -f qcow2 -O raw file.qcow2 file.raw
error: raw destination does not support bitmaps

- another way dest cannot support bitmaps
$ qemu-img convert --bitmaps -f qcow2 -O qcow2 -o compat=0.10 \
  src.qcow2 dst.qcow2
error: v2 qcow2 destination does not support bitmaps

- src and dest support bitmaps, but there are none
$ qemu-img measure --bitmaps -f qcow2 -O qcow2 file.qcow2
success, no modification to totals since no bitmaps to copy
$ qemu-img measure -f qcow2 -O qcow2 file.qcow2
success, separate line item for 'bitmap size: 0'
$ qemu-img convert --bitmaps -f qcow2 -O qcow2 src.qcow2 dst.qcow2
success, even though no bitmaps to copy

- src and dest support bitmaps, and there are some
$ qemu-img measure --bitmaps -f qcow2 -O qcow2 file.qcow2
success, totals modified in place to include bitmaps without separate line
$ qemu-img measure -f qcow2 -O qcow2 file.qcow2
success, separate line item added for 'bitmap size: NNN'
$ qemu-img convert --bitmaps -f qcow2 -O qcow2 src.qcow2 dst.qcow2
success, and bitmaps were copied

> > But in order to make patch 7 and 8
> > consistent with one another, I need to know which behavior is easier to
> > use: failing convert if the source lacks bitmaps (and thus measure would
> > suppress the bitmaps:0 output), or treating lack of bitmaps as nothing
> > additional to copy and thereby succeeding (and thus measure should
> > output bitmaps:0 to show that no additional space is needed because
> > nothing else will be copied, successfully).
>
> I don't think showing "bitmaps: 0" in measure is related to how --bitmaps
> behave in convert. If we will have --bitmaps in measure, we don't need to
> show "bitmaps" at all since "required" will include it.

Showing 'bitmaps: 0' in the QMP is how the CLI will know whether it was 
an error to request measuring with bitmaps.

> If we want to show bitmaps in measure, I think using the same logic is fine:
> - if format does not support bitmaps - fail
> - if format suppots bitmaps, show what we have - zero is valid result when
>    image does not have any bitmap.

More precisely, fail in QMP if either the source being measured cannot 
support bitmaps at all, or if the destination that determines the 
measurements cannot support bitmaps. But if both support bitmaps, then 
the QMP will output 0 if there happen to be no bitmaps to copy, and the 
CLI can add 'measure --bitmaps' to silently fold in a present bitmaps 
number (be it 0 or non-zero) into the other fields, or error if QMP did 
not include a bitmaps field.

> > And then back to my question: should 'measure --bitmaps' fail if there
> > are no bitmaps to be measured, or silently succeed and not change the
> > output size?
>
> For raw file yes (invalid request), for qcow2 file no, it should just
> add 0 since this is the actual
> size required for bitmaps in this image.

Okay, that is slightly different than what Max was describing (which was 
to output 'bitmaps: 0' in QMP even if the source is raw), but easy 
enough to be consistent on, and as this is your feature request, I can 
make v4 along those lines.

> > > > With the current way, to measure an image we need to do:
> > > >
> > > > qemu-img info --output json ...
> > > > check if image contains bitmaps
> > > > qemu-img measure --output json ...
> > > > check if bitmaps are reported.
> >
> > Why do you have to check 'qemu-img info' first?  If measure reports
> > bitmaps, then you know bitmaps can be copied;
>
> This works only if qemu-img measure will report "bitmaps": 0 when there
> are no bitmaps. Otherwise I don't know if this version does not report bitmaps
> because it does not understand them, or because there are no bitmaps.
>
> Using qemu-img info I can tell the difference if measure does not report 0.

But similarly, with the above rules, 'qemu-img measure --bitmaps' will 
either fail because qemu is too old, or fail because the source format 
cannot support bitmaps to be copied; and succeed if 'qemu-img convert 
--bitmaps' will also succeed.  All without having to check 'qemu-img 
info' to see if there were bitmaps.

> > if it doesn't, then you
> > can check info as a fallback path to compute size yourself - but
> > computing the size yourself doesn't help unless you also have fallbacks
> > to copy the bitmaps via QMP commands, because that qemu-img will also
> > lack 'qemu-img convert --bitmaps' or 'qemu-img bitmaps' to do it via
> > qemu-img.
>
> When we started to work on this it was not clear that we will have a
> way to measure
> bitmaps. If we are going to have support both in convert and measure,
> we can check
> capability only in convert or only in measure.

Yes, the point of this series is that it is an all-or-none improvement 
to qemu-img: all of the bitmap manipulations you are wanting will be 
included at the same time in qemu 5.1, and likely backported at the same 
time by whatever downstream distros that want backports included. 
Existing qemu-img 5.0 cannot manipulate bitmaps in any reasonable 
manner, at most it has 'qemu-img info' that tells you that bitmaps 
exist, but you're still stuck with a non-qemu-img fallback if there is 
no qemu-img support for manipulating bitmaps at all (including the 
fallback of just declaring incremental backups unsupported, and using 
full backups instead).


> > > (We could also let img_measure() in qemu-img add @bitmaps to @required
> > > if --bitmaps was given, so we wouldn’t have to pass the bool around; but
> > > I think letting qemu-img fix up a QAPI object filled by the block driver
> > > sounds wrong.  (Because that means the block driver didn’t fill it
> > > correctly.))
> >
> > If we only touch it up in the CLI, then we would have two forms of CLI
> > output:
> >
> > $ qemu-img measure -f qcow2 image.qcow2
> > required size: 8716288
> > fully allocated size: 8716288
> > bitmaps size: 1024
> > $ qemu-img measure -f qcow2 image.qcow2 --bitmaps
> > required size: 8717312
> > fully allocated size: 8717312
>
> I hope we will not have 2 forms. qemu-img is complicated enough ;-)

2 forms is the easiest way to add it.  And it is not that hard to 
explain.  Either you pass --bitmaps up front (no extra lines, but 
success only if measure saw bitmap support, even if the bitmaps occupy 0 
spze), or you don't pass --bitmaps up front (an extra bitmaps: line if 
bitmaps are supported, even if the size is 0, and no change to existing 
lines).  Just because 2 forms exist does not mean you are required to 
use both forms.

> > I'm leaning towards making v4 try a CLI-only 'measure --bitmaps', to see
> > if I can speed the discussion along with concrete patches for comparison.
>
> Thanks, that would be useful.

Okay, that's the 2-form approach.

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