[libvirt] [PATCH 0/3] snapshots: Allow <seclabel> for snapshot disk source

This is just a schema improvement, the code actually already does everything. Peter Krempa (3): docs: snapshot: Encourage people ot use disk 'target' to refer to disks docs: schemas: Add 'seclabel' for external disk snapshot tests: domainsnapshotxml2xml: make 'disk-seclabel' test operational docs/formatsnapshot.html.in | 18 +++++++++++++++--- docs/schemas/domainsnapshot.rng | 6 ++++++ ...-seclabel-invalid.xml => disk-seclabel.xml} | 0 .../domainsnapshotxml2xmlout/disk-seclabel.xml | 15 +++++++++++++++ tests/domainsnapshotxml2xmltest.c | 1 + 5 files changed, 37 insertions(+), 3 deletions(-) rename tests/domainsnapshotxml2xmlin/{disk-seclabel-invalid.xml => disk-seclabel.xml} (100%) create mode 100644 tests/domainsnapshotxml2xmlout/disk-seclabel.xml -- 2.21.0

Change the example and add a recommendation to use disk target rather than path. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/formatsnapshot.html.in | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/formatsnapshot.html.in b/docs/formatsnapshot.html.in index 1bbbf06205..a19e91b4d5 100644 --- a/docs/formatsnapshot.html.in +++ b/docs/formatsnapshot.html.in @@ -140,8 +140,8 @@ <dd>This sub-element describes the snapshot properties of a specific disk. The attribute <code>name</code> is mandatory, and must match either the <code><target - dev='name'/></code> or an unambiguous <code><source - file='name'/></code> of one of + dev='name'/></code> (recommended) or an unambiguous + <code><source file='name'/></code> of one of the <a href="formatdomain.html#elementsDisks">disk devices</a> specified for the domain at the time of the snapshot. The attribute <code>snapshot</code> is @@ -255,7 +255,7 @@ <domainsnapshot> <description>Snapshot of OS install and updates</description> <disks> - <disk name='/path/to/old'> + <disk name='vda'> <source file='/path/to/new'/> </disk> <disk name='vdb' snapshot='no'/> -- 2.21.0

On 6/20/19 8:51 AM, Peter Krempa wrote:
Change the example and add a recommendation to use disk target rather than path.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/formatsnapshot.html.in | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/docs/formatsnapshot.html.in b/docs/formatsnapshot.html.in index 1bbbf06205..a19e91b4d5 100644 --- a/docs/formatsnapshot.html.in +++ b/docs/formatsnapshot.html.in @@ -140,8 +140,8 @@ <dd>This sub-element describes the snapshot properties of a specific disk. The attribute <code>name</code> is mandatory, and must match either the <code><target - dev='name'/></code> or an unambiguous <code><source - file='name'/></code> of one of + dev='name'/></code> (recommended) or an unambiguous + <code><source file='name'/></code> of one of
I like it. I also know that for checkpoints, you wanted me to ONLY accept the disk target name rather than copying the snapshot practice of also supporting a file name. This gives me a stronger reason to follow up with that request. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org

Allow using seclabels the same way as disk images allow it. Currently the snapshot code copies the seclabels from the original image if no seclabel is provided. Also there's no code change required as the snapshot XML parser actually uses parts of the disk parser thus seclabels are already parsed and formatted and even applied thus this is just a formalization of our support for this. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- docs/formatsnapshot.html.in | 12 ++++++++++++ docs/schemas/domainsnapshot.rng | 6 ++++++ .../{disk-seclabel-invalid.xml => disk-seclabel.xml} | 0 3 files changed, 18 insertions(+) rename tests/domainsnapshotxml2xmlin/{disk-seclabel-invalid.xml => disk-seclabel.xml} (100%) diff --git a/docs/formatsnapshot.html.in b/docs/formatsnapshot.html.in index a19e91b4d5..92cc566467 100644 --- a/docs/formatsnapshot.html.in +++ b/docs/formatsnapshot.html.in @@ -170,6 +170,12 @@ snapshots, the original file name becomes the read-only snapshot, and the new file name contains the read-write delta of all disk changes since the snapshot. + <p/> + The <code>source</code> element also may contain the + <code>seclabel</code> element (described in the + <a href="formatdomain.html#seclabel">domain XML documentation</a>) + which can be used to override the domain security labeling policy + for <code>source</code>. </dd> <dt><code>driver</code></dt> <dd>An optional sub-element <code>driver</code>, @@ -177,6 +183,7 @@ as qcow2), of the new file created by the external snapshot of the new file. </dd> + <dt><code>seclabel</code></dt> </dl> <span class="since">Since 1.2.2</span> the <code>disk</code> element @@ -259,6 +266,11 @@ <source file='/path/to/new'/> </disk> <disk name='vdb' snapshot='no'/> + <disk name='vdc'> + <source file='/path/to/newc'> + <seclabel model='dac' relabel='no'/> + </source> + </disk> </disks> </domainsnapshot></pre> diff --git a/docs/schemas/domainsnapshot.rng b/docs/schemas/domainsnapshot.rng index 8863d99578..8e39feb229 100644 --- a/docs/schemas/domainsnapshot.rng +++ b/docs/schemas/domainsnapshot.rng @@ -157,6 +157,9 @@ <optional> <ref name='storageStartupPolicy'/> </optional> + <zeroOrMore> + <ref name='devSeclabel'/> + </zeroOrMore> <empty/> </element> </optional> @@ -173,6 +176,9 @@ <attribute name="dev"> <ref name="absFilePath"/> </attribute> + <zeroOrMore> + <ref name='devSeclabel'/> + </zeroOrMore> <empty/> </element> </optional> diff --git a/tests/domainsnapshotxml2xmlin/disk-seclabel-invalid.xml b/tests/domainsnapshotxml2xmlin/disk-seclabel.xml similarity index 100% rename from tests/domainsnapshotxml2xmlin/disk-seclabel-invalid.xml rename to tests/domainsnapshotxml2xmlin/disk-seclabel.xml -- 2.21.0

