[libvirt] -blockdev bug (and some questions)

[Peter Krempa]

On Sat, Oct 05, 2019 at 17:39:01 -0400, Cole Robinson wrote:
> Hi Peter + list,
> 
> I'm testing libvirt -blockdev, using your enablement patches from here:
> https://www.redhat.com/archives/libvir-list/2019-September/msg00524.html
> 
> I'm hitting an issue but the fix isn't clear to me. Take a VM with standard
> disk XML, using a qcow2 image with a backing file
> 
>     <disk type='file' device='disk'>
>       <driver name='qemu' type='qcow2'/>
>       <source file='/OVERLAY.qcow2'/>
>       <target dev='vda' bus='virtio'/>
>     </disk>
> 
> The VM starts fine, generated -blockdev options are:
> 
> -blockdev '{"driver":"file","filename":"/BACKING.qcow2","node-name":"libvirt-2-storage","auto-read-only":true,"discard":"unmap"}'
> \
> -blockdev '{"node-name":"libvirt-2-format","read-only":true,"driver":"qcow2","file":"libvirt-2-storage","backing":null}'
> \
> -blockdev '{"driver":"file","filename":"/OVERLAY.qcow2","node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}'
> \
> -blockdev '{"node-name":"libvirt-1-format","read-only":false,"driver":"qcow2","file":"libvirt-1-storage","backing":"libvirt-2-format"}'
> \
> 
> Stop the VM. Edit the disk XML to have a single stub <backingStore/>
> element:
> 
>     <disk type='file' device='disk'>
>       <driver name='qemu' type='qcow2'/>
>       <source file='/OVERLAY.qcow2'/>
>       <backingStore/>
>       <target dev='vda' bus='virtio'/>
>     </disk>
> 
> (FWIW my VM just happened to have this element in the XML, not sure where it
> came from, maybe some wayward XML edit at some point)
> 
> Start the VM. It starts, but it's not using the backing file, so things
> don't really work. The generate -blockdev bits are:
> 
> -blockdev '{"driver":"file","filename":"/OVERLAY.qcow2","node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}'
> \
> -blockdev '{"node-name":"libvirt-1-format","read-only":false,"driver":"qcow2","file":"libvirt-1-storage","backing":null}'
> \
> 
> This all works correctly with non-blockdev master. The difference comes from
> this code in qemuProcessPrepareHostStorage:

This is expected with blockdev. With blockdev you can put the backing
chain into the inactive XML and it will be honoured regardless of what
the qcow2 metadata says.

Your configuration says that the /OVERLAY.qcow2 image is supposed to
have no backing images and that will be exposed in the configuration.

Doing that was not possible without blockdev and thus we always
discarded the backing chain in the inactive XML and re-probed it.

> 
>   /* backing chain needs to be redetected if we aren't using blockdev */
>   if (!blockdev)
>       virStorageSourceBackingStoreClear(disk->src);
> 
> The <backingStore/> causes a disk->src->backingStore to be allocated by the
> domain_conf XML parser. On git master it is removed here. Later in
> qemuDomainDetermineDiskChain, it basically only probes the root image file
> if !disk->src->backingStore. Since the blockdev case doesn't call
> BackingStoreClear, the root image file is never probed in
> qemuDomainDetermineDiskChain, which causes the differing behavior.
> 
> Do you have a suggestion for a proper fix? I can try implementing it if
> you'd like, it will help me better understand this stuff.
> 
> More generally I have questions about why we track backingStore in the XML
> for an offline VM anyways? I presume this is intentional, but I don't
> understand the usecases. Can you provide more info or link me to somewhere
> if this has been discussed before?

As said above this is desired. The qcow2 metadata as created usually by
libvirt contains full absolute paths to images. This means that if you
want to move your image with the backing chain somewhere else you'd have
to tweak the metadata.

With blockdev you are able to fully configure this so putting a

     <disk type='file' device='disk'>
       <driver name='qemu' type='qcow2'/>
       <source file='/OVERLAY.qcow2'/>
       <backingStore type='file'>
         <source file='/backing.qcow2'/>
         </backingStore>
       </backingStore>
       <target dev='vda' bus='virtio'/>
     </disk>

will be honoured as written.

> 
> FWIW the XML docs claim backingStore is output only, but seems to contradict
> itself in the next sentence, and they aren't helping me much to understand
> the big picture: https://libvirt.org/formatdomain.html#elementsDisks

I didn't finish the docs for blockdev yet so I plan to address that
before enabling it.