Bug 1461638

Patches which allow to parse the new backing store format:

https://www.redhat.com/archives/libvir-list/2017-June/msg00730.html

Upstream fixes the json parser to support the new format from qemu 2.9 in commits:

commit b16133b114fd0d787de1cda6bf2ff0110c523a32
Author: Peter Krempa <pkrempa>
Date:   Thu Jun 15 17:44:18 2017 +0200

    util: storage: adapt to changes in JSON format for sheepdog
    
    Since qemu 2.9 the options changed from a monolithic string into fine
    grained options for the json pseudo-protocol object.

commit ea2c418ac30628d1db021f351f0ea5440ba5e4e4
Author: Peter Krempa <pkrempa>
Date:   Thu Jun 15 17:44:18 2017 +0200

    util: storage: adapt to changes in JSON format for ssh
    
    Since qemu 2.9 the options changed from a monolithic string into fine
    grained options for the json pseudo-protocol object.

commit 4fac5a1935041254964313ed722fa8d59f8fa2de
Author: Peter Krempa <pkrempa>
Date:   Thu Jun 15 17:44:18 2017 +0200

    util: storage: adapt to changes in JSON format for ceph/rbd
    
    Since qemu 2.9 the options changed from a monolithic string into fine
    grained options for the json pseudo-protocol object.

commit 35d23f90b2266adb93079755161515699a401932
Author: Peter Krempa <pkrempa>
Date:   Thu Jun 15 17:44:18 2017 +0200

    util: storage: adapt to changes in JSON format for NBD
    
    Since 2.9 the host and port for NBD are no longer directly under the
    json pseudo-protocol object, but rather belong to a sub-object called
    'server'.

commit b24bc54080b4bc444e60560c0db00c5867e74000
Author: Peter Krempa <pkrempa>
Date:   Thu Jun 15 17:12:01 2017 +0200

    util: storage: Add JSON parser for new options in iSCSI protocol
    
    Starting from qemu 2.9, more granular options are supported. Add parser
    for the relevant bits.
    
    With this patch libvirt is able to parse the host and target IQN of from
    the JSON pseudo-protocol specification.
    
    This corresponds to BlockdevOptionsIscsi in qemu qapi.

commit 299aff7e0cd52c50da518eeed676144d373e9281
Author: Peter Krempa <pkrempa>
Date:   Mon Jun 19 14:47:41 2017 +0200

    util: storage: Report errors when source host data is missing
    
    Merge the reporting of the missing source host data into the parser
    functions so that callers don't have to do it separately.

commit 49ed98a4579744568b5c57f65ae08034d5c9568f
Author: Peter Krempa <pkrempa>
Date:   Mon Jun 19 14:42:18 2017 +0200

    util: storage: Split out parsing of TCP network host from JSON pseudoprotocol
    
    Few backing protocols support only TCP. Split out the function which
    will correspond to parsing qemu's InetSocketAddressBase.

commit 1f915d40a220c6bb05d8630507eed045cabfba34
Author: Peter Krempa <pkrempa>
Date:   Mon Jun 19 14:37:47 2017 +0200

    util: storage: Add support for type 'inet' in virStorageSourceParseBackingJSONSocketAddress
    
    'SocketAddress' structure was changed to contain 'inet' instead of
    'tcp' since qemu commit c5f1ae3ae7b. Existing entries have a backward
    compatibility layer.
    
    Libvirt will parse 'inet' and 'tcp' as equivalents.

commit 6402f402d4577b5eacf00debe5e59c328bb58f75
Author: Peter Krempa <pkrempa>
Date:   Thu Jun 15 17:23:15 2017 +0200

    util: storage: make virStorageSourceParseBackingJSONGlusterHost universal
    
    The same json strucutre is used for NBD and sheepdog volumes for
    specifying of the host. Rename the function and fix up error messages to
    be more universal.

commit 506b80c84e4556296ea7ac499c77b9f4ee87079f
Author: Peter Krempa <pkrempa>
Date:   Thu Jun 15 17:09:26 2017 +0200

    util: storage: Add missing return to virStorageSourceParseBackingJSONGluster
    
    If the number of servers is not expected the code would report an error
    but would not return failure.

commit 236e1f7e8caf602f2c18525652b13f7b9ede4431
Author: Peter Krempa <pkrempa>
Date:   Fri Jun 16 17:07:55 2017 +0200

    util: storage: Output parsed network backing store string to debug log

Test on libvirt-3.2.0-11.virtcov.el7.x86_64 qemu-kvm-rhev-2.9.0-12.el7.x86_64

Use following script to test:

DOM=$1
virsh define $DOM.xml
# Attach rbd,nbd,iscsi backend
for i in vd{a..c}.xml;do
    virsh attach-device $DOM $i --config
done

virsh start $DOM && sleep 5
...
# Create snapshots
for i in s{1..3};do
    virsh snapshot-create-as $DOM $i --no-metadata --disk-only --diskspec vda,file=/tmp/vda.$i --diskspec vdb,file=/tmp/vdb.$i --diskspec vdc,file=/tmp/vdc.$i
