[Qemu-devel] [PATCH 0/2] fix the flaws of qemu_put_compression_data

[Qemu-devel] [PATCH 0/2] fix the flaws of qemu_put_compression_data

[Liang Li]

This patch fixed the flaws in qemu_put_compression_data function.
and cleanup the code based on the change.

Liang Li (2):
  qemu-file: fix flaws of qemu_put_compression_data
  migration: code clean up.

 migration/qemu-file.c | 10 +++++++++-
 migration/ram.c       | 20 ++++++++------------
 2 files changed, 17 insertions(+), 13 deletions(-)

-- 
1.9.1

[Qemu-devel] [PATCH 0/2] fix the flaws of qemu_put_compression_data

[Liang Li]

This patch fixed the flaws in qemu_put_compression_data function.
and cleanup the code based on the change.

Liang Li (2):
  qemu-file: fix flaws of qemu_put_compression_data
  migration: code clean up.

 migration/qemu-file.c | 10 +++++++++-
 migration/ram.c       | 20 ++++++++------------
 2 files changed, 17 insertions(+), 13 deletions(-)

-- 
1.9.1

[Qemu-devel] [PATCH 1/2] qemu-file: fix flaws of qemu_put_compression_data

[Liang Li]

There are some flaws in qemu_put_compression_data, this patch tries
to fix it. Now it can be used by other code.

