Re: [PATCH 0/3] qemu-img: Add convert --bitmaps

[Eric Blake <eblake@redhat.com>]

(adding Markus for a CLI question, look for [*])

On 4/16/20 1:20 PM, Nir Soffer wrote:
> On Thu, Apr 16, 2020 at 5:51 PM Eric Blake <eblake@redhat.com> wrote:
>>
>> Without this series, the process for copying one qcow2 image to
>> another including all of its bitmaps involves running qemu and doing
>> the copying by hand with a series of QMP commands.  This makes the
>> process a bit more convenient.
> 
> This seems good for copying an image chain from one storage to another,
> but I think we need a similar --bitmaps option to qemu-img measure to make
> this really useful.
> 
> Here is example use case showing how qemu-img measure is related:
> 
> Source chain:
> /dev/vg1/base
> /dev/vg1/top
> 
> Destination chain:
> /dev/vg2/base
> /dev/vg2/top
> 
> We create empty lvs with the same name on destination storage (/dev/vg2).
> 
> We measure the base lv using qemu-img measure for creating the target lv:
> 
>      qemu-img measure -f qcow2 -O qcow2 /dev/vg1/base
>      lvcreate -L required_size /dev/vg2/base
>      qemu-img create -f qcow2 /dev/vg2/base 10g
> 
> For the top lv we use the current size of the source lv - I think we
> should measure it instead but
> I'm not sure if qemu-img measure supports measuring a single image in a chain
> (maybe -o backing_file?).

qemu-measure --image-opts should be able to measure a single image by 
specifying image opts that purposefully treat the image as standalone 
rather than with its normal backing file included.  Let's see if I can 
whip up an example:

$ qemu-img create -f qcow2 img.base 100M
Formatting 'img.base', fmt=qcow2 size=104857600 cluster_size=65536 
lazy_refcounts=off refcount_bits=16
$ qemu-io -f qcow2 -c 'w 0 25m' img.base
wrote 26214400/26214400 bytes at offset 0
25 MiB, 1 ops; 00.24 sec (103.405 MiB/sec and 4.1362 ops/sec)
$ qemu-img create -f qcow2 -F qcow2 -b img.base img.top
Formatting 'img.top', fmt=qcow2 size=104857600 backing_file=img.base 
backing_fmt=qcow2 cluster_size=65536 lazy_refcounts=off refcount_bits=16
$ qemu-io -f qcow2 -c 'w 25m 25m' img.top
wrote 26214400/26214400 bytes at offset 26214400
25 MiB, 1 ops; 00.24 sec (103.116 MiB/sec and 4.1247 ops/sec)
$ qemu-img measure -f qcow2 -O qcow2 img.base
required size: 26542080
fully allocated size: 105185280
required size: 52756480
fully allocated size: 105185280

Okay, I can reproduce what you are seeing - measuring the top image 
defaults to measuring the full allocation of the entire chain, rather 
than the allocation of just the top image.  And now with --image-opts to 
the rescue:

$ qemu-img measure --image-opts -O qcow2 
driver=qcow2,backing=,file.driver=file,file.filename=img.top
qemu-img: warning: Use of "backing": "" is deprecated; use "backing": 
null instead
required size: 26542080
fully allocated size: 105185280

There you go - by forcing qemu to treat the overlay image as though it 
had no backing, you can then measure that image in isolation.

(*) Hmm - that warning about backing="" being deprecated is annoying, 
but I don't know any other way to use dotted command line syntax and 
still express that we want a QMP null.  I tried to see if I could inject 
an alternative backing driver, such as null-co, but was met with errors:

