11:35 a.m.
We special cased some behaviour and it broke on combination with other
special case.
Peter Krempa (4):
qemuxml2argvdata/disk-slices: Add test case for 'luks' encryption
qemu: block: Extract logic decision when to use a separate 'raw' layer
for slice
qemuBlockStorageSourceNeedsStorageSliceLayer: Deal with 'luks' files
qemu: block: Split up formatting of JSON props for 'raw' and 'luks'
drivers
src/qemu/qemu_block.c | 70 ++++++++++++++-----
src/qemu/qemu_block.h | 3 +
src/qemu/qemu_domain.c | 3 +-
.../disk-slices.x86_64-latest.args | 41 +++++++----
tests/qemuxml2argvdata/disk-slices.xml | 13 ++++
.../disk-slices.x86_64-latest.xml | 16 ++++-
6 files changed, 112 insertions(+), 34 deletions(-)
--
2.24.1
11:35 a.m.
New subject: [PATCH 1/4] qemuxml2argvdata/disk-slices: Add test case for 'luks' encryption
Since libvirt handles the luks encryption in a weird special way
(raw+encryption) we should really test that case with slices as well.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
.../disk-slices.x86_64-latest.args | 38 ++++++++++++-------
tests/qemuxml2argvdata/disk-slices.xml | 13 +++++++
.../disk-slices.x86_64-latest.xml | 16 +++++++-
3 files changed, 52 insertions(+), 15 deletions(-)
diff --git a/tests/qemuxml2argvdata/disk-slices.x86_64-latest.args
b/tests/qemuxml2argvdata/disk-slices.x86_64-latest.args
index 49daa293a4..75485c17a7 100644
--- a/tests/qemuxml2argvdata/disk-slices.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/disk-slices.x86_64-latest.args
@@ -29,25 +29,35 @@ file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
-boot strict=on \
-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \
-blockdev
'{"driver":"file","filename":"/var/lib/libvirt/images/raw.img",\
-"node-name":"libvirt-3-storage","auto-read-only":true,"discard":"unmap"}'
\
--blockdev
'{"node-name":"libvirt-3-format","read-only":false,"driver":"raw",\
-"offset":1234,"size":321,"file":"libvirt-3-storage"}'
\
--device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x2,drive=libvirt-3-format,\
+"node-name":"libvirt-4-storage","auto-read-only":true,"discard":"unmap"}'
\
+-blockdev
'{"node-name":"libvirt-4-format","read-only":false,"driver":"raw",\
+"offset":1234,"size":321,"file":"libvirt-4-storage"}'
\
+-device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x2,drive=libvirt-4-format,\
id=virtio-disk0,bootindex=1 \
-blockdev
'{"driver":"file","filename":"/var/lib/libvirt/images/raw.img",\
-"node-name":"libvirt-2-storage","auto-read-only":true,"discard":"unmap"}'
\
--blockdev
'{"driver":"raw","node-name":"libvirt-2-slice-sto","offset":9876,\
-"size":123456789,"file":"libvirt-2-storage","auto-read-only":true,\
+"node-name":"libvirt-3-storage","auto-read-only":true,"discard":"unmap"}'
\
+-blockdev
'{"driver":"raw","node-name":"libvirt-3-slice-sto","offset":9876,\
+"size":123456789,"file":"libvirt-3-storage","auto-read-only":true,\
"discard":"unmap"}' \
--blockdev
'{"node-name":"libvirt-2-format","read-only":true,"driver":"qcow2",\
-"file":"libvirt-2-slice-sto","backing":null}' \
+-blockdev
'{"node-name":"libvirt-3-format","read-only":true,"driver":"qcow2",\
+"file":"libvirt-3-slice-sto","backing":null}' \
-blockdev
'{"driver":"file","filename":"/var/lib/libvirt/images/overlay.qcow2",\
-"node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}'
\
--blockdev
'{"node-name":"libvirt-1-format","read-only":false,"driver":"qcow2",\
-"file":"libvirt-1-storage","backing":"libvirt-2-format"}'
\
--device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x3,drive=libvirt-1-format,\
+"node-name":"libvirt-2-storage","auto-read-only":true,"discard":"unmap"}'
\
+-blockdev
'{"node-name":"libvirt-2-format","read-only":false,"driver":"qcow2",\
+"file":"libvirt-2-storage","backing":"libvirt-3-format"}'
\
+-device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x3,drive=libvirt-2-format,\
id=virtio-disk1 \
--device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4 \
+-object secret,id=libvirt-1-format-encryption-secret0,\
+data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\
+keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \
+-blockdev
'{"driver":"file","filename":"/var/lib/libvirt/images/luks.img",\
+"node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}'
\
+-blockdev
'{"node-name":"libvirt-1-format","read-only":false,"driver":"luks",\
+"key-secret":"libvirt-1-format-encryption-secret0","offset":1234,"size":321,\
+"file":"libvirt-1-storage"}' \
+-device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x4,drive=libvirt-1-format,\
+id=virtio-disk2 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5 \
-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\
resourcecontrol=deny \
-msg timestamp=on
diff --git a/tests/qemuxml2argvdata/disk-slices.xml
b/tests/qemuxml2argvdata/disk-slices.xml
index cff7cc7ee4..5c6f29d154 100644
--- a/tests/qemuxml2argvdata/disk-slices.xml
+++ b/tests/qemuxml2argvdata/disk-slices.xml
@@ -38,6 +38,19 @@
</backingStore>
<target dev='vdb' bus='virtio'/>
</disk>
+ <disk type='file' device='disk'>
+ <driver name='qemu' type='raw'/>
+ <source file='/var/lib/libvirt/images/luks.img'>
+ <slices>
+ <slice type='storage' offset='1234' size='321'/>
+ </slices>
+ <encryption format='luks'>
+ <secret type='passphrase'
uuid='0a81f5b2-8403-7b23-c8d6-21ccc2f80d6f'/>
+ </encryption>
+ </source>
+ <backingStore/>
+ <target dev='vdc' bus='virtio'/>
+ </disk>
<controller type='usb'/>
<controller type='pci' model='pci-root'/>
<memballoon model='virtio'/>
diff --git a/tests/qemuxml2xmloutdata/disk-slices.x86_64-latest.xml
b/tests/qemuxml2xmloutdata/disk-slices.x86_64-latest.xml
index 65590df417..4f4abcda1f 100644
--- a/tests/qemuxml2xmloutdata/disk-slices.x86_64-latest.xml
+++ b/tests/qemuxml2xmloutdata/disk-slices.x86_64-latest.xml
@@ -43,6 +43,20 @@
<target dev='vdb' bus='virtio'/>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x03' function='0x0'/>
</disk>
+ <disk type='file' device='disk'>
+ <driver name='qemu' type='raw'/>
+ <source file='/var/lib/libvirt/images/luks.img'>
+ <slices>
+ <slice type='storage' offset='1234' size='321'/>
+ </slices>
+ <encryption format='luks'>
+ <secret type='passphrase'
uuid='0a81f5b2-8403-7b23-c8d6-21ccc2f80d6f'/>
+ </encryption>
+ </source>
+ <backingStore/>
+ <target dev='vdc' bus='virtio'/>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x04' function='0x0'/>
+ </disk>
<controller type='usb' index='0' model='piix3-uhci'>
<address type='pci' domain='0x0000' bus='0x00'
slot='0x01' function='0x2'/>
</controller>
@@ -50,7 +64,7 @@
<input type='mouse' bus='ps2'/>
<input type='keyboard' bus='ps2'/>
<memballoon model='virtio'>
- <address type='pci' domain='0x0000' bus='0x00'
slot='0x04' function='0x0'/>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x05' function='0x0'/>
</memballoon>
</devices>
</domain>
--
2.24.1
11:35 a.m.
New subject: [PATCH 2/4] qemu: block: Extract logic decision when to use a separate 'raw' layer for slice
Introduce qemuBlockStorageSourceNeedsStorageSliceLayer which will hold
the decision logic and fix all places that open-code it.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_block.c | 24 +++++++++++++++++++++---
src/qemu/qemu_block.h | 3 +++
src/qemu/qemu_domain.c | 3 +--
3 files changed, 25 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 4ed17b6df3..b5b34ab441 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -1446,8 +1446,7 @@ qemuBlockStorageSourceGetBlockdevProps(virStorageSourcePtr src,
g_autoptr(virJSONValue) props = NULL;
const char *storagenode = src->nodestorage;
- if (src->sliceStorage &&
- src->format != VIR_STORAGE_FILE_RAW)
+ if (qemuBlockStorageSourceNeedsStorageSliceLayer(src))
storagenode = src->sliceStorage->nodename;
if (!(props = qemuBlockStorageSourceGetBlockdevFormatProps(src)))
@@ -1568,7 +1567,7 @@ qemuBlockStorageSourceAttachPrepareBlockdev(virStorageSourcePtr
src,
data->storageNodeName = src->nodestorage;
data->formatNodeName = src->nodeformat;
- if (src->sliceStorage && src->format != VIR_STORAGE_FILE_RAW) {
+ if (qemuBlockStorageSourceNeedsStorageSliceLayer(src)) {
if (!(data->storageSliceProps =
qemuBlockStorageSourceGetBlockdevStorageSliceProps(src)))
return NULL;
@@ -3308,3 +3307,22 @@ qemuBlockReopenReadOnly(virDomainObjPtr vm,
return 0;
}
+
+/**
+ * qemuBlockStorageSourceNeedSliceLayer:
+ * @src: source to inspect
+ *
+ * Returns true if @src requires an extra 'raw' layer for handling of the
storage
+ * slice.
+ */
+bool
+qemuBlockStorageSourceNeedsStorageSliceLayer(const virStorageSource *src)
+{
+ if (!src->sliceStorage)
+ return false;
+
+ if (src->format != VIR_STORAGE_FILE_RAW)
+ return true;
+
+ return false;
+}
diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h
index 75b25bfea5..28475b25c1 100644
--- a/src/qemu/qemu_block.h
+++ b/src/qemu/qemu_block.h
@@ -254,3 +254,6 @@ int
qemuBlockReopenReadOnly(virDomainObjPtr vm,
virStorageSourcePtr src,
qemuDomainAsyncJob asyncJob);
+
+bool
+qemuBlockStorageSourceNeedsStorageSliceLayer(const virStorageSource *src);
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 0e2252f6cf..7dda986e3a 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -16590,8 +16590,7 @@ qemuDomainPrepareStorageSourceBlockdev(virDomainDiskDefPtr disk,
src->nodestorage = g_strdup_printf("libvirt-%u-storage", src->id);
src->nodeformat = g_strdup_printf("libvirt-%u-format", src->id);
- if (src->sliceStorage &&
- src->format != VIR_STORAGE_FILE_RAW)
+ if (qemuBlockStorageSourceNeedsStorageSliceLayer(src))
src->sliceStorage->nodename =
g_strdup_printf("libvirt-%u-slice-sto", src->id);
if (qemuDomainValidateStorageSource(src, priv->qemuCaps) < 0)
--
2.24.1
11:35 a.m.
New subject: [PATCH 3/4] qemuBlockStorageSourceNeedsStorageSliceLayer: Deal with 'luks' files
The 'luks' driver in qemu is as any other non-raw format driver and thus
doesn't support the properties for 'slice'. Since libvirt considers
luks files to be raw+encryption we need to special case them when
dealing with the slice.
https://bugzilla.redhat.com/show_bug.cgi?id=1814975
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_block.c | 4 ++++
tests/qemuxml2argvdata/disk-slices.x86_64-latest.args | 5 ++++-
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index b5b34ab441..ee15167215 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -3324,5 +3324,9 @@ qemuBlockStorageSourceNeedsStorageSliceLayer(const virStorageSource
*src)
if (src->format != VIR_STORAGE_FILE_RAW)
return true;
+ if (src->encryption &&
+ src->encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS)
+ return true;
+
return false;
}
diff --git a/tests/qemuxml2argvdata/disk-slices.x86_64-latest.args
b/tests/qemuxml2argvdata/disk-slices.x86_64-latest.args
index 75485c17a7..63bdaa58be 100644
--- a/tests/qemuxml2argvdata/disk-slices.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/disk-slices.x86_64-latest.args
@@ -52,9 +52,12 @@
data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\
keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \
-blockdev
'{"driver":"file","filename":"/var/lib/libvirt/images/luks.img",\
"node-name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}'
\
+-blockdev
'{"driver":"raw","node-name":"libvirt-1-slice-sto","offset":1234,\
+"size":321,"file":"libvirt-1-storage","auto-read-only":true,\
+"discard":"unmap"}' \
-blockdev
'{"node-name":"libvirt-1-format","read-only":false,"driver":"luks",\
"key-secret":"libvirt-1-format-encryption-secret0","offset":1234,"size":321,\
-"file":"libvirt-1-storage"}' \
+"file":"libvirt-1-slice-sto"}' \
-device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x4,drive=libvirt-1-format,\
id=virtio-disk2 \
-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5 \
--
2.24.1
11:35 a.m.
New subject: [PATCH 4/4] qemu: block: Split up formatting of JSON props for 'raw' and 'luks' drivers
qemuBlockStorageSourceGetFormatRawProps aggregated both formats but
since we now have props specific for either of those formats it's
unwanted to aggregate the code such way. Split out the 'luks' props
formatter into qemuBlockStorageSourceGetFormatLUKSProps.
The wrong separation demonstrates istself on formatting of the 'size'
and 'offset' attributes for the 'luks' driver which does not conform
to the qapi schema.
https://bugzilla.redhat.com/show_bug.cgi?id=1814975
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_block.c | 42 ++++++++++++-------
.../disk-slices.x86_64-latest.args | 2 +-
2 files changed, 29 insertions(+), 15 deletions(-)
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index ee15167215..5c85ddd44c 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -1200,24 +1200,32 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src,
static int
-qemuBlockStorageSourceGetFormatRawProps(virStorageSourcePtr src,
- virJSONValuePtr props)
+qemuBlockStorageSourceGetFormatLUKSProps(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;
+ if (!srcPriv || !srcPriv->encinfo || !srcPriv->encinfo->s.aes.alias) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("missing secret info for 'luks' driver"));
+ return -1;
}
if (virJSONValueObjectAdd(props,
- "s:driver", driver,
- "S:key-secret", secretalias, NULL) < 0)
+ "s:driver", "luks",
+ "s:key-secret",
srcPriv->encinfo->s.aes.alias,
+ NULL) < 0)
+ return -1;
+
+ return 0;
+}
+
+
+static int
+qemuBlockStorageSourceGetFormatRawProps(virStorageSourcePtr src,
+ virJSONValuePtr props)
+{
+ if (virJSONValueObjectAdd(props, "s:driver", "raw", NULL) <
0)
return -1;
/* Currently only storage slices are supported. We'll have to calculate
@@ -1371,8 +1379,14 @@ qemuBlockStorageSourceGetBlockdevFormatProps(virStorageSourcePtr
src)
/* 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)
- return NULL;
+ if (src->encryption &&
+ src->encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) {
+ if (qemuBlockStorageSourceGetFormatLUKSProps(src, props) < 0)
+ return NULL;
+ } else {
+ if (qemuBlockStorageSourceGetFormatRawProps(src, props) < 0)
+ return NULL;
+ }
break;
case VIR_STORAGE_FILE_QCOW2:
diff --git a/tests/qemuxml2argvdata/disk-slices.x86_64-latest.args
b/tests/qemuxml2argvdata/disk-slices.x86_64-latest.args
index 63bdaa58be..869b4c0af0 100644
--- a/tests/qemuxml2argvdata/disk-slices.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/disk-slices.x86_64-latest.args
@@ -56,7 +56,7 @@ keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \
"size":321,"file":"libvirt-1-storage","auto-read-only":true,\
"discard":"unmap"}' \
-blockdev
'{"node-name":"libvirt-1-format","read-only":false,"driver":"luks",\
-"key-secret":"libvirt-1-format-encryption-secret0","offset":1234,"size":321,\
+"key-secret":"libvirt-1-format-encryption-secret0",\
"file":"libvirt-1-slice-sto"}' \
-device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x4,drive=libvirt-1-format,\
id=virtio-disk2 \
--
2.24.1
12:03 p.m.
On a Thursday in 2020, Peter Krempa wrote:
We special cased some behaviour and it broke on combination with
other
special case.
Peter Krempa (4):
qemuxml2argvdata/disk-slices: Add test case for 'luks' encryption
qemu: block: Extract logic decision when to use a separate 'raw' layer
for slice
qemuBlockStorageSourceNeedsStorageSliceLayer: Deal with 'luks' files
qemu: block: Split up formatting of JSON props for 'raw' and 'luks'
drivers
src/qemu/qemu_block.c | 70 ++++++++++++++-----
src/qemu/qemu_block.h | 3 +
src/qemu/qemu_domain.c | 3 +-
.../disk-slices.x86_64-latest.args | 41 +++++++----
tests/qemuxml2argvdata/disk-slices.xml | 13 ++++
.../disk-slices.x86_64-latest.xml | 16 ++++-
6 files changed, 112 insertions(+), 34 deletions(-)
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
1709
days inactive
1709
days old
5 comments
2 participants
participants (2)
-
Ján Tomko -
Peter Krempa