diff mbox

migration: Fix regression with compression threads

Message ID 20170510114240.4302-1-quintela@redhat.com
State New
Headers show

Commit Message

Juan Quintela May 10, 2017, 11:42 a.m. UTC
Compression threads got broken on commit

  commit 247956946651ae0280f7b1ea88bb6237dd01c231
  Author: Juan Quintela <quintela@redhat.com>
  Date:   Tue Mar 21 11:45:01 2017 +0100

      ram: reorganize last_sent_block

On do_compress_ram_page() we use a different QEMUFile than the
migration one.  We need to pass it there.  The failure can be seen as:

(qemu) qemu-system-x86_64: Unknown combination of migration flags: 0
qemu-system-x86_64: error while loading state section id 3(ram)
qemu-system-x86_64: load of migration failed: Invalid argument

Please, review.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/ram.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

Comments

Peter Xu May 10, 2017, 12:35 p.m. UTC | #1
On Wed, May 10, 2017 at 01:42:40PM +0200, Juan Quintela wrote:
> Compression threads got broken on commit
> 
>   commit 247956946651ae0280f7b1ea88bb6237dd01c231
>   Author: Juan Quintela <quintela@redhat.com>
>   Date:   Tue Mar 21 11:45:01 2017 +0100
> 
>       ram: reorganize last_sent_block
> 
> On do_compress_ram_page() we use a different QEMUFile than the
> migration one.  We need to pass it there.  The failure can be seen as:
> 
> (qemu) qemu-system-x86_64: Unknown combination of migration flags: 0
> qemu-system-x86_64: error while loading state section id 3(ram)
> qemu-system-x86_64: load of migration failed: Invalid argument
> 
> Please, review.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>

Reviewed-by: Peter Xu <peterx@redhat.com>
Tested-by: Peter Xu <peterx@redhat.com>

(PS. besides this fix, we might have tiny problem when updating/using
 rs->last_sent_block in save_page_header(), since this function can be
 called simultaneously in different compression threads but we don't
 have any protection on the variable... anyway, this is not related to
 this fix after all, and iiuc we are safe right now as long as with
 the flush_compressed_data() in ram_save_compressed_page() when
 ramblock switches)

Thanks,

