Since the virStorageEncryptionPtr encryption; is a member of _virStorageSource it really should be allowed to be a subelement of the disk <source> for various disk formats: Source{File|Dir|Block|Volume} SourceProtocol{RBD|ISCSI|NBD|Gluster|Simple|HTTP} NB: Simple includes sheepdog, ftp, ftps, tftp That way we can set up to allow the <encryption> element to be formatted within the disk source. For now just allow the format in the RNG and read it in domain_conf. Modify the qemuxml2argvtest to add a parse failure when there is an <encryption> as a child of <disk> *and* an <encryption> as a child of <source>. The virschematest will read the new test files and validate from a RNG viewpoint things are fine. The luks-disks-source file has various formats to test, but not all valid/invalid. Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx> --- docs/schemas/domaincommon.rng | 30 ++++++++ src/conf/domain_conf.c | 56 +++++++++++++-- .../qemuxml2argv-luks-disks-source-both.xml | 40 +++++++++++ .../qemuxml2argv-luks-disks-source.xml | 81 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 1 + 5 files changed, 202 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-luks-disks-source-both.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-luks-disks-source.xml diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 139f1eea2..d1ef25b7b 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1469,6 +1469,9 @@ <optional> <ref name="storageStartupPolicy"/> </optional> + <optional> + <ref name="encryption"/> + </optional> <zeroOrMore> <ref name='devSeclabel'/> </zeroOrMore> @@ -1490,6 +1493,9 @@ <optional> <ref name="storageStartupPolicy"/> </optional> + <optional> + <ref name="encryption"/> + </optional> <zeroOrMore> <ref name='devSeclabel'/> </zeroOrMore> @@ -1509,6 +1515,9 @@ <optional> <ref name="storageStartupPolicy"/> </optional> + <optional> + <ref name="encryption"/> + </optional> <empty/> </element> </optional> @@ -1581,6 +1590,9 @@ <optional> <ref name="diskAuth"/> </optional> + <optional> + <ref name="encryption"/> + </optional> <empty/> </interleave> </element> @@ -1598,6 +1610,9 @@ <optional> <ref name="diskAuth"/> </optional> + <optional> + <ref name="encryption"/> + </optional> </element> </define> @@ -1611,6 +1626,9 @@ </attribute> <attribute name="name"/> <ref name="diskSourceNetworkHost"/> + <optional> + <ref name="encryption"/> + </optional> </element> </define> @@ -1626,6 +1644,9 @@ </attribute> <attribute name="name"/> <ref name="diskSourceNetworkHost"/> + <optional> + <ref name="encryption"/> + </optional> </element> </define> @@ -1638,6 +1659,9 @@ <attribute name="name"/> </optional> <ref name="diskSourceNetworkHost"/> + <optional> + <ref name="encryption"/> + </optional> </element> </define> @@ -1650,6 +1674,9 @@ <oneOrMore> <ref name="diskSourceNetworkHost"/> </oneOrMore> + <optional> + <ref name="encryption"/> + </optional> </element> </define> @@ -1690,6 +1717,9 @@ <optional> <ref name="storageStartupPolicy"/> </optional> + <optional> + <ref name="encryption"/> + </optional> <zeroOrMore> <ref name='devSeclabel'/> </zeroOrMore> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a3900488f..2a52462d0 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8135,6 +8135,29 @@ virDomainDiskSourceAuthParse(xmlNodePtr node, } +static int +virDomainDiskSourceEncryptionParse(xmlNodePtr node, + virStorageEncryptionPtr *encryptionsrc) +{ + xmlNodePtr child; + virStorageEncryptionPtr encryption = NULL; + + for (child = node->children; child; child = child->next) { + if (child->type == XML_ELEMENT_NODE && + virXMLNodeNameEqual(child, "encryption")) { + + if (!(encryption = virStorageEncryptionParseNode(node->doc, child))) + return -1; + + *encryptionsrc = encryption; + return 0; + } + } + + return 0; +} + + int virDomainDiskSourceParse(xmlNodePtr node, xmlXPathContextPtr ctxt, @@ -8224,6 +8247,9 @@ virDomainDiskSourceParse(xmlNodePtr node, if (virDomainDiskSourceAuthParse(node, &src->auth) < 0) goto cleanup; + if (virDomainDiskSourceEncryptionParse(node, &src->encryption) < 0) + goto cleanup; + /* People sometimes pass a bogus '' source path when they mean to omit the * source element completely (e.g. CDROM without media). This is just a * little compatibility check to help those broken apps */ @@ -8798,6 +8824,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, char *bus = NULL; char *devaddr = NULL; virStorageEncryptionPtr encryption = NULL; + bool diskEncryption = false; char *serial = NULL; char *startupPolicy = NULL; virStorageAuthDefPtr authdef = NULL; @@ -8861,6 +8888,15 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, goto error; } + /* Similarly for <encryption> - it's a child of <source> too + * and we cannot find in both places */ + if (diskEncryption && def->src->encryption) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("an <encryption> definition already found for " + "the <disk> definition")); + goto error; + } + source = true; startupPolicy = virXMLPropString(cur, "startupPolicy"); @@ -8943,12 +8979,20 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, virXMLNodeNameEqual(cur, "state")) { /* Legacy back-compat. Don't add any more attributes here */ devaddr = virXMLPropString(cur, "devaddr"); - } else if (encryption == NULL && + } else if (!diskEncryption && virXMLNodeNameEqual(cur, "encryption")) { - encryption = virStorageEncryptionParseNode(node->doc, - cur); - if (encryption == NULL) + diskEncryption = true; + if (!(encryption = virStorageEncryptionParseNode(node->doc, cur))) goto error; + + /* If we've already parsed <source> and found an <encryption> child, + * then generate an error to avoid ambiguity */ + if (source && def->src->encryption) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("an <encryption> definition already found for " + "disk source")); + goto error; + } } else if (!serial && virXMLNodeNameEqual(cur, "serial")) { serial = (char *)xmlNodeGetContent(cur); @@ -9165,8 +9209,8 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, target = NULL; if (diskAuth) VIR_STEAL_PTR(def->src->auth, authdef); - def->src->encryption = encryption; - encryption = NULL; + if (diskEncryption) + VIR_STEAL_PTR(def->src->encryption, encryption); def->domain_name = domain_name; domain_name = NULL; def->serial = serial; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-luks-disks-source-both.xml b/tests/qemuxml2argvdata/qemuxml2argv-luks-disks-source-both.xml new file mode 100644 index 000000000..c4b762a1e --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-luks-disks-source-both.xml @@ -0,0 +1,40 @@ +<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='raw'/> + <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'/> + <encryption format='luks'> + <secret type='passphrase' uuid='0a81f5b2-8403-7b23-c8d6-21ccc2f80d6f'/> + </encryption> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </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/qemuxml2argvdata/qemuxml2argv-luks-disks-source.xml b/tests/qemuxml2argvdata/qemuxml2argv-luks-disks-source.xml new file mode 100644 index 000000000..293877df9 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-luks-disks-source.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='raw'/> + <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='raw'/> + <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='raw'/> + <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='raw'/> + <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='raw'/> + <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 69548cc15..9a8caaa38 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1652,6 +1652,7 @@ mymain(void) DO_TEST_FAILURE("luks-disks", QEMU_CAPS_OBJECT_SECRET); # endif DO_TEST_PARSE_ERROR("luks-disk-invalid", NONE); + DO_TEST_PARSE_ERROR("luks-disks-source-both", QEMU_CAPS_OBJECT_SECRET); DO_TEST("memtune", NONE); DO_TEST("memtune-unlimited", NONE); -- 2.13.5 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list