[libvirt] [PATCH 3/4] util: storage: Don't treat files with missing backing store format as 'raw'

[Peter Krempa]

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 <pkrempa at redhat.com>
---
 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);
-- 
2.23.0