done
# Do shallow commit
virsh dumpxml $DOM| awk '/<disk/,/<\/disk/' > before-commit.log
for i in {1..3};do
    for j in vd{a..c};do
        virsh blockcommit $DOM $j --active --shallow --wait --verbose --pivot
        virsh dumpxml $DOM| awk '/<disk/,/<\/disk/'>$DOM-commit$i.log
    done
done
virsh destroy $DOM
virsh undefine $DOM
...

Only the third blockcommit on vdb fails:
+ virsh blockcommit V vdb --active --shallow --wait --verbose --pivot
error: internal error: unable to execute QEMU command 'block-commit': Block format 'nbd' used by node '#block208' does not support reopening files

Then check the logs. The dumpxml after the third commit seems incorrect: Missing <host/> element in source on vda.

<disk type='network' device='disk'>
      <driver name='qemu' type='raw'/>
      <source protocol='rbd' name='rbd/V'/>
      <backingStore/>
      <target dev='vda' bus='virtio'/>
      <alias name='virtio-disk0'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
    </disk>
    <disk type='file' device='disk'>
      <driver name='qemu' type='qcow2'/>
      <source file='/tmp/vdb.s1'/>
      <backingStore type='network' index='1'>
        <format type='raw'/>
        <source protocol='nbd'>
          <host name='10.66.5.92' port='10800'/>
        </source>
        <backingStore/>
      </backingStore>
      <target dev='vdb' bus='virtio'/>
      <alias name='virtio-disk1'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x08' function='0x0'/>
    </disk>
    <disk type='network' device='disk'>
      <driver name='qemu' type='raw'/>
      <source protocol='iscsi' name='iqn.2016-12.com.virttest:emulated-iscsi-noauth.target0/0'>
        <host name='10.66.5.92' port='3260'/>
      </source>
      <backingStore/>
      <target dev='vdc' bus='virtio'/>
      <alias name='virtio-disk2'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x09' function='0x0'/>
    </disk>

I will upload the scripts and logs later. Please check the result.

Created attachment 1290363 [details]
The test result on rhel7.3 and rhel7.4

To run the test, execute:
./commit.sh V

In RHEL7.3, the shallow blockcommit from vda[1] to base on rbd backend is forbidden. But the final result of disk xml is expected.

Hi Peter, there is another issue.
I see you add the patch that updates parsing new sheepdog format. But I find it doesn't work in libvirt:
# qemu-img create -f qcow2 -b 'json:{"driver": "raw", "file": {"server.host": "10.66.5.92", "server.port": "7000", "tag": "", "driver": "sheepdog", "server.type": "inet", "vdi": "Alice"}}' /var/lib/libvirt/images/sheepdog.qcow2
Formatting '/var/lib/libvirt/images/sheepdog.qcow2', fmt=qcow2 size=5368709120 backing_file=json:{"driver": "raw",, "file": {"server.host": "10.66.5.92",, "server.port": "7000",, "tag": "",, "driver": "sheepdog",, "server.type": "inet",, "vdi": "Alice"}} encryption=off cluster_size=65536 lazy_refcounts=off refcount_bits=16

# virsh pool-dumpxml default
<pool type='dir'>
  <name>default</name>
  <uuid>38795621-2398-4e87-8602-31ae5907db67</uuid>
  <capacity unit='bytes'>0</capacity>
  <allocation unit='bytes'>0</allocation>
  <available unit='bytes'>0</available>
  <source>
  </source>
  <target>
    <path>/var/lib/libvirt/images</path>
  </target>
</pool>

# virsh pool-refresh default
error: Failed to refresh pool default
error: invalid argument: missing remote server specification in JSON backing volume definition

Correct the mistake in comment10: all dumpxml of rbd missing <host/> after commits.

Okay, the problem is that if the server object is flattened ("server.host", "server.port") the code is not able to parse it.

For now please use only nested objects ("server":{"host":"123","port":"1234"})... which should work as expected.

You can also file a bug to track the deflattening of the 'server' subobject.