Signed-off-by: Liang Li <liang.z.li@intel.com>
---
 migration/qemu-file.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 0bbd257..ef9cd4a 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -616,7 +616,9 @@ ssize_t qemu_put_compression_data(QEMUFile *f, const uint8_t *p, size_t size,
     ssize_t blen = IO_BUF_SIZE - f->buf_index - sizeof(int32_t);
 
     if (blen < compressBound(size)) {
-        return 0;
+        if (f->ops->writev_buffer || f->ops->put_buffer) {
+            qemu_fflush(f);
+        }
     }
     if (compress2(f->buf + f->buf_index + sizeof(int32_t), (uLongf *)&blen,
                   (Bytef *)p, size, level) != Z_OK) {
@@ -624,7 +626,13 @@ ssize_t qemu_put_compression_data(QEMUFile *f, const uint8_t *p, size_t size,
         return 0;
     }
     qemu_put_be32(f, blen);
+    if (f->ops->writev_buffer) {
+        add_to_iovec(f, f->buf + f->buf_index, blen);
+    }
     f->buf_index += blen;
+    if (f->buf_index == IO_BUF_SIZE) {
+        qemu_fflush(f);
+    }
     return blen + sizeof(int32_t);
 }
 
-- 
1.9.1

[Qemu-devel] [PATCH 2/2] migration: code clean up.

[Liang Li]

Use qemu_put_compression_data to do the compression directly
instead of using do_compress_ram_page, avoid some data copy.
very small improvement, but the code looks better.

Signed-off-by: Liang Li <liang.z.li@intel.com>
---
 migration/ram.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 1eb155a..44b3edc 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -911,22 +911,18 @@ static int ram_save_compressed_page(QEMUFile *f, RAMBlock *block,
                                     uint64_t *bytes_transferred)
 {
     int pages = -1;
-    uint64_t bytes_xmit;
+    uint64_t bytes_xmit = 0;
     uint8_t *p;
     int ret;
 
     p = block->host + offset;
 
-    bytes_xmit = 0;
     ret = ram_control_save_page(f, block->offset,
                                 offset, TARGET_PAGE_SIZE, &bytes_xmit);
     if (bytes_xmit) {
         *bytes_transferred += bytes_xmit;
         pages = 1;
     }
-    if (block == last_sent_block) {
-        offset |= RAM_SAVE_FLAG_CONTINUE;
-    }
     if (ret != RAM_SAVE_CONTROL_NOT_SUPP) {
         if (ret != RAM_SAVE_CONTROL_DELAYED) {
             if (bytes_xmit > 0) {
@@ -946,17 +942,17 @@ static int ram_save_compressed_page(QEMUFile *f, RAMBlock *block,
             flush_compressed_data(f);
             pages = save_zero_page(f, block, offset, p, bytes_transferred);
             if (pages == -1) {
-                set_compress_params(&comp_param[0], block, offset);
-                /* Use the qemu thread to compress the data to make sure the
-                 * first page is sent out before other pages
-                 */
-                bytes_xmit = do_compress_ram_page(&comp_param[0]);
-                acct_info.norm_pages++;
-                qemu_put_qemu_file(f, comp_param[0].file);
+                /* Make sure the first page is sent out before other pages */
+                bytes_xmit = save_page_header(f, block, offset |
+                                              RAM_SAVE_FLAG_COMPRESS_PAGE);
+                bytes_xmit += qemu_put_compression_data(f, p, TARGET_PAGE_SIZE,
+                                                migrate_compress_level());
                 *bytes_transferred += bytes_xmit;
+                acct_info.norm_pages++;
                 pages = 1;
             }
         } else {
+            offset |= RAM_SAVE_FLAG_CONTINUE;
             pages = save_zero_page(f, block, offset, p, bytes_transferred);
             if (pages == -1) {
                 pages = compress_page_with_multi_thread(f, block, offset,
-- 
1.9.1

Re: [Qemu-devel] [PATCH 0/2] fix the flaws of qemu_put_compression_data

[Juan Quintela]

Liang Li <liang.z.li@intel.com> wrote:
> This patch fixed the flaws in qemu_put_compression_data function.
> and cleanup the code based on the change.

Hi

We are in hard freeze.  My understanding is that this are
"optimizations" that can wait for 2.6:
- my understanding from commit from message one and from quick look at
  the code is that this change is not needed for current users, is that correct?
- we avoid a copy at the beginning of each block (micro-optimization)

If my understanding is correct, I am delaying this two patches to 2.6.

What do you think?

Later, Juan.

BTW, amit address was wrongly typed, I just fixed it.


>
> Liang Li (2):
>   qemu-file: fix flaws of qemu_put_compression_data
>   migration: code clean up.
>
>  migration/qemu-file.c | 10 +++++++++-
>  migration/ram.c       | 20 ++++++++------------
>  2 files changed, 17 insertions(+), 13 deletions(-)

Re: [Qemu-devel] [PATCH 1/2] qemu-file: fix flaws of qemu_put_compression_data

[Amit Shah]

On (Fri) 04 Dec 2015 [11:53:09], Liang Li wrote:
> There are some flaws in qemu_put_compression_data, this patch tries
> to fix it. Now it can be used by other code.

Can you please write a better description here?  What are the flaws?
What is being fixed?  What other users, and how is it now safer for
the other users to use this code?

Thanks,

		Amit

Re: [Qemu-devel] [PATCH 0/2] fix the flaws of qemu_put_compression_data

[Li, Liang Z]

> Hi
> 
> We are in hard freeze.  My understanding is that this are "optimizations" that
> can wait for 2.6:
> - my understanding from commit from message one and from quick look at
>   the code is that this change is not needed for current users, is that correct?
> - we avoid a copy at the beginning of each block (micro-optimization)
> 
> If my understanding is correct, I am delaying this two patches to 2.6.
> 
> What do you think?
> 

Yes, you are wright. I think it's ok to delay to 2.6.

Thanks.

Liang.

Re: [Qemu-devel] [PATCH 1/2] qemu-file: fix flaws of qemu_put_compression_data

[Li, Liang Z]

> On (Fri) 04 Dec 2015 [11:53:09], Liang Li wrote:
> > There are some flaws in qemu_put_compression_data, this patch tries to
> > fix it. Now it can be used by other code.
> 
> Can you please write a better description here?  What are the flaws?
> What is being fixed?  What other users, and how is it now safer for the other
> users to use this code?
> 

no problem, will add it in next version.  

Thanks,

Liang

> Thanks,
> 
> 		Amit