$ ./qemu-img measure --image-opts -O qcow2 
driver=qcow2,backing.driver=null-co,file.driver=file,file.filename=img.top
qemu-img: Could not open 
'driver=qcow2,backing.driver=null-co,file.driver=file,file.filename=img.top': 
Could not open backing file: The only allowed filename for this driver 
is 'null-co://'
$ ./qemu-img measure --image-opts -O qcow2 
driver=qcow2,backing.driver=null-co,backing.file=null-co://,file.driver=file,file.filename=img.top
qemu-img: Could not open 
'driver=qcow2,backing.driver=null-co,backing.file=null-co://,file.driver=file,file.filename=img.top': 
Could not open backing file: The only allowed filename for this driver 
is 'null-co://'
$ ./qemu-img measure --image-opts -O qcow2 
driver=qcow2,backing.driver=null-co,backing.file.filename=null-co://,file.driver=file,file.filename=img.top
qemu-img: Could not open 
'driver=qcow2,backing.driver=null-co,backing.file.filename=null-co://,file.driver=file,file.filename=img.top': 
Could not open backing file: Block protocol 'null-co' doesn't support 
the option 'file.filename'

We don't want to support "" in the QMP syntax forever, but if the CLI 
syntax has to handle the empty string specially in order to get null 
passed to the QMP code, then so be it.

I also tried, but failed, to use JSON syntax.  I don't know why we 
haven't wired up --image-opts to use JSON syntax yet.

$ qemu-img measure --image-opts -O qcow2 '{"driver":"qcow2", "backing":null,
   "file":{"driver":"file", "filename":"img.top"}}'
qemu-img: Could not open '{"driver":"qcow2", "backing":null,
   "file":{"driver":"file", "filename":"img.top"}}': Cannot find 
device={"driver":"qcow2" nor node_name={"driver":"qcow2"

I guess there's always the pseudo-json protocol:

$ qemu-img measure -O qcow2 'json:{"driver":"qcow2", "backing":null,
   "file":{"driver":"file", "filename":"img.top"}}'
required size: 26542080
fully allocated size: 105185280


> 
>      lvcreate -L current_size /dev/vg2/top
>      qemu-img create -f qcow2 -b /dev/vg2/base -F qcow2 /dev/vg2/top 10g
> 
> And then convert the lvs one by one:
> 
>      qemu-img convert -f qcow2 -O qcow2 -n --bitmaps /dev/vg1/base /dev/vg2/base
>      qemu-img convert -f qcow2 -O qcow2 -n --bitmaps -B /dev/vg2/base
> /dev/vg1/top /dev/vg2/top
> 
> The first copy may fail with ENOSPC since qemu-img measure of the base
> does not consider the
> bitmaps in the required size.

Yes, that's a good argument for adding 'qemu-img measure --bitmaps'.  In 
the meantime...

> 
> So I think we need to add a similar --bitmaps option to qemu-img
> measure, hopefully reusing the
> same code to find and estimate the size of the bitmaps.
> 
> Maybe we can estimate the size using qemu-img info --bitmaps,

...you are correct, this works today.  Well, 'qemu-img info --bitmaps' 
doesn't exist, but 'qemu-img info' does output the number of bitmaps, as 
well as their granularity, and if you also assume that each bitmap is 
sized to match the virtual image size, you can compute the estimated 
space occupied by those bitmaps.

> but I
> think the right way to
> do this is in qemu-img measure.

Yes, even though we have existing multi-step code, being able to do it 
in a single step is justification for the improvement (although you may 
still end up having to code the multi-step mode yourself if you can't 
guarantee new-enough qemu-img - the addition of 'qemu-img measure 
--bitmaps' won't land until at least 4 months from now with qemu 5.1). 
Not to mention that the estimation computation (image size / granularity 
rounded up to the next cluster size, summed over all bitmaps) is hairy 
enough that it shouldn't have to be reimplemented by multiple layers of 
software.

> 
> We have also another use case when we collapsed an image chain to single image:
> 
> Source chain:
> /dev/vg1/base
> /dev/vg1/top
> 
> Destination:
> /dev/vg2/collapsed
> 
> In this case we measure the size of the entire chain (/dev/vg1/base <-
> /dev/vg1/top) and create
> /dev/vg2/collapsed in the correct size, and then we convert the chain using:
> 
>     qemu-img convert /dev/vg1/top /dev/vg2/collapsed

When I submitted my v1 patch, I had only tested with a single source 
image to a single destination.  But now that you mention flattening, 
it's easy enough for me to test of what happens (tl;dr: my patch only 
looks at bitmaps in the active layer of the source):

