1. Patchew
  2. Libvirt
  3. View series

[PATCH] virDomainDiskDefValidateSourceChainOne: Fix validation of 'data-file' nesting

Peter Krempa via Devel posted 1 patch 1 year ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/9101a2ea0f46088fa5f2660b7477942bf95b09e0.1743152957.git.pkrempa@redhat.com
src/conf/domain_validate.c                    |  2 +-
...sk-qcow2-datafile-store.x86_64-latest.args | 43 +++++++++++--------
...isk-qcow2-datafile-store.x86_64-latest.xml | 22 +++++++++-
.../disk-qcow2-datafile-store.xml             | 19 ++++++++
4 files changed, 66 insertions(+), 20 deletions(-)
[PATCH] virDomainDiskDefValidateSourceChainOne: Fix validation of 'data-file' nesting
Posted by Peter Krempa via Devel 1 year ago 
From: Peter Krempa <pkrempa@redhat.com>

As the 'dataStore' is internally represented as a virStorageSource
object it has provisions for nesting which is not supported.

When I've reviewed and modified the commit adding data file parsing
support I've added code that was supposed to reject any 'backingStore'
and 'dataStore' structures nested in a source of a 'dataStore'.

Unfortunately the check was broken as one of the terms checked the
presence of parent's 'backingStore' instead of the nesting.

Fix it and add tests.

Fixes: b3171cf8da3
Resolves: https://issues.redhat.com/browse/RHEL-85320
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/conf/domain_validate.c                    |  2 +-
 ...sk-qcow2-datafile-store.x86_64-latest.args | 43 +++++++++++--------
 ...isk-qcow2-datafile-store.x86_64-latest.xml | 22 +++++++++-
 .../disk-qcow2-datafile-store.xml             | 19 ++++++++
 4 files changed, 66 insertions(+), 20 deletions(-)

diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
index 396d1e400a..d0d4bc0bf4 100644
--- a/src/conf/domain_validate.c
+++ b/src/conf/domain_validate.c
@@ -562,7 +562,7 @@ virDomainDiskDefValidateSourceChainOne(const virStorageSource *src)
             return -1;
         }

