[libvirt] [PATCH 00/14] Fixes, cleanups and qcow2+luks support (blockdev-add saga)
This is a collection of patches from my blockdev-add branch which are basically ready. First few patches are refactors and cleanups which should be justified enough by itself even if the code which will use some of them was not posted yet. Other few patches add checks which will reject configurations which qemu would not accept anyways. The last part adds qcow2+luks support since adding it was rather trivial. Obviously as with raw+luks disks it works only without a backing chain. Peter Krempa (14): tests: utils: Tolerate NULL actual data in virTestCompareToFile conf: Refactor/rename virDomainDiskDefSourceParse conf: Don't require 'def' in virDomainDiskDefParse conf: Extract logic for updating 'detect_zeroes' mode qemu: domain: Add helper to initialize detected parts of the backing chain qemu: domain: Forbid storage type 'cow' in qemu qemu: domain: Forbid VIR_STORAGE_FILE_DIR as a disk format qemu: domain: Forbid VIR_STORAGE_FILE_ISO as a disk format qemu: caps: Add capability for LUKS encrypted qcow2 image support qemu: domain: Validate support for LUKS encryption of QCOW2 images qemu: command: Add support for qcow2 + luks tests: qemu: Test QCOW2 + LUKS support tests: qemuxml2argv: Allow testing of config processed at startup qemu: domain: Move initialization of disk cachemode for <shareable> disks src/conf/domain_conf.c | 58 +++++++---- src/conf/domain_conf.h | 11 +- src/libvirt_private.syms | 3 +- src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 33 +++--- src/qemu/qemu_domain.c | 111 +++++++++++++++++++-- src/qemu/qemu_domain.h | 6 ++ src/qemu/qemu_driver.c | 9 +- tests/Makefile.am | 1 + tests/qemucapabilitiesdata/caps_2.10.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml | 1 + tests/qemuxml2argvdata/disk-drive-fmt-cow.xml | 27 +++++ tests/qemuxml2argvdata/disk-drive-fmt-dir.xml | 27 +++++ tests/qemuxml2argvdata/disk-drive-fmt-iso.xml | 27 +++++ .../qemuxml2argvdata/luks-disks-source-qcow2.args | 66 ++++++++++++ tests/qemuxml2argvdata/luks-disks-source-qcow2.xml | 81 +++++++++++++++ tests/qemuxml2argvtest.c | 47 ++++++++- .../disk-drive-shared.xml | 56 +++++++++++ tests/testutils.c | 4 +- 27 files changed, 521 insertions(+), 58 deletions(-) create mode 100644 tests/qemuxml2argvdata/disk-drive-fmt-cow.xml create mode 100644 tests/qemuxml2argvdata/disk-drive-fmt-dir.xml create mode 100644 tests/qemuxml2argvdata/disk-drive-fmt-iso.xml create mode 100644 tests/qemuxml2argvdata/luks-disks-source-qcow2.args create mode 100644 tests/qemuxml2argvdata/luks-disks-source-qcow2.xml create mode 100644 tests/qemuxml2startupxmloutdata/disk-drive-shared.xml -- 2.16.2
The function docs state that 'strcontent' may be NULL, but code added in 3506f1ecfde did not use the 'cmpcontent' variable which was fixed and dereferenced it. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/testutils.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/testutils.c b/tests/testutils.c index 040ef1d2f7..4bd1b63755 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -800,8 +800,8 @@ virTestCompareToFile(const char *strcontent, if (filecontentLen > 0 && filecontent[filecontentLen - 1] == '\n' && - strcontent[strlen(strcontent) - 1] != '\n') { - if (virAsprintf(&fixedcontent, "%s\n", strcontent) < 0) + cmpcontent[strlen(cmpcontent) - 1] != '\n') { + if (virAsprintf(&fixedcontent, "%s\n", cmpcontent) < 0) goto failure; cmpcontent = fixedcontent; } -- 2.16.2
On Thu, Mar 29, 2018 at 01:50:58PM +0200, Peter Krempa wrote:
The function docs state that 'strcontent' may be NULL, but code added in 3506f1ecfde did not use the 'cmpcontent' variable which was fixed and dereferenced it.
easier to follow as: 3506f1ecfde dereferenced it instead of using the fixed 'cmpcontent' variable.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/testutils.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/testutils.c b/tests/testutils.c index 040ef1d2f7..4bd1b63755 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -800,8 +800,8 @@ virTestCompareToFile(const char *strcontent,
if (filecontentLen > 0 && filecontent[filecontentLen - 1] == '\n' && - strcontent[strlen(strcontent) - 1] != '\n') { - if (virAsprintf(&fixedcontent, "%s\n", strcontent) < 0) + cmpcontent[strlen(cmpcontent) - 1] != '\n') { + if (virAsprintf(&fixedcontent, "%s\n", cmpcontent) < 0)
ACK if you check if strlen(cmpcontent) > 0 as well. Jano
Make the function more usable by returning the full disk definition and fix the only caller for the new semantics. The new name for the function is virDomainDiskDefParse. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 33 ++++++++++++--------------------- src/conf/domain_conf.h | 8 ++++---- src/libvirt_private.syms | 2 +- src/qemu/qemu_driver.c | 9 +++++++-- 4 files changed, 24 insertions(+), 28 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ae7c0d9b71..e2bad48cc8 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -15864,44 +15864,35 @@ virDomainDeviceDefParse(const char *xmlStr, } -virStorageSourcePtr -virDomainDiskDefSourceParse(const char *xmlStr, - const virDomainDef *def, - virDomainXMLOptionPtr xmlopt, - unsigned int flags) +virDomainDiskDefPtr +virDomainDiskDefParse(const char *xmlStr, + const virDomainDef *def, + virDomainXMLOptionPtr xmlopt, + unsigned int flags) { xmlDocPtr xml; - xmlNodePtr node; xmlXPathContextPtr ctxt = NULL; virDomainDiskDefPtr disk = NULL; - virStorageSourcePtr ret = NULL; if (!(xml = virXMLParseStringCtxt(xmlStr, _("(disk_definition)"), &ctxt))) goto cleanup; - node = ctxt->node; - if (!virXMLNodeNameEqual(node, "disk")) { + if (!virXMLNodeNameEqual(ctxt->node, "disk")) { virReportError(VIR_ERR_XML_ERROR, _("expecting root element of 'disk', not '%s'"), - node->name); + ctxt->node->name); goto cleanup; } - flags |= VIR_DOMAIN_DEF_PARSE_DISK_SOURCE; - if (!(disk = virDomainDiskDefParseXML(xmlopt, node, ctxt, - NULL, def->seclabels, - def->nseclabels, - flags))) - goto cleanup; - - ret = disk->src; - disk->src = NULL; + disk = virDomainDiskDefParseXML(xmlopt, ctxt->node, ctxt, + NULL, def->seclabels, + def->nseclabels, + flags); cleanup: - virDomainDiskDefFree(disk); xmlFreeDoc(xml); xmlXPathFreeContext(ctxt); - return ret; + return disk; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 61379e50fe..650901c1f4 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2935,10 +2935,10 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(const char *xmlStr, virCapsPtr caps, virDomainXMLOptionPtr xmlopt, unsigned int flags); -virStorageSourcePtr virDomainDiskDefSourceParse(const char *xmlStr, - const virDomainDef *def, - virDomainXMLOptionPtr xmlopt, - unsigned int flags); +virDomainDiskDefPtr virDomainDiskDefParse(const char *xmlStr, + const virDomainDef *def, + virDomainXMLOptionPtr xmlopt, + unsigned int flags); virDomainDefPtr virDomainDefParseString(const char *xmlStr, virCapsPtr caps, virDomainXMLOptionPtr xmlopt, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 03fe3b315f..96a0a6de52 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -315,7 +315,7 @@ virDomainDiskDefCheckDuplicateInfo; virDomainDiskDefForeachPath; virDomainDiskDefFree; virDomainDiskDefNew; -virDomainDiskDefSourceParse; +virDomainDiskDefParse; virDomainDiskDetectZeroesTypeFromString; virDomainDiskDetectZeroesTypeToString; virDomainDiskDeviceTypeToString; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7bcc4936de..4e9add0ef7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17456,6 +17456,7 @@ qemuDomainBlockCopy(virDomainPtr dom, const char *disk, const char *destxml, unsigned long long bandwidth = 0; unsigned int granularity = 0; unsigned long long buf_size = 0; + virDomainDiskDefPtr diskdef = NULL; virStorageSourcePtr dest = NULL; size_t i; @@ -17508,14 +17509,18 @@ qemuDomainBlockCopy(virDomainPtr dom, const char *disk, const char *destxml, } } - if (!(dest = virDomainDiskDefSourceParse(destxml, vm->def, driver->xmlopt, - VIR_DOMAIN_DEF_PARSE_INACTIVE))) + if (!(diskdef = virDomainDiskDefParse(destxml, vm->def, driver->xmlopt, + VIR_DOMAIN_DEF_PARSE_INACTIVE | + VIR_DOMAIN_DEF_PARSE_DISK_SOURCE))) goto cleanup; + VIR_STEAL_PTR(dest, diskdef->src); + ret = qemuDomainBlockCopyCommon(vm, dom->conn, disk, dest, bandwidth, granularity, buf_size, flags, false); cleanup: + virDomainDiskDefFree(diskdef); virDomainObjEndAPI(&vm); return ret; } -- 2.16.2
On Thu, Mar 29, 2018 at 01:50:59PM +0200, Peter Krempa wrote:
Make the function more usable by returning the full disk definition and fix the only caller for the new semantics. The new name for the function is virDomainDiskDefParse.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 33 ++++++++++++--------------------- src/conf/domain_conf.h | 8 ++++---- src/libvirt_private.syms | 2 +- src/qemu/qemu_driver.c | 9 +++++++-- 4 files changed, 24 insertions(+), 28 deletions(-)
ACK Jano
In some use cases (mostly in tests) it is not required to check the seclabel definition validity. Add possibility to call virDomainDiskDefParse without the domain definition. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e2bad48cc8..ac3a3d9966 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -15873,6 +15873,8 @@ virDomainDiskDefParse(const char *xmlStr, xmlDocPtr xml; xmlXPathContextPtr ctxt = NULL; virDomainDiskDefPtr disk = NULL; + virSecurityLabelDefPtr *seclabels = NULL; + size_t nseclabels = 0; if (!(xml = virXMLParseStringCtxt(xmlStr, _("(disk_definition)"), &ctxt))) goto cleanup; @@ -15884,10 +15886,13 @@ virDomainDiskDefParse(const char *xmlStr, goto cleanup; } + if (def) { + seclabels = def->seclabels; + nseclabels = def->nseclabels; + } + disk = virDomainDiskDefParseXML(xmlopt, ctxt->node, ctxt, - NULL, def->seclabels, - def->nseclabels, - flags); + NULL, seclabels, nseclabels, flags); cleanup: xmlFreeDoc(xml); -- 2.16.2
On Thu, Mar 29, 2018 at 01:51:00PM +0200, Peter Krempa wrote:
In some use cases (mostly in tests) it is not required to check the seclabel definition validity. Add possibility to call virDomainDiskDefParse without the domain definition.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
ACK Jano
For some reason we've decided to silently translate the disk detect_zeroes mode if it would be invalid. Extract the logic so that it does not need to be copypasta'd across the code base. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 22 ++++++++++++++++++++++ src/conf/domain_conf.h | 3 +++ src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 15 ++------------- 4 files changed, 28 insertions(+), 13 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ac3a3d9966..ef16431aaf 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -29362,3 +29362,25 @@ virDomainDiskTranslateSourcePool(virDomainDiskDefPtr def) virStoragePoolDefFree(pooldef); return ret; } + + +/** + * virDomainDiskGetDetectZeroesMode: + * @discard: disk/image sector discard setting + * @detect_zeroes: disk/image zero sector detection mode + * + * As a convenience syntax, if discards are ignored and zero detection is set + * to 'unmap', then simply behave like zero detection is set to 'on'. But + * don't change it in the XML for easier adjustments. This behaviour is + * documented. + */ +int +virDomainDiskGetDetectZeroesMode(virDomainDiskDiscard discard, + virDomainDiskDetectZeroes detect_zeroes) +{ + if (discard != VIR_DOMAIN_DISK_DISCARD_UNMAP && + detect_zeroes == VIR_DOMAIN_DISK_DETECT_ZEROES_UNMAP) + return VIR_DOMAIN_DISK_DETECT_ZEROES_ON; + + return detect_zeroes; +} diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 650901c1f4..bd17bd99ae 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3528,5 +3528,8 @@ virDomainNetResolveActualType(virDomainNetDefPtr iface) int virDomainDiskTranslateSourcePool(virDomainDiskDefPtr def); +int +virDomainDiskGetDetectZeroesMode(virDomainDiskDiscard discard, + virDomainDiskDetectZeroes detect_zeroes); #endif /* __DOMAIN_CONF_H */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 96a0a6de52..8d5509a8a8 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -326,6 +326,7 @@ virDomainDiskErrorPolicyTypeToString; virDomainDiskFindByBusAndDst; virDomainDiskGeometryTransTypeFromString; virDomainDiskGeometryTransTypeToString; +virDomainDiskGetDetectZeroesMode; virDomainDiskGetDriver; virDomainDiskGetFormat; virDomainDiskGetSource; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 89fd08b642..6a13714eae 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1729,6 +1729,8 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, virQEMUCapsPtr qemuCaps) { virBuffer opt = VIR_BUFFER_INITIALIZER; + int detect_zeroes = virDomainDiskGetDetectZeroesMode(disk->discard, + disk->detect_zeroes); if (qemuBuildDriveSourceStr(disk, qemuCaps, &opt) < 0) goto error; @@ -1809,19 +1811,6 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, } if (disk->detect_zeroes) { - int detect_zeroes = disk->detect_zeroes; - - /* - * As a convenience syntax, if discards are ignored and - * zero detection is set to 'unmap', then simply behave - * like zero detection is set to 'on'. But don't change - * it in the XML for easier adjustments. This behaviour - * is documented. - */ - if (disk->discard != VIR_DOMAIN_DISK_DISCARD_UNMAP && - detect_zeroes == VIR_DOMAIN_DISK_DETECT_ZEROES_UNMAP) - detect_zeroes = VIR_DOMAIN_DISK_DETECT_ZEROES_ON; - virBufferAsprintf(&opt, ",detect-zeroes=%s", virDomainDiskDetectZeroesTypeToString(detect_zeroes)); } -- 2.16.2
On Thu, Mar 29, 2018 at 01:51:01PM +0200, Peter Krempa wrote:
For some reason we've decided to silently translate the disk detect_zeroes mode if it would be invalid. Extract the logic so that it does not need to be copypasta'd across the code base.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 22 ++++++++++++++++++++++ src/conf/domain_conf.h | 3 +++ src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 15 ++------------- 4 files changed, 28 insertions(+), 13 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ac3a3d9966..ef16431aaf 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -29362,3 +29362,25 @@ virDomainDiskTranslateSourcePool(virDomainDiskDefPtr def) virStoragePoolDefFree(pooldef); return ret; } + + +/** + * virDomainDiskGetDetectZeroesMode: + * @discard: disk/image sector discard setting + * @detect_zeroes: disk/image zero sector detection mode + * + * As a convenience syntax, if discards are ignored and zero detection is set + * to 'unmap', then simply behave like zero detection is set to 'on'. But + * don't change it in the XML for easier adjustments. This behaviour is + * documented.
Ah, that makes it okay.
+ */ +int +virDomainDiskGetDetectZeroesMode(virDomainDiskDiscard discard, + virDomainDiskDetectZeroes detect_zeroes) +{ + if (discard != VIR_DOMAIN_DISK_DISCARD_UNMAP && + detect_zeroes == VIR_DOMAIN_DISK_DETECT_ZEROES_UNMAP) + return VIR_DOMAIN_DISK_DETECT_ZEROES_ON; + + return detect_zeroes; +} diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 89fd08b642..6a13714eae 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1729,6 +1729,8 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, virQEMUCapsPtr qemuCaps) { virBuffer opt = VIR_BUFFER_INITIALIZER; + int detect_zeroes = virDomainDiskGetDetectZeroesMode(disk->discard, + disk->detect_zeroes);
if (qemuBuildDriveSourceStr(disk, qemuCaps, &opt) < 0) goto error; @@ -1809,19 +1811,6 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, }
if (disk->detect_zeroes) {
If you created an "accessor" for it, it should be used in the above condition as well. ACK Jano
It will be necessary to initialize various aspects for the detected members of the backing chain. Add a function that will handle it and call it from qemuDomainPrepareDiskSource and qemuDomainDetermineDiskChain Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 48 ++++++++++++++++++++++++++++++++++++++++++------ src/qemu/qemu_domain.h | 6 ++++++ 2 files changed, 48 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 580e0f830d..009fb9daf3 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -7399,6 +7399,7 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver, { virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); virStorageSourcePtr src = disk->src; + qemuDomainObjPrivatePtr priv = vm->privateData; int ret = -1; uid_t uid; gid_t gid; @@ -7467,6 +7468,10 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver, report_broken) < 0) goto cleanup; + /* fill in data for the rest of the chain */ + if (qemuDomainPrepareDiskSourceChain(disk, src, cfg, priv->qemuCaps) < 0) + goto cleanup; + ret = 0; cleanup: @@ -11803,6 +11808,41 @@ qemuDomainCheckMigrationCapabilities(virQEMUDriverPtr driver, } +/** + * qemuDomainPrepareDiskSourceChain: + * + * @disk: Disk config object + * @src: source to start from + * @cfg: qemu driver config object + * + * Prepares various aspects of the disk source and it's backing chain. This + * function should be also called for detected backing chains. If @src is NULL + * the root source is used. + */ +int +qemuDomainPrepareDiskSourceChain(virDomainDiskDefPtr disk, + virStorageSourcePtr src, + virQEMUDriverConfigPtr cfg, + virQEMUCapsPtr qemuCaps) +{ + virStorageSourcePtr n; + + if (!src) + src = disk->src; + + for (n = src; virStorageSourceIsBacking(n); n = n->backingStore) { + if (n->type == VIR_STORAGE_TYPE_NETWORK && + n->protocol == VIR_STORAGE_NET_PROTOCOL_GLUSTER && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_GLUSTER_DEBUG_LEVEL)) { + n->debug = true; + n->debugLevel = cfg->glusterDebugLevel; + } + } + + return 0; +} + + int qemuDomainPrepareDiskSource(virDomainDiskDefPtr disk, qemuDomainObjPrivatePtr priv, @@ -11814,12 +11854,8 @@ qemuDomainPrepareDiskSource(virDomainDiskDefPtr disk, if (qemuDomainSecretDiskPrepare(priv, disk) < 0) return -1; - if (disk->src->type == VIR_STORAGE_TYPE_NETWORK && - disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_GLUSTER && - virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_GLUSTER_DEBUG_LEVEL)) { - disk->src->debug = true; - disk->src->debugLevel = cfg->glusterDebugLevel; - } + if (qemuDomainPrepareDiskSourceChain(disk, NULL, cfg, priv->qemuCaps) < 0) + return -1; return 0; } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 63d9fb6d21..21e12f6594 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -991,6 +991,12 @@ int qemuDomainCheckMigrationCapabilities(virQEMUDriverPtr driver, virDomainObjPtr vm, qemuDomainAsyncJob asyncJob); +int +qemuDomainPrepareDiskSourceChain(virDomainDiskDefPtr disk, + virStorageSourcePtr src, + virQEMUDriverConfigPtr cfg, + virQEMUCapsPtr qemuCaps) + ATTRIBUTE_RETURN_CHECK; int qemuDomainPrepareDiskSource(virDomainDiskDefPtr disk, -- 2.16.2
On Thu, Mar 29, 2018 at 01:51:02PM +0200, Peter Krempa wrote:
It will be necessary to initialize various aspects for the detected members of the backing chain. Add a function that will handle it and call it from qemuDomainPrepareDiskSource and qemuDomainDetermineDiskChain
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 48 ++++++++++++++++++++++++++++++++++++++++++------ src/qemu/qemu_domain.h | 6 ++++++ 2 files changed, 48 insertions(+), 6 deletions(-)
ACK Jano
QEMU does not support it so save us hassle and forbid it right away. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 23 +++++++++++++++++++++++ tests/qemuxml2argvdata/disk-drive-fmt-cow.xml | 27 +++++++++++++++++++++++++++ tests/qemuxml2argvtest.c | 1 + 3 files changed, 51 insertions(+) create mode 100644 tests/qemuxml2argvdata/disk-drive-fmt-cow.xml diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 009fb9daf3..960a78f082 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4108,9 +4108,24 @@ qemuDomainDeviceDefValidateVideo(const virDomainVideoDef *video) } +static int +qemuDomainValidateStorageSource(virStorageSourcePtr src) +{ + if (src->format == VIR_STORAGE_FILE_COW) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("'cow' storage format is not supported")); + return -1; + } + + return 0; +} + + static int qemuDomainDeviceDefValidateDisk(const virDomainDiskDef *disk) { + virStorageSourcePtr n; + if (disk->src->shared && !disk->src->readonly) { if (disk->src->format <= VIR_STORAGE_FILE_AUTO) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -4127,6 +4142,11 @@ qemuDomainDeviceDefValidateDisk(const virDomainDiskDef *disk) } } + for (n = disk->src; virStorageSourceIsBacking(n); n = n->backingStore) { + if (qemuDomainValidateStorageSource(n) < 0) + return -1; + } + return 0; } @@ -11837,6 +11857,9 @@ qemuDomainPrepareDiskSourceChain(virDomainDiskDefPtr disk, n->debug = true; n->debugLevel = cfg->glusterDebugLevel; } + + if (qemuDomainValidateStorageSource(n) < 0) + return -1; } return 0; diff --git a/tests/qemuxml2argvdata/disk-drive-fmt-cow.xml b/tests/qemuxml2argvdata/disk-drive-fmt-cow.xml new file mode 100644 index 0000000000..91c62ea055 --- /dev/null +++ b/tests/qemuxml2argvdata/disk-drive-fmt-cow.xml @@ -0,0 +1,27 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <disk type='block' device='disk'> + <driver name='qemu' type='cow'/> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 308d71f725..5d7f2e15ba 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -912,6 +912,7 @@ mymain(void) QEMU_CAPS_NODEFCONFIG); DO_TEST("disk-drive-fmt-qcow", QEMU_CAPS_DRIVE_BOOT); + DO_TEST_PARSE_ERROR("disk-drive-fmt-cow", QEMU_CAPS_DRIVE_BOOT); DO_TEST("disk-drive-shared", QEMU_CAPS_DRIVE_SERIAL); DO_TEST_PARSE_ERROR("disk-drive-shared-qcow", NONE); -- 2.16.2
On Thu, Mar 29, 2018 at 01:51:03PM +0200, Peter Krempa wrote:
QEMU does not support it so save us hassle and forbid it right away.
*the hassle
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 23 +++++++++++++++++++++++ tests/qemuxml2argvdata/disk-drive-fmt-cow.xml | 27 +++++++++++++++++++++++++++ tests/qemuxml2argvtest.c | 1 + 3 files changed, 51 insertions(+) create mode 100644 tests/qemuxml2argvdata/disk-drive-fmt-cow.xml
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 009fb9daf3..960a78f082 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4108,9 +4108,24 @@ qemuDomainDeviceDefValidateVideo(const virDomainVideoDef *video) }
+static int +qemuDomainValidateStorageSource(virStorageSourcePtr src) +{ + if (src->format == VIR_STORAGE_FILE_COW) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("'cow' storage format is not supported"));
You could also say '🐄'.
+ return -1; + } + + return 0; +} + +
ACK regardless Jano
This is a storage driver type, which is not handled in qemu driver properly. For accessing directories, disk type 'dir' is used instead. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 7 +++++++ tests/qemuxml2argvdata/disk-drive-fmt-dir.xml | 27 +++++++++++++++++++++++++++ tests/qemuxml2argvtest.c | 1 + 3 files changed, 35 insertions(+) create mode 100644 tests/qemuxml2argvdata/disk-drive-fmt-dir.xml diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 960a78f082..88b4653da5 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4117,6 +4117,13 @@ qemuDomainValidateStorageSource(virStorageSourcePtr src) return -1; } + if (src->format == VIR_STORAGE_FILE_DIR) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("'directory' storage format is not directly suppored by qemu, " + "use 'dir' disk type instead")); + return -1; + } + return 0; } diff --git a/tests/qemuxml2argvdata/disk-drive-fmt-dir.xml b/tests/qemuxml2argvdata/disk-drive-fmt-dir.xml new file mode 100644 index 0000000000..ede3f8e186 --- /dev/null +++ b/tests/qemuxml2argvdata/disk-drive-fmt-dir.xml @@ -0,0 +1,27 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <disk type='block' device='disk'> + <driver name='qemu' type='dir'/> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 5d7f2e15ba..711bd1b661 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -913,6 +913,7 @@ mymain(void) DO_TEST("disk-drive-fmt-qcow", QEMU_CAPS_DRIVE_BOOT); DO_TEST_PARSE_ERROR("disk-drive-fmt-cow", QEMU_CAPS_DRIVE_BOOT); + DO_TEST_PARSE_ERROR("disk-drive-fmt-dir", QEMU_CAPS_DRIVE_BOOT); DO_TEST("disk-drive-shared", QEMU_CAPS_DRIVE_SERIAL); DO_TEST_PARSE_ERROR("disk-drive-shared-qcow", NONE); -- 2.16.2
On Thu, Mar 29, 2018 at 01:51:04PM +0200, Peter Krempa wrote:
This is a storage driver type, which is not handled in qemu driver properly. For accessing directories, disk type 'dir' is used instead.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 7 +++++++ tests/qemuxml2argvdata/disk-drive-fmt-dir.xml | 27 +++++++++++++++++++++++++++ tests/qemuxml2argvtest.c | 1 + 3 files changed, 35 insertions(+) create mode 100644 tests/qemuxml2argvdata/disk-drive-fmt-dir.xml
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 960a78f082..88b4653da5 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4117,6 +4117,13 @@ qemuDomainValidateStorageSource(virStorageSourcePtr src) return -1; }
+ if (src->format == VIR_STORAGE_FILE_DIR) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("'directory' storage format is not directly suppored by qemu, " + "use 'dir' disk type instead"));
No idea how to emojify that. ACK Jano
+ return -1; + } + return 0; }
This format is used by the storage driver and other hypervisors but qemu does not have nothion of the 'iso' format and libvirt does not translate it to anything useful, so it would not work anyways. Users should use 'raw' instead. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 7 +++++++ tests/qemuxml2argvdata/disk-drive-fmt-iso.xml | 27 +++++++++++++++++++++++++++ tests/qemuxml2argvtest.c | 1 + 3 files changed, 35 insertions(+) create mode 100644 tests/qemuxml2argvdata/disk-drive-fmt-iso.xml diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 88b4653da5..e02da9e608 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4124,6 +4124,13 @@ qemuDomainValidateStorageSource(virStorageSourcePtr src) return -1; } + if (src->format == VIR_STORAGE_FILE_ISO) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("storage format 'iso' is not directly suppored by qemu, " + "use 'raw' instead")); + return -1; + } + return 0; } diff --git a/tests/qemuxml2argvdata/disk-drive-fmt-iso.xml b/tests/qemuxml2argvdata/disk-drive-fmt-iso.xml new file mode 100644 index 0000000000..ad2825bfb2 --- /dev/null +++ b/tests/qemuxml2argvdata/disk-drive-fmt-iso.xml @@ -0,0 +1,27 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <disk type='file' device='disk'> + <driver name='qemu' type='iso'/> + <source file='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 711bd1b661..3d278c4bc2 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -914,6 +914,7 @@ mymain(void) QEMU_CAPS_DRIVE_BOOT); DO_TEST_PARSE_ERROR("disk-drive-fmt-cow", QEMU_CAPS_DRIVE_BOOT); DO_TEST_PARSE_ERROR("disk-drive-fmt-dir", QEMU_CAPS_DRIVE_BOOT); + DO_TEST_PARSE_ERROR("disk-drive-fmt-iso", QEMU_CAPS_DRIVE_BOOT); DO_TEST("disk-drive-shared", QEMU_CAPS_DRIVE_SERIAL); DO_TEST_PARSE_ERROR("disk-drive-shared-qcow", NONE); -- 2.16.2
On Thu, Mar 29, 2018 at 01:51:05PM +0200, Peter Krempa wrote:
This format is used by the storage driver and other hypervisors but qemu does not have nothion of the 'iso' format and libvirt does not translate
s/nothion/notion/
it to anything useful, so it would not work anyways. Users should use 'raw' instead.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 7 +++++++ tests/qemuxml2argvdata/disk-drive-fmt-iso.xml | 27 +++++++++++++++++++++++++++ tests/qemuxml2argvtest.c | 1 + 3 files changed, 35 insertions(+) create mode 100644 tests/qemuxml2argvdata/disk-drive-fmt-iso.xml
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 88b4653da5..e02da9e608 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4124,6 +4124,13 @@ qemuDomainValidateStorageSource(virStorageSourcePtr src) return -1; }
+ if (src->format == VIR_STORAGE_FILE_ISO) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("storage format 'iso' is not directly suppored by qemu, "
s/suppored/supported/
+ "use 'raw' instead")); + return -1;
ACK Jano
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_2.10.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml | 1 + 11 files changed, 12 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index e54dde69ab..959c27f3bf 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -466,6 +466,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, /* 285 */ "virtio-mouse-ccw", "virtio-tablet-ccw", + "qcow2-luks", ); @@ -1849,6 +1850,7 @@ static struct virQEMUCapsStringFlags virQEMUCapsQMPSchemaQueries[] = { { "blockdev-add/arg-type/+gluster/debug", QEMU_CAPS_GLUSTER_DEBUG_LEVEL}, { "blockdev-add/arg-type/+vxhs", QEMU_CAPS_VXHS}, { "blockdev-add/arg-type/+iscsi/password-secret", QEMU_CAPS_ISCSI_PASSWORD_SECRET }, + { "blockdev-add/arg-type/+qcow2/encrypt/+luks/key-secret", QEMU_CAPS_QCOW2_LUKS }, }; struct virQEMUCapsObjectTypeProps { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 3f3c29f8fb..2203c28aa0 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -450,6 +450,7 @@ typedef enum { /* 285 */ QEMU_CAPS_DEVICE_VIRTIO_MOUSE_CCW, /* -device virtio-mouse-ccw */ QEMU_CAPS_DEVICE_VIRTIO_TABLET_CCW, /* -device virtio-tablet-ccw */ + QEMU_CAPS_QCOW2_LUKS, /* qcow2 format support LUKS encryption */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_2.10.0.aarch64.xml index 17d388e815..7585e02da2 100644 --- a/tests/qemucapabilitiesdata/caps_2.10.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_2.10.0.aarch64.xml @@ -186,6 +186,7 @@ <flag name='isa-serial'/> <flag name='pl011'/> <flag name='dump-completed'/> + <flag name='qcow2-luks'/> <version>2010000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>303541</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml index 403b538628..153e199c41 100644 --- a/tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml @@ -185,6 +185,7 @@ <flag name='isa-serial'/> <flag name='machine.pseries.max-cpu-compat'/> <flag name='dump-completed'/> + <flag name='qcow2-luks'/> <version>2010000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>382824</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml index 1096495f00..c4be3fca51 100644 --- a/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml @@ -146,6 +146,7 @@ <flag name='disk-share-rw'/> <flag name='iscsi.password-secret'/> <flag name='dump-completed'/> + <flag name='qcow2-luks'/> <version>2010000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>303326</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml index dacb5c4599..4dd5602014 100644 --- a/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml @@ -229,6 +229,7 @@ <flag name='iscsi.password-secret'/> <flag name='isa-serial'/> <flag name='dump-completed'/> + <flag name='qcow2-luks'/> <version>2010000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>344938</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml index 70a35ef507..cbd645ae93 100644 --- a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml @@ -150,6 +150,7 @@ <flag name='virtio-keyboard-ccw'/> <flag name='virtio-mouse-ccw'/> <flag name='virtio-tablet-ccw'/> + <flag name='qcow2-luks'/> <version>2011000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>342058</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml index ff48293656..ec2eec17f4 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml @@ -187,6 +187,7 @@ <flag name='isa-serial'/> <flag name='pl011'/> <flag name='dump-completed'/> + <flag name='qcow2-luks'/> <version>2011090</version> <kvmVersion>0</kvmVersion> <microcodeVersion>342346</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml index ee7fb9e052..1122d6408b 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml @@ -185,6 +185,7 @@ <flag name='isa-serial'/> <flag name='machine.pseries.max-cpu-compat'/> <flag name='dump-completed'/> + <flag name='qcow2-luks'/> <version>2011090</version> <kvmVersion>0</kvmVersion> <microcodeVersion>419215</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml index b5b6b5b3b1..191b1e0e37 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml @@ -150,6 +150,7 @@ <flag name='virtio-keyboard-ccw'/> <flag name='virtio-mouse-ccw'/> <flag name='virtio-tablet-ccw'/> + <flag name='qcow2-luks'/> <version>2011090</version> <kvmVersion>0</kvmVersion> <microcodeVersion>0</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml index 334296e213..aa5de811e1 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml @@ -225,6 +225,7 @@ <flag name='iscsi.password-secret'/> <flag name='isa-serial'/> <flag name='dump-completed'/> + <flag name='qcow2-luks'/> <version>2011090</version> <kvmVersion>0</kvmVersion> <microcodeVersion>390060</microcodeVersion> -- 2.16.2
On Thu, Mar 29, 2018 at 01:51:06PM +0200, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_2.10.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml | 1 + 11 files changed, 12 insertions(+)
ACK Jano
Reject configurations when qemu would not support the image. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index e02da9e608..cbad7d0f4c 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4109,7 +4109,8 @@ qemuDomainDeviceDefValidateVideo(const virDomainVideoDef *video) static int -qemuDomainValidateStorageSource(virStorageSourcePtr src) +qemuDomainValidateStorageSource(virStorageSourcePtr src, + virQEMUCapsPtr qemuCaps) { if (src->format == VIR_STORAGE_FILE_COW) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -4131,12 +4132,22 @@ qemuDomainValidateStorageSource(virStorageSourcePtr src) return -1; } + if (src->format == VIR_STORAGE_FILE_QCOW2 && + src->encryption && + src->encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_QCOW2_LUKS)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("LUKS encrypted QCOW2 images are not suppored by this qemu")); + return -1; + } + return 0; } static int -qemuDomainDeviceDefValidateDisk(const virDomainDiskDef *disk) +qemuDomainDeviceDefValidateDisk(const virDomainDiskDef *disk, + virQEMUCapsPtr qemuCaps) { virStorageSourcePtr n; @@ -4157,7 +4168,7 @@ qemuDomainDeviceDefValidateDisk(const virDomainDiskDef *disk) } for (n = disk->src; virStorageSourceIsBacking(n); n = n->backingStore) { - if (qemuDomainValidateStorageSource(n) < 0) + if (qemuDomainValidateStorageSource(n, qemuCaps) < 0) return -1; } @@ -4988,7 +4999,7 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev, break; case VIR_DOMAIN_DEVICE_DISK: - ret = qemuDomainDeviceDefValidateDisk(dev->data.disk); + ret = qemuDomainDeviceDefValidateDisk(dev->data.disk, qemuCaps); break; case VIR_DOMAIN_DEVICE_CONTROLLER: @@ -11872,7 +11883,7 @@ qemuDomainPrepareDiskSourceChain(virDomainDiskDefPtr disk, n->debugLevel = cfg->glusterDebugLevel; } - if (qemuDomainValidateStorageSource(n) < 0) + if (qemuDomainValidateStorageSource(n, qemuCaps) < 0) return -1; } -- 2.16.2
On Thu, Mar 29, 2018 at 01:51:07PM +0200, Peter Krempa wrote:
Reject configurations when qemu would not support the image.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index e02da9e608..cbad7d0f4c 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4109,7 +4109,8 @@ qemuDomainDeviceDefValidateVideo(const virDomainVideoDef *video)
static int -qemuDomainValidateStorageSource(virStorageSourcePtr src) +qemuDomainValidateStorageSource(virStorageSourcePtr src, + virQEMUCapsPtr qemuCaps) { if (src->format == VIR_STORAGE_FILE_COW) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -4131,12 +4132,22 @@ qemuDomainValidateStorageSource(virStorageSourcePtr src) return -1; }
+ if (src->format == VIR_STORAGE_FILE_QCOW2 && + src->encryption && + src->encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_QCOW2_LUKS)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("LUKS encrypted QCOW2 images are not suppored by this qemu"));
s/qemu/QEMU/ ACK Jano
The old qcow2 encryption format was buggy, so the new approach is to use luks inside qcow2. As it turns out, it didn't require that many changes. It was necessary to fix the command line formatter to stop mangling the format when secrets are present and specify the encryption format and secret in correct format. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- You can easily create a qcow2+luks disk image by: qemu-img create --object secret,id=sec0,data=asdf -f qcow2 \ -o encrypt.format=luks,encrypt.key-secret=sec0 luks.qcow2 10M src/qemu/qemu_command.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6a13714eae..c1225591b3 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1525,6 +1525,7 @@ qemuBuildDriveSourceStr(virDomainDiskDefPtr disk, qemuDomainSecretInfoPtr encinfo = NULL; virJSONValuePtr srcprops = NULL; char *source = NULL; + bool rawluks = false; int ret = -1; if (srcpriv) { @@ -1598,14 +1599,21 @@ qemuBuildDriveSourceStr(virDomainDiskDefPtr disk, } virBufferAddLit(buf, ","); - if (encinfo) - virQEMUBuildLuksOpts(buf, &disk->src->encryption->encinfo, - encinfo->s.aes.alias); + if (encinfo) { + if (disk->src->format == VIR_STORAGE_FILE_RAW) { + virBufferAsprintf(buf, "key-secret=%s,", encinfo->s.aes.alias); + rawluks = true; + } else if (disk->src->format == VIR_STORAGE_FILE_QCOW2 && + disk->src->encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) { + virBufferAddLit(buf, "encrypt.format=luks,"); + virBufferAsprintf(buf, "encrypt.key-secret=%s,", encinfo->s.aes.alias); + } + } if (disk->src->format > 0 && disk->src->type != VIR_STORAGE_TYPE_DIR) { const char *qemuformat = virStorageFileFormatTypeToString(disk->src->format); - if (qemuDomainDiskHasEncryptionSecret(disk->src)) + if (rawluks) qemuformat = "luks"; virBufferAsprintf(buf, "format=%s,", qemuformat); } -- 2.16.2
On Thu, Mar 29, 2018 at 01:51:08PM +0200, Peter Krempa wrote:
The old qcow2 encryption format was buggy, so the new approach is to use luks inside qcow2. As it turns out, it didn't require that many changes.
It was necessary to fix the command line formatter to stop mangling the format when secrets are present and specify the encryption format and secret in correct format.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- You can easily create a qcow2+luks disk image by:
qemu-img create --object secret,id=sec0,data=asdf -f qcow2 \ -o encrypt.format=luks,encrypt.key-secret=sec0 luks.qcow2 10M
src/qemu/qemu_command.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-)
ACK Jano
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- .../qemuxml2argvdata/luks-disks-source-qcow2.args | 66 ++++++++++++++++++ tests/qemuxml2argvdata/luks-disks-source-qcow2.xml | 81 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 + 3 files changed, 149 insertions(+) create mode 100644 tests/qemuxml2argvdata/luks-disks-source-qcow2.args create mode 100644 tests/qemuxml2argvdata/luks-disks-source-qcow2.xml diff --git a/tests/qemuxml2argvdata/luks-disks-source-qcow2.args b/tests/qemuxml2argvdata/luks-disks-source-qcow2.args new file mode 100644 index 0000000000..001fc659bc --- /dev/null +++ b/tests/qemuxml2argvdata/luks-disks-source-qcow2.args @@ -0,0 +1,66 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-x86_64 \ +-name encryptdisk \ +-S \ +-object secret,id=masterKey0,format=raw,\ +file=/tmp/lib/domain--1-encryptdisk/master-key.aes \ +-M pc-i440fx-2.1 \ +-m 1024 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid 496898a6-e6ff-f7c8-5dc2-3cf410945ee9 \ +-nographic \ +-nodefaults \ +-chardev socket,id=charmonitor,\ +path=/tmp/lib/domain--1-encryptdisk/monitor.sock,server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline \ +-no-acpi \ +-boot c \ +-usb \ +-object secret,id=virtio-disk0-luks-secret0,\ +data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\ +keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \ +-drive file=/storage/guest_disks/encryptdisk,encrypt.format=luks,\ +encrypt.key-secret=virtio-disk0-luks-secret0,format=qcow2,if=none,\ +id=drive-virtio-disk0 \ +-device virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,\ +id=virtio-disk0 \ +-object secret,id=virtio-disk1-luks-secret0,\ +data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\ +keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \ +-drive file=/storage/guest_disks/encryptdisk2,encrypt.format=luks,\ +encrypt.key-secret=virtio-disk1-luks-secret0,format=qcow2,if=none,\ +id=drive-virtio-disk1 \ +-device virtio-blk-pci,bus=pci.0,addr=0x5,drive=drive-virtio-disk1,\ +id=virtio-disk1 \ +-object secret,id=virtio-disk2-luks-secret0,\ +data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\ +keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \ +-drive file=iscsi://myname:AQCVn5hO6HzFAhAAq0NCv8jtJcIcE+HOBlMQ1A@example.org:\ +6000/iqn.1992-01.com.example%3Astorage/1,encrypt.format=luks,\ +encrypt.key-secret=virtio-disk2-luks-secret0,format=qcow2,if=none,\ +id=drive-virtio-disk2 \ +-device virtio-blk-pci,bus=pci.0,addr=0x6,drive=drive-virtio-disk2,\ +id=virtio-disk2 \ +-object secret,id=virtio-disk3-luks-secret0,\ +data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\ +keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \ +-drive file=iscsi://iscsi.example.com:3260/demo-target/3,encrypt.format=luks,\ +encrypt.key-secret=virtio-disk3-luks-secret0,format=qcow2,if=none,\ +id=drive-virtio-disk3 \ +-device virtio-blk-pci,bus=pci.0,addr=0x7,drive=drive-virtio-disk3,\ +id=virtio-disk3 \ +-object secret,id=virtio-disk4-luks-secret0,\ +data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\ +keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \ +-drive 'file=rbd:pool/image:auth_supported=none:mon_host=mon1.example.org\:\ +6321\;mon2.example.org\:6322\;mon3.example.org\:6322,encrypt.format=luks,\ +encrypt.key-secret=virtio-disk4-luks-secret0,format=qcow2,if=none,\ +id=drive-virtio-disk4' \ +-device virtio-blk-pci,bus=pci.0,addr=0x8,drive=drive-virtio-disk4,\ +id=virtio-disk4 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/luks-disks-source-qcow2.xml b/tests/qemuxml2argvdata/luks-disks-source-qcow2.xml new file mode 100644 index 0000000000..92b31fb8bd --- /dev/null +++ b/tests/qemuxml2argvdata/luks-disks-source-qcow2.xml @@ -0,0 +1,81 @@ +<domain type='qemu'> + <name>encryptdisk</name> + <uuid>496898a6-e6ff-f7c8-5dc2-3cf410945ee9</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>524288</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc-i440fx-2.1'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <disk type='file' device='disk'> + <driver name='qemu' type='qcow2'/> + <source file='/storage/guest_disks/encryptdisk'> + <encryption format='luks'> + <secret type='passphrase' uuid='0a81f5b2-8403-7b23-c8d6-21ccc2f80d6f'/> + </encryption> + </source> + <target dev='vda' bus='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </disk> + <disk type='file' device='disk'> + <driver name='qemu' type='qcow2'/> + <source file='/storage/guest_disks/encryptdisk2'> + <encryption format='luks'> + <secret type='passphrase' usage='/storage/guest_disks/encryptdisk2'/> + </encryption> + </source> + <target dev='vdb' bus='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> + </disk> + <disk type='network' device='disk'> + <driver name='qemu' type='qcow2'/> + <source protocol='iscsi' name='iqn.1992-01.com.example:storage/1'> + <host name='example.org' port='6000'/> + <auth username='myname'> + <secret type='iscsi' uuid='0a81f5b2-8403-7b23-c8d6-21ccc2f80e80'/> + </auth> + <encryption format='luks'> + <secret type='passphrase' uuid='0a81f5b2-8403-7b23-c8d6-21ccc2f80f77'/> + </encryption> + </source> + <target dev='vdc' bus='virtio'/> + </disk> + <disk type='volume' device='disk'> + <driver name='qemu' type='qcow2'/> + <source pool='pool-iscsi' volume='unit:0:0:3' mode='direct'> + <encryption format='luks'> + <secret type='passphrase' uuid='0a81f5b2-8403-7b23-c8d6-21ccc2f80f80'/> + </encryption> + </source> + <target dev='vdd' bus='virtio'/> + </disk> + <disk type='network' device='disk'> + <driver name='qemu' type='qcow2'/> + <source protocol='rbd' name='pool/image'> + <host name='mon1.example.org' port='6321'/> + <host name='mon2.example.org' port='6322'/> + <host name='mon3.example.org' port='6322'/> + <encryption format='luks'> + <secret type='passphrase' uuid='0a81f5b2-8403-7b23-c8d6-21ccc2f80fb0'/> + </encryption> + </source> + <target dev='vde' bus='virtio'/> + </disk> + <controller type='usb' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 3d278c4bc2..896d104593 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1700,6 +1700,8 @@ mymain(void) # ifdef HAVE_GNUTLS_CIPHER_ENCRYPT DO_TEST("luks-disks", QEMU_CAPS_OBJECT_SECRET); DO_TEST("luks-disks-source", QEMU_CAPS_OBJECT_SECRET); + DO_TEST_PARSE_ERROR("luks-disks-source-qcow2", QEMU_CAPS_OBJECT_SECRET); + DO_TEST("luks-disks-source-qcow2", QEMU_CAPS_OBJECT_SECRET, QEMU_CAPS_QCOW2_LUKS); # else DO_TEST_FAILURE("luks-disks", QEMU_CAPS_OBJECT_SECRET); # endif -- 2.16.2
On Thu, Mar 29, 2018 at 01:51:09PM +0200, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- .../qemuxml2argvdata/luks-disks-source-qcow2.args | 66 ++++++++++++++++++ tests/qemuxml2argvdata/luks-disks-source-qcow2.xml | 81 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 + 3 files changed, 149 insertions(+) create mode 100644 tests/qemuxml2argvdata/luks-disks-source-qcow2.args create mode 100644 tests/qemuxml2argvdata/luks-disks-source-qcow2.xml
ACK Jano
Add a new kind of XML output test for the files in qemuxml2argvtest where we can validate setup and defaults applied when starting up the VM. This is achieved by formatting of the definition processed by the qemuxml2argvtest into a XML and it's compared against files in qemuxml2startupxmloutdata. This test is automatically executed if the output file is present and it's skipped otherwise. The first example test case is created from 'disk-drive-shared' test case. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/Makefile.am | 1 + tests/qemuxml2argvtest.c | 42 +++++++++++++++- .../disk-drive-shared.xml | 56 ++++++++++++++++++++++ 3 files changed, 97 insertions(+), 2 deletions(-) create mode 100644 tests/qemuxml2startupxmloutdata/disk-drive-shared.xml diff --git a/tests/Makefile.am b/tests/Makefile.am index 289ef35bdd..f2f5caed4f 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -127,6 +127,7 @@ EXTRA_DIST = \ qemuhotplugtestdomains \ qemumonitorjsondata \ qemuxml2argvdata \ + qemuxml2startupxmloutdata \ qemuxml2xmloutdata \ qemustatusxml2xmldata \ qemuqapischema.json \ diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 896d104593..7e7a937c8d 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -277,6 +277,7 @@ struct testInfo { unsigned int flags; unsigned int parseFlags; bool skipLegacyCPUs; + virDomainObjPtr vm; }; @@ -402,9 +403,39 @@ testUpdateQEMUCaps(const struct testInfo *info, static int -testCompareXMLToArgv(const void *data) +testCompareXMLToStartupXML(const void *data) { const struct testInfo *info = data; + unsigned int format_flags = VIR_DOMAIN_DEF_FORMAT_SECURE; + char *xml = NULL; + char *actual = NULL; + int ret = -1; + + if (virAsprintf(&xml, "%s/qemuxml2startupxmloutdata/%s.xml", + abs_srcdir, info->name) < 0) + goto cleanup; + + if (!virFileExists(xml)) { + ret = EXIT_AM_SKIP; + goto cleanup; + } + + if (!(actual = virDomainDefFormat(info->vm->def, NULL, format_flags))) + goto cleanup; + + ret = virTestCompareToFile(actual, xml); + + cleanup: + VIR_FREE(xml); + VIR_FREE(actual); + return ret; +} + + +static int +testCompareXMLToArgv(const void *data) +{ + struct testInfo *info = (void *) data; char *xml = NULL; char *args = NULL; char *migrateURI = NULL; @@ -532,6 +563,9 @@ testCompareXMLToArgv(const void *data) ret = 0; } + if (!(flags & FLAG_EXPECT_FAILURE) && ret == 0) + VIR_STEAL_PTR(info->vm, vm); + cleanup: VIR_FREE(log); VIR_FREE(actualargv); @@ -625,7 +659,7 @@ mymain(void) do { \ static struct testInfo info = { \ name, NULL, migrateFrom, migrateFd, (flags), parseFlags, \ - false \ + false, NULL \ }; \ info.skipLegacyCPUs = skipLegacyCPUs; \ if (testInitQEMUCaps(&info, gic) < 0) \ @@ -634,7 +668,11 @@ mymain(void) if (virTestRun("QEMU XML-2-ARGV " name, \ testCompareXMLToArgv, &info) < 0) \ ret = -1; \ + if (info.vm && virTestRun("QEMU XML-2-startup-XML " name, \ + testCompareXMLToStartupXML, &info) < 0) \ + ret = -1; \ virObjectUnref(info.qemuCaps); \ + virObjectUnref(info.vm); \ } while (0) # define DO_TEST(name, ...) \ diff --git a/tests/qemuxml2startupxmloutdata/disk-drive-shared.xml b/tests/qemuxml2startupxmloutdata/disk-drive-shared.xml new file mode 100644 index 0000000000..f4d2871ae7 --- /dev/null +++ b/tests/qemuxml2startupxmloutdata/disk-drive-shared.xml @@ -0,0 +1,56 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <shareable/> + <serial>XYZXYZXYZYXXYZYZYXYZY</serial> + <alias name='ide0-0-0'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <disk type='block' device='cdrom'> + <driver name='qemu' type='raw'/> + <source dev='/dev/HostVG/QEMUGuest2'/> + <target dev='hdc' bus='ide'/> + <readonly/> + <alias name='ide0-1-0'/> + <address type='drive' controller='0' bus='1' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'> + <alias name='usb'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='ide' index='0'> + <alias name='ide'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='pci' index='0' model='pci-root'> + <alias name='pci.0'/> + </controller> + <input type='mouse' bus='ps2'> + <alias name='input0'/> + </input> + <input type='keyboard' bus='ps2'> + <alias name='input1'/> + </input> + <memballoon model='virtio'> + <alias name='balloon0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </memballoon> + </devices> +</domain> -- 2.16.2
On Thu, Mar 29, 2018 at 01:51:10PM +0200, Peter Krempa wrote:
Add a new kind of XML output test for the files in qemuxml2argvtest where we can validate setup and defaults applied when starting up the VM.
This is achieved by formatting of the definition processed by the qemuxml2argvtest into a XML and it's compared against files in qemuxml2startupxmloutdata. This test is automatically executed if the output file is present and it's skipped otherwise.
The first example test case is created from 'disk-drive-shared' test case.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/Makefile.am | 1 + tests/qemuxml2argvtest.c | 42 +++++++++++++++- .../disk-drive-shared.xml | 56 ++++++++++++++++++++++ 3 files changed, 97 insertions(+), 2 deletions(-) create mode 100644 tests/qemuxml2startupxmloutdata/disk-drive-shared.xml
@@ -402,9 +403,39 @@ testUpdateQEMUCaps(const struct testInfo *info, static int -testCompareXMLToArgv(const void *data) +testCompareXMLToStartupXML(const void *data) { const struct testInfo *info = data; + unsigned int format_flags = VIR_DOMAIN_DEF_FORMAT_SECURE; + char *xml = NULL; + char *actual = NULL; + int ret = -1; + + if (virAsprintf(&xml, "%s/qemuxml2startupxmloutdata/%s.xml", + abs_srcdir, info->name) < 0) + goto cleanup; + + if (!virFileExists(xml)) { + ret = EXIT_AM_SKIP; + goto cleanup; + } + + if (!(actual = virDomainDefFormat(info->vm->def, NULL, format_flags))) + goto cleanup; + + ret = virTestCompareToFile(actual, xml);
double space
+ + cleanup: + VIR_FREE(xml); + VIR_FREE(actual); + return ret; +} + +
ACK Jano
The qemu command line generator code set disk caching of shareable disks to 'none' when formatting the command line silently. Move this code to a common place when preparing the domain definition for startup so that it does not have to be duplicated. The new test case shows that the actual cache mode will now be recorded in the live XML definition. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 2 -- src/qemu/qemu_domain.c | 11 +++++++++++ tests/qemuxml2startupxmloutdata/disk-drive-shared.xml | 2 +- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c1225591b3..9e74ec64e9 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1804,8 +1804,6 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, if (disk->cachemode) { virBufferAsprintf(&opt, ",cache=%s", qemuDiskCacheV2TypeToString(disk->cachemode)); - } else if (disk->src->shared && !disk->src->readonly) { - virBufferAddLit(&opt, ",cache=none"); } if (disk->copy_on_read) { diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index cbad7d0f4c..d78a3cb255 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -11891,11 +11891,22 @@ qemuDomainPrepareDiskSourceChain(virDomainDiskDefPtr disk, } +static void +qemuDomainPrepareDiskCachemode(virDomainDiskDefPtr disk) +{ + if (disk->cachemode == VIR_DOMAIN_DISK_CACHE_DEFAULT && + disk->src->shared && !disk->src->readonly) + disk->cachemode = VIR_DOMAIN_DISK_CACHE_DISABLE; +} + + int qemuDomainPrepareDiskSource(virDomainDiskDefPtr disk, qemuDomainObjPrivatePtr priv, virQEMUDriverConfigPtr cfg) { + qemuDomainPrepareDiskCachemode(disk); + if (qemuDomainPrepareDiskSourceTLS(disk->src, cfg) < 0) return -1; diff --git a/tests/qemuxml2startupxmloutdata/disk-drive-shared.xml b/tests/qemuxml2startupxmloutdata/disk-drive-shared.xml index f4d2871ae7..60b7eca788 100644 --- a/tests/qemuxml2startupxmloutdata/disk-drive-shared.xml +++ b/tests/qemuxml2startupxmloutdata/disk-drive-shared.xml @@ -15,7 +15,7 @@ <devices> <emulator>/usr/bin/qemu-system-i686</emulator> <disk type='block' device='disk'> - <driver name='qemu' type='raw'/> + <driver name='qemu' type='raw' cache='none'/> <source dev='/dev/HostVG/QEMUGuest1'/> <target dev='hda' bus='ide'/> <shareable/> -- 2.16.2
On Thu, Mar 29, 2018 at 01:51:11PM +0200, Peter Krempa wrote:
The qemu command line generator code set disk caching of shareable disks to 'none' when formatting the command line silently. Move this code to a common place when preparing the domain definition for startup so that it does not have to be duplicated.
The new test case shows that the actual cache mode will now be recorded in the live XML definition.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 2 -- src/qemu/qemu_domain.c | 11 +++++++++++ tests/qemuxml2startupxmloutdata/disk-drive-shared.xml | 2 +- 3 files changed, 12 insertions(+), 3 deletions(-)
ACK Jano
participants (2)
-
Ján Tomko -
Peter Krempa