Re: [Qemu-devel] [v5 02/12] migration: Add the framework of multi-thread compression

[Juan Quintela]

Liang Li <address@hidden> wrote:
> Add the code to create and destroy the multiple threads those will
> be used to do data compression. Left some functions empty to keep
> clearness, and the code will be added later.
>
> Signed-off-by: Liang Li <address@hidden>
> Signed-off-by: Yang Zhang <address@hidden>
> Reviewed-by: Dr.David Alan Gilbert <address@hidden>

And here I am again.

Reviewing patch 8, I found that we need to fix some things here.

> +static int ram_save_compressed_page(QEMUFile *f, RAMBlock *block,
> +                                    ram_addr_t offset, bool last_stage)
> +{
> +    int bytes_sent = -1;
> +
> +    /* To be done*/
> +
> +    return bytes_sent;
> +}

We have three return values, here, that are not the same that for normal
pages

 0: this is the 1st page for a particular thread, nothing to sent yet
 n > 0: we are sending the previous compresed page for the choosen
        thread

Notice that the only way that ram_save_page() can return 0 is for xbzrle
when a page has modified but it has exactly the same value that before.

(it can have been modified twice, +1, -1 or whatever)

Notice that ram_save_page() can only return 0 (duplicate page) or > 0
(real size written)

> +
>  /*
>   * ram_find_and_save_block: Finds a page to send and sends it to f
>   *
> @@ -679,7 +751,12 @@ static int ram_find_and_save_block(QEMUFile *f, bool 
> last_stage)
>                  ram_bulk_stage = false;
>              }
>          } else {
> -            bytes_sent = ram_save_page(f, block, offset, last_stage);
> +            if (migrate_use_compression()) {
> +                bytes_sent = ram_save_compressed_page(f, block, offset,
> +                                                      last_stage);
> +            } else {
> +                bytes_sent = ram_save_page(f, block, offset, last_stage);
> +            }


I need more context, this is the corrent code

        } else {
            bytes_sent = ram_save_page(f, block, offset, last_stage);

            /* if page is unmodified, continue to the next */
            if (bytes_sent > 0) {
                last_sent_block = block;
                break;
            }
        }

And we should change to:

        } else if (migrate_use_compression()) {
            bytes_sent = ram_save_compressed_page(f, block, offset,
                                                  last_stage);
            last_sent_block = block;
            break;
        } else {
            bytes_sent = ram_save_page(f, block, offset, last_stage);

            /* if page is unmodified, continue to the next */
            if (bytes_sent > 0) {
                last_sent_block = block;
                break;
            }
        }

This would mean that we don't need to arrange for the zero byte return
on qemu.