Bug 760547

Live snapshot and block pull also need to interact with an explicit backing file chain notation in the domain XML.  My current solution for block pull (see bug 638506) is currently quite weak, relying on the user to pass in a correct name; but if libvirt were properly tracking the backing chain, then libvirt could validate that the user is actually passing in a valid name.

*** Bug 1534490 has been marked as a duplicate of this bug. ***

Created attachment 1474884 [details]
The libvirtd log of comment 30

After the eject failure, the media is still in the xml:
# virsh dumpxml A|awk '/<disk/,/<\/disk/'          
    <disk type='file' device='cdrom'>
      <driver name='qemu' type='raw'/>
      <source file='/var/lib/libvirt/images/Fedora.iso' index='1'/>
      <backingStore/>
      <target dev='sdb' bus='scsi'/>
      <readonly/>
      <alias name='scsi0-0-0-1'/>
      <address type='drive' controller='0' bus='0' target='0' unit='1'/>
    </disk>

And the cdrom media could be found in VM.

> But the domblkinfo Physical value is equal to the virtual size of qcow2.

Sorry in comment38 and comment39. It should be 'NOT equal to' there. Because the physical size from domblkinfo is 10737418240 while the virtual size from qcow2 file is 9663676416.

Update the testing of various backends of cdrom update:
Version: libvirt-4.5.0-7.el7_rc.0518ef33a7.x86_64 qemu-kvm-rhev-2.12.0-11.el7.x86_64
I. Test cdrom live insert 
1. Start a VM with empty cdrom:
...
    <disk type='file' device='cdrom'>                                                                                                                              
      <driver name='qemu' type='raw'/>
      <source index='1'/>
      <target dev='sdb' bus='scsi'/>
      <readonly/>
      <alias name='ua-cdrom'/>
      <address type='drive' controller='0' bus='0' target='0' unit='1'/>
    </disk>
...

2. Try to update instert iso from various backends: file,luks,block,gluster,iscsi,rbd,https,nbd

for i in file-file.xml file-luks.xml network-gluster.xml network-iscsi.xml network-rbd.xml network-https.xml network-nbd.xml;do
    virsh update-device $VM $i
    virsh dumpxml $VM|awk '/<disk/,/<\/disk/'
done

But get error from all the backends:
error: Failed to update device from file-file.xml
error: internal error: argument key 'id' must not have null value


II. Try to update media:
1. Start a VM with file backend cdrom media:
    <disk type='file' device='cdrom'>
      <driver name='qemu' type='raw'/>
      <source file='/var/lib/libvirt/images/boot.iso'/>
      <target dev='sdb' bus='scsi'/>
      <readonly/>
      <alias name='ua-cdrom'/>
      <address type='drive' controller='0' bus='0' target='0' unit='1'/>
    </disk>

2. Update the media to different backends:
for i in file-luks.xml block-block.xml network-gluster.xml network-iscsi.xml network-rbd.xml network-https.xml network-nbd.xml;do
    virsh update-device $VM $i
    virsh dumpxml $VM|awk '/<disk/,/<\/disk/'
done

Get error from all backends:
error: Failed to complete action eject on media
error: internal error: argument key 'id' must not have null value

III. Eject the media from different backends:
1. Prepare the empty cdrom xml
# cat file-empty.xml 
    <disk type='file' device='cdrom'>
      <driver name='qemu' type='raw'/>
      <target dev='sdb' bus='scsi'/>
      <readonly/>
      <alias name='ua-cdrom'/>
      <address type='drive' controller='0' bus='0' target='0' unit='1'/>
    </disk>

2. Run following script:
for i in file-file.xml file-luks.xml block-block.xml network-gluster.xml network-iscsi.xml network-rbd.xml network-https.xml network-nbd.xml;do
    virsh update-device $VM $i --config
    virsh start $VM && sleep 50
    virsh update-device $VM $EMPTY
    virsh destroy $VM
done

Get the error above, too.
error: Failed to update device from file-empty.xml
error: internal error: argument key 'id' must not have null value

Created attachment 1479976 [details]
The xml and script in comment41

Here are the XMLs and scripts of updating cdrom. You can run ./run.sh to reproduce the issues in comment41.

From usptream perspective following missing pieces needs to done
* handling of block jobs events
* execute modification of backing chain when event is populated
* create snapshots, blockdev-create is already done
* ...
* and finally switch on the blockdev-add

Blockjob data storing and handling of blockjob events refactoring is done in this upstream patches:
https://www.redhat.com/archives/libvir-list/2018-December/msg00319.html

This is one of the major prerequisite for enabling blockdev-add support in libvirt.

commit c6a9e54ce3252196f1fc6aa9e57537a659646d18
Author: Peter Krempa <pkrempa>
Date:   Mon Jan 7 11:45:19 2019 +0100

    qemu: enable blockdev support

    Now that all pieces are in place (hopefully) let's enable -blockdev.

    We base the capability on presence of the fix for 'auto-read-only' on
    files so that blockdev works properly, mandate that qemu supports
    explicit SCSI id strings to avoid ABI regression and that the fix for
    'savevm' is present so that internal snapshots work.

