Try also the modern incarnation of the test. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- ...luks-disks-source-qcow2.x86_64-latest.args | 110 ++++++++++++++++++ tests/qemuxml2argvtest.c | 1 + 2 files changed, 111 insertions(+) create mode 100644 tests/qemuxml2argvdata/luks-disks-source-qcow2.x86_64-latest.args diff --git a/tests/qemuxml2argvdata/luks-disks-source-qcow2.x86_64-latest.args b/tests/qemuxml2argvdata/luks-disks-source-qcow2.x86_64-latest.args new file mode 100644 index 0000000000..021bcb6961 --- /dev/null +++ b/tests/qemuxml2argvdata/luks-disks-source-qcow2.x86_64-latest.args @@ -0,0 +1,110 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-encryptdisk \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-encryptdisk/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-encryptdisk/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-encryptdisk/.config \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-x86_64 \ +-name guest=encryptdisk,debug-threads=on \ +-S \ +-object secret,id=masterKey0,format=raw,\ +file=/tmp/lib/domain--1-encryptdisk/master-key.aes \ +-machine pc-i440fx-2.1,accel=tcg,usb=off,dump-guest-core=off \ +-cpu qemu64 \ +-m 1024 \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid 496898a6-e6ff-f7c8-5dc2-3cf410945ee9 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot strict=on \ +-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ +-object secret,id=libvirt-7-format-luks-secret0,\ +data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\ +keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \ +-blockdev '{"driver":"file","filename":"/storage/guest_disks/encryptdisk",\ +"node-name":"libvirt-7-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-7-format","read-only":false,"driver":"qcow2",\ +"encrypt":{"format":"luks","key-secret":"libvirt-7-format-luks-secret0"},\ +"file":"libvirt-7-storage"}' \ +-device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x4,drive=libvirt-7-format,\ +id=virtio-disk0,bootindex=1 \ +-object secret,id=libvirt-6-format-luks-secret0,\ +data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\ +keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \ +-blockdev '{"driver":"file","filename":"/storage/guest_disks/encryptdisk2",\ +"node-name":"libvirt-6-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-6-format","read-only":false,"driver":"qcow2",\ +"encrypt":{"format":"luks","key-secret":"libvirt-6-format-luks-secret0"},\ +"file":"libvirt-6-storage"}' \ +-device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x5,drive=libvirt-6-format,\ +id=virtio-disk1 \ +-object secret,id=libvirt-5-storage-secret0,\ +data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\ +keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \ +-object secret,id=libvirt-5-format-luks-secret0,\ +data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\ +keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \ +-blockdev '{"driver":"iscsi","portal":"example.org:6000",\ +"target":"iqn.1992-01.com.example:storage","lun":1,"transport":"tcp",\ +"user":"myname","password-secret":"libvirt-5-storage-secret0",\ +"node-name":"libvirt-5-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-5-format","read-only":false,"driver":"qcow2",\ +"encrypt":{"format":"luks","key-secret":"libvirt-5-format-luks-secret0"},\ +"file":"libvirt-5-storage"}' \ +-device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x2,drive=libvirt-5-format,\ +id=virtio-disk2 \ +-object secret,id=libvirt-4-format-luks-secret0,\ +data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\ +keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \ +-blockdev '{"driver":"iscsi","portal":"iscsi.example.com:3260",\ +"target":"demo-target","lun":3,"transport":"tcp",\ +"node-name":"libvirt-4-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-4-format","read-only":false,"driver":"qcow2",\ +"encrypt":{"format":"luks","key-secret":"libvirt-4-format-luks-secret0"},\ +"file":"libvirt-4-storage"}' \ +-device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x6,drive=libvirt-4-format,\ +id=virtio-disk3 \ +-object secret,id=libvirt-3-format-luks-secret0,\ +data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\ +keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \ +-blockdev '{"driver":"rbd","pool":"pool","image":"image",\ +"server":[{"host":"mon1.example.org","port":"6321"},{"host":"mon2.example.org",\ +"port":"6322"},{"host":"mon3.example.org","port":"6322"}],\ +"node-name":"libvirt-3-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-3-format","read-only":false,"driver":"qcow2",\ +"encrypt":{"format":"luks","key-secret":"libvirt-3-format-luks-secret0"},\ +"file":"libvirt-3-storage"}' \ +-device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x7,drive=libvirt-3-format,\ +id=virtio-disk4 \ +-object secret,id=libvirt-2-format-luks-secret0,\ +data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\ +keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \ +-blockdev '{"driver":"file","filename":"/storage/guest_disks/base.qcow2",\ +"node-name":"libvirt-2-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-2-format","read-only":true,"driver":"qcow2",\ +"encrypt":{"format":"luks","key-secret":"libvirt-2-format-luks-secret0"},\ +"file":"libvirt-2-storage","backing":null}' \ +-object secret,id=libvirt-1-format-luks-secret0,\ +data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\ +keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \ +-blockdev '{"driver":"file","filename":"/storage/guest_disks/encryptdisk5",\ +"node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \ +-blockdev '{"node-name":"libvirt-1-format","read-only":false,"driver":"qcow2",\ +"encrypt":{"format":"luks","key-secret":"libvirt-1-format-luks-secret0"},\ +"file":"libvirt-1-storage","backing":"libvirt-2-format"}' \ +-device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x8,drive=libvirt-1-format,\ +id=virtio-disk5 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\ +resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 8af2ba38d3..47c35f0b91 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1757,6 +1757,7 @@ mymain(void) 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); + DO_TEST_CAPS_LATEST("luks-disks-source-qcow2"); DO_TEST_PARSE_ERROR("luks-disk-invalid", NONE); DO_TEST_PARSE_ERROR("luks-disks-source-both", QEMU_CAPS_OBJECT_SECRET); -- 2.24.1

