[Qemu-devel] [PATCH] XBZRLE: Fix qemu crash when resize the xbzrle cache during migration

[Gonglei (Arei)]

It is likely to crash qemu when resize the xbzrle cache

during migration. Because the xbzrle cache will be modified

by migration thread and resize thread.

 

Test scene

step one: set the size of xbzrle cache 1GB.

step two: migrate vm which dirty memory continuously.

step three: set the size of xbzrle cache 0.125GB.

 

Signed-off-by: ChenLiang <address@hidden>

Signed-off-by: Gonglei <address@hidden>

---

arch_init.c                    |   42 ++++++++++++++++++++++++++++++++++++---

include/migration/page_cache.h |   14 +++++++++++++

page_cache.c                   |   13 ++++++++++++

3 files changed, 65 insertions(+), 4 deletions(-)

 

diff --git a/arch_init.c b/arch_init.c

index 80574a0..e2d2c72 100644

--- a/arch_init.c

+++ b/arch_init.c

@@ -164,26 +164,56 @@ static struct {

     uint8_t *encoded_buf;

     /* buffer for storing page content */

     uint8_t *current_buf;

-    /* Cache for XBZRLE */

+    /* Cache for XBZRLE, Protected by lock. */

     PageCache *cache;

+    QemuMutex lock;

} XBZRLE = {

     .encoded_buf = NULL,

     .current_buf = NULL,

     .cache = NULL,

};

+

/* buffer used for XBZRLE decoding */

static uint8_t *xbzrle_decoded_buf;

 

+static void XBZRLE_cache_lock(void)

+{

+    qemu_mutex_lock(&XBZRLE.lock);

+}

+

+static void XBZRLE_cache_unlock(void)

+{

+    qemu_mutex_unlock(&XBZRLE.lock);

+}

+

+

int64_t xbzrle_cache_resize(int64_t new_size)

{

+    PageCache *new_cache, *old_cache;

+

     if (new_size < TARGET_PAGE_SIZE) {

         return -1;

     }

 

     if (XBZRLE.cache != NULL) {

-        return cache_resize(XBZRLE.cache, new_size / TARGET_PAGE_SIZE) *

-            TARGET_PAGE_SIZE;

+        if (pow2floor(new_size) == cache_max_num_items(XBZRLE.cache)

+                                               *TARGET_PAGE_SIZE) {

+            goto ret;

+        }

+        new_cache = cache_init(new_size / TARGET_PAGE_SIZE,

+                                      cache_page_size(XBZRLE.cache));

+        if (!new_cache) {

+            DPRINTF("Error creating cache\n");

+            return -1;

+        }

+        XBZRLE_cache_lock();

+        old_cache = XBZRLE.cache;

+        XBZRLE.cache = new_cache;

+        XBZRLE_cache_unlock();

+        cache_fini(old_cache);

+        goto ret;

     }

+ret:

     return pow2floor(new_size);

}

 

@@ -522,6 +552,8 @@ static int ram_save_block(QEMUFile *f, bool last_stage)

             ret = ram_control_save_page(f, block->offset,

                                offset, TARGET_PAGE_SIZE, &bytes_sent);

 

+            XBZRLE_cache_lock();

+

             if (ret != RAM_SAVE_CONTROL_NOT_SUPP) {

                 if (ret != RAM_SAVE_CONTROL_DELAYED) {

                     if (bytes_sent > 0) {

@@ -553,6 +585,8 @@ static int ram_save_block(QEMUFile *f, bool last_stage)

                 acct_info.norm_pages++;

             }

 

+            XBZRLE_cache_unlock();

+

             /* if page is unmodified, continue to the next */

             if (bytes_sent > 0) {

                 last_sent_block = block;

@@ -681,7 +715,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque)

             XBZRLE.encoded_buf = NULL;

             return -1;

         }

-

+        qemu_mutex_init(&XBZRLE.lock);

         acct_clear();

     }

 

diff --git a/include/migration/page_cache.h b/include/migration/page_cache.h

index d156f0d..6be0103 100644

--- a/include/migration/page_cache.h

+++ b/include/migration/page_cache.h

@@ -79,4 +79,18 @@ int cache_insert(PageCache *cache, uint64_t addr, uint8_t *pdata);

  */

int64_t cache_resize(PageCache *cache, int64_t num_pages);

 

+/**

+ * cache_max_num_items: return the cache max num items.

+ *

+ * @cache pointer to the PageCache struct

+ */

+int64_t cache_max_num_items(PageCache *cache);

+

+/**

+ * cache_page_size: return the cache page size

+ *

+ * @cache pointer to the PageCache struct

+ */

+unsigned int cache_page_size(PageCache *cache);

+

#endif

diff --git a/page_cache.c b/page_cache.c

index 3ef6ee7..92e401c 100644

--- a/page_cache.c

+++ b/page_cache.c

@@ -234,3 +234,16 @@ int64_t cache_resize(PageCache *cache, int64_t new_num_pages)

 

     return cache->max_num_items;

}

+

+int64_t cache_max_num_items(PageCache *cache)

+{

+    g_assert(cache);

+    return cache->max_num_items;

+}

+

+unsigned int cache_page_size(PageCache *cache)

+{

+    g_assert(cache);

+    return cache->page_size;

+}

+

--

1.6.0.2

 

 

Best regards,

-Gonglei