On 6/20/19 8:51 AM, Peter Krempa wrote:
Allow using seclabels the same way as disk images allow it. Currently the snapshot code copies the seclabels from the original image if no seclabel is provided. Also there's no code change required as the snapshot XML parser actually uses parts of the disk parser thus seclabels are already parsed and formatted and even applied thus this is just a formalization of our support for this.
Yay! The backup code needs this too, so I get to reuse this change. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org

Now that we added the seclabels to the schema we can test that they are parsed and formatted correctly. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/domainsnapshotxml2xmlout/disk-seclabel.xml | 15 +++++++++++++++ tests/domainsnapshotxml2xmltest.c | 1 + 2 files changed, 16 insertions(+) create mode 100644 tests/domainsnapshotxml2xmlout/disk-seclabel.xml diff --git a/tests/domainsnapshotxml2xmlout/disk-seclabel.xml b/tests/domainsnapshotxml2xmlout/disk-seclabel.xml new file mode 100644 index 0000000000..989cfc4964 --- /dev/null +++ b/tests/domainsnapshotxml2xmlout/disk-seclabel.xml @@ -0,0 +1,15 @@ +<domainsnapshot> + <name>my snap name</name> + <description>!@#$%^</description> + <creationTime>581484660</creationTime> + <disks> + <disk name='hde' snapshot='external' type='file'> + <source file='/path/to/new2'> + <seclabel model='dac' relabel='no'/> + </source> + </disk> + </disks> + <domain> + <uuid>9d37b878-a7cc-9f9a-b78f-49b3abad25a8</uuid> + </domain> +</domainsnapshot> diff --git a/tests/domainsnapshotxml2xmltest.c b/tests/domainsnapshotxml2xmltest.c index c2c7bedb56..c34ab0bc89 100644 --- a/tests/domainsnapshotxml2xmltest.c +++ b/tests/domainsnapshotxml2xmltest.c @@ -183,6 +183,7 @@ mymain(void) DO_TEST_INOUT("external_vm", NULL, 1555419243, 0); DO_TEST_INOUT("disk_snapshot", NULL, 1555419243, 0); DO_TEST_INOUT("disk_driver_name_null", NULL, 1555419243, 0); + DO_TEST_INOUT("disk-seclabel", "9d37b878-a7cc-9f9a-b78f-49b3abad25a8", 581484660, 0); DO_TEST_IN("name_and_description", NULL); DO_TEST_IN("description_only", NULL); -- 2.21.0

On Thu, Jun 20, 2019 at 03:51:02PM +0200, Peter Krempa wrote:
This is just a schema improvement, the code actually already does everything.
Peter Krempa (3): docs: snapshot: Encourage people ot use disk 'target' to refer to disks docs: schemas: Add 'seclabel' for external disk snapshot tests: domainsnapshotxml2xml: make 'disk-seclabel' test operational
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
docs/formatsnapshot.html.in | 18 +++++++++++++++--- docs/schemas/domainsnapshot.rng | 6 ++++++ ...-seclabel-invalid.xml => disk-seclabel.xml} | 0 .../domainsnapshotxml2xmlout/disk-seclabel.xml | 15 +++++++++++++++ tests/domainsnapshotxml2xmltest.c | 1 + 5 files changed, 37 insertions(+), 3 deletions(-) rename tests/domainsnapshotxml2xmlin/{disk-seclabel-invalid.xml => disk-seclabel.xml} (100%) create mode 100644 tests/domainsnapshotxml2xmlout/disk-seclabel.xml
-- 2.21.0
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (3)
-
Eric Blake
-
Martin Kletzander
-
Peter Krempa