[libvirt] [PATCH 00/35] qemu: Add formatting of JSON objects for -blockdev (blockdev-add saga)

Diff to the RFC posting: - host_device and host_cdrom handling was added - modified code to format and parse node-names to status XML so that we can add other node names easier (for quorum, throttling, etc) - added test case for host_device Kevin, please sanity check the outputs whether they make sense. Thanks. Peter Krempa (35): storage: Properly track that backing chain members are readonly qemu: domain: Format storage source node names into private data util: storage: Add shadow copies of few disk properties to virStorageSource qemu: domain: Carefuly transfer configuration from disk to storage source qemu: block: Extract formatting of props for 'file' backend qemu: block: Handle iomode property for json 'file' driver utils: storage: Mark that a virStorageSource is going to be used as a floppy qemu: Move virtual FAT disk validation from command line builder qemu: block: Add support for accessing directories via the 'vvfat' driver util: file: Use only one #ifdef for __linux__ util: file: Add helper to determine whether a path is a CDROM qemu: domain: Store whether a virStorageSource is a host CDROM drive qemu: block: Properly handle block storage in JSON generator qemu: block: Propagate 'legacy' parameter when formatting disk backing qemu: block: Validate node-names for use with qemu qemu: block: Format cache modes for disk storage backends qemu: block: Format 'read-only' attribute for JSON disk protocol qemu: block: Always set discard for storage nodes qemu: block: Add support for creating 'format' layer for blockdev-add tests: qemublock: Rename variables in anticipation of new tests tests: Makefile: Sanitize entry for qemublocktest qemu: domain: Export qemuDomainDeviceDefValidateDisk qemu: domain: Tolerate NULL 'cfg' in qemuDomainPrepareDiskSourceChain tests: qemublock: Add testing of blockdev JSON property generator tests: qemublock: Add basic 'raw' file test tests: qemublock: Add tests for all other format without special options tests: qemublock: Add tests for basic backing chain formats tests: qemublock: Add test-case for the 'vvfat' driver in qemu tests: qemublock: Add test cases for 'aio' options of 'file' storage tests: qemublock: Add test for raw luks disk format tests: qemublock: basic qcow2 tests tests: qemublock: Add test combining authentication and encryption tests: qemublock: Test handling of 'unmap' and 'detect-zeroes' options tests: qemublock: Test handling of all cache modes tests: qemublock: Test handling of block devices src/conf/domain_conf.c | 3 + src/libvirt_private.syms | 1 + src/qemu/qemu_block.c | 430 ++++++++++++++++++++- src/qemu/qemu_block.h | 6 +- src/qemu/qemu_command.c | 16 +- src/qemu/qemu_domain.c | 61 ++- src/qemu/qemu_domain.h | 3 + src/qemu/qemu_driver.c | 3 + src/util/virfile.c | 61 ++- src/util/virfile.h | 2 + src/util/virstoragefile.c | 5 + src/util/virstoragefile.h | 13 + tests/Makefile.am | 11 +- tests/qemublocktest.c | 314 ++++++++++++++- .../xml2json/block-raw-noopts.json | 12 + .../xml2json/block-raw-noopts.xml | 12 + .../qemublocktestdata/xml2json/dir-fat-cache.json | 22 ++ tests/qemublocktestdata/xml2json/dir-fat-cache.xml | 13 + .../qemublocktestdata/xml2json/dir-fat-floppy.json | 14 + .../qemublocktestdata/xml2json/dir-fat-floppy.xml | 14 + .../xml2json/dir-fat-readonly.json | 14 + .../xml2json/dir-fat-readonly.xml | 13 + .../xml2json/file-backing_basic-aio_threads.json | 62 +++ .../xml2json/file-backing_basic-aio_threads.xml | 47 +++ .../file-backing_basic-cache-directsync.json | 91 +++++ .../file-backing_basic-cache-directsync.xml | 47 +++ .../xml2json/file-backing_basic-cache-none.json | 91 +++++ .../xml2json/file-backing_basic-cache-none.xml | 47 +++ .../xml2json/file-backing_basic-cache-unsafe.json | 91 +++++ .../xml2json/file-backing_basic-cache-unsafe.xml | 47 +++ .../file-backing_basic-cache-writeback.json | 91 +++++ .../file-backing_basic-cache-writeback.xml | 47 +++ .../file-backing_basic-cache-writethrough.json | 91 +++++ .../file-backing_basic-cache-writethrough.xml | 47 +++ .../xml2json/file-backing_basic-detect.json | 60 +++ .../xml2json/file-backing_basic-detect.xml | 47 +++ .../xml2json/file-backing_basic-noopts.json | 51 +++ .../xml2json/file-backing_basic-noopts.xml | 46 +++ .../xml2json/file-backing_basic-unmap-detect.json | 64 +++ .../xml2json/file-backing_basic-unmap-detect.xml | 47 +++ .../xml2json/file-backing_basic-unmap-discard.json | 0 .../xml2json/file-backing_basic-unmap-ignore.json | 64 +++ .../xml2json/file-backing_basic-unmap-ignore.xml | 47 +++ .../xml2json/file-backing_basic-unmap.json | 63 +++ .../xml2json/file-backing_basic-unmap.xml | 47 +++ .../xml2json/file-bochs-noopts.json | 12 + .../xml2json/file-bochs-noopts.xml | 12 + .../xml2json/file-cloop-noopts.json | 12 + .../xml2json/file-cloop-noopts.xml | 12 + .../xml2json/file-dmg-noopts.json | 12 + .../qemublocktestdata/xml2json/file-dmg-noopts.xml | 12 + .../xml2json/file-ploop-noopts.json | 12 + .../xml2json/file-ploop-noopts.xml | 12 + .../file-qcow2-backing-chain-encryption.json | 34 ++ .../file-qcow2-backing-chain-encryption.xml | 31 ++ .../xml2json/file-qcow2-backing-chain-noopts.json | 130 +++++++ .../xml2json/file-qcow2-backing-chain-noopts.xml | 113 ++++++ .../file-qcow2-backing-chain-unterminated.json | 25 ++ .../file-qcow2-backing-chain-unterminated.xml | 24 ++ .../xml2json/file-raw-aio_native.json | 21 + .../xml2json/file-raw-aio_native.xml | 12 + .../qemublocktestdata/xml2json/file-raw-luks.json | 13 + tests/qemublocktestdata/xml2json/file-raw-luks.xml | 15 + .../xml2json/file-raw-noopts.json | 12 + .../qemublocktestdata/xml2json/file-raw-noopts.xml | 12 + .../xml2json/file-vdi-noopts.json | 12 + .../qemublocktestdata/xml2json/file-vdi-noopts.xml | 12 + .../xml2json/file-vhd-noopts.json | 12 + .../qemublocktestdata/xml2json/file-vhd-noopts.xml | 12 + .../xml2json/file-vpc-noopts.json | 12 + .../qemublocktestdata/xml2json/file-vpc-noopts.xml | 12 + .../network-qcow2-backing-chain-cache-unsafe.json | 57 +++ .../network-qcow2-backing-chain-cache-unsafe.xml | 31 ++ ...etwork-qcow2-backing-chain-encryption_auth.json | 51 +++ ...network-qcow2-backing-chain-encryption_auth.xml | 40 ++ .../xml2json/nodename-long-format.xml | 12 + .../xml2json/nodename-long-protocol.xml | 12 + tests/qemustatusxml2xmldata/modern-in.xml | 4 + 78 files changed, 3155 insertions(+), 40 deletions(-) create mode 100644 tests/qemublocktestdata/xml2json/block-raw-noopts.json create mode 100644 tests/qemublocktestdata/xml2json/block-raw-noopts.xml create mode 100644 tests/qemublocktestdata/xml2json/dir-fat-cache.json create mode 100644 tests/qemublocktestdata/xml2json/dir-fat-cache.xml create mode 100644 tests/qemublocktestdata/xml2json/dir-fat-floppy.json create mode 100644 tests/qemublocktestdata/xml2json/dir-fat-floppy.xml create mode 100644 tests/qemublocktestdata/xml2json/dir-fat-readonly.json create mode 100644 tests/qemublocktestdata/xml2json/dir-fat-readonly.xml create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-aio_threads.json create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-aio_threads.xml create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-cache-directsync.json create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-cache-directsync.xml create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-cache-none.json create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-cache-none.xml create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-cache-unsafe.json create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-cache-unsafe.xml create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-cache-writeback.json create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-cache-writeback.xml create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-cache-writethrough.json create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-cache-writethrough.xml create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-detect.json create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-detect.xml create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-noopts.json create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-noopts.xml create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-unmap-detect.json create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-unmap-detect.xml create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-unmap-discard.json create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-unmap-ignore.json create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-unmap-ignore.xml create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-unmap.json create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-unmap.xml create mode 100644 tests/qemublocktestdata/xml2json/file-bochs-noopts.json create mode 100644 tests/qemublocktestdata/xml2json/file-bochs-noopts.xml create mode 100644 tests/qemublocktestdata/xml2json/file-cloop-noopts.json create mode 100644 tests/qemublocktestdata/xml2json/file-cloop-noopts.xml create mode 100644 tests/qemublocktestdata/xml2json/file-dmg-noopts.json create mode 100644 tests/qemublocktestdata/xml2json/file-dmg-noopts.xml create mode 100644 tests/qemublocktestdata/xml2json/file-ploop-noopts.json create mode 100644 tests/qemublocktestdata/xml2json/file-ploop-noopts.xml create mode 100644 tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-encryption.json create mode 100644 tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-encryption.xml create mode 100644 tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-noopts.json create mode 100644 tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-noopts.xml create mode 100644 tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-unterminated.json create mode 100644 tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-unterminated.xml create mode 100644 tests/qemublocktestdata/xml2json/file-raw-aio_native.json create mode 100644 tests/qemublocktestdata/xml2json/file-raw-aio_native.xml create mode 100644 tests/qemublocktestdata/xml2json/file-raw-luks.json create mode 100644 tests/qemublocktestdata/xml2json/file-raw-luks.xml create mode 100644 tests/qemublocktestdata/xml2json/file-raw-noopts.json create mode 100644 tests/qemublocktestdata/xml2json/file-raw-noopts.xml create mode 100644 tests/qemublocktestdata/xml2json/file-vdi-noopts.json create mode 100644 tests/qemublocktestdata/xml2json/file-vdi-noopts.xml create mode 100644 tests/qemublocktestdata/xml2json/file-vhd-noopts.json create mode 100644 tests/qemublocktestdata/xml2json/file-vhd-noopts.xml create mode 100644 tests/qemublocktestdata/xml2json/file-vpc-noopts.json create mode 100644 tests/qemublocktestdata/xml2json/file-vpc-noopts.xml create mode 100644 tests/qemublocktestdata/xml2json/network-qcow2-backing-chain-cache-unsafe.json create mode 100644 tests/qemublocktestdata/xml2json/network-qcow2-backing-chain-cache-unsafe.xml create mode 100644 tests/qemublocktestdata/xml2json/network-qcow2-backing-chain-encryption_auth.json create mode 100644 tests/qemublocktestdata/xml2json/network-qcow2-backing-chain-encryption_auth.xml create mode 100644 tests/qemublocktestdata/xml2json/nodename-long-format.xml create mode 100644 tests/qemublocktestdata/xml2json/nodename-long-protocol.xml -- 2.16.2