(In reply to Peter Krempa from comment #14)
> Okay, the problem is that if the server object is flattened ("server.host",
> "server.port") the code is not able to parse it.
> 
> For now please use only nested objects
> ("server":{"host":"123","port":"1234"})... which should work as expected.
> 
> You can also file a bug to track the deflattening of the 'server' subobject.

Sure. Libvirt can parse the backing file with its server is not flattened:
# qemu-img create -f qcow2 -b 'json:{"driver": "raw", "file": {"server":{"host": "10.66.5.92", "port": "7000", "type":"inet"}, "tag": "", "driver": "sheepdog", "vdi": "Alice"}}' /var/lib/libvirt/images/sheepdog.qcow2
Formatting '/var/lib/libvirt/images/sheepdog.qcow2', fmt=qcow2 size=5368709120 backing_file=json:{"driver": "raw",, "file": {"server":{"host": "10.66.5.92",, "port": "7000",, "type":"inet"},, "tag": "",, "driver": "sheepdog",, "vdi": "Alice"}} encryption=off cluster_size=65536 lazy_refcounts=off refcount_bits=16

# virsh pool-refresh default
Pool default refreshed

I will file a bug to track the flattened object issue. Please continue to check the rbd <host/> missing issue. Thanks.

For the RBD case, the JSON string generated by qemu after the snapshot (and written to the overlay file) does not conform to the schema documented by qemu. Thus it's a qemu bug.

json:{"driver": "raw", "file": {"pool": "rbd", "image": "V", "driver": "rbd", "=keyvalue-pairs": "[\"auth_supported\", \"none\", \"mon_host\", \"10.73.75.52\"]"}}

As seen, this does not contain the 'server' section, but contains an undocumented "=keyvalue-pairs".

Libvirt should though be able to parse a network XML that it generates, so that is a bug in libvirt.

(In reply to Peter Krempa from comment #16)
> For the RBD case, the JSON string generated by qemu after the snapshot (and
> written to the overlay file) does not conform to the schema documented by
> qemu. Thus it's a qemu bug.
> 
> json:{"driver": "raw", "file": {"pool": "rbd", "image": "V", "driver":
> "rbd", "=keyvalue-pairs": "[\"auth_supported\", \"none\", \"mon_host\",
> \"10.73.75.52\"]"}}
> 
> As seen, this does not contain the 'server' section, but contains an
> undocumented "=keyvalue-pairs".
> 
> Libvirt should though be able to parse a network XML that it generates, so
> that is a bug in libvirt.

I tried creating snapshots without "=keyvalue-pairs". This works well:
1. Prepare a VM with rbd. Create a backing file with new rbd json format
# virsh dumpxml V| awk '/<disk/,/<\/disk/'
    <disk type='network' device='disk'>
      <driver name='qemu' type='raw'/>
      <source protocol='rbd' name='rbd/V'>
        <host name='10.73.75.52' port='6789'/>
      </source>
      <target dev='vda' bus='virtio'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
    </disk>

# qemu-img create -f qcow2 -o backing_fmt=raw -b 'json:{"file": {"pool": "rbd", "image": "V", "driver": "rbd", "server":[{"host":"XX.XX.XX.XX","port":"6789"}]}}' /var/lib/libvirt/images/vda.s1
Formatting '/var/lib/libvirt/images/vda.s1', fmt=qcow2 size=10737418240 backing_file=json:{"file": {"pool": "rbd",, "image": "V",, "driver": "rbd",, "server":[{"host":"XX.XX.XX.XX",,"port":"6789"}]}} backing_fmt=raw encryption=off cluster_size=65536 lazy_refcounts=off refcount_bits=16


2. Start VM and create a snapshot reusing the snapshot file
# virsh start V
Domain V started

# virsh snapshot-create-as V s1 --reuse-external --no-metadata --disk-only --diskspec vda,file=/var/lib/libvirt/images/vda.s1
Domain snapshot s1 created

3. Create more snapshot and do shallow blockcommit.
# virsh snapshot-create-as V s2 --no-metadata --disk-only
Domain snapshot s2 created

# virsh dumpxml V| awk '/<disk/,/<\/disk/'                                                                                                     
    <disk type='file' device='disk'>
      <driver name='qemu' type='qcow2'/>
      <source file='/var/lib/libvirt/images/vda.s2'/>
      <backingStore type='file' index='1'>
        <format type='qcow2'/>
        <source file='/var/lib/libvirt/images/vda.s1'/>
        <backingStore type='network' index='2'>
          <format type='raw'/>
          <source protocol='rbd' name='rbd/V'>
            <host name='10.73.75.52' port='6789'/>
          </source>
          <backingStore/>
        </backingStore>
      </backingStore>
      <target dev='vda' bus='virtio'/>
      <alias name='virtio-disk0'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
    </disk>

# virsh blockcommit V vda --active --shallow --wait --verbose --pivot               
Block commit: [100 %]
Successfully pivoted

# virsh dumpxml V| awk '/<disk/,/<\/disk/'                           
    <disk type='file' device='disk'>
      <driver name='qemu' type='qcow2'/>
      <source file='/var/lib/libvirt/images/vda.s1'/>
      <backingStore type='network' index='1'>
        <format type='raw'/>
        <source protocol='rbd' name='rbd/V'>
          <host name='10.73.75.52' port='6789'/>
        </source>
        <backingStore/>
      </backingStore>
      <target dev='vda' bus='virtio'/>
      <alias name='virtio-disk0'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
    </disk>

# virsh blockcommit V vda --active --shallow --wait --verbose --pivot
Block commit: [100 %]
Successfully pivoted

# virsh dumpxml V| awk '/<disk/,/<\/disk/'                           
    <disk type='network' device='disk'>
      <driver name='qemu' type='raw'/>
      <source protocol='rbd' name='rbd/V'>
        <host name='10.73.75.52' port='6789'/>
      </source>
      <backingStore/>
      <target dev='vda' bus='virtio'/>
      <alias name='virtio-disk0'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
    </disk>

The <host/> element is not missing. So the issue in comment10 is because libvirt cannot parse "=keyvalue-pairs" in rbd back fie metadata.

According to comment10, comment16 and comment17, mark this bug as verified. For the issue of rbd and flatten json, I will file another bugs.

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/RHEA-2017:1846