Description of problem: The iscsi lun number in source name will be changed after guest restart with external snapshot. Version-Release number of selected component (if applicable): libvirt-3.9.0-7.el7.x86_64 qemu-kvm-rhev-2.10.0-15.el7.x86_64 How reproducible: 100% Steps to Reproduce: 1. Prepare a guest with iscsi network disk. # virsh dumpxml rhel7 | grep disk -a6 ... <disk type='network' device='disk'> <driver name='qemu' type='qcow2'/> <source protocol='iscsi' name='iqn.2003-01.org.linux-iscsi.localhost.x8664:sn.9cba196611e6/2'> <host name='**IP**' port='3260'/> <auth username='redhat'> <secret type='iscsi' usage='libvirtiscsi'/> </auth> </source> <target dev='vda' bus='virtio'/> <alias name='virtio-disk0'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </disk>... 2. Create external disk and check xml. # for i in 1 2 3 4;do virsh snapshot-create-as rhel7 s$i --disk-only --diskspec vda,file=/tmp/rhel7.s$i;done # virsh dumpxml rhel7 | grep disk -a12 ... <disk type='file' device='disk'> <driver name='qemu' type='qcow2'/> <source file='/tmp/rhel7.s4'/> <backingStore type='file' index='1'> <format type='qcow2'/> <source file='/tmp/rhel7.s3'/> <backingStore type='file' index='2'> <format type='qcow2'/> <source file='/tmp/rhel7.s2'/> <backingStore type='file' index='3'> <format type='qcow2'/> <source file='/tmp/rhel7.s1'/> <backingStore type='network' index='4'> <format type='qcow2'/> <source protocol='iscsi' name='iqn.2003-01.org.linux-iscsi.localhost.x8664:sn.9cba196611e6/2'> <host name='**IP**' port='3260'/> </source> </backingStore>... 3. Restart guest and check guest xml. # virsh destroy rhel7 # virsh start rhel7 # virsh dumpxml rhel7 | grep disk -a12 ... <disk type='file' device='disk'> <driver name='qemu' type='qcow2'/> <source file='/tmp/rhel7.s4'/> <backingStore type='file' index='2'> <format type='qcow2'/> <source file='/tmp/rhel7.s1'/> <backingStore type='network' index='3'> <format type='qcow2'/> <source protocol='iscsi' name='iqn.2003-01.org.linux-iscsi.localhost.x8664:sn.9cba196611e6/0'> <host name='**IP**' port='3260'/> </source> -- </backingStore>... Actual results: As above step 3. Expected results: # virsh dumpxml rhel7 | grep disk -a12 … <backingStore type='network' index='2'> <format type='qcow2'/> <source protocol='iscsi' name='iqn.2003-01.org.linux-iscsi.localhost.x8664:sn.9cba196611e6/2'> <host name='**IP**' port='3260'/> </source> </backingStore>... Additional info: After blockcommit/blockpull/blockcopy, it also will be changed.
Could you please post output of 'qemu-img info /tmp/rhel7.s1' when that reproduces? it looks like we parse something wrong from the file on the disk
After restart guest, check the info: # qemu-img info /tmp/lmn.s1 -U --backing-chain image: /tmp/lmn.s1 file format: qcow2 virtual size: 5.0G (5368709120 bytes) disk size: 196K cluster_size: 65536 backing file: json:{"driver": "qcow2", "file": {"lun": "2", "portal": "**IP**:3260", "driver": "iscsi", "transport": "tcp", "target": "iqn.2003-01.org.linux-iscsi.localhost.x8664:sn.9cba196611e6"}} backing file format: qcow2 Format specific information: compat: 1.1 lazy refcounts: false refcount bits: 16 corrupt: false image: json:{"driver": "qcow2", "file": {"lun": "2", "portal": "**IP**:3260", "driver": "iscsi", "transport": "tcp", "target": "iqn.2003-01.org.linux-iscsi.localhost.x8664:sn.9cba196611e6"}} file format: qcow2 virtual size: 5.0G (5368709120 bytes) disk size: unavailable cluster_size: 65536 Format specific information: compat: 1.1 lazy refcounts: false refcount bits: 16 corrupt: false
Thanks for the information. In the backing json string the 'lun' field is clearly a string: json:{"driver": "qcow2", "file": {"lun": "2", "portal": "**IP**:3260", "driver": "iscsi", "transport": "tcp", "target": "iqn.2003-01.org.linux-iscsi.localhost.x8664:sn.9cba196611e6"}} while the documentation in qemu (qapi/block-core.json) clearly states that 'lun' is an int: { 'struct': 'BlockdevOptionsIscsi', 'data': { 'transport': 'IscsiTransport', 'portal': 'str', 'target': 'str', '*lun': 'int', '*user': 'str', '*password-secret': 'str', '*initiator-name': 'str', '*header-digest': 'IscsiHeaderDigest', '*timeout': 'int' } } Libvirt is parsing it as an int and thus fails to find it and uses the default (0). Reassigning to qemu.
Well, nobody told libvirt to parse this string or that it matches the BlockdevOptionsIscsi structure... qemu accepts both strings and integers in json: strings because internally everything is converted into strings anyway (we're in the -drive legacy code path here, so no QAPI schema involved at all). Consequently, the generated json: strings treat all values as strings, too. Maybe this can be improved by going through a real QAPI object using the keyval visitor, if we can assume that the content always actually contains everything that the schema requires. In that case you'd actually get JSON numbers for integers in the schema. If that doesn't work and we actually need to use QAPI objects internally (which is something that we want, but it's really hard), don't hold your breath. It might be a few years then. Either way, if libvirt parses the strings, it should probably accept both integers and strings as we won't be able to retroactively change all qemu versions from 2.1 where the feature was introduced.
(In reply to Kevin Wolf from comment #7) > Well, nobody told libvirt to parse this string or that it matches the > BlockdevOptionsIscsi structure... qemu accepts both strings and integers in > json: strings because internally everything is converted into strings anyway > (we're in the -drive legacy code path here, so no QAPI schema involved at > all). Consequently, the generated json: strings treat all values as strings, > too. Well the fact that we parse it is a result of qemu starting to put those strings into the backing store field in the qcow images. Libvirt uses those to reconstruct the backing chain and since there's no other documentation than the QAPI schema it makes sense that people will use it. Okay I'll clone this BZ to fix the JSON parsers to accept both strings and numbers for the 'int' fields.
(In reply to Peter Krempa from comment #8) > Libvirt uses those to reconstruct the backing chain Do you still need to reconstruct the backing chain after the blockdev work?
Yes that will still need to be used. The blockdev work will allow users to specify the full backing chain, but we really can't force them to specify it fully. This means that we'll still need to support the case when the user specifies only the top level image and libvirt will need to reconstruct the chain for filesystem labelling purposes. Additionally we can't even record this into the persistent XML used for the next start since some usecases involve relatively linked images which can be accessed using multiple storage protocols and in such case we need to support users changing only the top level image path while the rest still needs to work.
For the record, a simpler reproducer is: $ qemu-img info --image-opts driver=null-co,size=512 image: json:{"driver": "null-co", "size": "512"} [...] @size is an int, so its value should be numeric, too. Moving to 7.6 because libvirt seems to have its workaround, and qemu itself can cope with false strings in json:{} filenames.
Sent an upstream series, although I’d say it’s in rather questionable state: http://lists.nongnu.org/archive/html/qemu-block/2018-05/msg00290.html
QEMU has been recently split into sub-components and as a one-time operation to avoid breakage of tools, we are setting the QEMU sub-component of this BZ to "General". Please review and change the sub-component if necessary the next time you review this BZ. Thanks
After evaluating this issue, there are no plans to address it further or fix it in an upcoming release. Therefore, it is being closed. If plans change such that this issue will be fixed in an upcoming release, then the bug can be reopened.
Tested qemu-img-5.2.0-7.module+el8.4.0+9943+d64b3717.x86_64, # qemu-img info --image-opts driver=null-co,size=512 image: json:{"driver": "null-co", "size": "512"} file format: null-co virtual size: 512 B (512 bytes) disk size: 0 B Disk size is numberic, the issue is fixed, QE agrees to close this bug.
Correct my last comment, this issue still exits, but as libvirt has workaround, agrees to close it.