v5.9.0-390-gc6a9e54ce3

This requires upstream qemu-4.2 or appropriate downstream.

Note that there are hundreds commits which implement various bits needed for this feature which are not isted here.

Since libvirt-5.6, QE have tested blockdev for many times. The test matrix:
- disk xml
    variants:
    - iotune
    - without_iotune

    - source
        variants:
        - no_slices
        - slices

        variants:
        - file
            variants:
            - * block
                variants:
                - common
            - pr-helper
            - * file
        - network
            variants:
            - * multi_host_rbd_cephx
                variants:
                - * no_ceph_snapshot(default)
                - * ceph_snapshot(<readonly/> required)
            - * multi_host_gluster
            - chap_iscsi
            - * https(readonly)
				- cookie, sslverify, query, readahead, timeout
            - * nbd                
                variants:
                - no_tls(default)
                - * tls
        - * nvme
        - pool_volume
            variants:        
            - Directory pool
            - Filesystem pool
            - Network file system pool
            - Logical volume pool
            - Disk pool
            - iSCSI pool
            - iSCSI direct pool
            - SCSI pool
            - Multipath pool
            - RBD pool
            - Gluster pool

        
    - driver
        variants:
        - * format_qcow2
        - * format_raw
        - * format_raw_luks
        - format_vmdk(<readonly/> required)
        - format_vhd(<readonly/> required)
        - format_vpc(<readonly/> required)
        
        variants:
        - * copy_on_read_on
        - copy_on_read_off(default)
   
        variants:
        - default
        - cache_directsync
        - * cache_none
        - cache_unsafe
        - * cache_writeback
        - * cache_writethrough
    
        variants:
        - default
        - discard_ignore
        - * discard_unmap
        
        variants:
        - default
        - * detect_zeroes_on
        - detect_zeroes_unmap
        
        variants:
        - default
        - * readonly_yes(exclude block*, snapshot* operations)
        
        variants:
        - default
        - * io_native
        - * io_threads
        
        variants:
        - * custom_alias
        - default
    
        variants:
        - * backing store: Refer to source matrix
        
- operations
    variants:
    - snapshot-create
        variants:
        - * disk_only
        - * disk_memory
        
        variants:
        - no_reuse_external
        - * reuse_external
        
        variants(type):
        - * file
        - * block
        - * multihost_gluster
        
    - * attach
        variants:
        - active
            - xml: refer to source matrix
        - inactive
            - xml: refer to source matrix
            
    - detach
        variants:
        - * active
            - xml: refer to disk xml matrix
        - inactive
            - xml: refer to disk xml matrix
    
    - update-device
        - * cdrom
            - xml: refer to disk xml matrix
                variants:
                - eject
                - insert
        - floppy(not supported in latest q35)
            - xml: refer to disk xml matrix
                variants:
                - eject
                - insert
            
    - * detach_alias
        variants:
        - active
            - xml: refer to disk xml matrix
        - inactive
            - xml: refer to disk xml matrix
    
    - blockcommit
        variants:
        - * commit_from_active_layer
            variants:
            - pivot
            - finish
        - * commit_from_inactive_layer

        variants:
        - * shallow
        - no_shallow(default)
        
        variants:
        - * bandwidth
        - no_bandwidth(default)

        variants:
        - * keep_relative
        - no_keep_relative(default)
        
    - blockcopy
        - * dest_xml: Refer to disk xml matrix
        
        variants:
        - * shallow
        - * no_shallow(default)
        
        variants:
        - * bandwidth
        - no_bandwidth(default)

        variants:
        - * keep_relative
        - no_keep_relative(default)
        
        variants:
        - * pivot
        - * finish

        variants:
        - * timeout
        - * granularity
        - * buf-size
        - transient-job
    
    - blockpull
        variants:
        - * bandwidth
        - no_bandwidth(default)

        variants:
        - * keep_relative
        - no_keep_relative(default)
        
    - * blkthreshold
    
    - * blkdeviotune
        - * inactive set(by xml)
        - * active set
      
    - block-resize
    
    - * domblkstat
    
    - * domblkinfo

    - * storage-migration
        - dest_xml: Refer to source xml
        variants:
        - * tls
        - no_tls

Known issues:
1. copy_on_read_on with blockcommit, blocked by https://bugzilla.redhat.com/show_bug.cgi?id=1594747
2. gluster external snapshots with multi-hosts xml, https://bugzilla.redhat.com/show_bug.cgi?id=1447694

Verified on libvirt-6.0.0-17.module+el8.2.0+6257+0d066c28.x86_64 qemu-kvm-4.2.0-17.module+el8.2.0+6141+0f540f16.x86_64

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2020:2017

Covered in following tp-libvirt cases:
virtual_disk*
*block_copy*
*block_commit*
*snapshot*
*attach*

*** Bug 1856244 has been marked as a duplicate of this bug. ***