Everything besides the top of the chain is readonly. Track this when parsing the XML and detecting the chain from the disk. Also fix the state when taking snapshots. All other cases where the top image is changed already preserve the readonly state from the original image. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 3 +++ src/qemu/qemu_driver.c | 3 +++ src/util/virstoragefile.c | 1 + 3 files changed, 7 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b0257068da..21a6c4785a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8734,6 +8734,9 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, if (VIR_ALLOC(backingStore) < 0) goto cleanup; + /* backing store is always read-only */ + backingStore->readonly = true; + /* terminator does not have a type */ if (!(type = virXMLPropString(ctxt->node, "type"))) { VIR_STEAL_PTR(src->backingStore, backingStore); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7484b00e23..542c625962 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14670,6 +14670,9 @@ qemuDomainSnapshotUpdateDiskSources(qemuDomainSnapshotDiskDataPtr dd, if (dd->initialized) virStorageFileDeinit(dd->src); + /* the old disk image is now readonly */ + dd->disk->src->readonly = true; + VIR_STEAL_PTR(dd->disk->src->relPath, dd->relPath); VIR_STEAL_PTR(dd->src->backingStore, dd->disk->src); VIR_STEAL_PTR(dd->disk->src, dd->src); diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 531540ac91..d6ad6e1d0f 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -3425,6 +3425,7 @@ virStorageSourceNewFromBacking(virStorageSourcePtr parent) if (virStorageSourceInitChainElement(ret, parent, true) < 0) goto error; + ret->readonly = true; ret->detected = true; } -- 2.16.2

On 04/25/2018 11:15 AM, Peter Krempa wrote:
Everything besides the top of the chain is readonly. Track this when parsing the XML and detecting the chain from the disk. Also fix the state when taking snapshots.
All other cases where the top image is changed already preserve the readonly state from the original image.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 3 +++ src/qemu/qemu_driver.c | 3 +++ src/util/virstoragefile.c | 1 + 3 files changed, 7 insertions(+)
Any concerns with virDomainDiskDefCheckABIStability? Since true would now be the default - is it possible a save file comparison causes issues? Reviewed-by: John Ferlan <jferlan@redhat.com> John

On Tue, May 01, 2018 at 20:06:40 -0400, John Ferlan wrote:
On 04/25/2018 11:15 AM, Peter Krempa wrote:
Everything besides the top of the chain is readonly. Track this when parsing the XML and detecting the chain from the disk. Also fix the state when taking snapshots.
All other cases where the top image is changed already preserve the readonly state from the original image.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 3 +++ src/qemu/qemu_driver.c | 3 +++ src/util/virstoragefile.c | 1 + 3 files changed, 7 insertions(+)
Any concerns with virDomainDiskDefCheckABIStability? Since true would now be the default - is it possible a save file comparison causes issues?
The ABI stability is checked only for the 'readonly' attribute of the top layer, since that may have implications visible from the guest. The rest of the backing chain definitely is not ABI.

Save and restore node names if we know them in the status XML so that we don't need to recalculate them or don't lose them in some cases. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 12 ++++++++++++ tests/qemustatusxml2xmldata/modern-in.xml | 4 ++++ 2 files changed, 16 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 326c939c85..2c5de9508b 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1937,6 +1937,9 @@ static int qemuStorageSourcePrivateDataParse(xmlXPathContextPtr ctxt, virStorageSourcePtr src) { + src->nodestorage = virXPathString("string(./nodenames/nodename[@type='storage']/@name)", ctxt); + src->nodeformat = virXPathString("string(./nodenames/nodename[@type='format']/@name)", ctxt); + if (virStorageSourcePrivateDataParseRelPath(ctxt, src) < 0) return -1; @@ -1948,6 +1951,15 @@ static int qemuStorageSourcePrivateDataFormat(virStorageSourcePtr src, virBufferPtr buf) { + if (src->nodestorage || src->nodeformat) { + virBufferAddLit(buf, "<nodenames>\n"); + virBufferAdjustIndent(buf, 2); + virBufferEscapeString(buf, "<nodename type='storage' name='%s'/>\n", src->nodestorage); + virBufferEscapeString(buf, "<nodename type='format' name='%s'/>\n", src->nodeformat); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</nodenames>\n"); + } + if (virStorageSourcePrivateDataFormatRelPath(src, buf) < 0) return -1; diff --git a/tests/qemustatusxml2xmldata/modern-in.xml b/tests/qemustatusxml2xmldata/modern-in.xml index c1e57618b6..d57e1f605f 100644 --- a/tests/qemustatusxml2xmldata/modern-in.xml +++ b/tests/qemustatusxml2xmldata/modern-in.xml @@ -309,6 +309,10 @@ <format type='qcow2'/> <source file='/var/lib/libvirt/images/base.qcow2'> <privateData> + <nodenames> + <nodename type='storage' name='test-storage'/> + <nodename type='format' name='test-format'/> + </nodenames> <relPath>base.qcow2</relPath> </privateData> </source> -- 2.16.2

On 04/25/2018 11:15 AM, Peter Krempa wrote:
Save and restore node names if we know them in the status XML so that we don't need to recalculate them or don't lose them in some cases.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 12 ++++++++++++ tests/qemustatusxml2xmldata/modern-in.xml | 4 ++++ 2 files changed, 16 insertions(+)
So, what would cause a need for recalculation? Or losing them? I assume it's "expensive" to query, but then I wonder what trap we could fall into by saving them. If we've needed to recalculate or lost them before, what's to say that same issue doesn't happen by saving them? I also assume they don't change after we've saved them, right? I'm not opposed to this, it's the OK so how are they to be used and then why if they could be lost or needed recalculation would we want to save them? Also, what assurances do we have then when they are to be saved that they aren't in a condition that caused this patch to be generated. If the only way to get the 'real' value is ask QEMU, then why not ask? It's not like an alias which we generated it's IIUC something QEMU generates and wishes to be referenced in that manner. Maybe it's just the wording of the commit that raised the questions ;-) John
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 326c939c85..2c5de9508b 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1937,6 +1937,9 @@ static int qemuStorageSourcePrivateDataParse(xmlXPathContextPtr ctxt, virStorageSourcePtr src) { + src->nodestorage = virXPathString("string(./nodenames/nodename[@type='storage']/@name)", ctxt); + src->nodeformat = virXPathString("string(./nodenames/nodename[@type='format']/@name)", ctxt); + if (virStorageSourcePrivateDataParseRelPath(ctxt, src) < 0) return -1;
@@ -1948,6 +1951,15 @@ static int qemuStorageSourcePrivateDataFormat(virStorageSourcePtr src, virBufferPtr buf) { + if (src->nodestorage || src->nodeformat) { + virBufferAddLit(buf, "<nodenames>\n"); + virBufferAdjustIndent(buf, 2); + virBufferEscapeString(buf, "<nodename type='storage' name='%s'/>\n", src->nodestorage); + virBufferEscapeString(buf, "<nodename type='format' name='%s'/>\n", src->nodeformat); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</nodenames>\n"); + } + if (virStorageSourcePrivateDataFormatRelPath(src, buf) < 0) return -1;
diff --git a/tests/qemustatusxml2xmldata/modern-in.xml b/tests/qemustatusxml2xmldata/modern-in.xml index c1e57618b6..d57e1f605f 100644 --- a/tests/qemustatusxml2xmldata/modern-in.xml +++ b/tests/qemustatusxml2xmldata/modern-in.xml @@ -309,6 +309,10 @@ <format type='qcow2'/> <source file='/var/lib/libvirt/images/base.qcow2'> <privateData> + <nodenames> + <nodename type='storage' name='test-storage'/> + <nodename type='format' name='test-format'/> + </nodenames> <relPath>base.qcow2</relPath> </privateData> </source>

On Tue, May 01, 2018 at 20:06:43 -0400, John Ferlan wrote:
On 04/25/2018 11:15 AM, Peter Krempa wrote:
Save and restore node names if we know them in the status XML so that we don't need to recalculate them or don't lose them in some cases.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 12 ++++++++++++ tests/qemustatusxml2xmldata/modern-in.xml | 4 ++++ 2 files changed, 16 insertions(+)
So, what would cause a need for recalculation? Or losing them?
I assume it's "expensive" to query, but then I wonder what trap we could fall into by saving them. If we've needed to recalculate or lost them before, what's to say that same issue doesn't happen by saving them?
Well. Currently we use node-names only for the 'block-threshold' event and we get them by querying them from qemu. Since they are not stored in the XML a libvirtd restart requires us to re-query them. In the future we will start generating and providing our own node names to the block layer, so it will be required that we remember them. Both re-generating and detecting is way more complicated than just storing them.
I also assume they don't change after we've saved them, right? I'm not opposed to this, it's the OK so how are they to be used and then why if they could be lost or needed recalculation would we want to save them? Also, what assurances do we have then when they are to be saved that they aren't in a condition that caused this patch to be generated.
No, they just are lost on restart.
If the only way to get the 'real' value is ask QEMU, then why not ask? It's not like an alias which we generated it's IIUC something QEMU generates and wishes to be referenced in that manner.
Maybe it's just the wording of the commit that raised the questions ;-)
Umm probably yes. I've had this commit around for a long time :)

On 05/02/2018 05:51 AM, Peter Krempa wrote:
On Tue, May 01, 2018 at 20:06:43 -0400, John Ferlan wrote:
On 04/25/2018 11:15 AM, Peter Krempa wrote:
Save and restore node names if we know them in the status XML so that we don't need to recalculate them or don't lose them in some cases.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 12 ++++++++++++ tests/qemustatusxml2xmldata/modern-in.xml | 4 ++++ 2 files changed, 16 insertions(+)
So, what would cause a need for recalculation? Or losing them?
I assume it's "expensive" to query, but then I wonder what trap we could fall into by saving them. If we've needed to recalculate or lost them before, what's to say that same issue doesn't happen by saving them?
Well. Currently we use node-names only for the 'block-threshold' event and we get them by querying them from qemu. Since they are not stored in the XML a libvirtd restart requires us to re-query them.
In the future we will start generating and providing our own node names to the block layer, so it will be required that we remember them.
Since we can re-query, then the real need to save them is to avoid the [time consuming, complicated] re-query - which is fine as long as they stay in sync!
Both re-generating and detecting is way more complicated than just storing them.
I also assume they don't change after we've saved them, right? I'm not opposed to this, it's the OK so how are they to be used and then why if they could be lost or needed recalculation would we want to save them? Also, what assurances do we have then when they are to be saved that they aren't in a condition that caused this patch to be generated.
No, they just are lost on restart.
I was too lazy to look last night 0-). I see qemuBlockNodeNamesDetect is called during Reconnect which seems to fill in those values.
If the only way to get the 'real' value is ask QEMU, then why not ask? It's not like an alias which we generated it's IIUC something QEMU generates and wishes to be referenced in that manner.
Maybe it's just the wording of the commit that raised the questions ;-)
Umm probably yes. I've had this commit around for a long time :)
In any case, thanks for the explanation. Then it's just some commit message wordsmithing to indicate we'll save to avoid the re-query. The above function already avoids re-query if nodeformat or nodestorage is already defined. Thus, Reviewed-by: John Ferlan <jferlan@redhat.com> John

Few things which are currently stored the virDomainDiskDef structure are actually relevant for the storage source as well. Add the fields with a note that they are just mirror of the values from the disk. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virstoragefile.c | 4 ++++ src/util/virstoragefile.h | 8 ++++++++ 2 files changed, 12 insertions(+) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index d6ad6e1d0f..72584404da 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2042,6 +2042,10 @@ virStorageSourceCopy(const virStorageSource *src, ret->detected = src->detected; ret->debugLevel = src->debugLevel; ret->debug = src->debug; + ret->iomode = src->iomode; + ret->cachemode = src->cachemode; + ret->discard = src->discard; + ret->detect_zeroes = src->detect_zeroes; /* storage driver metadata are not copied */ ret->drv = NULL; diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index d129e81978..b450da55ae 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -305,6 +305,14 @@ struct _virStorageSource { unsigned int debugLevel; bool debug; + + /* Libvirt currently stores the following properities in virDomainDiskDef. + * These instances are currently just copies from the parent definition and + * are not mapped back to the XML */ + int iomode; /* enum virDomainDiskIo */ + int cachemode; /* enum virDomainDiskCache */ + int discard; /* enum virDomainDiskDiscard */ + int detect_zeroes; /* enum virDomainDiskDetectZeroes */ }; -- 2.16.2

On 04/25/2018 11:15 AM, Peter Krempa wrote:
Few things which are currently stored the virDomainDiskDef structure are actually relevant for the storage source as well. Add the fields with a note that they are just mirror of the values from the disk.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virstoragefile.c | 4 ++++ src/util/virstoragefile.h | 8 ++++++++ 2 files changed, 12 insertions(+)
Reviewed-by: John Ferlan <jferlan@redhat.com> John

Some properties don't make sense to be configured for every single layer of the backing chain, but to avoid needing to pass the disk structure we will copy them to the individual virStorageSource-s Zero detection is applied only for the top layer image, while caching and iomode for all layers. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 2c5de9508b..109f4f7f36 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -11844,6 +11844,9 @@ qemuDomainPrepareDiskSourceChain(virDomainDiskDefPtr disk, if (!src) src = disk->src; + /* transfer properties valid only for the top level image */ + src->detect_zeroes = disk->detect_zeroes; + for (n = src; virStorageSourceIsBacking(n); n = n->backingStore) { if (n->type == VIR_STORAGE_TYPE_NETWORK && n->protocol == VIR_STORAGE_NET_PROTOCOL_GLUSTER && @@ -11854,6 +11857,11 @@ qemuDomainPrepareDiskSourceChain(virDomainDiskDefPtr disk, if (qemuDomainValidateStorageSource(n, qemuCaps) < 0) return -1; + + /* transfer properties valid for the full chain */ + n->iomode = disk->iomode; + n->cachemode = disk->cachemode; + n->discard = disk->discard; } return 0; -- 2.16.2

On 04/25/2018 11:15 AM, Peter Krempa wrote:
Some properties don't make sense to be configured for every single layer of the backing chain, but to avoid needing to pass the disk structure we will copy them to the individual virStorageSource-s
Source's ??? or just without the -s
Zero detection is applied only for the top layer image, while caching and iomode for all layers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 8 ++++++++ 1 file changed, 8 insertions(+)
Reviewed-by: John Ferlan <jferlan@redhat.com> John

'file' backend in qemu supports few more options than the current implementation. Extract it so that changes don't pollute the code. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index c0cf6a95ad..e7bd6c909d 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -974,6 +974,18 @@ qemuBlockStorageSourceGetSshProps(virStorageSourcePtr src) } +static virJSONValuePtr +qemuBlockStorageSourceGetFileProps(virStorageSourcePtr src) +{ + virJSONValuePtr ret = NULL; + + ignore_value(virJSONValueObjectCreate(&ret, + "s:driver", "file", + "s:filename", src->path, NULL) < 0); + return ret; +} + + /** * qemuBlockStorageSourceGetBackendProps: * @src: disk source @@ -991,9 +1003,7 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src) case VIR_STORAGE_TYPE_BLOCK: case VIR_STORAGE_TYPE_FILE: case VIR_STORAGE_TYPE_DIR: - if (virJSONValueObjectCreate(&fileprops, - "s:driver", "file", - "s:filename", src->path, NULL) < 0) + if (!(fileprops = qemuBlockStorageSourceGetFileProps(src))) return NULL; break; -- 2.16.2

On 04/25/2018 11:15 AM, Peter Krempa wrote:
'file' backend in qemu supports few more options than the current implementation. Extract it so that changes don't pollute the code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John

Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index e7bd6c909d..6cf41cf544 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -977,11 +977,17 @@ qemuBlockStorageSourceGetSshProps(virStorageSourcePtr src) static virJSONValuePtr qemuBlockStorageSourceGetFileProps(virStorageSourcePtr src) { + const char *iomode = NULL; virJSONValuePtr ret = NULL; + if (src->iomode != VIR_DOMAIN_DISK_IO_DEFAULT) + iomode = virDomainDiskIoTypeToString(src->iomode); + ignore_value(virJSONValueObjectCreate(&ret, "s:driver", "file", - "s:filename", src->path, NULL) < 0); + "s:filename", src->path, + "S:aio", iomode, + NULL) < 0); return ret; } -- 2.16.2

On 04/25/2018 11:15 AM, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John

Add a flag denoting that a virStorageSource is going to be used as a floppy image. This will be useful in cases where the user passes in files which shall be exposed as an image to the guest. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 3 +++ src/util/virstoragefile.h | 3 +++ 2 files changed, 6 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 109f4f7f36..f8254bfdd5 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -11862,6 +11862,9 @@ qemuDomainPrepareDiskSourceChain(virDomainDiskDefPtr disk, n->iomode = disk->iomode; n->cachemode = disk->cachemode; n->discard = disk->discard; + + if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) + n->floppyimg = true; } return 0; diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index b450da55ae..57d39f98c2 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -313,6 +313,9 @@ struct _virStorageSource { int cachemode; /* enum virDomainDiskCache */ int discard; /* enum virDomainDiskDiscard */ int detect_zeroes; /* enum virDomainDiskDetectZeroes */ + + bool floppyimg; /* set to true if the storage source is going to be used + as a source for floppy drive */ }; -- 2.16.2

On 04/25/2018 11:15 AM, Peter Krempa wrote:
Add a flag denoting that a virStorageSource is going to be used as a floppy image. This will be useful in cases where the user passes in files which shall be exposed as an image to the guest.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 3 +++ src/util/virstoragefile.h | 3 +++ 2 files changed, 6 insertions(+)
Reviewed-by: John Ferlan <jferlan@redhat.com> John

Move it to the validation callback and make it more robust. This will also put the checks in the correct place to use with -blockdev. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 14 -------------- src/qemu/qemu_domain.c | 25 +++++++++++++++++++++++++ 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 418729b988..ecf28d3d1d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1520,20 +1520,6 @@ qemuBuildDriveSourceStr(virDomainDiskDefPtr disk, /* for now the DIR based storage is handled by the magic FAT format */ if (actualType == VIR_STORAGE_TYPE_DIR) { - if (disk->src->format > 0 && - disk->src->format != VIR_STORAGE_FILE_FAT) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unsupported disk driver type for '%s'"), - virStorageFileFormatTypeToString(disk->src->format)); - goto cleanup; - } - - if (!disk->src->readonly) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("cannot create virtual FAT disks in read-write mode")); - goto cleanup; - } - virBufferAddLit(buf, "fat:"); if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index f8254bfdd5..9f94809fd1 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4128,6 +4128,8 @@ static int qemuDomainValidateStorageSource(virStorageSourcePtr src, virQEMUCapsPtr qemuCaps) { + int actualType = virStorageSourceGetActualType(src); + if (src->format == VIR_STORAGE_FILE_COW) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("'cow' storage format is not supported")); @@ -4157,6 +4159,29 @@ qemuDomainValidateStorageSource(virStorageSourcePtr src, return -1; } + if (src->format == VIR_STORAGE_FILE_FAT && + actualType != VIR_STORAGE_TYPE_DIR) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("storage format 'fat' is supported only with 'dir' " + "storage type")); + return -1; + } + + if (actualType == VIR_STORAGE_TYPE_DIR) { + if (src->format > 0 && + src->format != VIR_STORAGE_FILE_FAT) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("storage type 'dir' requires use of storage format 'fat'")); + return -1; + } + + if (!src->readonly) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("virtual FAT storage can't be accessed in read-write mode")); + return -1; + } + } + return 0; } -- 2.16.2

On 04/25/2018 11:15 AM, Peter Krempa wrote:
Move it to the validation callback and make it more robust. This will also put the checks in the correct place to use with -blockdev.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 14 -------------- src/qemu/qemu_domain.c | 25 +++++++++++++++++++++++++ 2 files changed, 25 insertions(+), 14 deletions(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John

Handle VIR_STORAGE_TYPE_DIR in qemuBlockStorageSourceGetBackendProps so that a 'vvfat' driver is used, which emulates a FAT filesystem containing the folders. qemu requires us to add it as a storage layer, since a 'raw' layer is usually put on top of it. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 6cf41cf544..516b006ce9 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -992,6 +992,25 @@ qemuBlockStorageSourceGetFileProps(virStorageSourcePtr src) } +static virJSONValuePtr +qemuBlockStorageSourceGetVvfatProps(virStorageSourcePtr src) +{ + virJSONValuePtr ret = NULL; + + /* libvirt currently does not handle the following attributes: + * '*fat-type': 'int' + * '*label': 'str' + */ + ignore_value(virJSONValueObjectCreate(&ret, + "s:driver", "vvfat", + "s:dir", src->path, + "b:floppy", src->floppyimg, + "b:rw", !src->readonly, NULL)); + + return ret; +} + + /** * qemuBlockStorageSourceGetBackendProps: * @src: disk source @@ -1008,11 +1027,17 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src) switch ((virStorageType) actualType) { case VIR_STORAGE_TYPE_BLOCK: case VIR_STORAGE_TYPE_FILE: - case VIR_STORAGE_TYPE_DIR: if (!(fileprops = qemuBlockStorageSourceGetFileProps(src))) return NULL; break; + case VIR_STORAGE_TYPE_DIR: + /* qemu handles directories by exposing them as a device with emulated + * FAT filesystem */ + if (!(fileprops = qemuBlockStorageSourceGetVvfatProps(src))) + return NULL; + break; + case VIR_STORAGE_TYPE_VOLUME: case VIR_STORAGE_TYPE_NONE: case VIR_STORAGE_TYPE_LAST: -- 2.16.2

On 04/25/2018 11:15 AM, Peter Krempa wrote:
Handle VIR_STORAGE_TYPE_DIR in qemuBlockStorageSourceGetBackendProps so that a 'vvfat' driver is used, which emulates a FAT filesystem containing the folders.
qemu requires us to add it as a storage layer, since a 'raw' layer is usually put on top of it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-)
So I suppose the only question I have for this one relates to whether vvfat is supported in QEMU 1.5 or later and that's been the expectation to use it for a long time, but libvirt wasn't doing the right thing? Assuming 1.5 or later and that we don't have to do anything special or use qemuBlockStorageSourceGetFileProps for some variant between 1.5 and when vvfat was first supported, Reviewed-by: John Ferlan <jferlan@redhat.com> John I'm half surprised we didn't have some test failure!

On Tue, May 01, 2018 at 20:21:11 -0400, John Ferlan wrote:
On 04/25/2018 11:15 AM, Peter Krempa wrote:
Handle VIR_STORAGE_TYPE_DIR in qemuBlockStorageSourceGetBackendProps so that a 'vvfat' driver is used, which emulates a FAT filesystem containing the folders.
qemu requires us to add it as a storage layer, since a 'raw' layer is usually put on top of it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-)
So I suppose the only question I have for this one relates to whether vvfat is supported in QEMU 1.5 or later and that's been the expectation to use it for a long time, but libvirt wasn't doing the right thing?
Actually this code is not used yet mostly and will be only once we switch to blockdev. Since blockdev support will require a much newer qemu anyways we are fine to change this now.
Assuming 1.5 or later and that we don't have to do anything special or use qemuBlockStorageSourceGetFileProps for some variant between 1.5 and when vvfat was first supported,
Currently we format it the old way by using -drive, which will continue to be supported.
Reviewed-by: John Ferlan <jferlan@redhat.com>
John
I'm half surprised we didn't have some test failure!
Well, since it was not used there wasn't much test coverage. The test coverage was mostly in the XML->json->XML test, which did not handle the vvfat case. This series adds better coverage at least for the formatter. This will also lessen the need to add a lot of xml->argv tests, since the convertor will be tested separately.
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virfile.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/util/virfile.c b/src/util/virfile.c index e12a584ca1..24f866525f 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -57,11 +57,10 @@ # include <linux/magic.h> # endif # include <sys/statfs.h> -#endif - -#if defined(__linux__) && HAVE_DECL_LO_FLAGS_AUTOCLEAR -# include <linux/loop.h> -# include <sys/ioctl.h> +# if HAVE_DECL_LO_FLAGS_AUTOCLEAR +# include <linux/loop.h> +# include <sys/ioctl.h> +# endif #endif #include "configmake.h" -- 2.16.2

On 04/25/2018 11:15 AM, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virfile.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John

Add detection mechanism which will allow to check whether a path to a block device is a physical CDROM drive. This will be useful once we will need to pass it to hypervisors. The linux implementation uses an ioctl to do the detection, while the fallback uses a simple string prefix match. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virfile.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++- src/util/virfile.h | 2 ++ 3 files changed, 58 insertions(+), 1 deletion(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d2728749fb..4f911c10a8 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1781,6 +1781,7 @@ virFileGetMountSubtree; virFileHasSuffix; virFileInData; virFileIsAbsPath; +virFileIsCdrom; virFileIsDir; virFileIsExecutable; virFileIsLink; diff --git a/src/util/virfile.c b/src/util/virfile.c index 24f866525f..ad59b7015b 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -59,8 +59,9 @@ # include <sys/statfs.h> # if HAVE_DECL_LO_FLAGS_AUTOCLEAR # include <linux/loop.h> -# include <sys/ioctl.h> # endif +# include <sys/ioctl.h> +# include <linux/cdrom.h> #endif #include "configmake.h" @@ -1938,6 +1939,59 @@ int virFileIsMountPoint(const char *file) } +#if defined(__linux__) +/** + * virFileIsCdrom: + * @filename: File to check + * + * Returns 1 if @filename is a cdrom device 0 if it is not a cdrom and -1 on + * error. 'errno' of the failure is preserved and no libvirt errors are + * reported. + */ +int +virFileIsCdrom(const char *filename) +{ + struct stat st; + int fd; + int ret = -1; + + if ((fd = open(filename, O_RDONLY | O_NONBLOCK)) < 0) + goto cleanup; + + if (fstat(fd, &st) < 0) + goto cleanup; + + if (!S_ISBLK(st.st_mode)) { + ret = 0; + goto cleanup; + } + + /* Attempt to detect via a CDROM specific ioctl */ + if (ioctl(fd, CDROM_DRIVE_STATUS, CDSL_CURRENT) >= 0) + ret = 1; + else + ret = 0; + + cleanup: + VIR_FORCE_CLOSE(fd); + return ret; +} + +#else + +int +virFileIsCdrom(const char *filename) +{ + if (STRPREFIX(filename, "/dev/cd", NULL) || + STRPREFIX(filename, "/dev/acd", NULL)) + return 1; + + return 0; +} + +#endif /* defined(__linux__) */ + + #if defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R static int virFileGetMountSubtreeImpl(const char *mtabpath, diff --git a/src/util/virfile.h b/src/util/virfile.h index cd2a3867c2..8ab7d2d6a6 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -207,6 +207,8 @@ enum { int virFileIsSharedFSType(const char *path, int fstypes) ATTRIBUTE_NONNULL(1); int virFileIsSharedFS(const char *path) ATTRIBUTE_NONNULL(1); int virFileIsMountPoint(const char *file) ATTRIBUTE_NONNULL(1); +int virFileIsCdrom(const char *filename) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; int virFileGetMountSubtree(const char *mtabpath, const char *prefix, -- 2.16.2

On 04/25/2018 11:15 AM, Peter Krempa wrote:
Add detection mechanism which will allow to check whether a path to a block device is a physical CDROM drive. This will be useful once we will need to pass it to hypervisors.
The linux implementation uses an ioctl to do the detection, while the fallback uses a simple string prefix match.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virfile.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++- src/util/virfile.h | 2 ++ 3 files changed, 58 insertions(+), 1 deletion(-)
Should this be a replacement for qemuDomainFilePathIsHostCDROM used for qemuDomainObjCheckDiskTaint? Not a problem with this code, but I think there should only be one place that we determine host CDROM and it doesn't matter to me the mechanism. Just trying to avoid multiple means to get the same answer. I'm going to stop here for now as it's late for me - besides the answer here affects patch 12 too in a way. John
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d2728749fb..4f911c10a8 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1781,6 +1781,7 @@ virFileGetMountSubtree; virFileHasSuffix; virFileInData; virFileIsAbsPath; +virFileIsCdrom; virFileIsDir; virFileIsExecutable; virFileIsLink; diff --git a/src/util/virfile.c b/src/util/virfile.c index 24f866525f..ad59b7015b 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -59,8 +59,9 @@ # include <sys/statfs.h> # if HAVE_DECL_LO_FLAGS_AUTOCLEAR # include <linux/loop.h> -# include <sys/ioctl.h> # endif +# include <sys/ioctl.h> +# include <linux/cdrom.h> #endif
#include "configmake.h" @@ -1938,6 +1939,59 @@ int virFileIsMountPoint(const char *file) }
+#if defined(__linux__) +/** + * virFileIsCdrom: + * @filename: File to check + * + * Returns 1 if @filename is a cdrom device 0 if it is not a cdrom and -1 on + * error. 'errno' of the failure is preserved and no libvirt errors are + * reported. + */ +int +virFileIsCdrom(const char *filename) +{ + struct stat st; + int fd; + int ret = -1; + + if ((fd = open(filename, O_RDONLY | O_NONBLOCK)) < 0) + goto cleanup; + + if (fstat(fd, &st) < 0) + goto cleanup; + + if (!S_ISBLK(st.st_mode)) { + ret = 0; + goto cleanup; + } + + /* Attempt to detect via a CDROM specific ioctl */ + if (ioctl(fd, CDROM_DRIVE_STATUS, CDSL_CURRENT) >= 0) + ret = 1; + else + ret = 0; + + cleanup: + VIR_FORCE_CLOSE(fd); + return ret; +} + +#else + +int +virFileIsCdrom(const char *filename) +{ + if (STRPREFIX(filename, "/dev/cd", NULL) || + STRPREFIX(filename, "/dev/acd", NULL)) + return 1; + + return 0; +} + +#endif /* defined(__linux__) */ + + #if defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R static int virFileGetMountSubtreeImpl(const char *mtabpath, diff --git a/src/util/virfile.h b/src/util/virfile.h index cd2a3867c2..8ab7d2d6a6 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -207,6 +207,8 @@ enum { int virFileIsSharedFSType(const char *path, int fstypes) ATTRIBUTE_NONNULL(1); int virFileIsSharedFS(const char *path) ATTRIBUTE_NONNULL(1); int virFileIsMountPoint(const char *file) ATTRIBUTE_NONNULL(1); +int virFileIsCdrom(const char *filename) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
int virFileGetMountSubtree(const char *mtabpath, const char *prefix,

On Tue, May 01, 2018 at 20:25:09 -0400, John Ferlan wrote:
On 04/25/2018 11:15 AM, Peter Krempa wrote:
Add detection mechanism which will allow to check whether a path to a block device is a physical CDROM drive. This will be useful once we will need to pass it to hypervisors.
The linux implementation uses an ioctl to do the detection, while the fallback uses a simple string prefix match.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virfile.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++- src/util/virfile.h | 2 ++ 3 files changed, 58 insertions(+), 1 deletion(-)
Should this be a replacement for qemuDomainFilePathIsHostCDROM used for qemuDomainObjCheckDiskTaint?
Very good point. This code is actually "inspired" by the code that qemu uses for CDROM detection, so I think we should actually use it instead of the string checks.
Not a problem with this code, but I think there should only be one place that we determine host CDROM and it doesn't matter to me the mechanism. Just trying to avoid multiple means to get the same answer.
I agree. If it is deemed that it's okay to do ioctl()s on the cdrom device for libvirt I'll gladly replace the existing code. The advantage of the ioctl based code is that it works regardless of the name of the device.

On 04/25/2018 11:15 AM, Peter Krempa wrote:
Add detection mechanism which will allow to check whether a path to a block device is a physical CDROM drive. This will be useful once we will need to pass it to hypervisors.
The linux implementation uses an ioctl to do the detection, while the fallback uses a simple string prefix match.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virfile.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++- src/util/virfile.h | 2 ++ 3 files changed, 58 insertions(+), 1 deletion(-)
Because it's "easier" to find and it sticks out more, please consider using CDROM instead Cdrom. You could also change @filename to be @path, but that's just a nit. I agree this is better than qemuDomainFilePathIsHostCDROM and should be used instead. Since it's the same logic as qemu it's a fairly safe bet for having no issues on all the various platforms w/r/t the ioctl - famous last words though for all the variants we compile on... Altering the existing code can be a followup - unless of course you have the urge to post a followup after this patch or the next one. Reviewed-by: John Ferlan <jferlan@redhat.com> John

Use virFileIsCdrom to detect whether a block device is a cdrom drive and store it in virStorageSource. This will be necessary to correctly create the 'host_cdrom' backend in qemu when using -blockdev. We assume that host_cdrom makes only sense when used directly as a raw image, but if a backing chain would be put in front of it, libvirt will use 'host_device' in that case. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 8 ++++++++ src/util/virstoragefile.h | 2 ++ 2 files changed, 10 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 9f94809fd1..86e40e13e0 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -7545,6 +7545,14 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver, VIR_ALLOC(src->backingStore) < 0) goto cleanup; + /* host cdrom requires special treatment in qemu, so we need to check + * whether a block device is a cdrom */ + if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM && + src->format == VIR_STORAGE_FILE_RAW && + virStorageSourceIsBlockLocal(src) && + virFileIsCdrom(src->path) == 1) + src->hostcdrom = true; + ret = 0; goto cleanup; } diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 57d39f98c2..c8a4a288b1 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -316,6 +316,8 @@ struct _virStorageSource { bool floppyimg; /* set to true if the storage source is going to be used as a source for floppy drive */ + + bool hostcdrom; /* backing device is a cdrom */ }; -- 2.16.2

On 04/25/2018 11:15 AM, Peter Krempa wrote:
Use virFileIsCdrom to detect whether a block device is a cdrom drive and store it in virStorageSource. This will be necessary to correctly create the 'host_cdrom' backend in qemu when using -blockdev.
We assume that host_cdrom makes only sense when used directly as a raw image, but if a backing chain would be put in front of it, libvirt will use 'host_device' in that case.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 8 ++++++++ src/util/virstoragefile.h | 2 ++ 2 files changed, 10 insertions(+)
If you do change the method name as suggested in 11, be sure to update this commit message too. Reviewed-by: John Ferlan <jferlan@redhat.com>

Block storage should actually be passed to qemu via 'host_device' or 'host_cdrom' according to the device type. There were no users of this behaviour so we thankfully can change it. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 516b006ce9..bf330f8238 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -977,14 +977,22 @@ qemuBlockStorageSourceGetSshProps(virStorageSourcePtr src) static virJSONValuePtr qemuBlockStorageSourceGetFileProps(virStorageSourcePtr src) { + const char *driver = "file"; const char *iomode = NULL; virJSONValuePtr ret = NULL; if (src->iomode != VIR_DOMAIN_DISK_IO_DEFAULT) iomode = virDomainDiskIoTypeToString(src->iomode); + if (virStorageSourceIsBlockLocal(src)) { + if (src->hostcdrom) + driver = "host_cdrom"; + else + driver = "host_device"; + } + ignore_value(virJSONValueObjectCreate(&ret, - "s:driver", "file", + "s:driver", driver, "s:filename", src->path, "S:aio", iomode, NULL) < 0); -- 2.16.2

On 04/25/2018 11:15 AM, Peter Krempa wrote:
Block storage should actually be passed to qemu via 'host_device' or 'host_cdrom' according to the device type. There were no users of this behaviour so we thankfully can change it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John

The gluster protocol in qemu uses two styles, one of which is legacy and not covered by the QAPI schema. To allow using of the new style in the blockdev-add code, add a parameter for qemuBlockStorageSourceGetBackendProps which will switch between the two modes. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 11 +++++++---- src/qemu/qemu_block.h | 3 ++- src/qemu/qemu_command.c | 2 +- tests/qemublocktest.c | 2 +- 4 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index bf330f8238..29fc7edf61 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -655,13 +655,14 @@ qemuBlockStorageSourceBuildHostsJSONInetSocketAddress(virStorageSourcePtr src) static virJSONValuePtr -qemuBlockStorageSourceGetGlusterProps(virStorageSourcePtr src) +qemuBlockStorageSourceGetGlusterProps(virStorageSourcePtr src, + bool legacy) { virJSONValuePtr servers = NULL; virJSONValuePtr props = NULL; virJSONValuePtr ret = NULL; - if (!(servers = qemuBlockStorageSourceBuildHostsJSONSocketAddress(src, true))) + if (!(servers = qemuBlockStorageSourceBuildHostsJSONSocketAddress(src, legacy))) return NULL; /* { driver:"gluster", @@ -1022,12 +1023,14 @@ qemuBlockStorageSourceGetVvfatProps(virStorageSourcePtr src) /** * qemuBlockStorageSourceGetBackendProps: * @src: disk source + * @legacy: use legacy formatting of attributes (for -drive / old qemus) * * Creates a JSON object describing the underlying storage or protocol of a * storage source. Returns NULL on error and reports an appropriate error message. */ virJSONValuePtr -qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src) +qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src, + bool legacy) { int actualType = virStorageSourceGetActualType(src); virJSONValuePtr fileprops = NULL; @@ -1054,7 +1057,7 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src) case VIR_STORAGE_TYPE_NETWORK: switch ((virStorageNetProtocol) src->protocol) { case VIR_STORAGE_NET_PROTOCOL_GLUSTER: - if (!(fileprops = qemuBlockStorageSourceGetGlusterProps(src))) + if (!(fileprops = qemuBlockStorageSourceGetGlusterProps(src, legacy))) return NULL; break; diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h index 45485733fc..0e674437f4 100644 --- a/src/qemu/qemu_block.h +++ b/src/qemu/qemu_block.h @@ -58,7 +58,8 @@ bool qemuBlockStorageSourceSupportsConcurrentAccess(virStorageSourcePtr src); virJSONValuePtr -qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src); +qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src, + bool legacy); virURIPtr qemuBlockStorageSourceGetURI(virStorageSourcePtr src); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index ecf28d3d1d..a917eb52b1 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1458,7 +1458,7 @@ qemuDiskSourceGetProps(virStorageSourcePtr src) virJSONValuePtr props; virJSONValuePtr ret; - if (!(props = qemuBlockStorageSourceGetBackendProps(src))) + if (!(props = qemuBlockStorageSourceGetBackendProps(src, true))) return NULL; if (virJSONValueObjectCreate(&ret, "a:file", &props, NULL) < 0) { diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index 99584c759c..bd628295ff 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -62,7 +62,7 @@ testBackingXMLjsonXML(const void *args) goto cleanup; } - if (!(backendprops = qemuBlockStorageSourceGetBackendProps(xmlsrc))) { + if (!(backendprops = qemuBlockStorageSourceGetBackendProps(xmlsrc, true))) { fprintf(stderr, "failed to format disk source json\n"); goto cleanup; } -- 2.16.2

On 04/25/2018 11:15 AM, Peter Krempa wrote:
The gluster protocol in qemu uses two styles, one of which is legacy and not covered by the QAPI schema. To allow using of the new style in the blockdev-add code, add a parameter for qemuBlockStorageSourceGetBackendProps which will switch between the two modes.
awkward to look at with line length limit used. Maybe just start a new paragraph with "To allow...".
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 11 +++++++---- src/qemu/qemu_block.h | 3 ++- src/qemu/qemu_command.c | 2 +- tests/qemublocktest.c | 2 +- 4 files changed, 11 insertions(+), 7 deletions(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John

qemu declares node-name as a 32 byte buffer and silently truncates anything longer than that. This is unacceptable for libvirt, so we need to make sure that we won't ever supply a node-name exceeding 31 chars. Add a function which will do the validation and use it to validate storage-protocol node names. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 29fc7edf61..e3ee169368 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -27,6 +27,24 @@ #define VIR_FROM_THIS VIR_FROM_QEMU +/* qemu declares the buffer for node names as a 32 byte array */ +static const size_t qemuBlockNodeNameBufSize = 32; + +static int +qemuBlockNodeNameValidate(const char *nn) +{ + if (!nn) + return 0; + + if (strlen(nn) >= qemuBlockNodeNameBufSize) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("node-name '%s' too long for qemu"), nn); + return -1; + } + + return 0; +} + static int qemuBlockNamedNodesArrayToHash(size_t pos ATTRIBUTE_UNUSED, @@ -1107,7 +1125,8 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src, break; } - if (virJSONValueObjectAdd(fileprops, "S:node-name", src->nodestorage, NULL) < 0) { + if (qemuBlockNodeNameValidate(src->nodestorage) < 0 || + virJSONValueObjectAdd(fileprops, "S:node-name", src->nodestorage, NULL) < 0) { virJSONValueFree(fileprops); return NULL; } -- 2.16.2

On 04/25/2018 11:15 AM, Peter Krempa wrote:
qemu declares node-name as a 32 byte buffer and silently truncates anything longer than that. This is unacceptable for libvirt, so we need to make sure that we won't ever supply a node-name exceeding 31 chars.
Add a function which will do the validation and use it to validate storage-protocol node names.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John

When used directly with blockdev-add/-blockdev the cache mode will need to be specified directly for every image rather than just for the disk itself. This implements the backing options 'direct' and 'no-flush'. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 45 +++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 41 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index e3ee169368..3c04707618 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -1038,6 +1038,35 @@ qemuBlockStorageSourceGetVvfatProps(virStorageSourcePtr src) } +static int +qemuBlockStorageSourceGetBlockdevGetCacheProps(virStorageSourcePtr src, + virJSONValuePtr props) +{ + virJSONValuePtr cacheobj; + bool direct = false; + bool noflush = false; + + if (src->cachemode == VIR_DOMAIN_DISK_CACHE_DEFAULT) + return 0; + + if (qemuDomainDiskCachemodeFlags(src->cachemode, NULL, &direct, &noflush) < 0) + return -1; + + if (virJSONValueObjectCreate(&cacheobj, + "b:direct", direct, + "b:no-flush", noflush, + NULL) < 0) + return -1; + + if (virJSONValueObjectAppend(props, "cache", cacheobj) < 0) { + virJSONValueFree(cacheobj); + return -1; + } + + return 0; +} + + /** * qemuBlockStorageSourceGetBackendProps: * @src: disk source @@ -1052,6 +1081,7 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src, { int actualType = virStorageSourceGetActualType(src); virJSONValuePtr fileprops = NULL; + virJSONValuePtr ret = NULL; switch ((virStorageType) actualType) { case VIR_STORAGE_TYPE_BLOCK: @@ -1126,10 +1156,17 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src, } if (qemuBlockNodeNameValidate(src->nodestorage) < 0 || - virJSONValueObjectAdd(fileprops, "S:node-name", src->nodestorage, NULL) < 0) { - virJSONValueFree(fileprops); - return NULL; + virJSONValueObjectAdd(fileprops, "S:node-name", src->nodestorage, NULL) < 0) + goto cleanup; + + if (!legacy) { + if (qemuBlockStorageSourceGetBlockdevGetCacheProps(src, fileprops) < 0) + goto cleanup; } - return fileprops; + VIR_STEAL_PTR(ret, fileprops); + + cleanup: + virJSONValueFree(fileprops); + return ret; } -- 2.16.2

On 04/25/2018 11:15 AM, Peter Krempa wrote:
When used directly with blockdev-add/-blockdev the cache mode will need to be specified directly for every image rather than just for the disk itself. This implements the backing options 'direct' and 'no-flush'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 45 +++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 41 insertions(+), 4 deletions(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John

This will be required when doing blockdev-add to conform with the approach qemu would chose to create the disks. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 3c04707618..b1f495b731 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -1162,6 +1162,9 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src, if (!legacy) { if (qemuBlockStorageSourceGetBlockdevGetCacheProps(src, fileprops) < 0) goto cleanup; + + if (virJSONValueObjectAdd(fileprops, "b:read-only", src->readonly, NULL) < 0) + goto cleanup; } VIR_STEAL_PTR(ret, fileprops); -- 2.16.2

On 04/25/2018 11:15 AM, Peter Krempa wrote:
This will be required when doing blockdev-add to conform with the approach qemu would chose to create the disks.
s/would chose/chooses/ If 'would' does stay, then it's still choose as chose would be past tense.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 3 +++ 1 file changed, 3 insertions(+)
Reviewed-by: John Ferlan <jferlan@redhat.com> John

Enabling discard for the storage node allows the format drivers to discard snapshots and other things, while configuration of the format layer actually decides whether to actually discard data on request from the host. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index b1f495b731..6e76571796 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -1163,7 +1163,10 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src, if (qemuBlockStorageSourceGetBlockdevGetCacheProps(src, fileprops) < 0) goto cleanup; - if (virJSONValueObjectAdd(fileprops, "b:read-only", src->readonly, NULL) < 0) + if (virJSONValueObjectAdd(fileprops, + "b:read-only", src->readonly, + "s:discard", "unmap", + NULL) < 0) goto cleanup; } -- 2.16.2

On 04/25/2018 11:15 AM, Peter Krempa wrote:
Enabling discard for the storage node allows the format drivers to discard snapshots and other things, while configuration of the format layer actually decides whether to actually discard data on request from the host.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
Isn't this related to the {disk|src}->discard value? Which we copied from patch 3. So you'd just be unconditionally setting here regardless of what was configured? John
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index b1f495b731..6e76571796 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -1163,7 +1163,10 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src, if (qemuBlockStorageSourceGetBlockdevGetCacheProps(src, fileprops) < 0) goto cleanup;
- if (virJSONValueObjectAdd(fileprops, "b:read-only", src->readonly, NULL) < 0) + if (virJSONValueObjectAdd(fileprops, + "b:read-only", src->readonly, + "s:discard", "unmap", + NULL) < 0) goto cleanup; }

On 05/02/2018 03:39 PM, John Ferlan wrote:
On 04/25/2018 11:15 AM, Peter Krempa wrote:
Enabling discard for the storage node allows the format drivers to discard snapshots and other things, while configuration of the format layer actually decides whether to actually discard data on request from the host.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
Isn't this related to the {disk|src}->discard value? Which we copied from patch 3.
So you'd just be unconditionally setting here regardless of what was configured?
John
Hmm... so it seems the answer to my question is in the next patch. TBH: The variation between names and knowing exactly which method is for what condition - it's well, mind boggling. The terminology of storage and format layer to go along with source source protocol and storage node without much code documenting makes things challenging to follow. So it seems for whatever reason this GetBackendProps is always wanting to use "unmap"; whereas, GetBlockdevFormatCommonProps may use "unmap" or "ignore". And then, in the next patch a blockdev props and a backend props are both generated for the same object - poof. So assuming this dance is correct, Reviewed-by: John Ferlan <jferlan@redhat.com> John

On Wed, May 02, 2018 at 15:39:07 -0400, John Ferlan wrote:
On 04/25/2018 11:15 AM, Peter Krempa wrote:
Enabling discard for the storage node allows the format drivers to discard snapshots and other things, while configuration of the format layer actually decides whether to actually discard data on request from the host.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
Isn't this related to the {disk|src}->discard value? Which we copied from patch 3.
So you'd just be unconditionally setting here regardless of what was configured?
The actual discard value will be used for the "format" (qcow2/raw/...) layer. QEMU always enables discard for the storage layers so that qcow2 can discard blocks from a deleted snapshot etc.

When using blockdev-add and friends, libvirt will need to create also properties for the qcow2/raw/... format handler in qemu. This patch adds the infrastructure and implements all formats known to libvirt including all properties which are expressed at the format level in qemu. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 296 ++++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_block.h | 3 + 2 files changed, 299 insertions(+) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 6e76571796..5231c2707a 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -1176,3 +1176,299 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src, virJSONValueFree(fileprops); return ret; } + + +static int +qemuBlockStorageSourceGetFormatRawProps(virStorageSourcePtr src, + virJSONValuePtr props) +{ + qemuDomainStorageSourcePrivatePtr srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src); + const char *driver = "raw"; + const char *secretalias = NULL; + + if (src->encryption && + src->encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS && + srcPriv && + srcPriv->encinfo) { + driver = "luks"; + secretalias = srcPriv->encinfo->s.aes.alias; + } + + /* currently unhandled properties for the 'raw' driver: + * 'offset' + * 'size' + */ + + if (virJSONValueObjectAdd(props, + "s:driver", driver, + "S:key-secret", secretalias, NULL) < 0) + return -1; + + return 0; +} + + +static int +qemuBlockStorageSourceGetCryptoProps(virStorageSourcePtr src, + virJSONValuePtr *encprops) +{ + qemuDomainStorageSourcePrivatePtr srcpriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src); + const char *encformat = NULL; + + *encprops = NULL; + + /* qemu requires encrypted secrets regardless of encryption method used when + * passed using the blockdev infrastructure, thus only + * VIR_DOMAIN_SECRET_INFO_TYPE_AES works here. The correct type needs to be + * instantiated elsewhere. */ + if (!src->encryption || + !srcpriv || + !srcpriv->encinfo || + srcpriv->encinfo->type != VIR_DOMAIN_SECRET_INFO_TYPE_AES) + return 0; + + switch ((virStorageEncryptionFormatType) src->encryption->format) { + case VIR_STORAGE_ENCRYPTION_FORMAT_QCOW: + encformat = "aes"; + break; + + case VIR_STORAGE_ENCRYPTION_FORMAT_LUKS: + encformat = "luks"; + break; + + case VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT: + case VIR_STORAGE_ENCRYPTION_FORMAT_LAST: + default: + virReportEnumRangeError(virStorageEncryptionFormatType, + src->encryption->format); + return -1; + } + + return virJSONValueObjectCreate(encprops, + "s:format", encformat, + "s:key-secret", srcpriv->encinfo->s.aes.alias, + NULL); +} + + +static int +qemuBlockStorageSourceGetFormatQcowGenericProps(virStorageSourcePtr src, + const char *format, + virJSONValuePtr props) +{ + virJSONValuePtr encprops = NULL; + int ret = -1; + + if (qemuBlockStorageSourceGetCryptoProps(src, &encprops) < 0) + return -1; + + if (virJSONValueObjectAdd(props, + "s:driver", format, + "A:encrypt", &encprops, NULL) < 0) + goto cleanup; + + ret = 0; + + cleanup: + virJSONValueFree(encprops); + return ret; +} + + +static int +qemuBlockStorageSourceGetFormatQcow2Props(virStorageSourcePtr src, + virJSONValuePtr props) +{ + /* currently unhandled qcow2 props: + * + * 'lazy-refcounts' + * 'pass-discard-request' + * 'pass-discard-snapshot' + * 'pass-discard-other' + * 'overlap-check' + * 'l2-cache-size' + * 'l2-cache-entry-size' + * 'refcount-cache-size' + * 'cache-clean-interval' + */ + + if (qemuBlockStorageSourceGetFormatQcowGenericProps(src, "qcow2", props) < 0) + return -1; + + return 0; +} + + +static virJSONValuePtr +qemuBlockStorageSourceGetBlockdevFormatCommonProps(virStorageSourcePtr src) +{ + const char *detectZeroes = NULL; + const char *discard = NULL; + int detectZeroesMode = virDomainDiskGetDetectZeroesMode(src->discard, + src->detect_zeroes); + virJSONValuePtr props = NULL; + virJSONValuePtr ret = NULL; + + if (qemuBlockNodeNameValidate(src->nodeformat) < 0) + return NULL; + + if (src->discard) + discard = virDomainDiskDiscardTypeToString(src->discard); + + if (detectZeroesMode) + detectZeroes = virDomainDiskDetectZeroesTypeToString(detectZeroesMode); + + /* currently unhandled global properties: + * '*force-share': 'bool' + */ + + if (virJSONValueObjectCreate(&props, + "s:node-name", src->nodeformat, + "b:read-only", src->readonly, + "S:discard", discard, + "S:detect-zeroes", detectZeroes, + NULL) < 0) + return NULL; + + if (qemuBlockStorageSourceGetBlockdevGetCacheProps(src, props) < 0) + goto cleanup; + + VIR_STEAL_PTR(ret, props); + + cleanup: + virJSONValueFree(props); + return ret; +} + + +static virJSONValuePtr +qemuBlockStorageSourceGetBlockdevFormatProps(virStorageSourcePtr src) +{ + const char *driver = NULL; + virJSONValuePtr props = NULL; + virJSONValuePtr ret = NULL; + + if (!(props = qemuBlockStorageSourceGetBlockdevFormatCommonProps(src))) + goto cleanup; + + switch ((virStorageFileFormat) src->format) { + case VIR_STORAGE_FILE_FAT: + /* The fat layer is emulated by the storage access layer, so we need to + * put a raw layer on top */ + case VIR_STORAGE_FILE_RAW: + if (qemuBlockStorageSourceGetFormatRawProps(src, props) < 0) + goto cleanup; + + break; + + case VIR_STORAGE_FILE_QCOW2: + if (qemuBlockStorageSourceGetFormatQcow2Props(src, props) < 0) + goto cleanup; + break; + + case VIR_STORAGE_FILE_QCOW: + if (qemuBlockStorageSourceGetFormatQcowGenericProps(src, "qcow", props) < 0) + goto cleanup; + break; + + /* formats without any special parameters */ + case VIR_STORAGE_FILE_PLOOP: + driver = "parallels"; + break; + case VIR_STORAGE_FILE_VHD: + driver = "vhdx"; + break; + case VIR_STORAGE_FILE_BOCHS: + case VIR_STORAGE_FILE_CLOOP: + case VIR_STORAGE_FILE_DMG: + case VIR_STORAGE_FILE_VDI: + case VIR_STORAGE_FILE_VPC: + case VIR_STORAGE_FILE_QED: + case VIR_STORAGE_FILE_VMDK: + driver = virStorageFileFormatTypeToString(src->format); + break; + + case VIR_STORAGE_FILE_AUTO_SAFE: + case VIR_STORAGE_FILE_AUTO: + case VIR_STORAGE_FILE_NONE: + case VIR_STORAGE_FILE_COW: + case VIR_STORAGE_FILE_ISO: + case VIR_STORAGE_FILE_DIR: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("mishandled storage format '%s'"), + virStorageFileFormatTypeToString(src->format)); + goto cleanup; + + case VIR_STORAGE_FILE_LAST: + default: + virReportEnumRangeError(virStorageFileFormat, src->format); + goto cleanup; + } + + if (driver && + virJSONValueObjectAdd(props, "s:driver", driver, NULL) < 0) + goto cleanup; + + VIR_STEAL_PTR(ret, props); + + cleanup: + virJSONValueFree(props); + + return ret; +} + + +/** + * qemuBlockStorageSourceGetBlockdevProps: + * + * @src: storage source to format + * + * Formats @src into a JSON object which can be used with blockdev-add or + * -blockdev. The formatted object contains both the storage and format layer + * in nested form including link to the backing chain layer if necessary. + */ +virJSONValuePtr +qemuBlockStorageSourceGetBlockdevProps(virStorageSourcePtr src) +{ + bool backingSupported = src->format >= VIR_STORAGE_FILE_BACKING; + virJSONValuePtr storage = NULL; + virJSONValuePtr props = NULL; + virJSONValuePtr ret = NULL; + + if (virStorageSourceHasBacking(src) && !backingSupported) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("storage format '%s' does not support backing store"), + virStorageFileFormatTypeToString(src->format)); + goto cleanup; + } + + if (!(props = qemuBlockStorageSourceGetBlockdevFormatProps(src))) + goto cleanup; + + if (!(storage = qemuBlockStorageSourceGetBackendProps(src, false))) + goto cleanup; + + if (virJSONValueObjectAppend(props, "file", storage) < 0) + goto cleanup; + storage = NULL; + + if (src->backingStore && backingSupported) { + if (virStorageSourceHasBacking(src)) { + if (virJSONValueObjectAppendString(props, "backing", + src->backingStore->nodeformat) < 0) + goto cleanup; + } else { + /* chain is terminated, indicate that no detection should happen + * in qemu */ + if (virJSONValueObjectAppendNull(props, "backing") < 0) + goto cleanup; + } + } + + VIR_STEAL_PTR(ret, props); + + cleanup: + virJSONValueFree(storage); + virJSONValueFree(props); + return ret; +} diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h index 0e674437f4..f819c6f907 100644 --- a/src/qemu/qemu_block.h +++ b/src/qemu/qemu_block.h @@ -64,4 +64,7 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src, virURIPtr qemuBlockStorageSourceGetURI(virStorageSourcePtr src); +virJSONValuePtr +qemuBlockStorageSourceGetBlockdevProps(virStorageSourcePtr src); + #endif /* __QEMU_BLOCK_H__ */ -- 2.16.2