> ---
>  migration/ram.c | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index 293d27c..995d1fc 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -436,20 +436,21 @@ void migrate_compress_threads_create(void)
>   * @offset: offset inside the block for the page
>   *          in the lower bits, it contains flags
>   */
> -static size_t save_page_header(RAMState *rs, RAMBlock *block, ram_addr_t offset)
> +static size_t save_page_header(RAMState *rs, QEMUFile *f,  RAMBlock *block,
> +                               ram_addr_t offset)
>  {
>      size_t size, len;
>  
>      if (block == rs->last_sent_block) {
>          offset |= RAM_SAVE_FLAG_CONTINUE;
>      }
> -    qemu_put_be64(rs->f, offset);
> +    qemu_put_be64(f, offset);
>      size = 8;
>  
>      if (!(offset & RAM_SAVE_FLAG_CONTINUE)) {
>          len = strlen(block->idstr);
> -        qemu_put_byte(rs->f, len);
> -        qemu_put_buffer(rs->f, (uint8_t *)block->idstr, len);
> +        qemu_put_byte(f, len);
> +        qemu_put_buffer(f, (uint8_t *)block->idstr, len);
>          size += 1 + len;
>          rs->last_sent_block = block;
>      }
> @@ -571,7 +572,7 @@ static int save_xbzrle_page(RAMState *rs, uint8_t **current_data,
>      }
>  
>      /* Send XBZRLE based compressed page */
> -    bytes_xbzrle = save_page_header(rs, block,
> +    bytes_xbzrle = save_page_header(rs, rs->f, block,
>                                      offset | RAM_SAVE_FLAG_XBZRLE);
>      qemu_put_byte(rs->f, ENCODING_FLAG_XBZRLE);
>      qemu_put_be16(rs->f, encoded_len);
> @@ -745,7 +746,7 @@ static int save_zero_page(RAMState *rs, RAMBlock *block, ram_addr_t offset,
>      if (is_zero_range(p, TARGET_PAGE_SIZE)) {
>          rs->zero_pages++;
>          rs->bytes_transferred +=
> -            save_page_header(rs, block, offset | RAM_SAVE_FLAG_COMPRESS);
> +            save_page_header(rs, rs->f, block, offset | RAM_SAVE_FLAG_COMPRESS);
>          qemu_put_byte(rs->f, 0);
>          rs->bytes_transferred += 1;
>          pages = 1;
> @@ -834,7 +835,7 @@ static int ram_save_page(RAMState *rs, PageSearchStatus *pss, bool last_stage)
>  
>      /* XBZRLE overflow or normal page */
>      if (pages == -1) {
> -        rs->bytes_transferred += save_page_header(rs, block,
> +        rs->bytes_transferred += save_page_header(rs, rs->f, block,
>                                                    offset | RAM_SAVE_FLAG_PAGE);
>          if (send_async) {
>              qemu_put_buffer_async(rs->f, p, TARGET_PAGE_SIZE,
> @@ -860,7 +861,7 @@ static int do_compress_ram_page(QEMUFile *f, RAMBlock *block,
>      int bytes_sent, blen;
>      uint8_t *p = block->host + (offset & TARGET_PAGE_MASK);
>  
> -    bytes_sent = save_page_header(rs, block, offset |
> +    bytes_sent = save_page_header(rs, f, block, offset |
>                                    RAM_SAVE_FLAG_COMPRESS_PAGE);
>      blen = qemu_put_compression_data(f, p, TARGET_PAGE_SIZE,
>                                       migrate_compress_level());
> @@ -991,7 +992,7 @@ static int ram_save_compressed_page(RAMState *rs, PageSearchStatus *pss,
>              pages = save_zero_page(rs, block, offset, p);
>              if (pages == -1) {
>                  /* Make sure the first page is sent out before other pages */
> -                bytes_xmit = save_page_header(rs, block, offset |
> +                bytes_xmit = save_page_header(rs, rs->f, block, offset |
>                                                RAM_SAVE_FLAG_COMPRESS_PAGE);
>                  blen = qemu_put_compression_data(rs->f, p, TARGET_PAGE_SIZE,
>                                                   migrate_compress_level());
> -- 
> 2.9.3
>
diff mbox

Patch

diff --git a/migration/ram.c b/migration/ram.c
index 293d27c..995d1fc 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -436,20 +436,21 @@  void migrate_compress_threads_create(void)
  * @offset: offset inside the block for the page
  *          in the lower bits, it contains flags
  */
-static size_t save_page_header(RAMState *rs, RAMBlock *block, ram_addr_t offset)
+static size_t save_page_header(RAMState *rs, QEMUFile *f,  RAMBlock *block,
+                               ram_addr_t offset)
 {
     size_t size, len;
 
     if (block == rs->last_sent_block) {
         offset |= RAM_SAVE_FLAG_CONTINUE;
     }
-    qemu_put_be64(rs->f, offset);
+    qemu_put_be64(f, offset);
     size = 8;
 
     if (!(offset & RAM_SAVE_FLAG_CONTINUE)) {
         len = strlen(block->idstr);
-        qemu_put_byte(rs->f, len);
-        qemu_put_buffer(rs->f, (uint8_t *)block->idstr, len);
+        qemu_put_byte(f, len);
+        qemu_put_buffer(f, (uint8_t *)block->idstr, len);
         size += 1 + len;
         rs->last_sent_block = block;
     }
@@ -571,7 +572,7 @@  static int save_xbzrle_page(RAMState *rs, uint8_t **current_data,
     }
 
     /* Send XBZRLE based compressed page */
-    bytes_xbzrle = save_page_header(rs, block,
+    bytes_xbzrle = save_page_header(rs, rs->f, block,
                                     offset | RAM_SAVE_FLAG_XBZRLE);
     qemu_put_byte(rs->f, ENCODING_FLAG_XBZRLE);
     qemu_put_be16(rs->f, encoded_len);
@@ -745,7 +746,7 @@  static int save_zero_page(RAMState *rs, RAMBlock *block, ram_addr_t offset,
     if (is_zero_range(p, TARGET_PAGE_SIZE)) {
         rs->zero_pages++;
         rs->bytes_transferred +=
-            save_page_header(rs, block, offset | RAM_SAVE_FLAG_COMPRESS);
+            save_page_header(rs, rs->f, block, offset | RAM_SAVE_FLAG_COMPRESS);
         qemu_put_byte(rs->f, 0);
         rs->bytes_transferred += 1;
         pages = 1;
@@ -834,7 +835,7 @@  static int ram_save_page(RAMState *rs, PageSearchStatus *pss, bool last_stage)
 
     /* XBZRLE overflow or normal page */
     if (pages == -1) {
-        rs->bytes_transferred += save_page_header(rs, block,
+        rs->bytes_transferred += save_page_header(rs, rs->f, block,
                                                   offset | RAM_SAVE_FLAG_PAGE);
         if (send_async) {
             qemu_put_buffer_async(rs->f, p, TARGET_PAGE_SIZE,
@@ -860,7 +861,7 @@  static int do_compress_ram_page(QEMUFile *f, RAMBlock *block,
     int bytes_sent, blen;
     uint8_t *p = block->host + (offset & TARGET_PAGE_MASK);
 
-    bytes_sent = save_page_header(rs, block, offset |
+    bytes_sent = save_page_header(rs, f, block, offset |
                                   RAM_SAVE_FLAG_COMPRESS_PAGE);
     blen = qemu_put_compression_data(f, p, TARGET_PAGE_SIZE,
                                      migrate_compress_level());
@@ -991,7 +992,7 @@  static int ram_save_compressed_page(RAMState *rs, PageSearchStatus *pss,
             pages = save_zero_page(rs, block, offset, p);
             if (pages == -1) {
                 /* Make sure the first page is sent out before other pages */
-                bytes_xmit = save_page_header(rs, block, offset |
+                bytes_xmit = save_page_header(rs, rs->f, block, offset |
                                               RAM_SAVE_FLAG_COMPRESS_PAGE);
                 blen = qemu_put_compression_data(rs->f, p, TARGET_PAGE_SIZE,
                                                  migrate_compress_level());