Message ID | 20170510114240.4302-1-quintela@redhat.com |
---|---|
State | New |
Headers | show |
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 --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());
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(-)