On 04/25/2018 11:15 AM, Peter Krempa wrote:
When using blockdev-add and friends, libvirt will need to create also properties for the qcow2/raw/... format handler in qemu. This patch adds the infrastructure and implements all formats known to libvirt including all properties which are expressed at the format level in qemu.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 296 ++++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_block.h | 3 + 2 files changed, 299 insertions(+)
There's a few inconsistencies with blank lines in switch/cases in qemuBlockStorageSourceGetBlockdevFormatProps that need adjustment, but otherwise, Reviewed-by: John Ferlan <jferlan@redhat.com> John [...]
+ +static virJSONValuePtr +qemuBlockStorageSourceGetBlockdevFormatProps(virStorageSourcePtr src) +{ + const char *driver = NULL; + virJSONValuePtr props = NULL; + virJSONValuePtr ret = NULL; + + if (!(props = qemuBlockStorageSourceGetBlockdevFormatCommonProps(src))) + goto cleanup; + + switch ((virStorageFileFormat) src->format) { + case VIR_STORAGE_FILE_FAT: + /* The fat layer is emulated by the storage access layer, so we need to + * put a raw layer on top */ + case VIR_STORAGE_FILE_RAW: + if (qemuBlockStorageSourceGetFormatRawProps(src, props) < 0) + goto cleanup; +
Unnecessary blank line.
+ break; + + case VIR_STORAGE_FILE_QCOW2: + if (qemuBlockStorageSourceGetFormatQcow2Props(src, props) < 0) + goto cleanup; + break; + + case VIR_STORAGE_FILE_QCOW: + if (qemuBlockStorageSourceGetFormatQcowGenericProps(src, "qcow", props) < 0) + goto cleanup; + break; + + /* formats without any special parameters */ + case VIR_STORAGE_FILE_PLOOP: + driver = "parallels"; + break;
add blank line
+ case VIR_STORAGE_FILE_VHD: + driver = "vhdx"; + break;
add blank line
+ case VIR_STORAGE_FILE_BOCHS: + case VIR_STORAGE_FILE_CLOOP: + case VIR_STORAGE_FILE_DMG: + case VIR_STORAGE_FILE_VDI: + case VIR_STORAGE_FILE_VPC: + case VIR_STORAGE_FILE_QED: + case VIR_STORAGE_FILE_VMDK: + driver = virStorageFileFormatTypeToString(src->format); + break; + + case VIR_STORAGE_FILE_AUTO_SAFE: + case VIR_STORAGE_FILE_AUTO: + case VIR_STORAGE_FILE_NONE: + case VIR_STORAGE_FILE_COW: + case VIR_STORAGE_FILE_ISO: + case VIR_STORAGE_FILE_DIR: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("mishandled storage format '%s'"), + virStorageFileFormatTypeToString(src->format)); + goto cleanup; + + case VIR_STORAGE_FILE_LAST: + default: + virReportEnumRangeError(virStorageFileFormat, src->format); + goto cleanup; + }
[...]

