[libvirt] -blockdev bug (and some questions)

[Cole Robinson]

On 10/7/19 2:07 AM, Peter Krempa wrote:
> 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.

Hmm interesting. Maybe we should be raising a Validate failure if any 
<backingStore> is in the persistent XML if -blockdev isn't supported? I 
don't know if it's actually possible but just the general idea that on 
libvirt upgrade the same XML can result in different image topology sent 
to a writeable VM sounds scary.

Thanks,
Cole