* [Qemu-devel] [PATCH] migration: Fix regression with compression threads
@ 2017-05-10 11:42 Juan Quintela
2017-05-10 12:35 ` Peter Xu
0 siblings, 1 reply; 2+ messages in thread
From: Juan Quintela @ 2017-05-10 11:42 UTC (permalink / raw)
To: qemu-devel; +Cc: dgilbert, lvivier, peterx
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(-)
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
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [Qemu-devel] [PATCH] migration: Fix regression with compression threads
2017-05-10 11:42 [Qemu-devel] [PATCH] migration: Fix regression with compression threads Juan Quintela
@ 2017-05-10 12:35 ` Peter Xu
0 siblings, 0 replies; 2+ messages in thread
From: Peter Xu @ 2017-05-10 12:35 UTC (permalink / raw)
To: Juan Quintela; +Cc: qemu-devel, dgilbert, lvivier
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
>
--
Peter Xu
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2017-05-10 12:36 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-05-10 11:42 [Qemu-devel] [PATCH] migration: Fix regression with compression threads Juan Quintela
2017-05-10 12:35 ` Peter Xu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).