New tests will add new data structures so rename the 'data' structure. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemublocktest.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index bd628295ff..7eef9f286a 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -117,15 +117,16 @@ static int mymain(void) { int ret = 0; - struct testBackingXMLjsonXMLdata data; + struct testBackingXMLjsonXMLdata xmljsonxmldata; virTestCounterReset("qemu storage source xml->json->xml "); #define TEST_JSON_FORMAT(tpe, xmlstr) \ do { \ - data.type = tpe; \ - data.xml = xmlstr; \ - if (virTestRun(virTestCounterNext(), testBackingXMLjsonXML, &data) < 0) \ + xmljsonxmldata.type = tpe; \ + xmljsonxmldata.xml = xmlstr; \ + if (virTestRun(virTestCounterNext(), testBackingXMLjsonXML, \ + &xmljsonxmldata) < 0) \ ret = -1; \ } while (0) -- 2.16.2

On 04/25/2018 11:15 AM, Peter Krempa wrote:
New tests will add new data structures so rename the 'data' structure.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemublocktest.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John

Remove gnulib from _LDADD and move LDADDS to replace it. Also reformat the _SOURCES so that they can be easily extended. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/Makefile.am | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index 7b93fbde69..67850349de 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -655,12 +655,14 @@ qemuhotplugtest_SOURCES = \ qemuhotplugtest_LDADD = libqemumonitortestutils.la $(qemu_LDADDS) $(LDADDS) qemublocktest_SOURCES = \ - qemublocktest.c testutils.h testutils.c -qemublocktest_LDADD = $(LDADDS) \ + qemublocktest.c \ + testutils.h testutils.c \ + $(NULL) +qemublocktest_LDADD = \ ../src/libvirt_conf.la \ ../src/libvirt_util.la \ $(qemu_LDADDS) \ - ../gnulib/lib/libgnu.la \ + $(LDADDS) \ $(NULL) domainsnapshotxml2xmltest_SOURCES = \ -- 2.16.2

On 04/25/2018 11:15 AM, Peter Krempa wrote:
Remove gnulib from _LDADD and move LDADDS to replace it. Also reformat the _SOURCES so that they can be easily extended.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/Makefile.am | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
ew, ah, some black magic... Looks like possibly adding $(PROBES_O), adding libvirt.la, and adding "-Wl,--no-copy-dt-needed-entries" to the test image creation. Dragging in a lot, but I see future patches will need it... Reviewed-by: John Ferlan <jferlan@redhat.com> John

It will be used in the qemublocktest. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_domain.h | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 86e40e13e0..079655de87 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4186,7 +4186,7 @@ qemuDomainValidateStorageSource(virStorageSourcePtr src, } -static int +int qemuDomainDeviceDefValidateDisk(const virDomainDiskDef *disk, virQEMUCapsPtr qemuCaps) { diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 2c27dfb9f3..2dccec264b 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -862,6 +862,9 @@ int qemuDomainSecretPrepare(virQEMUDriverPtr driver, int qemuDomainDefValidateDiskLunSource(const virStorageSource *src) ATTRIBUTE_NONNULL(1); +int qemuDomainDeviceDefValidateDisk(const virDomainDiskDef *disk, + virQEMUCapsPtr qemuCaps); + int qemuDomainPrepareChannel(virDomainChrDefPtr chr, const char *domainChannelTargetDir) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); -- 2.16.2

On 04/25/2018 11:15 AM, Peter Krempa wrote:
It will be used in the qemublocktest.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_domain.h | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John

The function will be reused in the test code where we don't care much that the gluster debug level can't be populated from the qemu config. Set the level only when 'cfg' is passed. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 079655de87..564e4e7957 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -11881,7 +11881,8 @@ qemuDomainPrepareDiskSourceChain(virDomainDiskDefPtr disk, src->detect_zeroes = disk->detect_zeroes; for (n = src; virStorageSourceIsBacking(n); n = n->backingStore) { - if (n->type == VIR_STORAGE_TYPE_NETWORK && + if (cfg && + n->type == VIR_STORAGE_TYPE_NETWORK && n->protocol == VIR_STORAGE_NET_PROTOCOL_GLUSTER && virQEMUCapsGet(qemuCaps, QEMU_CAPS_GLUSTER_DEBUG_LEVEL)) { n->debug = true; -- 2.16.2

On 04/25/2018 11:15 AM, Peter Krempa wrote:
The function will be reused in the test code where we don't care much that the gluster debug level can't be populated from the qemu config.
Set the level only when 'cfg' is passed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
Doubt very much anyone wants/needs gluster debugging in test output ;-) Reviewed-by: John Ferlan <jferlan@redhat.com> John

Add a test infrastructure that will allow testing the JSON object generator used for generating data to use with blockdev-add. The resulting disk including the backing chain is validated to conform to the QAPI schema and the expected output files. The first test cases make sure that libvirt will not allow nodenames exceeding 31 chars. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/Makefile.am | 3 + tests/qemublocktest.c | 227 +++++++++++++++++++++ .../xml2json/nodename-long-format.xml | 12 ++ .../xml2json/nodename-long-protocol.xml | 12 ++ 4 files changed, 254 insertions(+) create mode 100644 tests/qemublocktestdata/xml2json/nodename-long-format.xml create mode 100644 tests/qemublocktestdata/xml2json/nodename-long-protocol.xml diff --git a/tests/Makefile.am b/tests/Makefile.am index 67850349de..6a61296c44 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -119,6 +119,7 @@ EXTRA_DIST = \ oomtrace.pl \ qemuagentdata \ qemuargv2xmldata \ + qemublocktestdata \ qemucapabilitiesdata \ qemucaps2xmldata \ qemuhotplugtestcpus \ @@ -657,8 +658,10 @@ qemuhotplugtest_LDADD = libqemumonitortestutils.la $(qemu_LDADDS) $(LDADDS) qemublocktest_SOURCES = \ qemublocktest.c \ testutils.h testutils.c \ + testutilsqemu.h testutilsqemu.c \ $(NULL) qemublocktest_LDADD = \ + libqemumonitortestutils.la \ ../src/libvirt_conf.la \ ../src/libvirt_util.la \ $(qemu_LDADDS) \ diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index 7eef9f286a..161053a717 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -19,10 +19,15 @@ #include <stdlib.h> #include "testutils.h" +#include "testutilsqemu.h" +#include "testutilsqemuschema.h" #include "virstoragefile.h" #include "virstring.h" #include "virlog.h" #include "qemu/qemu_block.h" +#include "qemu/qemu_qapi.h" + +#include "qemu/qemu_command.h" #define VIR_FROM_THIS VIR_FROM_NONE @@ -113,11 +118,190 @@ testBackingXMLjsonXML(const void *args) } +struct testQemuDiskXMLToJSONData { + virQEMUDriverPtr driver; + virHashTablePtr schema; + virJSONValuePtr schemaroot; + const char *name; + bool fail; + + virJSONValuePtr *props; + size_t nprops; + + virQEMUCapsPtr qemuCaps; +}; + + +static void +testQemuDiskXMLToPropsClear(struct testQemuDiskXMLToJSONData *data) +{ + size_t i; + + for (i = 0; i < data->nprops; i++) + virJSONValueFree(data->props[i]); + + data->nprops = 0; + VIR_FREE(data->props); +} + + +static const char *testQemuDiskXMLToJSONPath = abs_srcdir "/qemublocktestdata/xml2json/"; + +static int +testQemuDiskXMLToProps(const void *opaque) +{ + struct testQemuDiskXMLToJSONData *data = (void *) opaque; + virDomainDiskDefPtr disk = NULL; + virStorageSourcePtr n; + virJSONValuePtr props = NULL; + char *xmlpath = NULL; + char *xmlstr = NULL; + int ret = -1; + + if (virAsprintf(&xmlpath, "%s%s.xml", + testQemuDiskXMLToJSONPath, data->name) < 0) + goto cleanup; + + if (virTestLoadFile(xmlpath, &xmlstr) < 0) + goto cleanup; + + /* qemu stores node names in the status XML portion */ + if (!(disk = virDomainDiskDefParse(xmlstr, NULL, data->driver->xmlopt, + VIR_DOMAIN_DEF_PARSE_STATUS))) + goto cleanup; + + if (qemuCheckDiskConfig(disk, data->qemuCaps) < 0 || + qemuDomainDeviceDefValidateDisk(disk, data->qemuCaps) < 0) { + VIR_TEST_VERBOSE("invalid configuration for disk\n"); + goto cleanup; + } + + if (qemuDomainPrepareDiskSourceChain(disk, NULL, NULL, data->qemuCaps) < 0) + goto cleanup; + + for (n = disk->src; virStorageSourceIsBacking(n); n = n->backingStore) { + if (!(props = qemuBlockStorageSourceGetBlockdevProps(n))) { + if (!data->fail) { + VIR_TEST_VERBOSE("failed to generate qemu blockdev props\n"); + goto cleanup; + } + } else if (data->fail) { + VIR_TEST_VERBOSE("qemu blockdev props should have failed\n"); + goto cleanup; + } + + if (VIR_APPEND_ELEMENT(data->props, data->nprops, props) < 0) + goto cleanup; + } + + ret = 0; + + cleanup: + virDomainDiskDefFree(disk); + VIR_FREE(xmlpath); + VIR_FREE(xmlstr); + return ret; +} + + +static int +testQemuDiskXMLToPropsValidateSchema(const void *opaque) +{ + struct testQemuDiskXMLToJSONData *data = (void *) opaque; + virBuffer debug = VIR_BUFFER_INITIALIZER; + char *propsstr = NULL; + char *debugmsg = NULL; + int ret = 0; + size_t i; + + if (data->fail) + return EXIT_AM_SKIP; + + for (i = 0; i < data->nprops; i++) { + if (testQEMUSchemaValidate(data->props[i], data->schemaroot, + data->schema, &debug) < 0) { + debugmsg = virBufferContentAndReset(&debug); + propsstr = virJSONValueToString(data->props[i], true); + VIR_TEST_VERBOSE("json does not conform to QAPI schema"); + VIR_TEST_DEBUG("json:\n%s\ndoes not match schema. Debug output:\n %s", + propsstr, NULLSTR(debugmsg)); + VIR_FREE(debugmsg); + VIR_FREE(propsstr); + ret = -1; + } + + virBufferFreeAndReset(&debug); + } + return ret; +} + + +static int +testQemuDiskXMLToPropsValidateFile(const void *opaque) +{ + struct testQemuDiskXMLToJSONData *data = (void *) opaque; + virBuffer buf = VIR_BUFFER_INITIALIZER; + char *jsonpath = NULL; + char *jsonstr = NULL; + char *actual = NULL; + int ret = -1; + size_t i; + + if (data->fail) + return EXIT_AM_SKIP; + + if (virAsprintf(&jsonpath, "%s%s.json", + testQemuDiskXMLToJSONPath, data->name) < 0) + goto cleanup; + + for (i = 0; i < data->nprops; i++) { + if (!(jsonstr = virJSONValueToString(data->props[i], true))) + goto cleanup; + + virBufferAdd(&buf, jsonstr, -1); + } + + if (virBufferCheckError(&buf) < 0) + goto cleanup; + + actual = virBufferContentAndReset(&buf); + + ret = virTestCompareToFile(actual, jsonpath); + + cleanup: + VIR_FREE(jsonpath); + VIR_FREE(jsonstr); + VIR_FREE(actual); + return ret; +} + + static int mymain(void) { int ret = 0; + virQEMUDriver driver; struct testBackingXMLjsonXMLdata xmljsonxmldata; + struct testQemuDiskXMLToJSONData diskxmljsondata; + char *capslatest_x86_64 = NULL; + virQEMUCapsPtr caps_x86_64 = NULL; + + if (qemuTestDriverInit(&driver) < 0) + return EXIT_FAILURE; + + diskxmljsondata.driver = &driver; + + if (!(capslatest_x86_64 = testQemuGetLatestCapsForArch(abs_srcdir "/qemucapabilitiesdata", + "x86_64", "xml"))) + return EXIT_FAILURE; + + VIR_TEST_VERBOSE("\nlatest caps x86_64: %s\n", capslatest_x86_64); + + if (!(caps_x86_64 = qemuTestParseCapabilitiesArch(virArchFromString("x86_64"), + capslatest_x86_64))) + return EXIT_FAILURE; + + diskxmljsondata.qemuCaps = caps_x86_64; virTestCounterReset("qemu storage source xml->json->xml "); @@ -183,6 +367,49 @@ mymain(void) " <host name='example.com' port='9999'/>\n" "</source>\n"); +#define TEST_DISK_TO_JSON_FULL(nme, fl) \ + do { \ + diskxmljsondata.name = nme; \ + diskxmljsondata.props = NULL; \ + diskxmljsondata.nprops = 0; \ + diskxmljsondata.fail = fl; \ + if (virTestRun("disk xml to props " nme, testQemuDiskXMLToProps, \ + &diskxmljsondata) < 0) \ + ret = -1; \ + if (virTestRun("disk xml to props validate schema " nme, \ + testQemuDiskXMLToPropsValidateSchema, &diskxmljsondata) < 0) \ + ret = -1; \ + if (virTestRun("disk xml to props validate file " nme, \ + testQemuDiskXMLToPropsValidateFile, &diskxmljsondata) < 0) \ + ret = -1; \ + testQemuDiskXMLToPropsClear(&diskxmljsondata); \ + } while (0) + +#define TEST_DISK_TO_JSON(nme) TEST_DISK_TO_JSON_FULL(nme, false) + + if (!(diskxmljsondata.schema = testQEMUSchemaLoad())) { + ret = -1; + goto cleanup; + } + + if (virQEMUQAPISchemaPathGet("blockdev-add/arg-type", + diskxmljsondata.schema, + &diskxmljsondata.schemaroot) < 0 || + !diskxmljsondata.schemaroot) { + VIR_TEST_VERBOSE("failed to find schema entry for blockdev-add\n"); + ret = -1; + goto cleanup; + } + + TEST_DISK_TO_JSON_FULL("nodename-long-format", true); + TEST_DISK_TO_JSON_FULL("nodename-long-protocol", true); + + cleanup: + virHashFree(diskxmljsondata.schema); + qemuTestDriverFree(&driver); + VIR_FREE(capslatest_x86_64); + virObjectUnref(caps_x86_64); + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/tests/qemublocktestdata/xml2json/nodename-long-format.xml b/tests/qemublocktestdata/xml2json/nodename-long-format.xml new file mode 100644 index 0000000000..8fccf850db --- /dev/null +++ b/tests/qemublocktestdata/xml2json/nodename-long-format.xml @@ -0,0 +1,12 @@ +<disk device='disk'> + <driver name='qemu' type='raw'/> + <source file='/path'> + <privateData> + <nodenames> + <nodename type='storage' name='test2'/> + <nodename type='format' name='0123456789ABCDEF0123456789ABCDEF'/> + </nodenames> + </privateData> + </source> + <target dev='vda'/> +</disk> diff --git a/tests/qemublocktestdata/xml2json/nodename-long-protocol.xml b/tests/qemublocktestdata/xml2json/nodename-long-protocol.xml new file mode 100644 index 0000000000..e60d988325 --- /dev/null +++ b/tests/qemublocktestdata/xml2json/nodename-long-protocol.xml @@ -0,0 +1,12 @@ +<disk device='disk'> + <driver name='qemu' type='raw'/> + <source file='/path'> + <privateData> + <nodenames> + <nodename type='storage' name='0123456789ABCDEF0123456789ABCDEF0'/> + <nodename type='format' name='test1'/> + </nodenames> + </privateData> + </source> + <target dev='vda'/> +</disk> -- 2.16.2

On 04/25/2018 11:15 AM, Peter Krempa wrote:
Add a test infrastructure that will allow testing the JSON object generator used for generating data to use with blockdev-add.
The resulting disk including the backing chain is validated to conform to the QAPI schema and the expected output files.
The first test cases make sure that libvirt will not allow nodenames exceeding 31 chars.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/Makefile.am | 3 + tests/qemublocktest.c | 227 +++++++++++++++++++++ .../xml2json/nodename-long-format.xml | 12 ++ .../xml2json/nodename-long-protocol.xml | 12 ++ 4 files changed, 254 insertions(+) create mode 100644 tests/qemublocktestdata/xml2json/nodename-long-format.xml create mode 100644 tests/qemublocktestdata/xml2json/nodename-long-protocol.xml
[...]]
+struct testQemuDiskXMLToJSONData { + virQEMUDriverPtr driver; + virHashTablePtr schema; + virJSONValuePtr schemaroot; + const char *name; + bool fail;
If there was a const char *expt; which would have at least a subset of the expected error message, then you'd really know if the failure was for what you were testing as opposed to some other random failure (without needing to turn on debug). Nothing that needs to change for an R-by, but just a random thought as I ran the test w/ debug mode for the errors: internal error: node-name '0123456789ABCDEF0123456789ABCDEF' too long for qemu and internal error: node-name '0123456789ABCDEF0123456789ABCDEF0' too long for qemu
+ + virJSONValuePtr *props; + size_t nprops; + + virQEMUCapsPtr qemuCaps; +}; + +
[...]
+static int +testQemuDiskXMLToPropsValidateFile(const void *opaque) +{ + struct testQemuDiskXMLToJSONData *data = (void *) opaque; + virBuffer buf = VIR_BUFFER_INITIALIZER; + char *jsonpath = NULL; + char *jsonstr = NULL; + char *actual = NULL; + int ret = -1; + size_t i; + + if (data->fail) + return EXIT_AM_SKIP; + + if (virAsprintf(&jsonpath, "%s%s.json", + testQemuDiskXMLToJSONPath, data->name) < 0) + goto cleanup; + + for (i = 0; i < data->nprops; i++) { + if (!(jsonstr = virJSONValueToString(data->props[i], true))) + goto cleanup; + + virBufferAdd(&buf, jsonstr, -1);
Coverity informed me @jsonstr would be leaked each time through the loop. Probably could just be local to this loop too.
+ } + + if (virBufferCheckError(&buf) < 0) + goto cleanup; + + actual = virBufferContentAndReset(&buf); + + ret = virTestCompareToFile(actual, jsonpath); + + cleanup: + VIR_FREE(jsonpath); + VIR_FREE(jsonstr); + VIR_FREE(actual); + return ret; +} + +
Reviewed-by: John Ferlan <jferlan@redhat.com> John [...]

Test the JSON props generator with a very simple 'raw' image with no other options. The node-names for the image are 31 bytes long so that we validate our node name detector. The top level disk image would generate the following '-drive' cmdline: -drive file=/var/lib/libvirt/images/i.img,format=raw,if=none,id=drive-dummy -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemublocktest.c | 2 ++ tests/qemublocktestdata/xml2json/file-raw-noopts.json | 12 ++++++++++++ tests/qemublocktestdata/xml2json/file-raw-noopts.xml | 12 ++++++++++++ 3 files changed, 26 insertions(+) create mode 100644 tests/qemublocktestdata/xml2json/file-raw-noopts.json create mode 100644 tests/qemublocktestdata/xml2json/file-raw-noopts.xml diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index 161053a717..c9a2f91992 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -404,6 +404,8 @@ mymain(void) TEST_DISK_TO_JSON_FULL("nodename-long-format", true); TEST_DISK_TO_JSON_FULL("nodename-long-protocol", true); + TEST_DISK_TO_JSON("file-raw-noopts"); + cleanup: virHashFree(diskxmljsondata.schema); qemuTestDriverFree(&driver); diff --git a/tests/qemublocktestdata/xml2json/file-raw-noopts.json b/tests/qemublocktestdata/xml2json/file-raw-noopts.json new file mode 100644 index 0000000000..25de571428 --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-raw-noopts.json @@ -0,0 +1,12 @@ +{ + "node-name": "0123456789ABCDEF0123456789ABCDE", + "read-only": false, + "driver": "raw", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/i.img", + "node-name": "0123456789ABCDEF0123456789ABCDE", + "read-only": false, + "discard": "unmap" + } +} diff --git a/tests/qemublocktestdata/xml2json/file-raw-noopts.xml b/tests/qemublocktestdata/xml2json/file-raw-noopts.xml new file mode 100644 index 0000000000..8255e64eb7 --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-raw-noopts.xml @@ -0,0 +1,12 @@ +<disk device='disk'> + <driver name='qemu' type='raw'/> + <source file='/var/lib/libvirt/images/i.img'> + <privateData> + <nodenames> + <nodename type='storage' name='0123456789ABCDEF0123456789ABCDE'/> + <nodename type='format' name='0123456789ABCDEF0123456789ABCDE'/> + </nodenames> + </privateData> + </source> + <target dev='vda'/> +</disk> -- 2.16.2

On 04/25/2018 11:15 AM, Peter Krempa wrote:
Test the JSON props generator with a very simple 'raw' image with no other options. The node-names for the image are 31 bytes long so that we validate our node name detector.
The top level disk image would generate the following '-drive' cmdline:
-drive file=/var/lib/libvirt/images/i.img,format=raw,if=none,id=drive-dummy -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy
One would think we'd be able to test that too ;-)
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemublocktest.c | 2 ++ tests/qemublocktestdata/xml2json/file-raw-noopts.json | 12 ++++++++++++ tests/qemublocktestdata/xml2json/file-raw-noopts.xml | 12 ++++++++++++ 3 files changed, 26 insertions(+) create mode 100644 tests/qemublocktestdata/xml2json/file-raw-noopts.json create mode 100644 tests/qemublocktestdata/xml2json/file-raw-noopts.xml
Reviewed-by: John Ferlan <jferlan@redhat.com> John

On Wed, May 02, 2018 at 18:39:44 -0400, John Ferlan wrote:
On 04/25/2018 11:15 AM, Peter Krempa wrote:
Test the JSON props generator with a very simple 'raw' image with no other options. The node-names for the image are 31 bytes long so that we validate our node name detector.
The top level disk image would generate the following '-drive' cmdline:
-drive file=/var/lib/libvirt/images/i.img,format=raw,if=none,id=drive-dummy -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy
One would think we'd be able to test that too ;-)
I've actually generated these by instrumenting the test function to call the old generators as well. The thing is that the mapping to -drive command line is valid only for now, when I'm striving to fully replace -drive. Once blockdev is used new features will be added only to the blockdev infrastructure so the mapping to -drive will become invalid. Thus I chose to not test this aspect.

Similarly to the 'raw' case add tests for bochs, cloop, dmg, ploop, vdi vhd, and vpc. Covering all supproted non-backing formats. Note that the JSON name for 'ploop' maps to 'parallels' and 'vhd' maps to 'vhdx'. Files added here would result in the followint configs: file-bochs-noopts.xml: -drive file=/path/to/i.img,format=bochs,if=none,id=drive-dummy -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy file-cloop-noopts.xml: -drive file=/path/to/i.img,format=cloop,if=none,id=drive-dummy -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy file-dmg-noopts.xml: -drive file=/path/to/i.img,format=dmg,if=none,id=drive-dummy -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy file-ploop-noopts.xml: -drive file=/path/to/i.img,format=ploop,if=none,id=drive-dummy -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy file-vdi-noopts.xml: -drive file=/path/to/i.img,format=vdi,if=none,id=drive-dummy -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy file-vhd-noopts.xml: -drive file=/path/to/i.img,format=vhd,if=none,id=drive-dummy -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy file-vpc-noopts.xml: -drive file=/path/to/i.img,format=vpc,if=none,id=drive-dummy -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemublocktest.c | 7 +++++++ tests/qemublocktestdata/xml2json/file-bochs-noopts.json | 12 ++++++++++++ tests/qemublocktestdata/xml2json/file-bochs-noopts.xml | 12 ++++++++++++ tests/qemublocktestdata/xml2json/file-cloop-noopts.json | 12 ++++++++++++ tests/qemublocktestdata/xml2json/file-cloop-noopts.xml | 12 ++++++++++++ tests/qemublocktestdata/xml2json/file-dmg-noopts.json | 12 ++++++++++++ tests/qemublocktestdata/xml2json/file-dmg-noopts.xml | 12 ++++++++++++ tests/qemublocktestdata/xml2json/file-ploop-noopts.json | 12 ++++++++++++ tests/qemublocktestdata/xml2json/file-ploop-noopts.xml | 12 ++++++++++++ tests/qemublocktestdata/xml2json/file-vdi-noopts.json | 12 ++++++++++++ tests/qemublocktestdata/xml2json/file-vdi-noopts.xml | 12 ++++++++++++ tests/qemublocktestdata/xml2json/file-vhd-noopts.json | 12 ++++++++++++ tests/qemublocktestdata/xml2json/file-vhd-noopts.xml | 12 ++++++++++++ tests/qemublocktestdata/xml2json/file-vpc-noopts.json | 12 ++++++++++++ tests/qemublocktestdata/xml2json/file-vpc-noopts.xml | 12 ++++++++++++ 15 files changed, 175 insertions(+) create mode 100644 tests/qemublocktestdata/xml2json/file-bochs-noopts.json create mode 100644 tests/qemublocktestdata/xml2json/file-bochs-noopts.xml create mode 100644 tests/qemublocktestdata/xml2json/file-cloop-noopts.json create mode 100644 tests/qemublocktestdata/xml2json/file-cloop-noopts.xml create mode 100644 tests/qemublocktestdata/xml2json/file-dmg-noopts.json create mode 100644 tests/qemublocktestdata/xml2json/file-dmg-noopts.xml create mode 100644 tests/qemublocktestdata/xml2json/file-ploop-noopts.json create mode 100644 tests/qemublocktestdata/xml2json/file-ploop-noopts.xml create mode 100644 tests/qemublocktestdata/xml2json/file-vdi-noopts.json create mode 100644 tests/qemublocktestdata/xml2json/file-vdi-noopts.xml create mode 100644 tests/qemublocktestdata/xml2json/file-vhd-noopts.json create mode 100644 tests/qemublocktestdata/xml2json/file-vhd-noopts.xml create mode 100644 tests/qemublocktestdata/xml2json/file-vpc-noopts.json create mode 100644 tests/qemublocktestdata/xml2json/file-vpc-noopts.xml diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index c9a2f91992..feb25c69e3 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -405,6 +405,13 @@ mymain(void) TEST_DISK_TO_JSON_FULL("nodename-long-protocol", true); TEST_DISK_TO_JSON("file-raw-noopts"); + TEST_DISK_TO_JSON("file-bochs-noopts"); + TEST_DISK_TO_JSON("file-cloop-noopts"); + TEST_DISK_TO_JSON("file-dmg-noopts"); + TEST_DISK_TO_JSON("file-ploop-noopts"); + TEST_DISK_TO_JSON("file-vdi-noopts"); + TEST_DISK_TO_JSON("file-vhd-noopts"); + TEST_DISK_TO_JSON("file-vpc-noopts"); cleanup: virHashFree(diskxmljsondata.schema); diff --git a/tests/qemublocktestdata/xml2json/file-bochs-noopts.json b/tests/qemublocktestdata/xml2json/file-bochs-noopts.json new file mode 100644 index 0000000000..22e2560998 --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-bochs-noopts.json @@ -0,0 +1,12 @@ +{ + "node-name": "test1", + "read-only": false, + "driver": "bochs", + "file": { + "driver": "file", + "filename": "/path/to/i.img", + "node-name": "test2", + "read-only": false, + "discard": "unmap" + } +} diff --git a/tests/qemublocktestdata/xml2json/file-bochs-noopts.xml b/tests/qemublocktestdata/xml2json/file-bochs-noopts.xml new file mode 100644 index 0000000000..23da6c4feb --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-bochs-noopts.xml @@ -0,0 +1,12 @@ +<disk device='disk'> + <driver name='qemu' type='bochs'/> + <source file='/path/to/i.img'> + <privateData> + <nodenames> + <nodename type='storage' name='test2'/> + <nodename type='format' name='test1'/> + </nodenames> + </privateData> + </source> + <target dev='vda'/> +</disk> diff --git a/tests/qemublocktestdata/xml2json/file-cloop-noopts.json b/tests/qemublocktestdata/xml2json/file-cloop-noopts.json new file mode 100644 index 0000000000..b72bb5df56 --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-cloop-noopts.json @@ -0,0 +1,12 @@ +{ + "node-name": "test1", + "read-only": false, + "driver": "cloop", + "file": { + "driver": "file", + "filename": "/path/to/i.img", + "node-name": "test2", + "read-only": false, + "discard": "unmap" + } +} diff --git a/tests/qemublocktestdata/xml2json/file-cloop-noopts.xml b/tests/qemublocktestdata/xml2json/file-cloop-noopts.xml new file mode 100644 index 0000000000..4dff296983 --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-cloop-noopts.xml @@ -0,0 +1,12 @@ +<disk device='disk'> + <driver name='qemu' type='cloop'/> + <source file='/path/to/i.img'> + <privateData> + <nodenames> + <nodename type='storage' name='test2'/> + <nodename type='format' name='test1'/> + </nodenames> + </privateData> + </source> + <target dev='vda'/> +</disk> diff --git a/tests/qemublocktestdata/xml2json/file-dmg-noopts.json b/tests/qemublocktestdata/xml2json/file-dmg-noopts.json new file mode 100644 index 0000000000..9f2912b8d3 --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-dmg-noopts.json @@ -0,0 +1,12 @@ +{ + "node-name": "test1", + "read-only": false, + "driver": "dmg", + "file": { + "driver": "file", + "filename": "/path/to/i.img", + "node-name": "test2", + "read-only": false, + "discard": "unmap" + } +} diff --git a/tests/qemublocktestdata/xml2json/file-dmg-noopts.xml b/tests/qemublocktestdata/xml2json/file-dmg-noopts.xml new file mode 100644 index 0000000000..3650a1758f --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-dmg-noopts.xml @@ -0,0 +1,12 @@ +<disk device='disk'> + <driver name='qemu' type='dmg'/> + <source file='/path/to/i.img'> + <privateData> + <nodenames> + <nodename type='storage' name='test2'/> + <nodename type='format' name='test1'/> + </nodenames> + </privateData> + </source> + <target dev='vda'/> +</disk> diff --git a/tests/qemublocktestdata/xml2json/file-ploop-noopts.json b/tests/qemublocktestdata/xml2json/file-ploop-noopts.json new file mode 100644 index 0000000000..64006d28a3 --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-ploop-noopts.json @@ -0,0 +1,12 @@ +{ + "node-name": "test1", + "read-only": false, + "driver": "parallels", + "file": { + "driver": "file", + "filename": "/path/to/i.img", + "node-name": "test2", + "read-only": false, + "discard": "unmap" + } +} diff --git a/tests/qemublocktestdata/xml2json/file-ploop-noopts.xml b/tests/qemublocktestdata/xml2json/file-ploop-noopts.xml new file mode 100644 index 0000000000..5435c4d8d7 --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-ploop-noopts.xml @@ -0,0 +1,12 @@ +<disk device='disk'> + <driver name='qemu' type='ploop'/> + <source file='/path/to/i.img'> + <privateData> + <nodenames> + <nodename type='storage' name='test2'/> + <nodename type='format' name='test1'/> + </nodenames> + </privateData> + </source> + <target dev='vda'/> +</disk> diff --git a/tests/qemublocktestdata/xml2json/file-vdi-noopts.json b/tests/qemublocktestdata/xml2json/file-vdi-noopts.json new file mode 100644 index 0000000000..ce8e359c91 --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-vdi-noopts.json @@ -0,0 +1,12 @@ +{ + "node-name": "test1", + "read-only": false, + "driver": "vdi", + "file": { + "driver": "file", + "filename": "/path/to/i.img", + "node-name": "test2", + "read-only": false, + "discard": "unmap" + } +} diff --git a/tests/qemublocktestdata/xml2json/file-vdi-noopts.xml b/tests/qemublocktestdata/xml2json/file-vdi-noopts.xml new file mode 100644 index 0000000000..3fa489e24b --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-vdi-noopts.xml @@ -0,0 +1,12 @@ +<disk device='disk'> + <driver name='qemu' type='vdi'/> + <source file='/path/to/i.img'> + <privateData> + <nodenames> + <nodename type='storage' name='test2'/> + <nodename type='format' name='test1'/> + </nodenames> + </privateData> + </source> + <target dev='vda'/> +</disk> diff --git a/tests/qemublocktestdata/xml2json/file-vhd-noopts.json b/tests/qemublocktestdata/xml2json/file-vhd-noopts.json new file mode 100644 index 0000000000..d4b8e1f55a --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-vhd-noopts.json @@ -0,0 +1,12 @@ +{ + "node-name": "test1", + "read-only": false, + "driver": "vhdx", + "file": { + "driver": "file", + "filename": "/path/to/i.img", + "node-name": "test2", + "read-only": false, + "discard": "unmap" + } +} diff --git a/tests/qemublocktestdata/xml2json/file-vhd-noopts.xml b/tests/qemublocktestdata/xml2json/file-vhd-noopts.xml new file mode 100644 index 0000000000..25fd1ba7b0 --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-vhd-noopts.xml @@ -0,0 +1,12 @@ +<disk device='disk'> + <driver name='qemu' type='vhd'/> + <source file='/path/to/i.img'> + <privateData> + <nodenames> + <nodename type='storage' name='test2'/> + <nodename type='format' name='test1'/> + </nodenames> + </privateData> + </source> + <target dev='vda'/> +</disk> diff --git a/tests/qemublocktestdata/xml2json/file-vpc-noopts.json b/tests/qemublocktestdata/xml2json/file-vpc-noopts.json new file mode 100644 index 0000000000..be1ec795a7 --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-vpc-noopts.json @@ -0,0 +1,12 @@ +{ + "node-name": "test1", + "read-only": false, + "driver": "vpc", + "file": { + "driver": "file", + "filename": "/path/to/i.img", + "node-name": "test2", + "read-only": false, + "discard": "unmap" + } +} diff --git a/tests/qemublocktestdata/xml2json/file-vpc-noopts.xml b/tests/qemublocktestdata/xml2json/file-vpc-noopts.xml new file mode 100644 index 0000000000..d761bb33cc --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-vpc-noopts.xml @@ -0,0 +1,12 @@ +<disk device='disk'> + <driver name='qemu' type='vpc'/> + <source file='/path/to/i.img'> + <privateData> + <nodenames> + <nodename type='storage' name='test2'/> + <nodename type='format' name='test1'/> + </nodenames> + </privateData> + </source> + <target dev='vda'/> +</disk> -- 2.16.2

On 04/25/2018 11:15 AM, Peter Krempa wrote:
Similarly to the 'raw' case add tests for bochs, cloop, dmg, ploop, vdi vhd, and vpc. Covering all supproted non-backing formats.
supported
Note that the JSON name for 'ploop' maps to 'parallels' and 'vhd' maps to 'vhdx'.
Files added here would result in the followint configs:
file-bochs-noopts.xml: -drive file=/path/to/i.img,format=bochs,if=none,id=drive-dummy -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy
file-cloop-noopts.xml: -drive file=/path/to/i.img,format=cloop,if=none,id=drive-dummy -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy
file-dmg-noopts.xml: -drive file=/path/to/i.img,format=dmg,if=none,id=drive-dummy -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy
file-ploop-noopts.xml: -drive file=/path/to/i.img,format=ploop,if=none,id=drive-dummy -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy
file-vdi-noopts.xml: -drive file=/path/to/i.img,format=vdi,if=none,id=drive-dummy -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy
file-vhd-noopts.xml: -drive file=/path/to/i.img,format=vhd,if=none,id=drive-dummy -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy
file-vpc-noopts.xml: -drive file=/path/to/i.img,format=vpc,if=none,id=drive-dummy -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemublocktest.c | 7 +++++++ tests/qemublocktestdata/xml2json/file-bochs-noopts.json | 12 ++++++++++++ tests/qemublocktestdata/xml2json/file-bochs-noopts.xml | 12 ++++++++++++ tests/qemublocktestdata/xml2json/file-cloop-noopts.json | 12 ++++++++++++ tests/qemublocktestdata/xml2json/file-cloop-noopts.xml | 12 ++++++++++++ tests/qemublocktestdata/xml2json/file-dmg-noopts.json | 12 ++++++++++++ tests/qemublocktestdata/xml2json/file-dmg-noopts.xml | 12 ++++++++++++ tests/qemublocktestdata/xml2json/file-ploop-noopts.json | 12 ++++++++++++ tests/qemublocktestdata/xml2json/file-ploop-noopts.xml | 12 ++++++++++++ tests/qemublocktestdata/xml2json/file-vdi-noopts.json | 12 ++++++++++++ tests/qemublocktestdata/xml2json/file-vdi-noopts.xml | 12 ++++++++++++ tests/qemublocktestdata/xml2json/file-vhd-noopts.json | 12 ++++++++++++ tests/qemublocktestdata/xml2json/file-vhd-noopts.xml | 12 ++++++++++++ tests/qemublocktestdata/xml2json/file-vpc-noopts.json | 12 ++++++++++++ tests/qemublocktestdata/xml2json/file-vpc-noopts.xml | 12 ++++++++++++ 15 files changed, 175 insertions(+) create mode 100644 tests/qemublocktestdata/xml2json/file-bochs-noopts.json create mode 100644 tests/qemublocktestdata/xml2json/file-bochs-noopts.xml create mode 100644 tests/qemublocktestdata/xml2json/file-cloop-noopts.json create mode 100644 tests/qemublocktestdata/xml2json/file-cloop-noopts.xml create mode 100644 tests/qemublocktestdata/xml2json/file-dmg-noopts.json create mode 100644 tests/qemublocktestdata/xml2json/file-dmg-noopts.xml create mode 100644 tests/qemublocktestdata/xml2json/file-ploop-noopts.json create mode 100644 tests/qemublocktestdata/xml2json/file-ploop-noopts.xml create mode 100644 tests/qemublocktestdata/xml2json/file-vdi-noopts.json create mode 100644 tests/qemublocktestdata/xml2json/file-vdi-noopts.xml create mode 100644 tests/qemublocktestdata/xml2json/file-vhd-noopts.json create mode 100644 tests/qemublocktestdata/xml2json/file-vhd-noopts.xml create mode 100644 tests/qemublocktestdata/xml2json/file-vpc-noopts.json create mode 100644 tests/qemublocktestdata/xml2json/file-vpc-noopts.xml
Reviewed-by: John Ferlan <jferlan@redhat.com> John

Formats supporting backing chain such as qed, vmdk, don't have any other parameters than the backing store and 'qcow' has only encryption params which will be tested extra. Add this test case so they are covered since any further test cases will mainly care about 'qcow2' and 'raw'. The top level disk image would generate the following '-drive' cmdline: -drive file=/var/lib/libvirt/images/a,format=qed,if=none,id=drive-dummy -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemublocktest.c | 2 + .../xml2json/file-backing_basic-noopts.json | 51 ++++++++++++++++++++++ .../xml2json/file-backing_basic-noopts.xml | 46 +++++++++++++++++++ 3 files changed, 99 insertions(+) create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-noopts.json create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-noopts.xml diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index feb25c69e3..881d7c17ee 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -413,6 +413,8 @@ mymain(void) TEST_DISK_TO_JSON("file-vhd-noopts"); TEST_DISK_TO_JSON("file-vpc-noopts"); + TEST_DISK_TO_JSON("file-backing_basic-noopts"); + cleanup: virHashFree(diskxmljsondata.schema); qemuTestDriverFree(&driver); diff --git a/tests/qemublocktestdata/xml2json/file-backing_basic-noopts.json b/tests/qemublocktestdata/xml2json/file-backing_basic-noopts.json new file mode 100644 index 0000000000..3285a6ec67 --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-backing_basic-noopts.json @@ -0,0 +1,51 @@ +{ + "node-name": "node-a-f", + "read-only": false, + "driver": "qed", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/a", + "node-name": "node-a-s", + "read-only": false, + "discard": "unmap" + }, + "backing": "node-b-f" +} +{ + "node-name": "node-b-f", + "read-only": true, + "driver": "qcow", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/b", + "node-name": "node-b-s", + "read-only": true, + "discard": "unmap" + }, + "backing": "node-c-f" +} +{ + "node-name": "node-c-f", + "read-only": true, + "driver": "vmdk", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/c", + "node-name": "node-c-s", + "read-only": true, + "discard": "unmap" + }, + "backing": "node-d-f" +} +{ + "node-name": "node-d-f", + "read-only": true, + "driver": "raw", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/d", + "node-name": "node-d-s", + "read-only": true, + "discard": "unmap" + } +} diff --git a/tests/qemublocktestdata/xml2json/file-backing_basic-noopts.xml b/tests/qemublocktestdata/xml2json/file-backing_basic-noopts.xml new file mode 100644 index 0000000000..7c48433aec --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-backing_basic-noopts.xml @@ -0,0 +1,46 @@ +<disk type='file' device='disk'> + <driver name='qemu' type='qed'/> + <source file='/var/lib/libvirt/images/a'> + <privateData> + <nodenames> + <nodename type='storage' name='node-a-s'/> + <nodename type='format' name='node-a-f'/> + </nodenames> + </privateData> + </source> + <backingStore type='file' index='1'> + <format type='qcow'/> + <source file='/var/lib/libvirt/images/b'> + <privateData> + <nodenames> + <nodename type='storage' name='node-b-s'/> + <nodename type='format' name='node-b-f'/> + </nodenames> + </privateData> + </source> + <backingStore type='file' index='2'> + <format type='vmdk'/> + <source file='/var/lib/libvirt/images/c'> + <privateData> + <nodenames> + <nodename type='storage' name='node-c-s'/> + <nodename type='format' name='node-c-f'/> + </nodenames> + </privateData> + </source> + <backingStore type='file' index='3'> + <format type='raw'/> + <source file='/var/lib/libvirt/images/d'> + <privateData> + <nodenames> + <nodename type='storage' name='node-d-s'/> + <nodename type='format' name='node-d-f'/> + </nodenames> + </privateData> + </source> + <backingStore/> + </backingStore> + </backingStore> + </backingStore> + <target dev='vda'/> +</disk> -- 2.16.2

On 04/25/2018 11:15 AM, Peter Krempa wrote:
Formats supporting backing chain such as qed, vmdk, don't have any other parameters than the backing store and 'qcow' has only encryption params which will be tested extra. Add this test case so they are covered since any further test cases will mainly care about 'qcow2' and 'raw'.
The top level disk image would generate the following '-drive' cmdline:
-drive file=/var/lib/libvirt/images/a,format=qed,if=none,id=drive-dummy -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemublocktest.c | 2 + .../xml2json/file-backing_basic-noopts.json | 51 ++++++++++++++++++++++ .../xml2json/file-backing_basic-noopts.xml | 46 +++++++++++++++++++ 3 files changed, 99 insertions(+) create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-noopts.json create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-noopts.xml
Reviewed-by: John Ferlan <jferlan@redhat.com> John

Test mapping of the 'FAT' disk format to 'vvfat' in qemu. The top level disk image would generate the following '-drive' cmdline: dir-fat-readonly.xml: -drive file=fat:/var/somefiles,if=none,id=drive-dummy,readonly=on -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemublocktest.c | 2 ++ tests/qemublocktestdata/xml2json/dir-fat-floppy.json | 14 ++++++++++++++ tests/qemublocktestdata/xml2json/dir-fat-floppy.xml | 14 ++++++++++++++ tests/qemublocktestdata/xml2json/dir-fat-readonly.json | 14 ++++++++++++++ tests/qemublocktestdata/xml2json/dir-fat-readonly.xml | 13 +++++++++++++ 5 files changed, 57 insertions(+) create mode 100644 tests/qemublocktestdata/xml2json/dir-fat-floppy.json create mode 100644 tests/qemublocktestdata/xml2json/dir-fat-floppy.xml create mode 100644 tests/qemublocktestdata/xml2json/dir-fat-readonly.json create mode 100644 tests/qemublocktestdata/xml2json/dir-fat-readonly.xml diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index 881d7c17ee..81a2261dab 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -414,6 +414,8 @@ mymain(void) TEST_DISK_TO_JSON("file-vpc-noopts"); TEST_DISK_TO_JSON("file-backing_basic-noopts"); + TEST_DISK_TO_JSON("dir-fat-readonly"); + TEST_DISK_TO_JSON("dir-fat-floppy"); cleanup: virHashFree(diskxmljsondata.schema); diff --git a/tests/qemublocktestdata/xml2json/dir-fat-floppy.json b/tests/qemublocktestdata/xml2json/dir-fat-floppy.json new file mode 100644 index 0000000000..9685acaa8b --- /dev/null +++ b/tests/qemublocktestdata/xml2json/dir-fat-floppy.json @@ -0,0 +1,14 @@ +{ + "node-name": "node-f", + "read-only": true, + "driver": "raw", + "file": { + "driver": "vvfat", + "dir": "/var/somefiles", + "floppy": true, + "rw": false, + "node-name": "node-s", + "read-only": true, + "discard": "unmap" + } +} diff --git a/tests/qemublocktestdata/xml2json/dir-fat-floppy.xml b/tests/qemublocktestdata/xml2json/dir-fat-floppy.xml new file mode 100644 index 0000000000..8f085a1832 --- /dev/null +++ b/tests/qemublocktestdata/xml2json/dir-fat-floppy.xml @@ -0,0 +1,14 @@ +<disk type='dir' device='floppy'> + <driver name='qemu' type='fat'/> + <source dir='/var/somefiles'> + <privateData> + <nodenames> + <nodename type='storage' name='node-s'/> + <nodename type='format' name='node-f'/> + </nodenames> + </privateData> + </source> + <target dev='fda'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + <readonly/> +</disk> diff --git a/tests/qemublocktestdata/xml2json/dir-fat-readonly.json b/tests/qemublocktestdata/xml2json/dir-fat-readonly.json new file mode 100644 index 0000000000..7d35d255cc --- /dev/null +++ b/tests/qemublocktestdata/xml2json/dir-fat-readonly.json @@ -0,0 +1,14 @@ +{ + "node-name": "node-f", + "read-only": true, + "driver": "raw", + "file": { + "driver": "vvfat", + "dir": "/var/somefiles", + "floppy": false, + "rw": false, + "node-name": "node-s", + "read-only": true, + "discard": "unmap" + } +} diff --git a/tests/qemublocktestdata/xml2json/dir-fat-readonly.xml b/tests/qemublocktestdata/xml2json/dir-fat-readonly.xml new file mode 100644 index 0000000000..9ac24b5d41 --- /dev/null +++ b/tests/qemublocktestdata/xml2json/dir-fat-readonly.xml @@ -0,0 +1,13 @@ +<disk type='dir' device='disk'> + <driver name='qemu' type='fat'/> + <source dir='/var/somefiles'> + <privateData> + <nodenames> + <nodename type='storage' name='node-s'/> + <nodename type='format' name='node-f'/> + </nodenames> + </privateData> + </source> + <target dev='vda'/> + <readonly/> +</disk> -- 2.16.2

On 04/25/2018 11:15 AM, Peter Krempa wrote:
Test mapping of the 'FAT' disk format to 'vvfat' in qemu.
The top level disk image would generate the following '-drive' cmdline:
dir-fat-readonly.xml: -drive file=fat:/var/somefiles,if=none,id=drive-dummy,readonly=on -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy
May as well show dir-fat-floppy.xml too
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemublocktest.c | 2 ++ tests/qemublocktestdata/xml2json/dir-fat-floppy.json | 14 ++++++++++++++ tests/qemublocktestdata/xml2json/dir-fat-floppy.xml | 14 ++++++++++++++ tests/qemublocktestdata/xml2json/dir-fat-readonly.json | 14 ++++++++++++++ tests/qemublocktestdata/xml2json/dir-fat-readonly.xml | 13 +++++++++++++ 5 files changed, 57 insertions(+) create mode 100644 tests/qemublocktestdata/xml2json/dir-fat-floppy.json create mode 100644 tests/qemublocktestdata/xml2json/dir-fat-floppy.xml create mode 100644 tests/qemublocktestdata/xml2json/dir-fat-readonly.json create mode 100644 tests/qemublocktestdata/xml2json/dir-fat-readonly.xml
Reviewed-by: John Ferlan <jferlan@redhat.com> John

Test that the 'aio' option is applied correctly for the 'file' protocol backend and across the backing chain. The top level disk image would generate the following '-drive' cmdline: file-backing_basic-aio_threads: -drive file=/var/lib/libvirt/images/a,format=qcow,if=none,id=drive-dummy,aio=threads -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy file-raw-aio_native: -drive file=/path/to/i.img,format=raw,if=none,id=drive-dummy,cache=none,aio=native -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy,write-cache=on Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemublocktest.c | 2 + .../xml2json/file-backing_basic-aio_threads.json | 62 ++++++++++++++++++++++ .../xml2json/file-backing_basic-aio_threads.xml | 47 ++++++++++++++++ .../xml2json/file-raw-aio_native.json | 21 ++++++++ .../xml2json/file-raw-aio_native.xml | 12 +++++ 5 files changed, 144 insertions(+) create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-aio_threads.json create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-aio_threads.xml create mode 100644 tests/qemublocktestdata/xml2json/file-raw-aio_native.json create mode 100644 tests/qemublocktestdata/xml2json/file-raw-aio_native.xml diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index 81a2261dab..a79ffa90d8 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -416,6 +416,8 @@ mymain(void) TEST_DISK_TO_JSON("file-backing_basic-noopts"); TEST_DISK_TO_JSON("dir-fat-readonly"); TEST_DISK_TO_JSON("dir-fat-floppy"); + TEST_DISK_TO_JSON("file-raw-aio_native"); + TEST_DISK_TO_JSON("file-backing_basic-aio_threads"); cleanup: virHashFree(diskxmljsondata.schema); diff --git a/tests/qemublocktestdata/xml2json/file-backing_basic-aio_threads.json b/tests/qemublocktestdata/xml2json/file-backing_basic-aio_threads.json new file mode 100644 index 0000000000..dcaf905085 --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-backing_basic-aio_threads.json @@ -0,0 +1,62 @@ +{ + "node-name": "node-a-f", + "read-only": false, + "driver": "qcow", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/a", + "aio": "threads", + "node-name": "node-a-s", + "read-only": false, + "discard": "unmap" + }, + "backing": "node-b-f" +} +{ + "node-name": "node-b-f", + "read-only": true, + "driver": "qed", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/b", + "aio": "threads", + "node-name": "node-b-s", + "read-only": true, + "discard": "unmap" + }, + "backing": "node-c-f" +} +{ + "node-name": "node-c-f", + "read-only": true, + "driver": "vmdk", + "file": { + "driver": "gluster", + "volume": "images", + "path": "c", + "server": [ + { + "type": "inet", + "host": "test.org", + "port": "24007" + } + ], + "node-name": "node-c-s", + "read-only": true, + "discard": "unmap" + }, + "backing": "node-d-f" +} +{ + "node-name": "node-d-f", + "read-only": true, + "driver": "raw", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/d", + "aio": "threads", + "node-name": "node-d-s", + "read-only": true, + "discard": "unmap" + } +} diff --git a/tests/qemublocktestdata/xml2json/file-backing_basic-aio_threads.xml b/tests/qemublocktestdata/xml2json/file-backing_basic-aio_threads.xml new file mode 100644 index 0000000000..ad84fab720 --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-backing_basic-aio_threads.xml @@ -0,0 +1,47 @@ +<disk type='file' device='disk'> + <driver name='qemu' type='qcow' io='threads'/> + <source file='/var/lib/libvirt/images/a'> + <privateData> + <nodenames> + <nodename type='storage' name='node-a-s'/> + <nodename type='format' name='node-a-f'/> + </nodenames> + </privateData> + </source> + <backingStore type='file' index='1'> + <format type='qed'/> + <source file='/var/lib/libvirt/images/b'> + <privateData> + <nodenames> + <nodename type='storage' name='node-b-s'/> + <nodename type='format' name='node-b-f'/> + </nodenames> + </privateData> + </source> + <backingStore type='network' index='2'> + <format type='vmdk'/> + <source protocol='gluster' name='images/c'> + <host name='test.org'/> + <privateData> + <nodenames> + <nodename type='storage' name='node-c-s'/> + <nodename type='format' name='node-c-f'/> + </nodenames> + </privateData> + </source> + <backingStore type='file' index='3'> + <format type='raw'/> + <source file='/var/lib/libvirt/images/d'> + <privateData> + <nodenames> + <nodename type='storage' name='node-d-s'/> + <nodename type='format' name='node-d-f'/> + </nodenames> + </privateData> + </source> + <backingStore/> + </backingStore> + </backingStore> + </backingStore> + <target dev='vda'/> +</disk> diff --git a/tests/qemublocktestdata/xml2json/file-raw-aio_native.json b/tests/qemublocktestdata/xml2json/file-raw-aio_native.json new file mode 100644 index 0000000000..2752e0b204 --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-raw-aio_native.json @@ -0,0 +1,21 @@ +{ + "node-name": "test1", + "read-only": false, + "cache": { + "direct": true, + "no-flush": false + }, + "driver": "raw", + "file": { + "driver": "file", + "filename": "/path/to/i.img", + "aio": "native", + "node-name": "test2", + "cache": { + "direct": true, + "no-flush": false + }, + "read-only": false, + "discard": "unmap" + } +} diff --git a/tests/qemublocktestdata/xml2json/file-raw-aio_native.xml b/tests/qemublocktestdata/xml2json/file-raw-aio_native.xml new file mode 100644 index 0000000000..470b60f26b --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-raw-aio_native.xml @@ -0,0 +1,12 @@ +<disk device='disk'> + <driver name='qemu' type='raw' io='native' cache='none'/> + <source file='/path/to/i.img'> + <privateData> + <nodenames> + <nodename type='storage' name='test2'/> + <nodename type='format' name='test1'/> + </nodenames> + </privateData> + </source> + <target dev='vda'/> +</disk> -- 2.16.2

On 04/25/2018 11:15 AM, Peter Krempa wrote:
Test that the 'aio' option is applied correctly for the 'file' protocol backend and across the backing chain.
The top level disk image would generate the following '-drive' cmdline:
file-backing_basic-aio_threads: -drive file=/var/lib/libvirt/images/a,format=qcow,if=none,id=drive-dummy,aio=threads -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy
file-raw-aio_native: -drive file=/path/to/i.img,format=raw,if=none,id=drive-dummy,cache=none,aio=native -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy,write-cache=on
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemublocktest.c | 2 + .../xml2json/file-backing_basic-aio_threads.json | 62 ++++++++++++++++++++++ .../xml2json/file-backing_basic-aio_threads.xml | 47 ++++++++++++++++ .../xml2json/file-raw-aio_native.json | 21 ++++++++ .../xml2json/file-raw-aio_native.xml | 12 +++++ 5 files changed, 144 insertions(+) create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-aio_threads.json create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-aio_threads.xml create mode 100644 tests/qemublocktestdata/xml2json/file-raw-aio_native.json create mode 100644 tests/qemublocktestdata/xml2json/file-raw-aio_native.xml
Taking the leap of faith that the results are "expected" :-)... At least I know gluster shouldn't be getting the aio based on code! Reviewed-by: John Ferlan <jferlan@redhat.com> John
diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index 81a2261dab..a79ffa90d8 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -416,6 +416,8 @@ mymain(void) TEST_DISK_TO_JSON("file-backing_basic-noopts"); TEST_DISK_TO_JSON("dir-fat-readonly"); TEST_DISK_TO_JSON("dir-fat-floppy"); + TEST_DISK_TO_JSON("file-raw-aio_native"); + TEST_DISK_TO_JSON("file-backing_basic-aio_threads");
cleanup: virHashFree(diskxmljsondata.schema); diff --git a/tests/qemublocktestdata/xml2json/file-backing_basic-aio_threads.json b/tests/qemublocktestdata/xml2json/file-backing_basic-aio_threads.json new file mode 100644 index 0000000000..dcaf905085 --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-backing_basic-aio_threads.json @@ -0,0 +1,62 @@ +{ + "node-name": "node-a-f", + "read-only": false, + "driver": "qcow", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/a", + "aio": "threads", + "node-name": "node-a-s", + "read-only": false, + "discard": "unmap" + }, + "backing": "node-b-f" +} +{ + "node-name": "node-b-f", + "read-only": true, + "driver": "qed", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/b", + "aio": "threads", + "node-name": "node-b-s", + "read-only": true, + "discard": "unmap" + }, + "backing": "node-c-f" +} +{ + "node-name": "node-c-f", + "read-only": true, + "driver": "vmdk", + "file": { + "driver": "gluster", + "volume": "images", + "path": "c", + "server": [ + { + "type": "inet", + "host": "test.org", + "port": "24007" + } + ], + "node-name": "node-c-s", + "read-only": true, + "discard": "unmap" + }, + "backing": "node-d-f" +} +{ + "node-name": "node-d-f", + "read-only": true, + "driver": "raw", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/d", + "aio": "threads", + "node-name": "node-d-s", + "read-only": true, + "discard": "unmap" + } +} diff --git a/tests/qemublocktestdata/xml2json/file-backing_basic-aio_threads.xml b/tests/qemublocktestdata/xml2json/file-backing_basic-aio_threads.xml new file mode 100644 index 0000000000..ad84fab720 --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-backing_basic-aio_threads.xml @@ -0,0 +1,47 @@ +<disk type='file' device='disk'> + <driver name='qemu' type='qcow' io='threads'/> + <source file='/var/lib/libvirt/images/a'> + <privateData> + <nodenames> + <nodename type='storage' name='node-a-s'/> + <nodename type='format' name='node-a-f'/> + </nodenames> + </privateData> + </source> + <backingStore type='file' index='1'> + <format type='qed'/> + <source file='/var/lib/libvirt/images/b'> + <privateData> + <nodenames> + <nodename type='storage' name='node-b-s'/> + <nodename type='format' name='node-b-f'/> + </nodenames> + </privateData> + </source> + <backingStore type='network' index='2'> + <format type='vmdk'/> + <source protocol='gluster' name='images/c'> + <host name='test.org'/> + <privateData> + <nodenames> + <nodename type='storage' name='node-c-s'/> + <nodename type='format' name='node-c-f'/> + </nodenames> + </privateData> + </source> + <backingStore type='file' index='3'> + <format type='raw'/> + <source file='/var/lib/libvirt/images/d'> + <privateData> + <nodenames> + <nodename type='storage' name='node-d-s'/> + <nodename type='format' name='node-d-f'/> + </nodenames> + </privateData> + </source> + <backingStore/> + </backingStore> + </backingStore> + </backingStore> + <target dev='vda'/> +</disk> diff --git a/tests/qemublocktestdata/xml2json/file-raw-aio_native.json b/tests/qemublocktestdata/xml2json/file-raw-aio_native.json new file mode 100644 index 0000000000..2752e0b204 --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-raw-aio_native.json @@ -0,0 +1,21 @@ +{ + "node-name": "test1", + "read-only": false, + "cache": { + "direct": true, + "no-flush": false + }, + "driver": "raw", + "file": { + "driver": "file", + "filename": "/path/to/i.img", + "aio": "native", + "node-name": "test2", + "cache": { + "direct": true, + "no-flush": false + }, + "read-only": false, + "discard": "unmap" + } +} diff --git a/tests/qemublocktestdata/xml2json/file-raw-aio_native.xml b/tests/qemublocktestdata/xml2json/file-raw-aio_native.xml new file mode 100644 index 0000000000..470b60f26b --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-raw-aio_native.xml @@ -0,0 +1,12 @@ +<disk device='disk'> + <driver name='qemu' type='raw' io='native' cache='none'/> + <source file='/path/to/i.img'> + <privateData> + <nodenames> + <nodename type='storage' name='test2'/> + <nodename type='format' name='test1'/> + </nodenames> + </privateData> + </source> + <target dev='vda'/> +</disk>

Apart from adding test data add a function which sets up fake secrets for the test. The top level disk image would generate the following '-drive' cmdline: -drive file=/path/luks.img,key-secret=test1-encalias,format=luks,if=none,id=drive-dummy -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemublocktest.c | 42 ++++++++++++++++++++++ .../qemublocktestdata/xml2json/file-raw-luks.json | 13 +++++++ tests/qemublocktestdata/xml2json/file-raw-luks.xml | 15 ++++++++ 3 files changed, 70 insertions(+) create mode 100644 tests/qemublocktestdata/xml2json/file-raw-luks.json create mode 100644 tests/qemublocktestdata/xml2json/file-raw-luks.xml diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index a79ffa90d8..55d028e9fb 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -145,6 +145,44 @@ testQemuDiskXMLToPropsClear(struct testQemuDiskXMLToJSONData *data) } +static int +testQemuDiskXMLToJSONFakeSecrets(virStorageSourcePtr src) +{ + qemuDomainStorageSourcePrivatePtr srcpriv; + + if (!src->privateData && + !(src->privateData = qemuDomainStorageSourcePrivateNew())) + return -1; + + srcpriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src); + + if (src->auth) { + if (VIR_ALLOC(srcpriv->secinfo) < 0) + return -1; + + srcpriv->secinfo->type = VIR_DOMAIN_SECRET_INFO_TYPE_AES; + if (VIR_STRDUP(srcpriv->secinfo->s.aes.username, src->auth->username) < 0) + return -1; + + if (virAsprintf(&srcpriv->secinfo->s.aes.alias, "%s-secalias", + NULLSTR(src->nodestorage)) < 0) + return -1; + } + + if (src->encryption) { + if (VIR_ALLOC(srcpriv->encinfo) < 0) + return -1; + + srcpriv->encinfo->type = VIR_DOMAIN_SECRET_INFO_TYPE_AES; + if (virAsprintf(&srcpriv->encinfo->s.aes.alias, "%s-encalias", + NULLSTR(src->nodeformat)) < 0) + return -1; + } + + return 0; +} + + static const char *testQemuDiskXMLToJSONPath = abs_srcdir "/qemublocktestdata/xml2json/"; static int @@ -180,6 +218,9 @@ testQemuDiskXMLToProps(const void *opaque) goto cleanup; for (n = disk->src; virStorageSourceIsBacking(n); n = n->backingStore) { + if (testQemuDiskXMLToJSONFakeSecrets(n) < 0) + goto cleanup; + if (!(props = qemuBlockStorageSourceGetBlockdevProps(n))) { if (!data->fail) { VIR_TEST_VERBOSE("failed to generate qemu blockdev props\n"); @@ -418,6 +459,7 @@ mymain(void) TEST_DISK_TO_JSON("dir-fat-floppy"); TEST_DISK_TO_JSON("file-raw-aio_native"); TEST_DISK_TO_JSON("file-backing_basic-aio_threads"); + TEST_DISK_TO_JSON("file-raw-luks"); cleanup: virHashFree(diskxmljsondata.schema); diff --git a/tests/qemublocktestdata/xml2json/file-raw-luks.json b/tests/qemublocktestdata/xml2json/file-raw-luks.json new file mode 100644 index 0000000000..e3d9c4c26b --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-raw-luks.json @@ -0,0 +1,13 @@ +{ + "node-name": "test1", + "read-only": false, + "driver": "luks", + "key-secret": "test1-encalias", + "file": { + "driver": "file", + "filename": "/path/luks.img", + "node-name": "test2", + "read-only": false, + "discard": "unmap" + } +} diff --git a/tests/qemublocktestdata/xml2json/file-raw-luks.xml b/tests/qemublocktestdata/xml2json/file-raw-luks.xml new file mode 100644 index 0000000000..f446b03bdb --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-raw-luks.xml @@ -0,0 +1,15 @@ +<disk device='disk'> + <driver name='qemu' type='raw'/> + <source file='/path/luks.img'> + <encryption format='luks'> + <secret type='passphrase' uuid='0a81f5b2-8403-7b23-c8d6-21ccc2f80d6f'/> + </encryption> + <privateData> + <nodenames> + <nodename type='storage' name='test2'/> + <nodename type='format' name='test1'/> + </nodenames> + </privateData> + </source> + <target dev='vda'/> +</disk> -- 2.16.2

On 04/25/2018 11:15 AM, Peter Krempa wrote:
Apart from adding test data add a function which sets up fake secrets for the test.
The top level disk image would generate the following '-drive' cmdline:
-drive file=/path/luks.img,key-secret=test1-encalias,format=luks,if=none,id=drive-dummy -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemublocktest.c | 42 ++++++++++++++++++++++ .../qemublocktestdata/xml2json/file-raw-luks.json | 13 +++++++ tests/qemublocktestdata/xml2json/file-raw-luks.xml | 15 ++++++++ 3 files changed, 70 insertions(+) create mode 100644 tests/qemublocktestdata/xml2json/file-raw-luks.json create mode 100644 tests/qemublocktestdata/xml2json/file-raw-luks.xml
Later on auth gets tested, so that's fine. Unless you want to make a "simple" auth test to partner with this simple luks encryption one. Reviewed-by: John Ferlan <jferlan@redhat.com> John

On Wed, May 02, 2018 at 19:07:27 -0400, John Ferlan wrote:
On 04/25/2018 11:15 AM, Peter Krempa wrote:
Apart from adding test data add a function which sets up fake secrets for the test.
The top level disk image would generate the following '-drive' cmdline:
-drive file=/path/luks.img,key-secret=test1-encalias,format=luks,if=none,id=drive-dummy -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemublocktest.c | 42 ++++++++++++++++++++++ .../qemublocktestdata/xml2json/file-raw-luks.json | 13 +++++++ tests/qemublocktestdata/xml2json/file-raw-luks.xml | 15 ++++++++ 3 files changed, 70 insertions(+) create mode 100644 tests/qemublocktestdata/xml2json/file-raw-luks.json create mode 100644 tests/qemublocktestdata/xml2json/file-raw-luks.xml
Later on auth gets tested, so that's fine. Unless you want to make a "simple" auth test to partner with this simple luks encryption one.
My motivation here was to add a simple case along with the code faking the authentication/encryption data, so that it can be seen and then add the cases separately. Also this is an artifact of approach I took while implementing it. I originally added this case to test the implementation of encryption I've done. Later when I implemented everything I've added the full test.
Reviewed-by: John Ferlan <jferlan@redhat.com>
John

Add tests for backing chain handling, including a very long chain which is fully specified in the XML and an unterminated chain. The top level disk image would generate the following '-drive': file-qcow2-backing-chain-encryption.xml: -drive file=/var/lib/libvirt/images/a,encrypt.format=luks, encrypt.key-secret=node-b-f-encalias,format=qcow2,if=none,id=drive-dummy -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy file-qcow2-backing-chain-noopts.xml: -drive file=/var/lib/libvirt/images/rhel7.3.1507297895,format=qcow2,if=none,id=drive-dummy -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy file-qcow2-backing-chain-unterminated.xml: -drive file=/var/lib/libvirt/images/rhel7.3.1507297895,format=qcow2,if=none,id=drive-dummy -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemublocktest.c | 3 + .../file-qcow2-backing-chain-encryption.json | 34 ++++++ .../file-qcow2-backing-chain-encryption.xml | 31 +++++ .../xml2json/file-qcow2-backing-chain-noopts.json | 130 +++++++++++++++++++++ .../xml2json/file-qcow2-backing-chain-noopts.xml | 113 ++++++++++++++++++ .../file-qcow2-backing-chain-unterminated.json | 25 ++++ .../file-qcow2-backing-chain-unterminated.xml | 24 ++++ 7 files changed, 360 insertions(+) create mode 100644 tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-encryption.json create mode 100644 tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-encryption.xml create mode 100644 tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-noopts.json create mode 100644 tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-noopts.xml create mode 100644 tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-unterminated.json create mode 100644 tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-unterminated.xml diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index 55d028e9fb..44d76bebbb 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -460,6 +460,9 @@ mymain(void) TEST_DISK_TO_JSON("file-raw-aio_native"); TEST_DISK_TO_JSON("file-backing_basic-aio_threads"); TEST_DISK_TO_JSON("file-raw-luks"); + TEST_DISK_TO_JSON("file-qcow2-backing-chain-noopts"); + TEST_DISK_TO_JSON("file-qcow2-backing-chain-unterminated"); + TEST_DISK_TO_JSON("file-qcow2-backing-chain-encryption"); cleanup: virHashFree(diskxmljsondata.schema); diff --git a/tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-encryption.json b/tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-encryption.json new file mode 100644 index 0000000000..94e2ecd1e2 --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-encryption.json @@ -0,0 +1,34 @@ +{ + "node-name": "node-b-f", + "read-only": false, + "driver": "qcow2", + "encrypt": { + "format": "luks", + "key-secret": "node-b-f-encalias" + }, + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/a", + "node-name": "node-a-s", + "read-only": false, + "discard": "unmap" + }, + "backing": "node-b-f" +} +{ + "node-name": "node-b-f", + "read-only": true, + "driver": "qcow2", + "encrypt": { + "format": "aes", + "key-secret": "node-b-f-encalias" + }, + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/b", + "node-name": "node-b-s", + "read-only": true, + "discard": "unmap" + }, + "backing": null +} diff --git a/tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-encryption.xml b/tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-encryption.xml new file mode 100644 index 0000000000..a1292284bf --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-encryption.xml @@ -0,0 +1,31 @@ +<disk type='file' device='disk'> + <driver name='qemu' type='qcow2'/> + <source file='/var/lib/libvirt/images/a'> + <encryption format='luks'> + <secret type='passphrase' uuid='0a81f5b2-8403-7b23-c8d6-21ccc2f80d6f'/> + </encryption> + <privateData> + <nodenames> + <nodename type='storage' name='node-a-s'/> + <nodename type='format' name='node-b-f'/> + </nodenames> + </privateData> + </source> + <backingStore type='file' index='1'> + <format type='qcow2'/> + <source file='/var/lib/libvirt/images/b'> + <privateData> + <nodenames> + <nodename type='storage' name='node-b-s'/> + <nodename type='format' name='node-b-f'/> + </nodenames> + </privateData> + <encryption format='qcow'> + <secret type='passphrase' uuid='0a81f5b2-8403-7b23-c8d6-21ccc2f80d6f'/> + </encryption> + </source> + <backingStore/> + </backingStore> + <target dev='vda' bus='virtio'/> + <alias name='virtio-disk0'/> +</disk> diff --git a/tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-noopts.json b/tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-noopts.json new file mode 100644 index 0000000000..3e7c08f080 --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-noopts.json @@ -0,0 +1,130 @@ +{ + "node-name": "#block126", + "read-only": false, + "driver": "qcow2", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/rhel7.3.1507297895", + "node-name": "#block004", + "read-only": false, + "discard": "unmap" + }, + "backing": "#block313" +} +{ + "node-name": "#block313", + "read-only": true, + "driver": "qcow2", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/rhel7.3.1484071872", + "node-name": "#block256", + "read-only": true, + "discard": "unmap" + }, + "backing": "#block556" +} +{ + "node-name": "#block556", + "read-only": true, + "driver": "qcow2", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/rhel7.3.1483615252", + "node-name": "#block418", + "read-only": true, + "discard": "unmap" + }, + "backing": "#block767" +} +{ + "node-name": "#block767", + "read-only": true, + "driver": "qcow2", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/rhel7.3.1483605924", + "node-name": "#block624", + "read-only": true, + "discard": "unmap" + }, + "backing": "#block937" +} +{ + "node-name": "#block937", + "read-only": true, + "driver": "qcow2", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/rhel7.3.1483605920", + "node-name": "#block869", + "read-only": true, + "discard": "unmap" + }, + "backing": "#block1157" +} +{ + "node-name": "#block1157", + "read-only": true, + "driver": "qcow2", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/rhel7.3.1483546244", + "node-name": "#block1047", + "read-only": true, + "discard": "unmap" + }, + "backing": "#block1392" +} +{ + "node-name": "#block1392", + "read-only": true, + "driver": "qcow2", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/rhel7.3.1483545901", + "node-name": "#block1279", + "read-only": true, + "discard": "unmap" + }, + "backing": "#block1523" +} +{ + "node-name": "#block1523", + "read-only": true, + "driver": "qcow2", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/rhel7.3.1483545313", + "node-name": "#block1444", + "read-only": true, + "discard": "unmap" + }, + "backing": "#block1742" +} +{ + "node-name": "#block1742", + "read-only": true, + "driver": "qcow2", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/rhel7.3.1483536402", + "node-name": "#block1602", + "read-only": true, + "discard": "unmap" + }, + "backing": "#block1909" +} +{ + "node-name": "#block1909", + "read-only": true, + "driver": "qcow2", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/rhel7.3.qcow2", + "node-name": "#block1864", + "read-only": true, + "discard": "unmap" + }, + "backing": null +} diff --git a/tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-noopts.xml b/tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-noopts.xml new file mode 100644 index 0000000000..12db679f37 --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-noopts.xml @@ -0,0 +1,113 @@ +<disk type='file' device='disk'> + <driver name='qemu' type='qcow2'/> + <source file='/var/lib/libvirt/images/rhel7.3.1507297895'> + <privateData> + <nodenames> + <nodename type='storage' name='#block004'/> + <nodename type='format' name='#block126'/> + </nodenames> + </privateData> + </source> + <backingStore type='file' index='1'> + <format type='qcow2'/> + <source file='/var/lib/libvirt/images/rhel7.3.1484071872'> + <privateData> + <nodenames> + <nodename type='storage' name='#block256'/> + <nodename type='format' name='#block313'/> + </nodenames> + </privateData> + </source> + <backingStore type='file' index='2'> + <format type='qcow2'/> + <source file='/var/lib/libvirt/images/rhel7.3.1483615252'> + <privateData> + <nodenames> + <nodename type='storage' name='#block418'/> + <nodename type='format' name='#block556'/> + </nodenames> + </privateData> + </source> + <backingStore type='file' index='3'> + <format type='qcow2'/> + <source file='/var/lib/libvirt/images/rhel7.3.1483605924'> + <privateData> + <nodenames> + <nodename type='storage' name='#block624'/> + <nodename type='format' name='#block767'/> + </nodenames> + </privateData> + </source> + <backingStore type='file' index='4'> + <format type='qcow2'/> + <source file='/var/lib/libvirt/images/rhel7.3.1483605920'> + <privateData> + <nodenames> + <nodename type='storage' name='#block869'/> + <nodename type='format' name='#block937'/> + </nodenames> + </privateData> + </source> + <backingStore type='file' index='5'> + <format type='qcow2'/> + <source file='/var/lib/libvirt/images/rhel7.3.1483546244'> + <privateData> + <nodenames> + <nodename type='storage' name='#block1047'/> + <nodename type='format' name='#block1157'/> + </nodenames> + </privateData> + </source> + <backingStore type='file' index='6'> + <format type='qcow2'/> + <source file='/var/lib/libvirt/images/rhel7.3.1483545901'> + <privateData> + <nodenames> + <nodename type='storage' name='#block1279'/> + <nodename type='format' name='#block1392'/> + </nodenames> + </privateData> + </source> + <backingStore type='file' index='7'> + <format type='qcow2'/> + <source file='/var/lib/libvirt/images/rhel7.3.1483545313'> + <privateData> + <nodenames> + <nodename type='storage' name='#block1444'/> + <nodename type='format' name='#block1523'/> + </nodenames> + </privateData> + </source> + <backingStore type='file' index='8'> + <format type='qcow2'/> + <source file='/var/lib/libvirt/images/rhel7.3.1483536402'> + <privateData> + <nodenames> + <nodename type='storage' name='#block1602'/> + <nodename type='format' name='#block1742'/> + </nodenames> + </privateData> + </source> + <backingStore type='file' index='9'> + <format type='qcow2'/> + <source file='/var/lib/libvirt/images/rhel7.3.qcow2'> + <privateData> + <nodenames> + <nodename type='storage' name='#block1864'/> + <nodename type='format' name='#block1909'/> + </nodenames> + </privateData> + </source> + <backingStore/> + </backingStore> + </backingStore> + </backingStore> + </backingStore> + </backingStore> + </backingStore> + </backingStore> + </backingStore> + </backingStore> + <target dev='vda' bus='virtio'/> + <alias name='virtio-disk0'/> +</disk> diff --git a/tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-unterminated.json b/tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-unterminated.json new file mode 100644 index 0000000000..8fcdc48bb0 --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-unterminated.json @@ -0,0 +1,25 @@ +{ + "node-name": "#block126", + "read-only": false, + "driver": "qcow2", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/rhel7.3.1507297895", + "node-name": "#block004", + "read-only": false, + "discard": "unmap" + }, + "backing": "#block313" +} +{ + "node-name": "#block313", + "read-only": true, + "driver": "qcow2", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/rhel7.3.1484071872", + "node-name": "#block256", + "read-only": true, + "discard": "unmap" + } +} diff --git a/tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-unterminated.xml b/tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-unterminated.xml new file mode 100644 index 0000000000..7520cfe86a --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-unterminated.xml @@ -0,0 +1,24 @@ +<disk type='file' device='disk'> + <driver name='qemu' type='qcow2'/> + <source file='/var/lib/libvirt/images/rhel7.3.1507297895'> + <privateData> + <nodenames> + <nodename type='storage' name='#block004'/> + <nodename type='format' name='#block126'/> + </nodenames> + </privateData> + </source> + <backingStore type='file' index='1'> + <format type='qcow2'/> + <source file='/var/lib/libvirt/images/rhel7.3.1484071872'> + <privateData> + <nodenames> + <nodename type='storage' name='#block256'/> + <nodename type='format' name='#block313'/> + </nodenames> + </privateData> + </source> + </backingStore> + <target dev='vda' bus='virtio'/> + <alias name='virtio-disk0'/> +</disk> -- 2.16.2

On 04/25/2018 11:15 AM, Peter Krempa wrote:
Add tests for backing chain handling, including a very long chain which is fully specified in the XML and an unterminated chain.
The top level disk image would generate the following '-drive':
file-qcow2-backing-chain-encryption.xml: -drive file=/var/lib/libvirt/images/a,encrypt.format=luks, encrypt.key-secret=node-b-f-encalias,format=qcow2,if=none,id=drive-dummy -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy
file-qcow2-backing-chain-noopts.xml: -drive file=/var/lib/libvirt/images/rhel7.3.1507297895,format=qcow2,if=none,id=drive-dummy -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy
file-qcow2-backing-chain-unterminated.xml: -drive file=/var/lib/libvirt/images/rhel7.3.1507297895,format=qcow2,if=none,id=drive-dummy -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemublocktest.c | 3 + .../file-qcow2-backing-chain-encryption.json | 34 ++++++ .../file-qcow2-backing-chain-encryption.xml | 31 +++++ .../xml2json/file-qcow2-backing-chain-noopts.json | 130 +++++++++++++++++++++ .../xml2json/file-qcow2-backing-chain-noopts.xml | 113 ++++++++++++++++++ .../file-qcow2-backing-chain-unterminated.json | 25 ++++ .../file-qcow2-backing-chain-unterminated.xml | 24 ++++ 7 files changed, 360 insertions(+) create mode 100644 tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-encryption.json create mode 100644 tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-encryption.xml create mode 100644 tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-noopts.json create mode 100644 tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-noopts.xml create mode 100644 tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-unterminated.json create mode 100644 tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-unterminated.xml
hahahaha... basic... hahahaha and perhaps a more real-life example from -noopts output "#blockNNN"... Too bad one of those rhel7.3 paths didn't have 3.14159265359 ... Reviewed-by: John Ferlan <jferlan@redhat.com> John

On Wed, May 02, 2018 at 19:21:39 -0400, John Ferlan wrote:
On 04/25/2018 11:15 AM, Peter Krempa wrote:
Add tests for backing chain handling, including a very long chain which is fully specified in the XML and an unterminated chain.
The top level disk image would generate the following '-drive':
file-qcow2-backing-chain-encryption.xml: -drive file=/var/lib/libvirt/images/a,encrypt.format=luks, encrypt.key-secret=node-b-f-encalias,format=qcow2,if=none,id=drive-dummy -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy
file-qcow2-backing-chain-noopts.xml: -drive file=/var/lib/libvirt/images/rhel7.3.1507297895,format=qcow2,if=none,id=drive-dummy -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy
file-qcow2-backing-chain-unterminated.xml: -drive file=/var/lib/libvirt/images/rhel7.3.1507297895,format=qcow2,if=none,id=drive-dummy -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemublocktest.c | 3 + .../file-qcow2-backing-chain-encryption.json | 34 ++++++ .../file-qcow2-backing-chain-encryption.xml | 31 +++++ .../xml2json/file-qcow2-backing-chain-noopts.json | 130 +++++++++++++++++++++ .../xml2json/file-qcow2-backing-chain-noopts.xml | 113 ++++++++++++++++++ .../file-qcow2-backing-chain-unterminated.json | 25 ++++ .../file-qcow2-backing-chain-unterminated.xml | 24 ++++ 7 files changed, 360 insertions(+) create mode 100644 tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-encryption.json create mode 100644 tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-encryption.xml create mode 100644 tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-noopts.json create mode 100644 tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-noopts.xml create mode 100644 tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-unterminated.json create mode 100644 tests/qemublocktestdata/xml2json/file-qcow2-backing-chain-unterminated.xml
hahahaha... basic... hahahaha
Oh, right. That was the patch subject prior to adding more of the tests :)
and perhaps a more real-life example from -noopts output "#blockNNN"...
Oh, yeah, the node names are super fake (or in the case of the -noopts test harvested from a real qemu run and then dumped from the status XML) in all of the tests. The libvirt generated node names will be different, but for the sake of testing this should be enough.
Too bad one of those rhel7.3 paths didn't have 3.14159265359 ...
Well, the number after dot is the timestamp when the snapshot was taken. Your example is still possible: $ date --date='@14159265359' Sun Sep 9 10:15:59 CET 2418 If you drop the 9 at the end to get to the correct magnitude you'll get: $ date --date='@1415926535' Fri Nov 14 01:55:35 CET 2014 sooo, in 2014, there was no rhel 7.3, and I hope that no sane person will use rhel 7.3 in year 2418. Maybe rhel 107.3 will be possible at that time though ;)

iscsi and rbd support authentication of the connection. Combine it with encryption of qcow2. The top level disk image would generate the following '-drive' cmdline: -drive file=rbd:rbdpool/rbdimg:id=testuser-rbd:auth_supported=cephx\;none: mon_host=host1.example.com\;host2.example.com, file.password-secret=node-a-s-secalias,encrypt.format=luks, encrypt.key-secret=node-b-f-encalias,format=qcow2, if=none,id=drive-dummy -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemublocktest.c | 1 + ...etwork-qcow2-backing-chain-encryption_auth.json | 51 ++++++++++++++++++++++ ...network-qcow2-backing-chain-encryption_auth.xml | 40 +++++++++++++++++ 3 files changed, 92 insertions(+) create mode 100644 tests/qemublocktestdata/xml2json/network-qcow2-backing-chain-encryption_auth.json create mode 100644 tests/qemublocktestdata/xml2json/network-qcow2-backing-chain-encryption_auth.xml diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index 44d76bebbb..34509be543 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -463,6 +463,7 @@ mymain(void) TEST_DISK_TO_JSON("file-qcow2-backing-chain-noopts"); TEST_DISK_TO_JSON("file-qcow2-backing-chain-unterminated"); TEST_DISK_TO_JSON("file-qcow2-backing-chain-encryption"); + TEST_DISK_TO_JSON("network-qcow2-backing-chain-encryption_auth"); cleanup: virHashFree(diskxmljsondata.schema); diff --git a/tests/qemublocktestdata/xml2json/network-qcow2-backing-chain-encryption_auth.json b/tests/qemublocktestdata/xml2json/network-qcow2-backing-chain-encryption_auth.json new file mode 100644 index 0000000000..f307ba8805 --- /dev/null +++ b/tests/qemublocktestdata/xml2json/network-qcow2-backing-chain-encryption_auth.json @@ -0,0 +1,51 @@ +{ + "node-name": "node-b-f", + "read-only": false, + "driver": "qcow2", + "encrypt": { + "format": "luks", + "key-secret": "node-b-f-encalias" + }, + "file": { + "driver": "rbd", + "pool": "rbdpool", + "image": "rbdimg", + "server": [ + { + "host": "host1.example.com", + "port": "0" + }, + { + "host": "host2.example.com", + "port": "0" + } + ], + "user": "testuser-rbd", + "node-name": "node-a-s", + "read-only": false, + "discard": "unmap" + }, + "backing": "node-b-f" +} +{ + "node-name": "node-b-f", + "read-only": true, + "driver": "qcow2", + "encrypt": { + "format": "aes", + "key-secret": "node-b-f-encalias" + }, + "file": { + "driver": "iscsi", + "portal": "example.org:3260", + "target": "iscsitarget", + "lun": 1, + "transport": "tcp", + "user": "testuser-iscsi", + "password-secret": "node-b-s-secalias", + "node-name": "node-b-s", + "read-only": true, + "discard": "unmap" + }, + "backing": null +} diff --git a/tests/qemublocktestdata/xml2json/network-qcow2-backing-chain-encryption_auth.xml b/tests/qemublocktestdata/xml2json/network-qcow2-backing-chain-encryption_auth.xml new file mode 100644 index 0000000000..775886801b --- /dev/null +++ b/tests/qemublocktestdata/xml2json/network-qcow2-backing-chain-encryption_auth.xml @@ -0,0 +1,40 @@ +<disk type='network' device='disk'> + <driver name='qemu' type='qcow2'/> + <source protocol='rbd' name='rbdpool/rbdimg'> + <host name='host1.example.com'/> + <host name='host2.example.com'/> + <encryption format='luks'> + <secret type='passphrase' uuid='0a81f5b2-8403-7b23-c8d6-21ccc2f80d6f'/> + </encryption> + <auth username='testuser-rbd'> + <secret type='ceph' usage='testuser-rbd-secret'/> + </auth> + <privateData> + <nodenames> + <nodename type='storage' name='node-a-s'/> + <nodename type='format' name='node-b-f'/> + </nodenames> + </privateData> + </source> + <backingStore type='network' index='1'> + <format type='qcow2'/> + <source protocol='iscsi' name='iscsitarget/1'> + <host name='example.org'/> + <privateData> + <nodenames> + <nodename type='storage' name='node-b-s'/> + <nodename type='format' name='node-b-f'/> + </nodenames> + </privateData> + <encryption format='qcow'> + <secret type='passphrase' uuid='0a81f5b2-8403-7b23-c8d6-21ccc2f80d6f'/> + </encryption> + <auth username='testuser-iscsi'> + <secret type='iscsi' usage='testuser-iscsi-secret'/> + </auth> + </source> + <backingStore/> + </backingStore> + <target dev='vda' bus='virtio'/> + <alias name='virtio-disk0'/> +</disk> -- 2.16.2

On 04/25/2018 11:15 AM, Peter Krempa wrote:
iscsi and rbd support authentication of the connection. Combine it with encryption of qcow2.
The top level disk image would generate the following '-drive' cmdline:
-drive file=rbd:rbdpool/rbdimg:id=testuser-rbd:auth_supported=cephx\;none: mon_host=host1.example.com\;host2.example.com, file.password-secret=node-a-s-secalias,encrypt.format=luks, encrypt.key-secret=node-b-f-encalias,format=qcow2, if=none,id=drive-dummy -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemublocktest.c | 1 + ...etwork-qcow2-backing-chain-encryption_auth.json | 51 ++++++++++++++++++++++ ...network-qcow2-backing-chain-encryption_auth.xml | 40 +++++++++++++++++ 3 files changed, 92 insertions(+) create mode 100644 tests/qemublocktestdata/xml2json/network-qcow2-backing-chain-encryption_auth.json create mode 100644 tests/qemublocktestdata/xml2json/network-qcow2-backing-chain-encryption_auth.xml
The iSCSI target IQN listed here probably isn't valid, but no big deal. You could prefix with something like "iqn.2016-09.com.example:" - changes output a bit. Reviewed-by: John Ferlan <jferlan@redhat.com> John qcow encrypted iSCSI chained with a LUKS encrypted RBD... That's a trick!

The test cases would correspond to the following -drive command lines: file-backing_basic-detect.xml: -drive file=/var/lib/libvirt/images/a,format=qcow,if=none,id=drive-dummy,detect-zeroes=on -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy file-backing_basic-unmap-detect.xml: -drive file=/var/lib/libvirt/images/a,format=qcow,if=none,id=drive-dummy,discard=unmap,detect-zeroes=unmap -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy file-backing_basic-unmap-ignore.xml: -drive file=/var/lib/libvirt/images/a,format=qcow,if=none,id=drive-dummy,discard=ignore,detect-zeroes=on -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy file-backing_basic-unmap.xml: -drive file=/var/lib/libvirt/images/a,format=qcow,if=none,id=drive-dummy,discard=unmap -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy --- tests/qemublocktest.c | 5 ++ .../xml2json/file-backing_basic-detect.json | 60 ++++++++++++++++++++ .../xml2json/file-backing_basic-detect.xml | 47 ++++++++++++++++ .../xml2json/file-backing_basic-unmap-detect.json | 64 ++++++++++++++++++++++ .../xml2json/file-backing_basic-unmap-detect.xml | 47 ++++++++++++++++ .../xml2json/file-backing_basic-unmap-discard.json | 0 .../xml2json/file-backing_basic-unmap-ignore.json | 64 ++++++++++++++++++++++ .../xml2json/file-backing_basic-unmap-ignore.xml | 47 ++++++++++++++++ .../xml2json/file-backing_basic-unmap.json | 63 +++++++++++++++++++++ .../xml2json/file-backing_basic-unmap.xml | 47 ++++++++++++++++ 10 files changed, 444 insertions(+) create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-detect.json create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-detect.xml create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-unmap-detect.json create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-unmap-detect.xml create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-unmap-discard.json create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-unmap-ignore.json create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-unmap-ignore.xml create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-unmap.json create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-unmap.xml diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index 34509be543..4a2051da57 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -465,6 +465,11 @@ mymain(void) TEST_DISK_TO_JSON("file-qcow2-backing-chain-encryption"); TEST_DISK_TO_JSON("network-qcow2-backing-chain-encryption_auth"); + TEST_DISK_TO_JSON("file-backing_basic-unmap"); + TEST_DISK_TO_JSON("file-backing_basic-unmap-detect"); + TEST_DISK_TO_JSON("file-backing_basic-unmap-ignore"); + TEST_DISK_TO_JSON("file-backing_basic-detect"); + cleanup: virHashFree(diskxmljsondata.schema); qemuTestDriverFree(&driver); diff --git a/tests/qemublocktestdata/xml2json/file-backing_basic-detect.json b/tests/qemublocktestdata/xml2json/file-backing_basic-detect.json new file mode 100644 index 0000000000..c1e4c40757 --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-backing_basic-detect.json @@ -0,0 +1,60 @@ +{ + "node-name": "node-a-f", + "read-only": false, + "detect-zeroes": "on", + "driver": "qcow", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/a", + "node-name": "node-a-s", + "read-only": false, + "discard": "unmap" + }, + "backing": "node-b-f" +} +{ + "node-name": "node-b-f", + "read-only": true, + "driver": "qed", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/b", + "node-name": "node-b-s", + "read-only": true, + "discard": "unmap" + }, + "backing": "node-c-f" +} +{ + "node-name": "node-c-f", + "read-only": true, + "driver": "vmdk", + "file": { + "driver": "gluster", + "volume": "images", + "path": "c", + "server": [ + { + "type": "inet", + "host": "test.org", + "port": "24007" + } + ], + "node-name": "node-c-s", + "read-only": true, + "discard": "unmap" + }, + "backing": "node-d-f" +} +{ + "node-name": "node-d-f", + "read-only": true, + "driver": "raw", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/d", + "node-name": "node-d-s", + "read-only": true, + "discard": "unmap" + } +} diff --git a/tests/qemublocktestdata/xml2json/file-backing_basic-detect.xml b/tests/qemublocktestdata/xml2json/file-backing_basic-detect.xml new file mode 100644 index 0000000000..eaf89d316a --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-backing_basic-detect.xml @@ -0,0 +1,47 @@ +<disk type='file' device='disk'> + <driver name='qemu' type='qcow' detect_zeroes='unmap'/> + <source file='/var/lib/libvirt/images/a'> + <privateData> + <nodenames> + <nodename type='storage' name='node-a-s'/> + <nodename type='format' name='node-a-f'/> + </nodenames> + </privateData> + </source> + <backingStore type='file' index='1'> + <format type='qed'/> + <source file='/var/lib/libvirt/images/b'> + <privateData> + <nodenames> + <nodename type='storage' name='node-b-s'/> + <nodename type='format' name='node-b-f'/> + </nodenames> + </privateData> + </source> + <backingStore type='network' index='2'> + <format type='vmdk'/> + <source protocol='gluster' name='images/c'> + <host name='test.org'/> + <privateData> + <nodenames> + <nodename type='storage' name='node-c-s'/> + <nodename type='format' name='node-c-f'/> + </nodenames> + </privateData> + </source> + <backingStore type='file' index='3'> + <format type='raw'/> + <source file='/var/lib/libvirt/images/d'> + <privateData> + <nodenames> + <nodename type='storage' name='node-d-s'/> + <nodename type='format' name='node-d-f'/> + </nodenames> + </privateData> + </source> + <backingStore/> + </backingStore> + </backingStore> + </backingStore> + <target dev='vda'/> +</disk> diff --git a/tests/qemublocktestdata/xml2json/file-backing_basic-unmap-detect.json b/tests/qemublocktestdata/xml2json/file-backing_basic-unmap-detect.json new file mode 100644 index 0000000000..b00008dd54 --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-backing_basic-unmap-detect.json @@ -0,0 +1,64 @@ +{ + "node-name": "node-a-f", + "read-only": false, + "discard": "unmap", + "detect-zeroes": "unmap", + "driver": "qcow", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/a", + "node-name": "node-a-s", + "read-only": false, + "discard": "unmap" + }, + "backing": "node-b-f" +} +{ + "node-name": "node-b-f", + "read-only": true, + "discard": "unmap", + "driver": "qed", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/b", + "node-name": "node-b-s", + "read-only": true, + "discard": "unmap" + }, + "backing": "node-c-f" +} +{ + "node-name": "node-c-f", + "read-only": true, + "discard": "unmap", + "driver": "vmdk", + "file": { + "driver": "gluster", + "volume": "images", + "path": "c", + "server": [ + { + "type": "inet", + "host": "test.org", + "port": "24007" + } + ], + "node-name": "node-c-s", + "read-only": true, + "discard": "unmap" + }, + "backing": "node-d-f" +} +{ + "node-name": "node-d-f", + "read-only": true, + "discard": "unmap", + "driver": "raw", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/d", + "node-name": "node-d-s", + "read-only": true, + "discard": "unmap" + } +} diff --git a/tests/qemublocktestdata/xml2json/file-backing_basic-unmap-detect.xml b/tests/qemublocktestdata/xml2json/file-backing_basic-unmap-detect.xml new file mode 100644 index 0000000000..4638a78541 --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-backing_basic-unmap-detect.xml @@ -0,0 +1,47 @@ +<disk type='file' device='disk'> + <driver name='qemu' type='qcow' discard='unmap' detect_zeroes='unmap'/> + <source file='/var/lib/libvirt/images/a'> + <privateData> + <nodenames> + <nodename type='storage' name='node-a-s'/> + <nodename type='format' name='node-a-f'/> + </nodenames> + </privateData> + </source> + <backingStore type='file' index='1'> + <format type='qed'/> + <source file='/var/lib/libvirt/images/b'> + <privateData> + <nodenames> + <nodename type='storage' name='node-b-s'/> + <nodename type='format' name='node-b-f'/> + </nodenames> + </privateData> + </source> + <backingStore type='network' index='2'> + <format type='vmdk'/> + <source protocol='gluster' name='images/c'> + <host name='test.org'/> + <privateData> + <nodenames> + <nodename type='storage' name='node-c-s'/> + <nodename type='format' name='node-c-f'/> + </nodenames> + </privateData> + </source> + <backingStore type='file' index='3'> + <format type='raw'/> + <source file='/var/lib/libvirt/images/d'> + <privateData> + <nodenames> + <nodename type='storage' name='node-d-s'/> + <nodename type='format' name='node-d-f'/> + </nodenames> + </privateData> + </source> + <backingStore/> + </backingStore> + </backingStore> + </backingStore> + <target dev='vda'/> +</disk> diff --git a/tests/qemublocktestdata/xml2json/file-backing_basic-unmap-discard.json b/tests/qemublocktestdata/xml2json/file-backing_basic-unmap-discard.json new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/qemublocktestdata/xml2json/file-backing_basic-unmap-ignore.json b/tests/qemublocktestdata/xml2json/file-backing_basic-unmap-ignore.json new file mode 100644 index 0000000000..5419deafc0 --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-backing_basic-unmap-ignore.json @@ -0,0 +1,64 @@ +{ + "node-name": "node-a-f", + "read-only": false, + "discard": "ignore", + "detect-zeroes": "on", + "driver": "qcow", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/a", + "node-name": "node-a-s", + "read-only": false, + "discard": "unmap" + }, + "backing": "node-b-f" +} +{ + "node-name": "node-b-f", + "read-only": true, + "discard": "ignore", + "driver": "qed", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/b", + "node-name": "node-b-s", + "read-only": true, + "discard": "unmap" + }, + "backing": "node-c-f" +} +{ + "node-name": "node-c-f", + "read-only": true, + "discard": "ignore", + "driver": "vmdk", + "file": { + "driver": "gluster", + "volume": "images", + "path": "c", + "server": [ + { + "type": "inet", + "host": "test.org", + "port": "24007" + } + ], + "node-name": "node-c-s", + "read-only": true, + "discard": "unmap" + }, + "backing": "node-d-f" +} +{ + "node-name": "node-d-f", + "read-only": true, + "discard": "ignore", + "driver": "raw", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/d", + "node-name": "node-d-s", + "read-only": true, + "discard": "unmap" + } +} diff --git a/tests/qemublocktestdata/xml2json/file-backing_basic-unmap-ignore.xml b/tests/qemublocktestdata/xml2json/file-backing_basic-unmap-ignore.xml new file mode 100644 index 0000000000..b2d599fc6c --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-backing_basic-unmap-ignore.xml @@ -0,0 +1,47 @@ +<disk type='file' device='disk'> + <driver name='qemu' type='qcow' discard='ignore' detect_zeroes='on'/> + <source file='/var/lib/libvirt/images/a'> + <privateData> + <nodenames> + <nodename type='storage' name='node-a-s'/> + <nodename type='format' name='node-a-f'/> + </nodenames> + </privateData> + </source> + <backingStore type='file' index='1'> + <format type='qed'/> + <source file='/var/lib/libvirt/images/b'> + <privateData> + <nodenames> + <nodename type='storage' name='node-b-s'/> + <nodename type='format' name='node-b-f'/> + </nodenames> + </privateData> + </source> + <backingStore type='network' index='2'> + <format type='vmdk'/> + <source protocol='gluster' name='images/c'> + <host name='test.org'/> + <privateData> + <nodenames> + <nodename type='storage' name='node-c-s'/> + <nodename type='format' name='node-c-f'/> + </nodenames> + </privateData> + </source> + <backingStore type='file' index='3'> + <format type='raw'/> + <source file='/var/lib/libvirt/images/d'> + <privateData> + <nodenames> + <nodename type='storage' name='node-d-s'/> + <nodename type='format' name='node-d-f'/> + </nodenames> + </privateData> + </source> + <backingStore/> + </backingStore> + </backingStore> + </backingStore> + <target dev='vda'/> +</disk> diff --git a/tests/qemublocktestdata/xml2json/file-backing_basic-unmap.json b/tests/qemublocktestdata/xml2json/file-backing_basic-unmap.json new file mode 100644 index 0000000000..1efc462c06 --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-backing_basic-unmap.json @@ -0,0 +1,63 @@ +{ + "node-name": "node-a-f", + "read-only": false, + "discard": "unmap", + "driver": "qcow", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/a", + "node-name": "node-a-s", + "read-only": false, + "discard": "unmap" + }, + "backing": "node-b-f" +} +{ + "node-name": "node-b-f", + "read-only": true, + "discard": "unmap", + "driver": "qed", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/b", + "node-name": "node-b-s", + "read-only": true, + "discard": "unmap" + }, + "backing": "node-c-f" +} +{ + "node-name": "node-c-f", + "read-only": true, + "discard": "unmap", + "driver": "vmdk", + "file": { + "driver": "gluster", + "volume": "images", + "path": "c", + "server": [ + { + "type": "inet", + "host": "test.org", + "port": "24007" + } + ], + "node-name": "node-c-s", + "read-only": true, + "discard": "unmap" + }, + "backing": "node-d-f" +} +{ + "node-name": "node-d-f", + "read-only": true, + "discard": "unmap", + "driver": "raw", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/d", + "node-name": "node-d-s", + "read-only": true, + "discard": "unmap" + } +} diff --git a/tests/qemublocktestdata/xml2json/file-backing_basic-unmap.xml b/tests/qemublocktestdata/xml2json/file-backing_basic-unmap.xml new file mode 100644 index 0000000000..68484991c1 --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-backing_basic-unmap.xml @@ -0,0 +1,47 @@ +<disk type='file' device='disk'> + <driver name='qemu' type='qcow' discard='unmap'/> + <source file='/var/lib/libvirt/images/a'> + <privateData> + <nodenames> + <nodename type='storage' name='node-a-s'/> + <nodename type='format' name='node-a-f'/> + </nodenames> + </privateData> + </source> + <backingStore type='file' index='1'> + <format type='qed'/> + <source file='/var/lib/libvirt/images/b'> + <privateData> + <nodenames> + <nodename type='storage' name='node-b-s'/> + <nodename type='format' name='node-b-f'/> + </nodenames> + </privateData> + </source> + <backingStore type='network' index='2'> + <format type='vmdk'/> + <source protocol='gluster' name='images/c'> + <host name='test.org'/> + <privateData> + <nodenames> + <nodename type='storage' name='node-c-s'/> + <nodename type='format' name='node-c-f'/> + </nodenames> + </privateData> + </source> + <backingStore type='file' index='3'> + <format type='raw'/> + <source file='/var/lib/libvirt/images/d'> + <privateData> + <nodenames> + <nodename type='storage' name='node-d-s'/> + <nodename type='format' name='node-d-f'/> + </nodenames> + </privateData> + </source> + <backingStore/> + </backingStore> + </backingStore> + </backingStore> + <target dev='vda'/> +</disk> -- 2.16.2

