[Qemu-devel] [PATCH V3 1/7] block/qemu-img: Refine and export infinite loop checking in collect_image_info_list()

[Xu Wang]

If there is a loop exists in the backing file chain, many problems
could be caused by it, such as no response and segment fault during
system boot. Hence stopping backing file loop appear is very necessary.
This patch refine and export loop checking function from collect_image_
info_list() to block.c and build a independent function named bdrv_
backing_file_loop_check().

Signed-off-by: Xu Wang <address@hidden>
---
 block.c               | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++
 include/block/block.h |  4 +++
 qemu-img.c            | 29 +++++----------
 3 files changed, 110 insertions(+), 20 deletions(-)

diff --git a/block.c b/block.c
index 183fec8..c1d060d 100644
--- a/block.c
+++ b/block.c
@@ -4433,6 +4433,103 @@ bdrv_acct_done(BlockDriverState *bs, BlockAcctCookie 
*cookie)
     bs->total_time_ns[cookie->type] += get_clock() - cookie->start_time_ns;
 }
 
+static gboolean str_equal_func(gconstpointer a, gconstpointer b)
+{
+    return strcmp(a, b) == 0;
+}
+
+/**
+ * Check backing file chain if there is a loop in it and build list of
+ * ImageInfo if needed.
+ *
+ * @filename: topmost image filename, absolute or relative path is OK.
+ * @fmt: topmost image format (may be NULL to autodetect)
+ * @backing_file: if this value is set, @filename inode (-1 if it doesn't
+ *                exist) will insert into hash table directly. Loop check
+ *                starts from it. Absolute or relative path is OK for it.
+ * @backing_format: format of backing file
+ *
+ * Returns: true for backing file loop or error happened, false for no loop.
+ */
+bool bdrv_backing_file_loop_check(const char *filename, const char *fmt,
+                                  const char *backing_file,
+                                  const char *backing_format) {
+    GHashTable *inodes;
+    BlockDriverState *bs;
+    BlockDriver *drv;
+    struct stat sbuf;
+    long inode = 0;
+    int ret;
+    char fbuf[1024];
+
+    inodes = g_hash_table_new_full(g_str_hash, str_equal_func, NULL, NULL);
+
+    if (backing_file) {
+        /* Check if file exists. */
+        if (access(filename, F_OK)) {
+            inode = -1;
+        } else {
+            if (stat(filename, &sbuf) == -1) {
+                error_report("Get file %s stat failed.", filename);
+                goto err;
+            }
+            inode = (long)sbuf.st_ino;
+        }
+
+        filename = backing_file;
+        fmt = backing_format;
+        g_hash_table_insert(inodes, (gpointer)&inode, NULL);
+    }
+
+    while (filename && (filename[0] != '\0')) {
+        if (stat(filename, &sbuf) == -1) {
+                error_report("Get file %s stat failed.", filename);
+                goto err;
+        }
+        inode = (long)sbuf.st_ino;
+
+        if (g_hash_table_lookup_extended(inodes, &inode, NULL, NULL)) {
+            error_report("Backing file '%s' creates an infinite loop.",
+                         filename);
+            goto err;
+        }
+        g_hash_table_insert(inodes, (gpointer)&inode, NULL);
+
+        bs = bdrv_new("image");
+
+        if (fmt) {
+            drv = bdrv_find_format(fmt);
+            if (!drv) {
+                error_report("Unknown file format '%s'", fmt);
+                bdrv_delete(bs);
+                goto err;
+            }
+        } else {
+            drv = NULL;
+        }
+
+        ret = bdrv_open(bs, filename, NULL,
+                        BDRV_O_CACHE_WB | BDRV_O_NO_BACKING, drv);
+        if (ret < 0) {
+            error_report("Could not open '%s': %s", filename, strerror(-ret));
+            bdrv_delete(bs);
+            goto err;
+        }
+
+        bdrv_get_backing_filename(bs, fbuf, sizeof(fbuf));
+        filename = fbuf;
+        fmt = NULL;
+
+        bdrv_delete(bs);
+    }
+    g_hash_table_destroy(inodes);
+    return false;
+
+err:
+    g_hash_table_destroy(inodes);
+    return true;
+}
+
 void bdrv_img_create(const char *filename, const char *fmt,
                      const char *base_filename, const char *base_fmt,
                      char *options, uint64_t img_size, int flags,
diff --git a/include/block/block.h b/include/block/block.h
index dd8eca1..3dc29bb 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -333,6 +333,10 @@ int bdrv_save_vmstate(BlockDriverState *bs, const uint8_t 
*buf,
 int bdrv_load_vmstate(BlockDriverState *bs, uint8_t *buf,
                       int64_t pos, int size);
 
+bool bdrv_backing_file_loop_check(const char *filename, const char *fmt,
+                                  const char *backing_file,
+                                  const char *backing_format);
+
 void bdrv_img_create(const char *filename, const char *fmt,
                      const char *base_filename, const char *base_fmt,
                      char *options, uint64_t img_size, int flags,
diff --git a/qemu-img.c b/qemu-img.c
index f8c97d3..19cac5e 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1620,11 +1620,6 @@ static void dump_human_image_info_list(ImageInfoList 
*list)
     }
 }
 
-static gboolean str_equal_func(gconstpointer a, gconstpointer b)
-{
-    return strcmp(a, b) == 0;
-}
-
 /**
  * Open an image file chain and return an ImageInfoList
  *
@@ -1642,24 +1637,19 @@ static ImageInfoList *collect_image_info_list(const 
char *filename,
                                               bool chain)
 {
     ImageInfoList *head = NULL;
+    BlockDriverState *bs;
+    ImageInfoList *elem;
     ImageInfoList **last = &head;
-    GHashTable *filenames;
+    ImageInfo *info;
     Error *err = NULL;
 
-    filenames = g_hash_table_new_full(g_str_hash, str_equal_func, NULL, NULL);
+    /* check if loop exists and build ImageInfoList */
+    if (chain &&
+        bdrv_backing_file_loop_check(filename, fmt, NULL, NULL)) {
+        goto err;
+    }
 
     while (filename) {
-        BlockDriverState *bs;
-        ImageInfo *info;
-        ImageInfoList *elem;
-
-        if (g_hash_table_lookup_extended(filenames, filename, NULL, NULL)) {
-            error_report("Backing file '%s' creates an infinite loop.",
-                         filename);
-            goto err;
-        }
-        g_hash_table_insert(filenames, (gpointer)filename, NULL);
-
         bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_NO_BACKING,
                            false, false);
         if (!bs) {
@@ -1692,12 +1682,11 @@ static ImageInfoList *collect_image_info_list(const 
char *filename,
             }
         }
     }
-    g_hash_table_destroy(filenames);
+
     return head;
 
 err:
     qapi_free_ImageInfoList(head);
-    g_hash_table_destroy(filenames);
     return NULL;
 }
 
-- 
1.8.1.4