-        if (src->dataFileStore->dataFileStore || src->backingStore) {
+        if (src->dataFileStore->dataFileStore || src->dataFileStore->backingStore) {
             virReportError(VIR_ERR_XML_ERROR, "%s",
                            _("The <source> of <dataStore> can't have another nested <dataStore> or <backingStore> element"));
             return -1;
diff --git a/tests/qemuxmlconfdata/disk-qcow2-datafile-store.x86_64-latest.args b/tests/qemuxmlconfdata/disk-qcow2-datafile-store.x86_64-latest.args
index 5a64246af6..74f2d1a090 100644
--- a/tests/qemuxmlconfdata/disk-qcow2-datafile-store.x86_64-latest.args
+++ b/tests/qemuxmlconfdata/disk-qcow2-datafile-store.x86_64-latest.args
@@ -27,25 +27,32 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \
 -no-shutdown \
 -boot strict=on \
 -device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \
--blockdev '{"driver":"file","filename":"/tmp/data-file-store","node-name":"libvirt-9-storage","read-only":false}' \
--blockdev '{"driver":"nbd","server":{"type":"unix","path":"/path/to/sock"},"export":"Volume2/Image","node-name":"libvirt-8-storage","auto-read-only":true,"discard":"unmap"}' \
+-blockdev '{"driver":"file","filename":"/tmp/data-file-store","node-name":"libvirt-13-storage","read-only":false}' \
+-blockdev '{"driver":"nbd","server":{"type":"unix","path":"/path/to/sock"},"export":"Volume2/Image","node-name":"libvirt-12-storage","auto-read-only":true,"discard":"unmap"}' \
+-blockdev '{"node-name":"libvirt-12-format","read-only":false,"driver":"qcow2","data-file":"libvirt-13-storage","file":"libvirt-12-storage"}' \
+-device '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x2","drive":"libvirt-12-format","id":"virtio-disk0","bootindex":1}' \
+-blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1","node-name":"libvirt-11-storage","read-only":false}' \
+-blockdev '{"driver":"nbd","server":{"type":"unix","path":"/var/run/nbdsock"},"export":"bar","node-name":"libvirt-10-storage","auto-read-only":true,"discard":"unmap"}' \
+-blockdev '{"node-name":"libvirt-10-format","read-only":false,"driver":"qcow2","data-file":"libvirt-11-storage","file":"libvirt-10-storage"}' \
+-device '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x3","drive":"libvirt-10-format","id":"virtio-disk1"}' \
+-blockdev '{"driver":"nbd","server":{"type":"unix","path":"/path/to/sock/datafile"},"export":"Volume2/ImageDataFile","node-name":"libvirt-9-storage","read-only":false}' \
+-blockdev '{"driver":"file","filename":"/var/lib/libvirt/images/rhel7.1484071879","node-name":"libvirt-8-storage","auto-read-only":true,"discard":"unmap"}' \
 -blockdev '{"node-name":"libvirt-8-format","read-only":false,"driver":"qcow2","data-file":"libvirt-9-storage","file":"libvirt-8-storage"}' \
--device '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x2","drive":"libvirt-8-format","id":"virtio-disk0","bootindex":1}' \
--blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1","node-name":"libvirt-7-storage","read-only":false}' \
--blockdev '{"driver":"nbd","server":{"type":"unix","path":"/var/run/nbdsock"},"export":"bar","node-name":"libvirt-6-storage","auto-read-only":true,"discard":"unmap"}' \
--blockdev '{"node-name":"libvirt-6-format","read-only":false,"driver":"qcow2","data-file":"libvirt-7-storage","file":"libvirt-6-storage"}' \
--device '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x3","drive":"libvirt-6-format","id":"virtio-disk1"}' \
--blockdev '{"driver":"nbd","server":{"type":"unix","path":"/path/to/sock/datafile"},"export":"Volume2/ImageDataFile","node-name":"libvirt-5-storage","read-only":false}' \
--blockdev '{"driver":"file","filename":"/var/lib/libvirt/images/rhel7.1484071879","node-name":"libvirt-4-storage","auto-read-only":true,"discard":"unmap"}' \
--blockdev '{"node-name":"libvirt-4-format","read-only":false,"driver":"qcow2","data-file":"libvirt-5-storage","file":"libvirt-4-storage"}' \
--device '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x4","drive":"libvirt-4-format","id":"virtio-disk2"}' \
--blockdev '{"driver":"file","filename":"/tmp/data-file-store-2","node-name":"libvirt-3-storage","read-only":true}' \
--blockdev '{"driver":"file","filename":"/var/lib/libvirt/images/rhel7.1484071877","node-name":"libvirt-2-storage","auto-read-only":true,"discard":"unmap"}' \
--blockdev '{"node-name":"libvirt-2-format","read-only":true,"driver":"qcow2","data-file":"libvirt-3-storage","file":"libvirt-2-storage"}' \
--blockdev '{"driver":"file","filename":"/var/lib/libvirt/images/rhel7.1484071880","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"}' \
--device '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x5","drive":"libvirt-1-format","id":"virtio-disk3"}' \
+-device '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x4","drive":"libvirt-8-format","id":"virtio-disk2"}' \
+-blockdev '{"driver":"file","filename":"/tmp/data-file-store-2","node-name":"libvirt-7-storage","read-only":true}' \
+-blockdev '{"driver":"file","filename":"/var/lib/libvirt/images/rhel7.1484071877","node-name":"libvirt-6-storage","auto-read-only":true,"discard":"unmap"}' \
+-blockdev '{"node-name":"libvirt-6-format","read-only":true,"driver":"qcow2","data-file":"libvirt-7-storage","file":"libvirt-6-storage"}' \
+-blockdev '{"driver":"file","filename":"/var/lib/libvirt/images/rhel7.1484071880","node-name":"libvirt-5-storage","auto-read-only":true,"discard":"unmap"}' \
+-blockdev '{"node-name":"libvirt-5-format","read-only":false,"driver":"qcow2","file":"libvirt-5-storage","backing":"libvirt-6-format"}' \
+-device '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x5","drive":"libvirt-5-format","id":"virtio-disk3"}' \
+-blockdev '{"driver":"file","filename":"/var/lib/libvirt/images/datastore_1","aio":"native","node-name":"libvirt-4-storage","read-only":true,"discard":"unmap","cache":{"direct":true,"no-flush":false}}' \
+-blockdev '{"driver":"file","filename":"/var/lib/libvirt/images/base-with-data-file.qcow","aio":"native","node-name":"libvirt-3-storage","auto-read-only":true,"discard":"unmap","cache":{"direct":true,"no-flush":false}}' \
+-blockdev '{"node-name":"libvirt-3-format","read-only":true,"discard":"unmap","cache":{"direct":true,"no-flush":false},"driver":"qcow2","data-file":"libvirt-4-storage","file":"libvirt-3-storage"}' \
+-blockdev '{"driver":"file","filename":"/var/lib/libvirt/images/datastore_2","aio":"native","node-name":"libvirt-2-storage","read-only":false,"discard":"unmap","cache":{"direct":true,"no-flush":false}}' \
+-blockdev '{"driver":"file","filename":"/var/lib/libvirt/images/datastore.qcow2","aio":"native","node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap","cache":{"direct":true,"no-flush":false}}' \
+-blockdev '{"node-name":"libvirt-1-format","read-only":false,"discard":"unmap","cache":{"direct":true,"no-flush":false},"driver":"qcow2","data-file":"libvirt-2-storage","file":"libvirt-1-storage","backing":"libvirt-3-format"}' \
+-device '{"driver":"virtio-blk-pci","bus":"pci.0","addr":"0x6","drive":"libvirt-1-format","id":"virtio-disk4","write-cache":"on"}' \
 -audiodev '{"id":"audio1","driver":"none"}' \
--device '{"driver":"virtio-balloon-pci","id":"balloon0","bus":"pci.0","addr":"0x6"}' \
+-device '{"driver":"virtio-balloon-pci","id":"balloon0","bus":"pci.0","addr":"0x7"}' \
 -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
 -msg timestamp=on