On 04/25/2018 11:15 AM, Peter Krempa wrote:
The test cases would correspond to the following -drive command lines:
file-backing_basic-detect.xml: -drive file=/var/lib/libvirt/images/a,format=qcow,if=none,id=drive-dummy,detect-zeroes=on -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy
file-backing_basic-unmap-detect.xml: -drive file=/var/lib/libvirt/images/a,format=qcow,if=none,id=drive-dummy,discard=unmap,detect-zeroes=unmap -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy
file-backing_basic-unmap-ignore.xml: -drive file=/var/lib/libvirt/images/a,format=qcow,if=none,id=drive-dummy,discard=ignore,detect-zeroes=on -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy
file-backing_basic-unmap.xml: -drive file=/var/lib/libvirt/images/a,format=qcow,if=none,id=drive-dummy,discard=unmap -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy --- tests/qemublocktest.c | 5 ++ .../xml2json/file-backing_basic-detect.json | 60 ++++++++++++++++++++ .../xml2json/file-backing_basic-detect.xml | 47 ++++++++++++++++ .../xml2json/file-backing_basic-unmap-detect.json | 64 ++++++++++++++++++++++ .../xml2json/file-backing_basic-unmap-detect.xml | 47 ++++++++++++++++ .../xml2json/file-backing_basic-unmap-discard.json | 0
^^ You have a stray
.../xml2json/file-backing_basic-unmap-ignore.json | 64 ++++++++++++++++++++++ .../xml2json/file-backing_basic-unmap-ignore.xml | 47 ++++++++++++++++ .../xml2json/file-backing_basic-unmap.json | 63 +++++++++++++++++++++ .../xml2json/file-backing_basic-unmap.xml | 47 ++++++++++++++++ 10 files changed, 444 insertions(+) create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-detect.json create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-detect.xml create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-unmap-detect.json create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-unmap-detect.xml create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-unmap-discard.json create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-unmap-ignore.json create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-unmap-ignore.xml create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-unmap.json create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-unmap.xml
Discarding the stray... (sorry)... Yet another case where there's trust that you have the right output here... Reviewed-by: John Ferlan <jferlan@redhat.com> John

