4:22 a.m.
Peter Krempa (2):
docs: backup: Remove references to push backup to network disk
backup: Allow 'encryption' of backups and scratch images
docs/formatbackup.html.in | 19 ++++--
docs/schemas/domainbackup.rng | 65 +++++++++++++++----
.../backup-pull-encrypted.xml | 30 +++++++++
.../backup-push-encrypted.xml | 29 +++++++++
.../backup-pull-encrypted.xml | 30 +++++++++
.../backup-push-encrypted.xml | 29 +++++++++
tests/genericxml2xmltest.c | 3 +
7 files changed, 187 insertions(+), 18 deletions(-)
create mode 100644 tests/domainbackupxml2xmlin/backup-pull-encrypted.xml
create mode 100644 tests/domainbackupxml2xmlin/backup-push-encrypted.xml
create mode 100644 tests/domainbackupxml2xmlout/backup-pull-encrypted.xml
create mode 100644 tests/domainbackupxml2xmlout/backup-push-encrypted.xml
--
2.26.0
4:22 a.m.
New subject: [PATCH 1/2] docs: backup: Remove references to push backup to network disk
It was never implemented and for now I don't think there's demand to do
it. Remove the reference.
https://bugzilla.redhat.com/show_bug.cgi?id=1812100
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
docs/formatbackup.html.in | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/docs/formatbackup.html.in b/docs/formatbackup.html.in
index 55acd13ddc..87744bac98 100644
--- a/docs/formatbackup.html.in
+++ b/docs/formatbackup.html.in
@@ -97,12 +97,11 @@
<dt><code>type</code></dt>
<dd>A mandatory attribute to describe the type of the
disk, except when <code>backup='no'</code> is
- used. Valid values include <code>file</code>,
- <code>block</code>, or <code>network</code>.
+ used. Valid values include <code>file</code>, or
+ <code>block</code>.
Similar to a disk declaration for a domain, the choice of type
controls what additional sub-elements are needed to describe
- the destination (such as <code>protocol</code> for a
- network destination).</dd>
+ the destination.
<dt><code>target</code></dt>
<dd>Valid only for push mode backups, this is the
primary sub-element that describes the file name of
--
2.26.0
12:36 p.m.
New subject: [PATCH 1/2] docs: backup: Remove references to push backup to network disk
On 4/14/20 4:22 AM, Peter Krempa wrote:
It was never implemented and for now I don't think there's
demand to do
it. Remove the reference.
https://bugzilla.redhat.com/show_bug.cgi?id=1812100
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
docs/formatbackup.html.in | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/docs/formatbackup.html.in b/docs/formatbackup.html.in
index 55acd13ddc..87744bac98 100644
--- a/docs/formatbackup.html.in
+++ b/docs/formatbackup.html.in
@@ -97,12 +97,11 @@
<dt><code>type</code></dt>
<dd>A mandatory attribute to describe the type of the
disk, except when <code>backup='no'</code> is
- used. Valid values include <code>file</code>,
- <code>block</code>, or <code>network</code>.
+ used. Valid values include <code>file</code>, or
+ <code>block</code>.
I think we should implement block, rather than delete it. It matters
for the same reason that it matters in the destination of block copy: if
you want to set a highest-byte watermark threshold (to be warned by qemu
when it is time to resize the disk larger), you NEED a block device, not
a file. But libvirt treats all <disk type='file'> as files, even when
opening /path/to/device; you HAVE to use <disk type='block'> when
specifying a block device to get the behaviors needed for handling it as
a block device rather than a file.
Similar to a disk declaration for a domain, the
choice of type
controls what additional sub-elements are needed to describe
- the destination (such as <code>protocol</code> for a
- network destination).</dd>
+ the destination.
<dt><code>target</code></dt>
<dd>Valid only for push mode backups, this is the
primary sub-element that describes the file name of
I'm inclined to NACK this patch.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
2:06 p.m.
New subject: [PATCH 1/2] docs: backup: Remove references to push backup to network disk
On Tue, Apr 14, 2020 at 12:36:56 -0500, Eric Blake wrote:
On 4/14/20 4:22 AM, Peter Krempa wrote:
> It was never implemented and for now I don't think there's demand to do
> it. Remove the reference.
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1812100
>
> Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
> ---
> docs/formatbackup.html.in | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/docs/formatbackup.html.in b/docs/formatbackup.html.in
> index 55acd13ddc..87744bac98 100644
> --- a/docs/formatbackup.html.in
> +++ b/docs/formatbackup.html.in
> @@ -97,12 +97,11 @@
> <dt><code>type</code></dt>
> <dd>A mandatory attribute to describe the type of the
> disk, except when <code>backup='no'</code> is
> - used. Valid values include <code>file</code>,
> - <code>block</code>, or
<code>network</code>.
> + used. Valid values include <code>file</code>, or
> + <code>block</code>.
I think we should implement block, rather than delete it. It matters for
the same reason that it matters in the destination of block copy: if you
I'm deleting 'network'. Block is implemented, working and tested.
want to set a highest-byte watermark threshold (to be warned by qemu
when it
is time to resize the disk larger), you NEED a block device, not a file.
You can do this on a file too.
But libvirt treats all <disk type='file'> as files,
even when opening
/path/to/device; you HAVE to use <disk type='block'> when specifying a
block
device to get the behaviors needed for handling it as a block device rather
than a file.
> Similar to a disk declaration for a domain, the choice of type
> controls what additional sub-elements are needed to describe
> - the destination (such as <code>protocol</code> for a
> - network destination).</dd>
> + the destination.
> <dt><code>target</code></dt>
> <dd>Valid only for push mode backups, this is the
> primary sub-element that describes the file name of
>
I'm inclined to NACK this patch.
Wouldn't mean that much since it's necessary to add schema if you want
it.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
2:29 p.m.
New subject: [PATCH 1/2] docs: backup: Remove references to push backup to network disk
On 4/14/20 2:06 PM, Peter Krempa wrote:
On Tue, Apr 14, 2020 at 12:36:56 -0500, Eric Blake wrote:
> On 4/14/20 4:22 AM, Peter Krempa wrote:
>> It was never implemented and for now I don't think there's demand to do
>> it. Remove the reference.
>>
>> https://bugzilla.redhat.com/show_bug.cgi?id=1812100
>>
>> Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
>> ---
>> docs/formatbackup.html.in | 7 +++----
>> 1 file changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/docs/formatbackup.html.in b/docs/formatbackup.html.in
>> index 55acd13ddc..87744bac98 100644
>> --- a/docs/formatbackup.html.in
>> +++ b/docs/formatbackup.html.in
>> @@ -97,12 +97,11 @@
>> <dt><code>type</code></dt>
>> <dd>A mandatory attribute to describe the type of the
>> disk, except when <code>backup='no'</code>
is
>> - used. Valid values include <code>file</code>,
>> - <code>block</code>, or
<code>network</code>.
>> + used. Valid values include <code>file</code>, or
>> + <code>block</code>.
>
> I think we should implement block, rather than delete it. It matters for
> the same reason that it matters in the destination of block copy: if you
I'm deleting 'network'. Block is implemented, working and tested.
Then shame on me for misreading the patch.
Okay, we may someday add network, but for now making the documentation
match what we have as existing implementation makes sense.
> want to set a highest-byte watermark threshold (to be warned by qemu when it
> is time to resize the disk larger), you NEED a block device, not a file.
You can do this on a file too.
I seem to recall differences in how the two behave at the qemu level;
but it doesn't affect this patch if we have the means for telling
libvirt which of the two (file or block) we meant.
> I'm inclined to NACK this patch.
Wouldn't mean that much since it's necessary to add schema if you want
it.
NACK withdrawn; you've convinced me that the patch (dropping 'network',
not 'block') is reasonable for matching what we have.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
4:22 a.m.
New subject: [PATCH 2/2] backup: Allow 'encryption' of backups and scratch images
Add the appropriate entries into the schema to allow encryption of the
backup or scratch image. Since we use blockdev internals for everything
no changes to the code are actually necessary.
https://bugzilla.redhat.com/show_bug.cgi?id=1811906
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
docs/formatbackup.html.in | 14 +++-
docs/schemas/domainbackup.rng | 65 +++++++++++++++----
.../backup-pull-encrypted.xml | 30 +++++++++
.../backup-push-encrypted.xml | 29 +++++++++
.../backup-pull-encrypted.xml | 30 +++++++++
.../backup-push-encrypted.xml | 29 +++++++++
tests/genericxml2xmltest.c | 3 +
7 files changed, 185 insertions(+), 15 deletions(-)
create mode 100644 tests/domainbackupxml2xmlin/backup-pull-encrypted.xml
create mode 100644 tests/domainbackupxml2xmlin/backup-push-encrypted.xml
create mode 100644 tests/domainbackupxml2xmlout/backup-pull-encrypted.xml
create mode 100644 tests/domainbackupxml2xmlout/backup-push-encrypted.xml
diff --git a/docs/formatbackup.html.in b/docs/formatbackup.html.in
index 87744bac98..9e69d8f7d3 100644
--- a/docs/formatbackup.html.in
+++ b/docs/formatbackup.html.in
@@ -101,7 +101,7 @@
<code>block</code>.
Similar to a disk declaration for a domain, the choice of type
controls what additional sub-elements are needed to describe
- the destination.
+ the destination.</dd>
<dt><code>target</code></dt>
<dd>Valid only for push mode backups, this is the
primary sub-element that describes the file name of
@@ -110,7 +110,8 @@
disk. An optional sub-element <code>driver</code> can
also be used, with an attribute <code>type</code> to
specify a destination format different from
- qcow2. </dd>
+ qcow2. See documentation for <code>scratch</code> below for
+ additional configuration.</dd>
<dt><code>scratch</code></dt>
<dd>Valid only for pull mode backups, this is the
primary sub-element that describes the file name of
@@ -130,7 +131,14 @@
used without modification. The file is not deleted after the
backup but the contents of the file don't make sense outside
of the backup. The same applies for the block device which
- must be formatted appropriately.</dd>
+ must be formatted appropriately.
+
+ Similarly to the domain
+ <a
href="formatdomain.html#elementsDisks"><code>disk</code></a>
+ definition <code>scratch</code> and
<code>target</code> can
+ contain <code>seclabel</code> and/or
<code>encryption</code>
+ subelements to configure the corresponding properties.
+ </dd>
</dl>
</dd>
</dl>
diff --git a/docs/schemas/domainbackup.rng b/docs/schemas/domainbackup.rng
index 395ea841f9..ac5b12c463 100644
--- a/docs/schemas/domainbackup.rng
+++ b/docs/schemas/domainbackup.rng
@@ -7,6 +7,27 @@
<include href='domaincommon.rng'/>
+ <define name='backupEncryption'>
+ <element name='encryption'>
+ <attribute name='format'>
+ <choice>
+ <value>luks</value>
+ </choice>
+ </attribute>
+ <interleave>
+ <ref name='secret'/>
+ <optional>
+ <element name='cipher'>
+ <ref name='keycipher'/>
+ </element>
+ <element name='ivgen'>
+ <ref name='keyivgen'/>
+ </element>
+ </optional>
+ </interleave>
+ </element>
+ </define>
+
<define name='domainbackup'>
<element name='domainbackup'>
<interleave>
@@ -123,9 +144,14 @@
<attribute name='file'>
<ref name='absFilePath'/>
</attribute>
- <zeroOrMore>
- <ref name='devSeclabel'/>
- </zeroOrMore>
+ <interleave>
+ <zeroOrMore>
+ <ref name='devSeclabel'/>
+ </zeroOrMore>
+ <optional>
+ <ref name='backupEncryption'/>
+ </optional>
+ </interleave>
</element>
</optional>
<ref name='backupPushDriver'/>
@@ -142,9 +168,14 @@
<attribute name='dev'>
<ref name='absFilePath'/>
</attribute>
- <zeroOrMore>
- <ref name='devSeclabel'/>
- </zeroOrMore>
+ <interleave>
+ <zeroOrMore>
+ <ref name='devSeclabel'/>
+ </zeroOrMore>
+ <optional>
+ <ref name='backupEncryption'/>
+ </optional>
+ </interleave>
</element>
</optional>
<ref name='backupPushDriver'/>
@@ -192,9 +223,14 @@
<attribute name='file'>
<ref name='absFilePath'/>
</attribute>
- <zeroOrMore>
- <ref name='devSeclabel'/>
- </zeroOrMore>
+ <interleave>
+ <zeroOrMore>
+ <ref name='devSeclabel'/>
+ </zeroOrMore>
+ <optional>
+ <ref name='backupEncryption'/>
+ </optional>
+ </interleave>
</element>
<ref name='backupPullDriver'/>
</interleave>
@@ -210,9 +246,14 @@
<attribute name='dev'>
<ref name='absFilePath'/>
</attribute>
- <zeroOrMore>
- <ref name='devSeclabel'/>
- </zeroOrMore>
+ <interleave>
+ <zeroOrMore>
+ <ref name='devSeclabel'/>
+ </zeroOrMore>
+ <optional>
+ <ref name='backupEncryption'/>
+ </optional>
+ </interleave>
</element>
<ref name='backupPullDriver'/>
</interleave>
diff --git a/tests/domainbackupxml2xmlin/backup-pull-encrypted.xml
b/tests/domainbackupxml2xmlin/backup-pull-encrypted.xml
new file mode 100644
index 0000000000..1469189a37
--- /dev/null
+++ b/tests/domainbackupxml2xmlin/backup-pull-encrypted.xml
@@ -0,0 +1,30 @@
+<domainbackup mode="pull">
+ <incremental>1525889631</incremental>
+ <server transport='tcp' name='localhost' port='10809'/>
+ <disks>
+ <disk name='vda' type='file' exportname='test-vda'
exportbitmap='blah'>
+ <driver type='qcow2'/>
+ <scratch file='/path/to/file'>
+ <encryption format='luks'>
+ <secret type='passphrase'
uuid='0a81f5b2-8403-7b23-c8d6-21ccc2f80d6f'/>
+ </encryption>
+ </scratch>
+ </disk>
+ <disk name='vdb' type='file' exportname='test-vda'
exportbitmap='blah'>
+ <driver type='qcow2'/>
+ <scratch file='/path/to/file'>
+ <encryption format='luks'>
+ <secret type='passphrase' usage='/storage/backup/vdb'/>
+ </encryption>
+ </scratch>
+ </disk>
+ <disk name='vdc' type='block'>
+ <driver type='qcow2'/>
+ <scratch dev='/dev/block'>
+ <encryption format='luks'>
+ <secret type='passphrase' usage='/storage/backup/vdc'/>
+ </encryption>
+ </scratch>
+ </disk>
+ </disks>
+</domainbackup>
diff --git a/tests/domainbackupxml2xmlin/backup-push-encrypted.xml
b/tests/domainbackupxml2xmlin/backup-push-encrypted.xml
new file mode 100644
index 0000000000..121cfd7fa9
--- /dev/null
+++ b/tests/domainbackupxml2xmlin/backup-push-encrypted.xml
@@ -0,0 +1,29 @@
+<domainbackup mode="push">
+ <incremental>1525889631</incremental>
+ <disks>
+ <disk name='vda' type='file'>
+ <driver type='qcow2'/>
+ <target file='/path/to/file'>
+ <encryption format='luks'>
+ <secret type='passphrase'
uuid='0a81f5b2-8403-7b23-c8d6-21ccc2f80d6f'/>
+ </encryption>
+ </target>
+ </disk>
+ <disk name='vdb' type='file'>
+ <driver type='raw'/>
+ <target file='/path/to/file'>
+ <encryption format='luks'>
+ <secret type='passphrase' usage='/storage/backup/vdb'/>
+ </encryption>
+ </target>
+ </disk>
+ <disk name='vdc' type='block'>
+ <driver type='qcow2'/>
+ <target dev='/dev/block'>
+ <encryption format='luks'>
+ <secret type='passphrase' usage='/storage/backup/vdc'/>
+ </encryption>
+ </target>
+ </disk>
+ </disks>
+</domainbackup>
diff --git a/tests/domainbackupxml2xmlout/backup-pull-encrypted.xml
b/tests/domainbackupxml2xmlout/backup-pull-encrypted.xml
new file mode 100644
index 0000000000..81519bfcb5
--- /dev/null
+++ b/tests/domainbackupxml2xmlout/backup-pull-encrypted.xml
@@ -0,0 +1,30 @@
+<domainbackup mode='pull'>
+ <incremental>1525889631</incremental>
+ <server transport='tcp' name='localhost' port='10809'/>
+ <disks>
+ <disk name='vda' backup='yes' type='file'
exportname='test-vda' exportbitmap='blah'>
+ <driver type='qcow2'/>
+ <scratch file='/path/to/file'>
+ <encryption format='luks'>
+ <secret type='passphrase'
uuid='0a81f5b2-8403-7b23-c8d6-21ccc2f80d6f'/>
+ </encryption>
+ </scratch>
+ </disk>
+ <disk name='vdb' backup='yes' type='file'
exportname='test-vda' exportbitmap='blah'>
+ <driver type='qcow2'/>
+ <scratch file='/path/to/file'>
+ <encryption format='luks'>
+ <secret type='passphrase' usage='/storage/backup/vdb'/>
+ </encryption>
+ </scratch>
+ </disk>
+ <disk name='vdc' backup='yes' type='block'>
+ <driver type='qcow2'/>
+ <scratch dev='/dev/block'>
+ <encryption format='luks'>
+ <secret type='passphrase' usage='/storage/backup/vdc'/>
+ </encryption>
+ </scratch>
+ </disk>
+ </disks>
+</domainbackup>
diff --git a/tests/domainbackupxml2xmlout/backup-push-encrypted.xml
b/tests/domainbackupxml2xmlout/backup-push-encrypted.xml
new file mode 100644
index 0000000000..a955340964
--- /dev/null
+++ b/tests/domainbackupxml2xmlout/backup-push-encrypted.xml
@@ -0,0 +1,29 @@
+<domainbackup mode='push'>
+ <incremental>1525889631</incremental>
+ <disks>
+ <disk name='vda' backup='yes' type='file'>
+ <driver type='qcow2'/>
+ <target file='/path/to/file'>
+ <encryption format='luks'>
+ <secret type='passphrase'
uuid='0a81f5b2-8403-7b23-c8d6-21ccc2f80d6f'/>
+ </encryption>
+ </target>
+ </disk>
+ <disk name='vdb' backup='yes' type='file'>
+ <driver type='raw'/>
+ <target file='/path/to/file'>
+ <encryption format='luks'>
+ <secret type='passphrase' usage='/storage/backup/vdb'/>
+ </encryption>
+ </target>
+ </disk>
+ <disk name='vdc' backup='yes' type='block'>
+ <driver type='qcow2'/>
+ <target dev='/dev/block'>
+ <encryption format='luks'>
+ <secret type='passphrase' usage='/storage/backup/vdc'/>
+ </encryption>
+ </target>
+ </disk>
+ </disks>
+</domainbackup>
diff --git a/tests/genericxml2xmltest.c b/tests/genericxml2xmltest.c
index 501bcdb0a1..74e520522b 100644
--- a/tests/genericxml2xmltest.c
+++ b/tests/genericxml2xmltest.c
@@ -192,8 +192,11 @@ mymain(void)
DO_TEST_BACKUP("empty");
DO_TEST_BACKUP("backup-pull");
DO_TEST_BACKUP("backup-pull-seclabel");
+ DO_TEST_BACKUP("backup-pull-encrypted");
DO_TEST_BACKUP("backup-push");
DO_TEST_BACKUP("backup-push-seclabel");
+ DO_TEST_BACKUP("backup-push-encrypted");
+
virObjectUnref(caps);
virObjectUnref(xmlopt);
--
2.26.0
10:07 a.m.
New subject: [PATCH 2/2] backup: Allow 'encryption' of backups and scratch images
On Tue, Apr 14, 2020 at 11:22:44AM +0200, Peter Krempa wrote:
Add the appropriate entries into the schema to allow encryption of
the
backup or scratch image. Since we use blockdev internals for everything
no changes to the code are actually necessary.
https://bugzilla.redhat.com/show_bug.cgi?id=1811906
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
docs/formatbackup.html.in | 14 +++-
docs/schemas/domainbackup.rng | 65 +++++++++++++++----
.../backup-pull-encrypted.xml | 30 +++++++++
.../backup-push-encrypted.xml | 29 +++++++++
.../backup-pull-encrypted.xml | 30 +++++++++
.../backup-push-encrypted.xml | 29 +++++++++
tests/genericxml2xmltest.c | 3 +
7 files changed, 185 insertions(+), 15 deletions(-)
create mode 100644 tests/domainbackupxml2xmlin/backup-pull-encrypted.xml
create mode 100644 tests/domainbackupxml2xmlin/backup-push-encrypted.xml
create mode 100644 tests/domainbackupxml2xmlout/backup-pull-encrypted.xml
create mode 100644 tests/domainbackupxml2xmlout/backup-push-encrypted.xml
diff --git a/docs/formatbackup.html.in b/docs/formatbackup.html.in
index 87744bac98..9e69d8f7d3 100644
--- a/docs/formatbackup.html.in
+++ b/docs/formatbackup.html.in
@@ -101,7 +101,7 @@
<code>block</code>.
Similar to a disk declaration for a domain, the choice of type
controls what additional sub-elements are needed to describe
- the destination.
+ the destination.</dd>
^This hunk needs to go into the previous commit, otherwise the previous patch
breaks the build.
With that fixed (to both patches):
Reviewed-by: Erik Skultety <eskultet(a)redhat.com>
10:03 a.m.
On a Tuesday in 2020, Peter Krempa wrote:
Peter Krempa (2):
docs: backup: Remove references to push backup to network disk
backup: Allow 'encryption' of backups and scratch images
docs/formatbackup.html.in | 19 ++++--
docs/schemas/domainbackup.rng | 65 +++++++++++++++----
.../backup-pull-encrypted.xml | 30 +++++++++
.../backup-push-encrypted.xml | 29 +++++++++
.../backup-pull-encrypted.xml | 30 +++++++++
.../backup-push-encrypted.xml | 29 +++++++++
tests/genericxml2xmltest.c | 3 +
7 files changed, 187 insertions(+), 18 deletions(-)
create mode 100644 tests/domainbackupxml2xmlin/backup-pull-encrypted.xml
create mode 100644 tests/domainbackupxml2xmlin/backup-push-encrypted.xml
create mode 100644 tests/domainbackupxml2xmlout/backup-pull-encrypted.xml
create mode 100644 tests/domainbackupxml2xmlout/backup-push-encrypted.xml
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
1717
days inactive
1717
days old
7 comments
4 participants
participants (4)
-
Eric Blake -
Erik Skultety -
Ján Tomko -
Peter Krempa