[Qemu-devel] [PATCH 1/2] Refine and export infinite loop checking in collect_image_info_list()

[Xu Wang]

From: Xu Wang <address@hidden>

Signed-off-by: Xu Wang <address@hidden>
---
 qemu-img.c | 110 +++++++++++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 89 insertions(+), 21 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 809b4f1..0bc265d 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -324,6 +324,86 @@ static int add_old_style_options(const char *fmt, 
QEMUOptionParameter *list,
     return 0;
 }
 
+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
+ * @fmt: topmost image format (may be NULL to autodetect)
+ * @head: head of ImageInfo list. If not need please set head to null.
+ * @chain: true  - enumerate entire backing file chain
+ *         false - only topmost image file
+ * @backing_file: if this value is set, filename will insert into hash
+ *                table directly. open and seek backing file start from it.
+ *
+ * If return true, stands for a backing file loop exists or some error
+ * happend. If return false, everything is ok.
+ */
+static bool backing_file_loop_check(const char *filename, const char *fmt,
+                             bool chain, const char *backing_file) {
+    GHashTable *filenames;
+    BlockDriverState *bs;
+    ImageInfo *info;
+    Error *err = NULL;
+
+    filenames = g_hash_table_new_full(g_str_hash, str_equal_func, NULL, NULL);
+
+    /* If backing file exists, filename will insert into hash table and seek
+     * the whole backing file chain from @backing_file.
+     */
+    if (backing_file) {
+        g_hash_table_insert(filenames, (gpointer)filename, NULL);
+        filename = backing_file;
+    }
+
+    while (filename) {
+        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) {
+            goto err;
+        }
+
+        bdrv_query_image_info(bs, &info, &err);
+        if (error_is_set(&err)) {
+            error_report("%s", error_get_pretty(err));
+            error_free(err);
+            goto err;
+        }
+
+        bdrv_delete(bs);
+
+        filename = fmt = NULL;
+        if (chain) {
+            if (info->has_full_backing_filename) {
+                filename = info->full_backing_filename;
+            } else if (info->has_backing_filename) {
+                filename = info->backing_filename;
+            }
+            if (info->has_backing_filename_format) {
+                fmt = info->backing_filename_format;
+            }
+        }
+    }
+    g_hash_table_destroy(filenames);
+    return false;
+
+err:
+    g_hash_table_destroy(filenames);
+    return true;
+}
+
 static int img_create(int argc, char **argv)
 {
     int c;
@@ -1620,11 +1700,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 +1717,18 @@ 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 (backing_file_loop_check(filename, fmt, chain, 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 +1761,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