From 3615e8b39badf2a526996a69dc91a92b04cf262e Mon Sep 17 00:00:00 2001 From: Peter Krempa Date: Tue, 17 Dec 2019 17:04:04 +0100 Subject: [PATCH] util: storage: Don't treat files with missing backing store format as 'raw' Assuming that the backing image format is raw is wrong when doing image detection: 1) In -drive mode qemu will still probe the image format of the backing image. This means it will try to open a backing file of the image which will fail if a more advanced security model is in use. 2) In blockdev mode the image will be opened as raw actually which is wrong since it might be qcow. Not opening the backing images will also end up in the guest seeing corrupted data. Rather than attempt to solve various corner cases when us assuming the storage file being raw and actually being right forbid startup when the guest image doesn't have the format specified in the metadata. https://bugzilla.redhat.com/show_bug.cgi?id=1588373 Signed-off-by: Peter Krempa Reviewed-by: Michal Privoznik --- src/util/virstoragefile.c | 21 +++++++++++++++++---- tests/virstoragetest.c | 2 +- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 3a3de314b8..e280a646b7 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -4981,12 +4981,25 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src, goto cleanup; } - if (backingFormat == VIR_STORAGE_FILE_AUTO) + backingStore->format = backingFormat; + + 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) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("format of backing image '%s' of image '%s' was not specified in the image metadata"), + src->backingStoreRaw, NULLSTR(src->path)); + return -1; + } + backingStore->format = VIR_STORAGE_FILE_RAW; - else if (backingFormat == VIR_STORAGE_FILE_AUTO_SAFE) + } + + if (backingStore->format == VIR_STORAGE_FILE_AUTO_SAFE) backingStore->format = VIR_STORAGE_FILE_AUTO; - else - backingStore->format = backingFormat; if ((ret = virStorageFileGetMetadataRecurse(backingStore, parent, uid, gid, diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 93f3d1604a..b9d4a45cdd 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -751,7 +751,7 @@ mymain(void) .format = VIR_STORAGE_FILE_QCOW2, }; TEST_CHAIN(abswrap, VIR_STORAGE_FILE_QCOW2, - (&wrap_as_raw, &qcow2_as_raw), EXP_PASS); + (&wrap_as_raw, &qcow2_as_raw), EXP_FAIL); /* Rewrite qcow2 to a missing backing file, with backing type */ virCommandFree(cmd);