diff --git a/tests/qemuxmlconfdata/disk-qcow2-datafile-store.x86_64-latest.xml b/tests/qemuxmlconfdata/disk-qcow2-datafile-store.x86_64-latest.xml
index a026749faf..a4bfcb29e0 100644
--- a/tests/qemuxmlconfdata/disk-qcow2-datafile-store.x86_64-latest.xml
+++ b/tests/qemuxmlconfdata/disk-qcow2-datafile-store.x86_64-latest.xml
@@ -69,6 +69,26 @@
       <target dev='vdd' bus='virtio'/>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/>
     </disk>
+    <disk type='file' device='disk'>
+      <driver name='qemu' type='qcow2' cache='none' io='native' discard='unmap'/>
+      <source file='/var/lib/libvirt/images/datastore.qcow2'>
+        <dataStore type='file'>
+          <format type='raw'/>
+          <source file='/var/lib/libvirt/images/datastore_2'/>
+        </dataStore>
+      </source>
+      <backingStore type='file'>
+        <format type='qcow2'/>
+        <source file='/var/lib/libvirt/images/base-with-data-file.qcow'>
+          <dataStore type='file'>
+            <format type='raw'/>
+            <source file='/var/lib/libvirt/images/datastore_1'/>
+          </dataStore>
+        </source>
+      </backingStore>
+      <target dev='vde' bus='virtio'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/>
+    </disk>
     <controller type='usb' index='0' model='piix3-uhci'>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
     </controller>
@@ -77,7 +97,7 @@
     <input type='keyboard' bus='ps2'/>
     <audio id='1' type='none'/>
     <memballoon model='virtio'>
-      <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/>
     </memballoon>
   </devices>
 </domain>
diff --git a/tests/qemuxmlconfdata/disk-qcow2-datafile-store.xml b/tests/qemuxmlconfdata/disk-qcow2-datafile-store.xml
index dff5f15158..9f2671a3e7 100644
--- a/tests/qemuxmlconfdata/disk-qcow2-datafile-store.xml
+++ b/tests/qemuxmlconfdata/disk-qcow2-datafile-store.xml
@@ -62,6 +62,25 @@
       </backingStore>
       <target dev='vdd' bus='virtio'/>
     </disk>
+    <disk type='file' device='disk'>
+      <driver name='qemu' type='qcow2' cache='none' io='native' discard='unmap'/>
+      <source file='/var/lib/libvirt/images/datastore.qcow2'>
+        <dataStore type='file'>
+          <format type='raw'/>
+          <source file='/var/lib/libvirt/images/datastore_2'/>
+        </dataStore>
+      </source>
+      <backingStore type='file'>
+        <format type='qcow2'/>
+        <source file='/var/lib/libvirt/images/base-with-data-file.qcow'>
+          <dataStore type='file'>
+            <format type='raw'/>
+            <source file='/var/lib/libvirt/images/datastore_1'/>
+          </dataStore>
+        </source>
+      </backingStore>
+      <target dev='vde' bus='virtio'/>
+    </disk>
     <controller type='usb' index='0'/>
     <controller type='pci' index='0' model='pci-root'/>
     <memballoon model='virtio'/>
-- 
2.49.0
Re: [PATCH] virDomainDiskDefValidateSourceChainOne: Fix validation of 'data-file' nesting
Posted by Jiri Denemark via Devel 1 year ago 
On Fri, Mar 28, 2025 at 10:09:17 +0100, Peter Krempa wrote:
> From: Peter Krempa <pkrempa@redhat.com>
> 
> As the 'dataStore' is internally represented as a virStorageSource
> object it has provisions for nesting which is not supported.
> 
> When I've reviewed and modified the commit adding data file parsing
> support I've added code that was supposed to reject any 'backingStore'
> and 'dataStore' structures nested in a source of a 'dataStore'.
> 
> Unfortunately the check was broken as one of the terms checked the
> presence of parent's 'backingStore' instead of the nesting.
> 
> Fix it and add tests.
> 
> Fixes: b3171cf8da3
> Resolves: https://issues.redhat.com/browse/RHEL-85320
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
>  src/conf/domain_validate.c                    |  2 +-
>  ...sk-qcow2-datafile-store.x86_64-latest.args | 43 +++++++++++--------
>  ...isk-qcow2-datafile-store.x86_64-latest.xml | 22 +++++++++-
>  .../disk-qcow2-datafile-store.xml             | 19 ++++++++
>  4 files changed, 66 insertions(+), 20 deletions(-)

Reviewed-by: Jiri Denemark <jdenemar@redhat.com>

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.