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 <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 3a3de314b8471be0a0dd3fb1be73a38444757663..e280a646b792ac2c9111a385bbeb84f42e59a378 100644 (file)
--- 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 93f3d1604a2d936f1eb1e76eb9a8cd31f81d2282..b9d4a45cddfc98bdabe59e7ff2ef5de07e924d15 100644 (file)
--- 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);