forked from rpms/libvirt
149 lines
6.0 KiB
Diff
149 lines
6.0 KiB
Diff
From 1e3921f1065972dc236bc726f19be37492a145c6 Mon Sep 17 00:00:00 2001
|
|
Message-Id: <1e3921f1065972dc236bc726f19be37492a145c6@dist-git>
|
|
From: Peter Krempa <pkrempa@redhat.com>
|
|
Date: Fri, 28 Feb 2020 10:24:43 +0100
|
|
Subject: [PATCH] virStorageFileGetMetadataRecurse: Allow format probing under
|
|
special circumstances
|
|
MIME-Version: 1.0
|
|
Content-Type: text/plain; charset=UTF-8
|
|
Content-Transfer-Encoding: 8bit
|
|
|
|
Allow format probing to work around lazy clients which did not specify
|
|
their format in the overlay. Format probing will be allowed only, if we
|
|
are able to probe the image, the probing result was successful and the
|
|
probed image does not have any backing or data file.
|
|
|
|
This relaxes the restrictions which were imposed in commit 3615e8b39bad
|
|
in cases when we know that the image probing will not result in security
|
|
issues or data corruption.
|
|
|
|
We perform the image format detection and in the case that we were able
|
|
to probe the format and the format does not specify a backing store (or
|
|
doesn't support backing store) we can use this format.
|
|
|
|
With pre-blockdev configurations this will restore the previous
|
|
behaviour for the images mentioned above as qemu would probe the format
|
|
anyways. It also improves error reporting compared to the old state as
|
|
we now report that the backing chain will be broken in case when there
|
|
is a backing file.
|
|
|
|
In blockdev configurations this ensures that libvirt will not cause data
|
|
corruption by ending the chain prematurely without notifying the user,
|
|
but still allows the old semantics when the users forgot to specify the
|
|
format.
|
|
|
|
Users thus don't have to re-invent when image format detection is safe
|
|
to do.
|
|
|
|
The price for this is that libvirt will need to keep the image format
|
|
detector still current and working or replace it by invocation of
|
|
qemu-img.
|
|
|
|
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
|
|
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
|
|
(cherry picked from commit ae9e6c2a2b75d958995c661f7bb64ed4353a6404)
|
|
|
|
https://bugzilla.redhat.com/show_bug.cgi?id=1798148
|
|
Message-Id: <77d1d21a0ce64fd49c501b1ed3667307cbab0b73.1582881363.git.pkrempa@redhat.com>
|
|
Reviewed-by: Ján Tomko <jtomko@redhat.com>
|
|
---
|
|
src/util/virstoragefile.c | 52 ++++++++++++++++++++++-----------------
|
|
1 file changed, 30 insertions(+), 22 deletions(-)
|
|
|
|
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
|
|
index 0bfd9e16e9..b88763b267 100644
|
|
--- a/src/util/virstoragefile.c
|
|
+++ b/src/util/virstoragefile.c
|
|
@@ -4986,6 +4986,7 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
|
|
virHashTablePtr cycle,
|
|
unsigned int depth)
|
|
{
|
|
+ virStorageFileFormat orig_format = src->format;
|
|
size_t headerLen;
|
|
int rv;
|
|
g_autofree char *buf = NULL;
|
|
@@ -4995,10 +4996,17 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
|
|
NULLSTR(src->path), src->format,
|
|
(unsigned int)uid, (unsigned int)gid);
|
|
|
|
+ if (src->format == VIR_STORAGE_FILE_AUTO_SAFE)
|
|
+ src->format = VIR_STORAGE_FILE_AUTO;
|
|
+
|
|
/* exit if we can't load information about the current image */
|
|
rv = virStorageFileSupportsBackingChainTraversal(src);
|
|
- if (rv <= 0)
|
|
+ if (rv <= 0) {
|
|
+ if (orig_format == VIR_STORAGE_FILE_AUTO)
|
|
+ return -2;
|
|
+
|
|
return rv;
|
|
+ }
|
|
|
|
if (virStorageFileGetMetadataRecurseReadHeader(src, parent, uid, gid,
|
|
&buf, &headerLen, cycle) < 0)
|
|
@@ -5007,6 +5015,18 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
|
|
if (virStorageFileGetMetadataInternal(src, buf, headerLen) < 0)
|
|
return -1;
|
|
|
|
+ /* If we probed the format we MUST ensure that nothing else than the current
|
|
+ * image (this includes both backing files and external data store) is
|
|
+ * considered for security labelling and/or recursion. */
|
|
+ if (orig_format == VIR_STORAGE_FILE_AUTO) {
|
|
+ if (src->backingStoreRaw || src->externalDataStoreRaw) {
|
|
+ src->format = VIR_STORAGE_FILE_RAW;
|
|
+ VIR_FREE(src->backingStoreRaw);
|
|
+ VIR_FREE(src->externalDataStoreRaw);
|
|
+ return -2;
|
|
+ }
|
|
+ }
|
|
+
|
|
if (src->backingStoreRaw) {
|
|
if ((rv = virStorageSourceNewFromBacking(src, &backingStore)) < 0)
|
|
return -1;
|
|
@@ -5015,33 +5035,21 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
|
|
if (rv == 1)
|
|
return 0;
|
|
|
|
- if (backingStore->format == VIR_STORAGE_FILE_AUTO) {
|
|
- /* Assuming the backing store to be raw can lead to failures. We do
|
|
- * it only when we must not report an error to prevent losing VMs.
|
|
- * Otherwise report an error.
|
|
- */
|
|
- if (report_broken) {
|
|
+ if ((rv = virStorageFileGetMetadataRecurse(backingStore, parent,
|
|
+ uid, gid,
|
|
+ report_broken,
|
|
+ cycle, depth + 1)) < 0) {
|
|
+ if (!report_broken)
|
|
+ return 0;
|
|
+
|
|
+ if (rv == -2) {
|
|
virReportError(VIR_ERR_OPERATION_INVALID,
|
|
_("format of backing image '%s' of image '%s' was not specified in the image metadata "
|
|
"(See https://libvirt.org/kbase/backing_chains.html for troubleshooting)"),
|
|
src->backingStoreRaw, NULLSTR(src->path));
|
|
- return -1;
|
|
}
|
|
|
|
- backingStore->format = VIR_STORAGE_FILE_RAW;
|
|
- }
|
|
-
|
|
- if (backingStore->format == VIR_STORAGE_FILE_AUTO_SAFE)
|
|
- backingStore->format = VIR_STORAGE_FILE_AUTO;
|
|
-
|
|
- if (virStorageFileGetMetadataRecurse(backingStore, parent,
|
|
- uid, gid,
|
|
- report_broken,
|
|
- cycle, depth + 1) < 0) {
|
|
- if (report_broken)
|
|
- return -1;
|
|
- else
|
|
- return 0;
|
|
+ return -1;
|
|
}
|
|
|
|
backingStore->id = depth;
|
|
--
|
|
2.25.1
|
|
|