[PATCH 2/5] conf: fix detection of infinite backing loop

[Eric Blake <eblake@xxxxxxxxxx>]

While trying to refactor the backing file chain, I noticed that
if you have a self-referential qcow2 file via a relative name:

qemu-img create -f qcow2 loop 10M
qemu-img rebase -u -f qcow2 -F qcow2 -b loop loop

then libvirt was creating a chain 2 deep before realizing it
had hit a loop; furthermore, virStorageFileChainCheckBroken
was not identifying the chain as broken.  With this patch,
the loop is detected when the chain is only 1 deep; still
enough for storage volume XML to display the file, but now
with a proper error report about where the loop was found.

* src/util/virstoragefile.c (virStorageFileGetMetadataRecurse):
Mark chain broken if OOM or infinite loop occurs.

Signed-off-by: Eric Blake <eblake@xxxxxxxxxx>
---
 src/util/virstoragefile.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 017717c..39e4c19 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1074,15 +1074,21 @@ virStorageFileGetMetadataRecurse(const char *path, const char *directory,
               path, format, (int)uid, (int)gid, allow_probe);

     virStorageFileMetadataPtr ret = NULL;
+    char *canonical = canonicalize_file_name(path);

-    if (virHashLookup(cycle, path)) {
+    /* Hash by canonical name */
+    if (!canonical) {
+        virReportOOMError();
+        goto cleanup;
+    }
+    if (virHashLookup(cycle, canonical)) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("backing store for %s is self-referential"),
                        path);
-        return NULL;
+        goto cleanup;
     }
-    if (virHashAddEntry(cycle, path, (void *)1) < 0)
-        return NULL;
+    if (virHashAddEntry(cycle, canonical, (void *)1) < 0)
+        goto cleanup;

     if ((fd = virFileOpenAs(path, O_RDONLY, 0, uid, gid, 0)) < 0) {
         virReportSystemError(-fd, _("Failed to open file '%s'"), path);
@@ -1106,8 +1112,15 @@ virStorageFileGetMetadataRecurse(const char *path, const char *directory,
                                                             uid, gid,
                                                             allow_probe,
                                                             cycle);
+        if (!ret->backingMeta) {
+            /* If we failed to get backing data, mark the chain broken */
+            ret->backingStoreFormat = VIR_STORAGE_FILE_NONE;
+            VIR_FREE(ret->backingStore);
+        }
     }

+ cleanup:
+    VIR_FREE(canonical);
     return ret;
 }

@@ -1176,7 +1189,8 @@ virStorageFileChainGetBroken(virStorageFileMetadataPtr chain,

     tmp = chain;
     while (tmp) {
-        /* Break if no backing store or backing store is not file */
+        /* Break if no backing store, backing store is not file, or
+         * other problem such as infinite loop */
        if (!tmp->backingStoreRaw)
            break;
        if (!tmp->backingStore) {
-- 
1.9.0

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list