$ qemu-img create -f qcow2 img.base 100M
Formatting 'img.base', fmt=qcow2 size=104857600 cluster_size=65536 
lazy_refcounts=off refcount_bits=16
$ qemu-img create -f qcow2 -F qcow2 -b img.base img.top
Formatting 'img.top', fmt=qcow2 size=104857600 backing_file=img.base 
backing_fmt=qcow2 cluster_size=65536 lazy_refcounts=off refcount_bits=16
$ qemu-kvm --nographic --nodefaults --qmp stdio
{'execute':'qmp_capabilities'}
{'execute':'blockdev-add','arguments':{'driver':'qcow2',
  'node-name':'base','file':{'driver':'file','filename':'img.base'}}}
{'execute':'blockdev-add','arguments':{'driver':'qcow2',
  'node-name':'top','file':{'driver':'file','filename':'img.top'},
  'backing':'base'}}
{'execute':'block-dirty-bitmap-add','arguments':{'node':'base',
  'name':'b1','persistent':true}}
{'execute':'block-dirty-bitmap-add','arguments':{'node':'base',
  'name':'b2','persistent':true}}
{'execute':'block-dirty-bitmap-add','arguments':{'node':'top',
  'name':'b2','persistent':true}}
{'execute':'block-dirty-bitmap-add','arguments':{'node':'top',
  'name':'b3','persistent':true}}
{'execute':'quit'}
$ qemu-img convert -f qcow2 -O qcow2 --bitmaps img.top img.flat
$ qemu-img info img.flat
image: img.flat
file format: qcow2
virtual size: 100 MiB (104857600 bytes)
disk size: 208 KiB
cluster_size: 65536
Format specific information:
     compat: 1.1
     lazy refcounts: false
     bitmaps:
         [0]:
             flags:
                 [0]: auto
             name: b3
             granularity: 65536
         [1]:
             flags:
                 [0]: auto
             name: b2
             granularity: 65536
     refcount bits: 16
     corrupt: false

It only copied the bitmaps from the active layer, not all bitmaps from 
all layers.  If we want more complicated handling (such as whether to 
pull in bitmaps deeper in the backing chain, rename bitmaps, policies on 
merging duplicated bitmap names across nodes vs. failure, etc), we 
really should be designing 'qemu-img bitmap' that gives full 
command-line control over all sorts of bitmap operations, rather than 
trying to further overload 'qemu-img convert'.

> 
> Currently we use this for exporting images, for example when creating
> templates, or as a simple
> backup. In this case we don't need to copy the bitmaps in the target
> image - this is a new image
> not used by any VM. Copying the bitmaps may also be non-trivial since
> we may have the bitmaps
> with the same names in several layers (e.g. result of live snapshot).
> 
> So I think using --bitmaps should be disabled when doing this kind of
> convert. We can handle this
> on our side easily, but I think this should fail or log a warning on
> qemu-img, or require merging of
> bitmaps with same names during the copy. I did not check if you
> already handle this.

My patch only copies top-most bitmaps.  If you are using qemu-img 
convert to flatten a chain, we'll need something else to control what to 
do with bitmaps across that chain.

> 
> Finally we also have a use case when we copy the chain as is to new or
> same storage, but
> we create a new vm. In this case I don't think the backup history
> makes sense for the new
> vm, so we don't need to copy the bitmaps.
> 
> I will review the rest of the patches next week and can maybe give
> this some testing.
> 

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