The test cases would correspond to the following -drive command lines: dir-fat-cache.xml: -drive file=fat:/var/somefiles,if=none,id=drive-dummy,readonly=on,cache=directsync -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy,write-cache=off file-backing_basic-cache-directsync.xml: -drive file=/var/lib/libvirt/images/a,format=qcow2,if=none,id=drive-dummy,cache=directsync -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy,write-cache=off file-backing_basic-cache-none.xml: -drive file=/var/lib/libvirt/images/a,format=qcow2,if=none,id=drive-dummy,cache=none -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy,write-cache=on file-backing_basic-cache-unsafe.xml: -drive file=/var/lib/libvirt/images/a,format=qcow2,if=none,id=drive-dummy,cache=unsafe -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy,write-cache=on file-backing_basic-cache-writeback.xml: -drive file=/var/lib/libvirt/images/a,format=qcow2,if=none,id=drive-dummy,cache=writeback -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy,write-cache=on file-backing_basic-cache-writethrough.xml: -drive file=/var/lib/libvirt/images/a,format=qcow2,if=none,id=drive-dummy,cache=writethrough -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy,write-cache=off network-qcow2-backing-chain-cache-unsafe.xml: -drive file=rbd:rbdpool/rbdimg:id=testuser-rbd:auth_supported=cephx\;none: mon_host=host1.example.com\;host2.example.com, file.password-secret=node-a-s-secalias,format=qcow2, if=none,id=drive-dummy,cache=directsync -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy,write-cache=off --- tests/qemublocktest.c | 8 ++ .../qemublocktestdata/xml2json/dir-fat-cache.json | 22 ++++++ tests/qemublocktestdata/xml2json/dir-fat-cache.xml | 13 ++++ .../file-backing_basic-cache-directsync.json | 91 ++++++++++++++++++++++ .../file-backing_basic-cache-directsync.xml | 47 +++++++++++ .../xml2json/file-backing_basic-cache-none.json | 91 ++++++++++++++++++++++ .../xml2json/file-backing_basic-cache-none.xml | 47 +++++++++++ .../xml2json/file-backing_basic-cache-unsafe.json | 91 ++++++++++++++++++++++ .../xml2json/file-backing_basic-cache-unsafe.xml | 47 +++++++++++ .../file-backing_basic-cache-writeback.json | 91 ++++++++++++++++++++++ .../file-backing_basic-cache-writeback.xml | 47 +++++++++++ .../file-backing_basic-cache-writethrough.json | 91 ++++++++++++++++++++++ .../file-backing_basic-cache-writethrough.xml | 47 +++++++++++ .../network-qcow2-backing-chain-cache-unsafe.json | 57 ++++++++++++++ .../network-qcow2-backing-chain-cache-unsafe.xml | 31 ++++++++ 15 files changed, 821 insertions(+) create mode 100644 tests/qemublocktestdata/xml2json/dir-fat-cache.json create mode 100644 tests/qemublocktestdata/xml2json/dir-fat-cache.xml create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-cache-directsync.json create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-cache-directsync.xml create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-cache-none.json create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-cache-none.xml create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-cache-unsafe.json create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-cache-unsafe.xml create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-cache-writeback.json create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-cache-writeback.xml create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-cache-writethrough.json create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-cache-writethrough.xml create mode 100644 tests/qemublocktestdata/xml2json/network-qcow2-backing-chain-cache-unsafe.json create mode 100644 tests/qemublocktestdata/xml2json/network-qcow2-backing-chain-cache-unsafe.xml diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index 4a2051da57..a888eab524 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -470,6 +470,14 @@ mymain(void) TEST_DISK_TO_JSON("file-backing_basic-unmap-ignore"); TEST_DISK_TO_JSON("file-backing_basic-detect"); + TEST_DISK_TO_JSON("file-backing_basic-cache-none"); + TEST_DISK_TO_JSON("file-backing_basic-cache-writethrough"); + TEST_DISK_TO_JSON("file-backing_basic-cache-writeback"); + TEST_DISK_TO_JSON("file-backing_basic-cache-directsync"); + TEST_DISK_TO_JSON("file-backing_basic-cache-unsafe"); + TEST_DISK_TO_JSON("network-qcow2-backing-chain-cache-unsafe"); + TEST_DISK_TO_JSON("dir-fat-cache"); + cleanup: virHashFree(diskxmljsondata.schema); qemuTestDriverFree(&driver); diff --git a/tests/qemublocktestdata/xml2json/dir-fat-cache.json b/tests/qemublocktestdata/xml2json/dir-fat-cache.json new file mode 100644 index 0000000000..8c0a045ed5 --- /dev/null +++ b/tests/qemublocktestdata/xml2json/dir-fat-cache.json @@ -0,0 +1,22 @@ +{ + "node-name": "node-f", + "read-only": true, + "cache": { + "direct": true, + "no-flush": false + }, + "driver": "raw", + "file": { + "driver": "vvfat", + "dir": "/var/somefiles", + "floppy": false, + "rw": false, + "node-name": "node-s", + "cache": { + "direct": true, + "no-flush": false + }, + "read-only": true, + "discard": "unmap" + } +} diff --git a/tests/qemublocktestdata/xml2json/dir-fat-cache.xml b/tests/qemublocktestdata/xml2json/dir-fat-cache.xml new file mode 100644 index 0000000000..18e652a45f --- /dev/null +++ b/tests/qemublocktestdata/xml2json/dir-fat-cache.xml @@ -0,0 +1,13 @@ +<disk type='dir' device='disk'> + <driver name='qemu' type='fat' cache='directsync'/> + <source dir='/var/somefiles'> + <privateData> + <nodenames> + <nodename type='storage' name='node-s'/> + <nodename type='format' name='node-f'/> + </nodenames> + </privateData> + </source> + <target dev='vda'/> + <readonly/> +</disk> diff --git a/tests/qemublocktestdata/xml2json/file-backing_basic-cache-directsync.json b/tests/qemublocktestdata/xml2json/file-backing_basic-cache-directsync.json new file mode 100644 index 0000000000..023caf013f --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-backing_basic-cache-directsync.json @@ -0,0 +1,91 @@ +{ + "node-name": "node-a-f", + "read-only": false, + "cache": { + "direct": true, + "no-flush": false + }, + "driver": "qcow2", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/a", + "node-name": "node-a-s", + "cache": { + "direct": true, + "no-flush": false + }, + "read-only": false, + "discard": "unmap" + }, + "backing": "node-b-f" +} +{ + "node-name": "node-b-f", + "read-only": true, + "cache": { + "direct": true, + "no-flush": false + }, + "driver": "qcow2", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/b", + "node-name": "node-b-s", + "cache": { + "direct": true, + "no-flush": false + }, + "read-only": true, + "discard": "unmap" + }, + "backing": "node-c-f" +} +{ + "node-name": "node-c-f", + "read-only": true, + "cache": { + "direct": true, + "no-flush": false + }, + "driver": "qcow2", + "file": { + "driver": "gluster", + "volume": "images", + "path": "c", + "server": [ + { + "type": "inet", + "host": "test.org", + "port": "24007" + } + ], + "node-name": "node-c-s", + "cache": { + "direct": true, + "no-flush": false + }, + "read-only": true, + "discard": "unmap" + }, + "backing": "node-d-f" +} +{ + "node-name": "node-d-f", + "read-only": true, + "cache": { + "direct": true, + "no-flush": false + }, + "driver": "raw", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/d", + "node-name": "node-d-s", + "cache": { + "direct": true, + "no-flush": false + }, + "read-only": true, + "discard": "unmap" + } +} diff --git a/tests/qemublocktestdata/xml2json/file-backing_basic-cache-directsync.xml b/tests/qemublocktestdata/xml2json/file-backing_basic-cache-directsync.xml new file mode 100644 index 0000000000..cf4197ea86 --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-backing_basic-cache-directsync.xml @@ -0,0 +1,47 @@ +<disk type='file' device='disk'> + <driver name='qemu' type='qcow2' cache='directsync'/> + <source file='/var/lib/libvirt/images/a'> + <privateData> + <nodenames> + <nodename type='storage' name='node-a-s'/> + <nodename type='format' name='node-a-f'/> + </nodenames> + </privateData> + </source> + <backingStore type='file' index='1'> + <format type='qcow2'/> + <source file='/var/lib/libvirt/images/b'> + <privateData> + <nodenames> + <nodename type='storage' name='node-b-s'/> + <nodename type='format' name='node-b-f'/> + </nodenames> + </privateData> + </source> + <backingStore type='network' index='2'> + <format type='qcow2'/> + <source protocol='gluster' name='images/c'> + <host name='test.org'/> + <privateData> + <nodenames> + <nodename type='storage' name='node-c-s'/> + <nodename type='format' name='node-c-f'/> + </nodenames> + </privateData> + </source> + <backingStore type='file' index='3'> + <format type='raw'/> + <source file='/var/lib/libvirt/images/d'> + <privateData> + <nodenames> + <nodename type='storage' name='node-d-s'/> + <nodename type='format' name='node-d-f'/> + </nodenames> + </privateData> + </source> + <backingStore/> + </backingStore> + </backingStore> + </backingStore> + <target dev='vda'/> +</disk> diff --git a/tests/qemublocktestdata/xml2json/file-backing_basic-cache-none.json b/tests/qemublocktestdata/xml2json/file-backing_basic-cache-none.json new file mode 100644 index 0000000000..023caf013f --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-backing_basic-cache-none.json @@ -0,0 +1,91 @@ +{ + "node-name": "node-a-f", + "read-only": false, + "cache": { + "direct": true, + "no-flush": false + }, + "driver": "qcow2", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/a", + "node-name": "node-a-s", + "cache": { + "direct": true, + "no-flush": false + }, + "read-only": false, + "discard": "unmap" + }, + "backing": "node-b-f" +} +{ + "node-name": "node-b-f", + "read-only": true, + "cache": { + "direct": true, + "no-flush": false + }, + "driver": "qcow2", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/b", + "node-name": "node-b-s", + "cache": { + "direct": true, + "no-flush": false + }, + "read-only": true, + "discard": "unmap" + }, + "backing": "node-c-f" +} +{ + "node-name": "node-c-f", + "read-only": true, + "cache": { + "direct": true, + "no-flush": false + }, + "driver": "qcow2", + "file": { + "driver": "gluster", + "volume": "images", + "path": "c", + "server": [ + { + "type": "inet", + "host": "test.org", + "port": "24007" + } + ], + "node-name": "node-c-s", + "cache": { + "direct": true, + "no-flush": false + }, + "read-only": true, + "discard": "unmap" + }, + "backing": "node-d-f" +} +{ + "node-name": "node-d-f", + "read-only": true, + "cache": { + "direct": true, + "no-flush": false + }, + "driver": "raw", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/d", + "node-name": "node-d-s", + "cache": { + "direct": true, + "no-flush": false + }, + "read-only": true, + "discard": "unmap" + } +} diff --git a/tests/qemublocktestdata/xml2json/file-backing_basic-cache-none.xml b/tests/qemublocktestdata/xml2json/file-backing_basic-cache-none.xml new file mode 100644 index 0000000000..240afa85d2 --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-backing_basic-cache-none.xml @@ -0,0 +1,47 @@ +<disk type='file' device='disk'> + <driver name='qemu' type='qcow2' cache='none'/> + <source file='/var/lib/libvirt/images/a'> + <privateData> + <nodenames> + <nodename type='storage' name='node-a-s'/> + <nodename type='format' name='node-a-f'/> + </nodenames> + </privateData> + </source> + <backingStore type='file' index='1'> + <format type='qcow2'/> + <source file='/var/lib/libvirt/images/b'> + <privateData> + <nodenames> + <nodename type='storage' name='node-b-s'/> + <nodename type='format' name='node-b-f'/> + </nodenames> + </privateData> + </source> + <backingStore type='network' index='2'> + <format type='qcow2'/> + <source protocol='gluster' name='images/c'> + <host name='test.org'/> + <privateData> + <nodenames> + <nodename type='storage' name='node-c-s'/> + <nodename type='format' name='node-c-f'/> + </nodenames> + </privateData> + </source> + <backingStore type='file' index='3'> + <format type='raw'/> + <source file='/var/lib/libvirt/images/d'> + <privateData> + <nodenames> + <nodename type='storage' name='node-d-s'/> + <nodename type='format' name='node-d-f'/> + </nodenames> + </privateData> + </source> + <backingStore/> + </backingStore> + </backingStore> + </backingStore> + <target dev='vda'/> +</disk> diff --git a/tests/qemublocktestdata/xml2json/file-backing_basic-cache-unsafe.json b/tests/qemublocktestdata/xml2json/file-backing_basic-cache-unsafe.json new file mode 100644 index 0000000000..5f6b94a9d5 --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-backing_basic-cache-unsafe.json @@ -0,0 +1,91 @@ +{ + "node-name": "node-a-f", + "read-only": false, + "cache": { + "direct": false, + "no-flush": true + }, + "driver": "qcow2", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/a", + "node-name": "node-a-s", + "cache": { + "direct": false, + "no-flush": true + }, + "read-only": false, + "discard": "unmap" + }, + "backing": "node-b-f" +} +{ + "node-name": "node-b-f", + "read-only": true, + "cache": { + "direct": false, + "no-flush": true + }, + "driver": "qcow2", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/b", + "node-name": "node-b-s", + "cache": { + "direct": false, + "no-flush": true + }, + "read-only": true, + "discard": "unmap" + }, + "backing": "node-c-f" +} +{ + "node-name": "node-c-f", + "read-only": true, + "cache": { + "direct": false, + "no-flush": true + }, + "driver": "qcow2", + "file": { + "driver": "gluster", + "volume": "images", + "path": "c", + "server": [ + { + "type": "inet", + "host": "test.org", + "port": "24007" + } + ], + "node-name": "node-c-s", + "cache": { + "direct": false, + "no-flush": true + }, + "read-only": true, + "discard": "unmap" + }, + "backing": "node-d-f" +} +{ + "node-name": "node-d-f", + "read-only": true, + "cache": { + "direct": false, + "no-flush": true + }, + "driver": "raw", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/d", + "node-name": "node-d-s", + "cache": { + "direct": false, + "no-flush": true + }, + "read-only": true, + "discard": "unmap" + } +} diff --git a/tests/qemublocktestdata/xml2json/file-backing_basic-cache-unsafe.xml b/tests/qemublocktestdata/xml2json/file-backing_basic-cache-unsafe.xml new file mode 100644 index 0000000000..e18d33ce3c --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-backing_basic-cache-unsafe.xml @@ -0,0 +1,47 @@ +<disk type='file' device='disk'> + <driver name='qemu' type='qcow2' cache='unsafe'/> + <source file='/var/lib/libvirt/images/a'> + <privateData> + <nodenames> + <nodename type='storage' name='node-a-s'/> + <nodename type='format' name='node-a-f'/> + </nodenames> + </privateData> + </source> + <backingStore type='file' index='1'> + <format type='qcow2'/> + <source file='/var/lib/libvirt/images/b'> + <privateData> + <nodenames> + <nodename type='storage' name='node-b-s'/> + <nodename type='format' name='node-b-f'/> + </nodenames> + </privateData> + </source> + <backingStore type='network' index='2'> + <format type='qcow2'/> + <source protocol='gluster' name='images/c'> + <host name='test.org'/> + <privateData> + <nodenames> + <nodename type='storage' name='node-c-s'/> + <nodename type='format' name='node-c-f'/> + </nodenames> + </privateData> + </source> + <backingStore type='file' index='3'> + <format type='raw'/> + <source file='/var/lib/libvirt/images/d'> + <privateData> + <nodenames> + <nodename type='storage' name='node-d-s'/> + <nodename type='format' name='node-d-f'/> + </nodenames> + </privateData> + </source> + <backingStore/> + </backingStore> + </backingStore> + </backingStore> + <target dev='vda'/> +</disk> diff --git a/tests/qemublocktestdata/xml2json/file-backing_basic-cache-writeback.json b/tests/qemublocktestdata/xml2json/file-backing_basic-cache-writeback.json new file mode 100644 index 0000000000..9dc7d38850 --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-backing_basic-cache-writeback.json @@ -0,0 +1,91 @@ +{ + "node-name": "node-a-f", + "read-only": false, + "cache": { + "direct": false, + "no-flush": false + }, + "driver": "qcow2", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/a", + "node-name": "node-a-s", + "cache": { + "direct": false, + "no-flush": false + }, + "read-only": false, + "discard": "unmap" + }, + "backing": "node-b-f" +} +{ + "node-name": "node-b-f", + "read-only": true, + "cache": { + "direct": false, + "no-flush": false + }, + "driver": "qcow2", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/b", + "node-name": "node-b-s", + "cache": { + "direct": false, + "no-flush": false + }, + "read-only": true, + "discard": "unmap" + }, + "backing": "node-c-f" +} +{ + "node-name": "node-c-f", + "read-only": true, + "cache": { + "direct": false, + "no-flush": false + }, + "driver": "qcow2", + "file": { + "driver": "gluster", + "volume": "images", + "path": "c", + "server": [ + { + "type": "inet", + "host": "test.org", + "port": "24007" + } + ], + "node-name": "node-c-s", + "cache": { + "direct": false, + "no-flush": false + }, + "read-only": true, + "discard": "unmap" + }, + "backing": "node-d-f" +} +{ + "node-name": "node-d-f", + "read-only": true, + "cache": { + "direct": false, + "no-flush": false + }, + "driver": "raw", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/d", + "node-name": "node-d-s", + "cache": { + "direct": false, + "no-flush": false + }, + "read-only": true, + "discard": "unmap" + } +} diff --git a/tests/qemublocktestdata/xml2json/file-backing_basic-cache-writeback.xml b/tests/qemublocktestdata/xml2json/file-backing_basic-cache-writeback.xml new file mode 100644 index 0000000000..7040388040 --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-backing_basic-cache-writeback.xml @@ -0,0 +1,47 @@ +<disk type='file' device='disk'> + <driver name='qemu' type='qcow2' cache='writeback'/> + <source file='/var/lib/libvirt/images/a'> + <privateData> + <nodenames> + <nodename type='storage' name='node-a-s'/> + <nodename type='format' name='node-a-f'/> + </nodenames> + </privateData> + </source> + <backingStore type='file' index='1'> + <format type='qcow2'/> + <source file='/var/lib/libvirt/images/b'> + <privateData> + <nodenames> + <nodename type='storage' name='node-b-s'/> + <nodename type='format' name='node-b-f'/> + </nodenames> + </privateData> + </source> + <backingStore type='network' index='2'> + <format type='qcow2'/> + <source protocol='gluster' name='images/c'> + <host name='test.org'/> + <privateData> + <nodenames> + <nodename type='storage' name='node-c-s'/> + <nodename type='format' name='node-c-f'/> + </nodenames> + </privateData> + </source> + <backingStore type='file' index='3'> + <format type='raw'/> + <source file='/var/lib/libvirt/images/d'> + <privateData> + <nodenames> + <nodename type='storage' name='node-d-s'/> + <nodename type='format' name='node-d-f'/> + </nodenames> + </privateData> + </source> + <backingStore/> + </backingStore> + </backingStore> + </backingStore> + <target dev='vda'/> +</disk> diff --git a/tests/qemublocktestdata/xml2json/file-backing_basic-cache-writethrough.json b/tests/qemublocktestdata/xml2json/file-backing_basic-cache-writethrough.json new file mode 100644 index 0000000000..9dc7d38850 --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-backing_basic-cache-writethrough.json @@ -0,0 +1,91 @@ +{ + "node-name": "node-a-f", + "read-only": false, + "cache": { + "direct": false, + "no-flush": false + }, + "driver": "qcow2", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/a", + "node-name": "node-a-s", + "cache": { + "direct": false, + "no-flush": false + }, + "read-only": false, + "discard": "unmap" + }, + "backing": "node-b-f" +} +{ + "node-name": "node-b-f", + "read-only": true, + "cache": { + "direct": false, + "no-flush": false + }, + "driver": "qcow2", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/b", + "node-name": "node-b-s", + "cache": { + "direct": false, + "no-flush": false + }, + "read-only": true, + "discard": "unmap" + }, + "backing": "node-c-f" +} +{ + "node-name": "node-c-f", + "read-only": true, + "cache": { + "direct": false, + "no-flush": false + }, + "driver": "qcow2", + "file": { + "driver": "gluster", + "volume": "images", + "path": "c", + "server": [ + { + "type": "inet", + "host": "test.org", + "port": "24007" + } + ], + "node-name": "node-c-s", + "cache": { + "direct": false, + "no-flush": false + }, + "read-only": true, + "discard": "unmap" + }, + "backing": "node-d-f" +} +{ + "node-name": "node-d-f", + "read-only": true, + "cache": { + "direct": false, + "no-flush": false + }, + "driver": "raw", + "file": { + "driver": "file", + "filename": "/var/lib/libvirt/images/d", + "node-name": "node-d-s", + "cache": { + "direct": false, + "no-flush": false + }, + "read-only": true, + "discard": "unmap" + } +} diff --git a/tests/qemublocktestdata/xml2json/file-backing_basic-cache-writethrough.xml b/tests/qemublocktestdata/xml2json/file-backing_basic-cache-writethrough.xml new file mode 100644 index 0000000000..d2fe1b780b --- /dev/null +++ b/tests/qemublocktestdata/xml2json/file-backing_basic-cache-writethrough.xml @@ -0,0 +1,47 @@ +<disk type='file' device='disk'> + <driver name='qemu' type='qcow2' cache='writethrough'/> + <source file='/var/lib/libvirt/images/a'> + <privateData> + <nodenames> + <nodename type='storage' name='node-a-s'/> + <nodename type='format' name='node-a-f'/> + </nodenames> + </privateData> + </source> + <backingStore type='file' index='1'> + <format type='qcow2'/> + <source file='/var/lib/libvirt/images/b'> + <privateData> + <nodenames> + <nodename type='storage' name='node-b-s'/> + <nodename type='format' name='node-b-f'/> + </nodenames> + </privateData> + </source> + <backingStore type='network' index='2'> + <format type='qcow2'/> + <source protocol='gluster' name='images/c'> + <host name='test.org'/> + <privateData> + <nodenames> + <nodename type='storage' name='node-c-s'/> + <nodename type='format' name='node-c-f'/> + </nodenames> + </privateData> + </source> + <backingStore type='file' index='3'> + <format type='raw'/> + <source file='/var/lib/libvirt/images/d'> + <privateData> + <nodenames> + <nodename type='storage' name='node-d-s'/> + <nodename type='format' name='node-d-f'/> + </nodenames> + </privateData> + </source> + <backingStore/> + </backingStore> + </backingStore> + </backingStore> + <target dev='vda'/> +</disk> diff --git a/tests/qemublocktestdata/xml2json/network-qcow2-backing-chain-cache-unsafe.json b/tests/qemublocktestdata/xml2json/network-qcow2-backing-chain-cache-unsafe.json new file mode 100644 index 0000000000..de4be359cb --- /dev/null +++ b/tests/qemublocktestdata/xml2json/network-qcow2-backing-chain-cache-unsafe.json @@ -0,0 +1,57 @@ +{ + "node-name": "node-b-f", + "read-only": false, + "cache": { + "direct": true, + "no-flush": false + }, + "driver": "qcow2", + "file": { + "driver": "rbd", + "pool": "rbdpool", + "image": "rbdimg", + "server": [ + { + "host": "host1.example.com", + "port": "0" + }, + { + "host": "host2.example.com", + "port": "0" + } + ], + "user": "testuser-rbd", + "node-name": "node-a-s", + "cache": { + "direct": true, + "no-flush": false + }, + "read-only": false, + "discard": "unmap" + }, + "backing": "node-b-f" +} +{ + "node-name": "node-b-f", + "read-only": true, + "cache": { + "direct": true, + "no-flush": false + }, + "driver": "qcow2", + "file": { + "driver": "iscsi", + "portal": "example.org:3260", + "target": "iscsitarget", + "lun": 1, + "transport": "tcp", + "node-name": "node-b-s", + "cache": { + "direct": true, + "no-flush": false + }, + "read-only": true, + "discard": "unmap" + }, + "backing": null +} diff --git a/tests/qemublocktestdata/xml2json/network-qcow2-backing-chain-cache-unsafe.xml b/tests/qemublocktestdata/xml2json/network-qcow2-backing-chain-cache-unsafe.xml new file mode 100644 index 0000000000..b4ba7470af --- /dev/null +++ b/tests/qemublocktestdata/xml2json/network-qcow2-backing-chain-cache-unsafe.xml @@ -0,0 +1,31 @@ +<disk type='network' device='disk'> + <driver name='qemu' type='qcow2' cache='directsync'/> + <source protocol='rbd' name='rbdpool/rbdimg'> + <host name='host1.example.com'/> + <host name='host2.example.com'/> + <auth username='testuser-rbd'> + <secret type='ceph' usage='testuser-rbd-secret'/> + </auth> + <privateData> + <nodenames> + <nodename type='storage' name='node-a-s'/> + <nodename type='format' name='node-b-f'/> + </nodenames> + </privateData> + </source> + <backingStore type='network' index='1'> + <format type='qcow2'/> + <source protocol='iscsi' name='iscsitarget/1'> + <host name='example.org'/> + <privateData> + <nodenames> + <nodename type='storage' name='node-b-s'/> + <nodename type='format' name='node-b-f'/> + </nodenames> + </privateData> + </source> + <backingStore/> + </backingStore> + <target dev='vda' bus='virtio'/> + <alias name='virtio-disk0'/> +</disk> -- 2.16.2