Historically there are two places where we format authentication and encryption for a disk. The logich which formats it for backing files was flawed though and didn't format it at all. This worked if the image became a backing file through the means of a snapshot but not directly. Force formatting of the source and encryption for any non-disk case to fix the issue. This caused problems in many places as we use the formatter to copy the definition. Effectively any copy lost the secret definition. https://bugzilla.redhat.com/show_bug.cgi?id=1789310 https://bugzilla.redhat.com/show_bug.cgi?id=1788898 Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/conf/backup_conf.c | 2 +- src/conf/domain_conf.c | 13 ++++++++----- src/conf/domain_conf.h | 1 + src/conf/snapshot_conf.c | 2 +- src/qemu/qemu_domain.c | 4 ++-- tests/qemublocktest.c | 2 +- .../luks-disks-source-qcow2.x86_64-latest.xml | 6 +++++- tests/virstoragetest.c | 2 +- 8 files changed, 20 insertions(+), 12 deletions(-) diff --git a/src/conf/backup_conf.c b/src/conf/backup_conf.c index aa11967d2a..61dc8cd4b2 100644 --- a/src/conf/backup_conf.c +++ b/src/conf/backup_conf.c @@ -338,7 +338,7 @@ virDomainBackupDiskDefFormat(virBufferPtr buf, virStorageFileFormatTypeToString(disk->store->format)); if (virDomainDiskSourceFormat(&childBuf, disk->store, sourcename, - 0, false, storageSourceFormatFlags, NULL) < 0) + 0, false, storageSourceFormatFlags, true, NULL) < 0) return -1; } diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1290241923..ca5bbc3f35 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -24189,6 +24189,8 @@ virDomainDiskSourceFormatPrivateData(virBufferPtr buf, * @policy: startup policy attribute value, if necessary * @attrIndex: the 'index' attribute of <source> is formatted if true * @flags: XML formatter flags + * @formatsecrets: Force formatting of <auth> and <encryption> even if they + * were inherited * @xmlopt: XML formatter callbacks * * Formats @src into a <source> element. Note that this doesn't format the @@ -24201,6 +24203,7 @@ virDomainDiskSourceFormat(virBufferPtr buf, int policy, bool attrIndex, unsigned int flags, + bool formatsecrets, virDomainXMLOptionPtr xmlopt) { g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; @@ -24257,13 +24260,13 @@ virDomainDiskSourceFormat(virBufferPtr buf, * <auth> for a volume source type. The <auth> information is * kept in the storage pool and would be overwritten anyway. * So avoid formatting it for volumes. */ - if (src->auth && src->authInherited && + if (src->auth && (src->authInherited || formatsecrets) && src->type != VIR_STORAGE_TYPE_VOLUME) virStorageAuthDefFormat(&childBuf, src->auth); /* If we found encryption as a child of <source>, then format it * as we found it. */ - if (src->encryption && src->encryptionInherited && + if (src->encryption && (src->encryptionInherited || formatsecrets) && virStorageEncryptionFormat(&childBuf, src->encryption) < 0) return -1; @@ -24324,7 +24327,7 @@ virDomainDiskBackingStoreFormat(virBufferPtr buf, virBufferAsprintf(&childBuf, "<format type='%s'/>\n", virStorageFileFormatTypeToString(backingStore->format)); if (virDomainDiskSourceFormat(&childBuf, backingStore, "source", 0, false, - flags, xmlopt) < 0) + flags, true, xmlopt) < 0) return -1; if (virDomainDiskBackingStoreFormat(&childBuf, backingStore, xmlopt, flags) < 0) @@ -24486,7 +24489,7 @@ virDomainDiskDefFormatMirror(virBufferPtr buf, virBufferEscapeString(&childBuf, "<format type='%s'/>\n", formatStr); if (virDomainDiskSourceFormat(&childBuf, disk->mirror, "source", 0, true, - flags, xmlopt) < 0) + flags, true, xmlopt) < 0) return -1; if (virDomainDiskBackingStoreFormat(&childBuf, disk->mirror, xmlopt, flags) < 0) @@ -24585,7 +24588,7 @@ virDomainDiskDefFormat(virBufferPtr buf, virStorageAuthDefFormat(buf, def->src->auth); if (virDomainDiskSourceFormat(buf, def->src, "source", def->startupPolicy, - true, flags, xmlopt) < 0) + true, flags, false, xmlopt) < 0) return -1; /* Don't format backingStore to inactive XMLs until the code for diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 76d6ef8e48..6ae89fa498 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3118,6 +3118,7 @@ int virDomainDiskSourceFormat(virBufferPtr buf, int policy, bool attrIndex, unsigned int flags, + bool formatsecrets, virDomainXMLOptionPtr xmlopt); int diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index 2bd4d6a276..37b5c2fdf7 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -818,7 +818,7 @@ virDomainSnapshotDiskDefFormat(virBufferPtr buf, if (disk->src->format > 0) virBufferEscapeString(buf, "<driver type='%s'/>\n", virStorageFileFormatTypeToString(disk->src->format)); - if (virDomainDiskSourceFormat(buf, disk->src, "source", 0, false, 0, + if (virDomainDiskSourceFormat(buf, disk->src, "source", 0, false, 0, true, xmlopt) < 0) return -1; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 1f358544ab..a6dde15bad 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2547,7 +2547,7 @@ qemuDomainObjPrivateXMLFormatBlockjobFormatSource(virBufferPtr buf, virStorageTypeToString(src->type), virStorageFileFormatTypeToString(src->format)); - if (virDomainDiskSourceFormat(&childBuf, src, "source", 0, true, xmlflags, xmlopt) < 0) + if (virDomainDiskSourceFormat(&childBuf, src, "source", 0, true, xmlflags, true, xmlopt) < 0) return -1; if (chain && @@ -2746,7 +2746,7 @@ qemuDomainObjPrivateXMLFormatNBDMigrationSource(virBufferPtr buf, virStorageFileFormatTypeToString(src->format)); if (virDomainDiskSourceFormat(&childBuf, src, "source", 0, false, - VIR_DOMAIN_DEF_FORMAT_STATUS, xmlopt) < 0) + VIR_DOMAIN_DEF_FORMAT_STATUS, true, xmlopt) < 0) return -1; virXMLFormatElement(buf, "migrationSource", &attrBuf, &childBuf); diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index 3e9edb85f0..7ff6a6b17b 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -90,7 +90,7 @@ testBackingXMLjsonXML(const void *args) return -1; } - if (virDomainDiskSourceFormat(&buf, jsonsrc, "source", 0, false, 0, + if (virDomainDiskSourceFormat(&buf, jsonsrc, "source", 0, false, 0, true, NULL) < 0 || !(actualxml = virBufferContentAndReset(&buf))) { fprintf(stderr, "failed to format disk source xml\n"); diff --git a/tests/qemuxml2xmloutdata/luks-disks-source-qcow2.x86_64-latest.xml b/tests/qemuxml2xmloutdata/luks-disks-source-qcow2.x86_64-latest.xml index 33e6d02976..aa13fe5dfa 100644 --- a/tests/qemuxml2xmloutdata/luks-disks-source-qcow2.x86_64-latest.xml +++ b/tests/qemuxml2xmloutdata/luks-disks-source-qcow2.x86_64-latest.xml @@ -83,7 +83,11 @@ </source> <backingStore type='file'> <format type='qcow2'/> - <source file='/storage/guest_disks/base.qcow2'/> + <source file='/storage/guest_disks/base.qcow2'> + <encryption format='luks'> + <secret type='passphrase' uuid='0a81f5b2-8403-7b23-c8d6-21ccc2f80d6f'/> + </encryption> + </source> <backingStore/> </backingStore> <target dev='vdf' bus='virtio'/> diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 0e274ad1b7..2862758752 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -612,7 +612,7 @@ testBackingParse(const void *args) return -1; } - if (virDomainDiskSourceFormat(&buf, src, "source", 0, false, 0, NULL) < 0 || + if (virDomainDiskSourceFormat(&buf, src, "source", 0, false, 0, true, NULL) < 0 || !(xml = virBufferContentAndReset(&buf))) { fprintf(stderr, "failed to format disk source xml\n"); return -1; -- 2.24.1