On 04/25/2018 11:15 AM, Peter Krempa wrote:
The test cases would correspond to the following -drive command lines:
dir-fat-cache.xml: -drive file=fat:/var/somefiles,if=none,id=drive-dummy,readonly=on,cache=directsync -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy,write-cache=off
file-backing_basic-cache-directsync.xml: -drive file=/var/lib/libvirt/images/a,format=qcow2,if=none,id=drive-dummy,cache=directsync -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy,write-cache=off
file-backing_basic-cache-none.xml: -drive file=/var/lib/libvirt/images/a,format=qcow2,if=none,id=drive-dummy,cache=none -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy,write-cache=on
file-backing_basic-cache-unsafe.xml: -drive file=/var/lib/libvirt/images/a,format=qcow2,if=none,id=drive-dummy,cache=unsafe -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy,write-cache=on
file-backing_basic-cache-writeback.xml: -drive file=/var/lib/libvirt/images/a,format=qcow2,if=none,id=drive-dummy,cache=writeback -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy,write-cache=on
file-backing_basic-cache-writethrough.xml: -drive file=/var/lib/libvirt/images/a,format=qcow2,if=none,id=drive-dummy,cache=writethrough -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy,write-cache=off
network-qcow2-backing-chain-cache-unsafe.xml: -drive file=rbd:rbdpool/rbdimg:id=testuser-rbd:auth_supported=cephx\;none: mon_host=host1.example.com\;host2.example.com, file.password-secret=node-a-s-secalias,format=qcow2, if=none,id=drive-dummy,cache=directsync -device virtio-blk-pci,scsi=off,drive=drive-dummy,id=dummy,write-cache=off --- tests/qemublocktest.c | 8 ++ .../qemublocktestdata/xml2json/dir-fat-cache.json | 22 ++++++ tests/qemublocktestdata/xml2json/dir-fat-cache.xml | 13 ++++ .../file-backing_basic-cache-directsync.json | 91 ++++++++++++++++++++++ .../file-backing_basic-cache-directsync.xml | 47 +++++++++++ .../xml2json/file-backing_basic-cache-none.json | 91 ++++++++++++++++++++++ .../xml2json/file-backing_basic-cache-none.xml | 47 +++++++++++ .../xml2json/file-backing_basic-cache-unsafe.json | 91 ++++++++++++++++++++++ .../xml2json/file-backing_basic-cache-unsafe.xml | 47 +++++++++++ .../file-backing_basic-cache-writeback.json | 91 ++++++++++++++++++++++ .../file-backing_basic-cache-writeback.xml | 47 +++++++++++ .../file-backing_basic-cache-writethrough.json | 91 ++++++++++++++++++++++ .../file-backing_basic-cache-writethrough.xml | 47 +++++++++++ .../network-qcow2-backing-chain-cache-unsafe.json | 57 ++++++++++++++ .../network-qcow2-backing-chain-cache-unsafe.xml | 31 ++++++++ 15 files changed, 821 insertions(+) create mode 100644 tests/qemublocktestdata/xml2json/dir-fat-cache.json create mode 100644 tests/qemublocktestdata/xml2json/dir-fat-cache.xml create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-cache-directsync.json create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-cache-directsync.xml create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-cache-none.json create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-cache-none.xml create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-cache-unsafe.json create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-cache-unsafe.xml create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-cache-writeback.json create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-cache-writeback.xml create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-cache-writethrough.json create mode 100644 tests/qemublocktestdata/xml2json/file-backing_basic-cache-writethrough.xml create mode 100644 tests/qemublocktestdata/xml2json/network-qcow2-backing-chain-cache-unsafe.json create mode 100644 tests/qemublocktestdata/xml2json/network-qcow2-backing-chain-cache-unsafe.xml
I'm glad someone knows the rules around here - guess we won't really know how right things are until testing gets ahold of this. Again nothing "basic" about these things - I'm really glad none of the were named complex or in-too-deep ;-) [...]
diff --git a/tests/qemublocktestdata/xml2json/network-qcow2-backing-chain-cache-unsafe.json b/tests/qemublocktestdata/xml2json/network-qcow2-backing-chain-cache-unsafe.json new file mode 100644 index 0000000000..de4be359cb --- /dev/null +++ b/tests/qemublocktestdata/xml2json/network-qcow2-backing-chain-cache-unsafe.json @@ -0,0 +1,57 @@ +{ + "node-name": "node-b-f", + "read-only": false, + "cache": { + "direct": true, + "no-flush": false + }, + "driver": "qcow2", + "file": { + "driver": "rbd", + "pool": "rbdpool", + "image": "rbdimg", + "server": [ + { + "host": "host1.example.com", + "port": "0" + }, + { + "host": "host2.example.com", + "port": "0" + } + ], + "user": "testuser-rbd", + "node-name": "node-a-s", + "cache": { + "direct": true, + "no-flush": false + }, + "read-only": false, + "discard": "unmap" + }, + "backing": "node-b-f" +} +{ + "node-name": "node-b-f", + "read-only": true, + "cache": { + "direct": true, + "no-flush": false + }, + "driver": "qcow2", + "file": { + "driver": "iscsi", + "portal": "example.org:3260", + "target": "iscsitarget", + "lun": 1, + "transport": "tcp", + "node-name": "node-b-s", + "cache": { + "direct": true, + "no-flush": false + }, + "read-only": true, + "discard": "unmap" + }, + "backing": null +} diff --git a/tests/qemublocktestdata/xml2json/network-qcow2-backing-chain-cache-unsafe.xml b/tests/qemublocktestdata/xml2json/network-qcow2-backing-chain-cache-unsafe.xml new file mode 100644 index 0000000000..b4ba7470af --- /dev/null +++ b/tests/qemublocktestdata/xml2json/network-qcow2-backing-chain-cache-unsafe.xml @@ -0,0 +1,31 @@ +<disk type='network' device='disk'> + <driver name='qemu' type='qcow2' cache='directsync'/>
^^^ this was supposed to be 'unsafe' wasn't it?
+ <source protocol='rbd' name='rbdpool/rbdimg'> + <host name='host1.example.com'/> + <host name='host2.example.com'/> + <auth username='testuser-rbd'> + <secret type='ceph' usage='testuser-rbd-secret'/> + </auth> + <privateData> + <nodenames> + <nodename type='storage' name='node-a-s'/> + <nodename type='format' name='node-b-f'/> + </nodenames> + </privateData> + </source> + <backingStore type='network' index='1'> + <format type='qcow2'/> + <source protocol='iscsi' name='iscsitarget/1'> + <host name='example.org'/> + <privateData> + <nodenames> + <nodename type='storage' name='node-b-s'/> + <nodename type='format' name='node-b-f'/> + </nodenames> + </privateData> + </source> + <backingStore/> + </backingStore> + <target dev='vda' bus='virtio'/> + <alias name='virtio-disk0'/> +</disk>
With I think a minor adjustment to use 'unsafe'... and updated output... Reviewed-by: John Ferlan <jferlan@redhat.com> John

Make sure that 'host_device' is generated for type='block'. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemublocktest.c | 2 ++ tests/qemublocktestdata/xml2json/block-raw-noopts.json | 12 ++++++++++++ tests/qemublocktestdata/xml2json/block-raw-noopts.xml | 12 ++++++++++++ 3 files changed, 26 insertions(+) create mode 100644 tests/qemublocktestdata/xml2json/block-raw-noopts.json create mode 100644 tests/qemublocktestdata/xml2json/block-raw-noopts.xml diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index a888eab524..08f165d7e0 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -478,6 +478,8 @@ mymain(void) TEST_DISK_TO_JSON("network-qcow2-backing-chain-cache-unsafe"); TEST_DISK_TO_JSON("dir-fat-cache"); + TEST_DISK_TO_JSON("block-raw-noopts"); + cleanup: virHashFree(diskxmljsondata.schema); qemuTestDriverFree(&driver); diff --git a/tests/qemublocktestdata/xml2json/block-raw-noopts.json b/tests/qemublocktestdata/xml2json/block-raw-noopts.json new file mode 100644 index 0000000000..25bf77d5aa --- /dev/null +++ b/tests/qemublocktestdata/xml2json/block-raw-noopts.json @@ -0,0 +1,12 @@ +{ + "node-name": "0123456789ABCDEF0123456789ABCDE", + "read-only": false, + "driver": "raw", + "file": { + "driver": "host_device", + "filename": "/dev/blah", + "node-name": "0123456789ABCDEF0123456789ABCDE", + "read-only": false, + "discard": "unmap" + } +} diff --git a/tests/qemublocktestdata/xml2json/block-raw-noopts.xml b/tests/qemublocktestdata/xml2json/block-raw-noopts.xml new file mode 100644 index 0000000000..2f319d26d0 --- /dev/null +++ b/tests/qemublocktestdata/xml2json/block-raw-noopts.xml @@ -0,0 +1,12 @@ +<disk device='disk' type='block'> + <driver name='qemu' type='raw'/> + <source dev='/dev/blah'> + <privateData> + <nodenames> + <nodename type='storage' name='0123456789ABCDEF0123456789ABCDE'/> + <nodename type='format' name='0123456789ABCDEF0123456789ABCDE'/> + </nodenames> + </privateData> + </source> + <target dev='vda'/> +</disk> -- 2.16.2

On 04/25/2018 11:15 AM, Peter Krempa wrote:
Make sure that 'host_device' is generated for type='block'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemublocktest.c | 2 ++ tests/qemublocktestdata/xml2json/block-raw-noopts.json | 12 ++++++++++++ tests/qemublocktestdata/xml2json/block-raw-noopts.xml | 12 ++++++++++++ 3 files changed, 26 insertions(+) create mode 100644 tests/qemublocktestdata/xml2json/block-raw-noopts.json create mode 100644 tests/qemublocktestdata/xml2json/block-raw-noopts.xml
Reviewed-by: John Ferlan <jferlan@redhat.com> John

Am 25.04.2018 um 17:15 hat Peter Krempa geschrieben:
Diff to the RFC posting: - host_device and host_cdrom handling was added - modified code to format and parse node-names to status XML so that we can add other node names easier (for quorum, throttling, etc) - added test case for host_device
Kevin, please sanity check the outputs whether they make sense. Thanks.
The diff between the applied v1 series and the RFC series looks sane to me. Kevin
participants (3)
-
John Ferlan
-
Kevin Wolf
-
Peter Krempa