[Qemu-devel] [v2 0/2] Fix flaw of qemu_put_compression_data

[Qemu-devel] [v2 0/2] Fix flaw of qemu_put_compression_data

[Liang Li]

The implementation of qemu_put_compression_data only consider the case
QEMUFile is writable, it can't work with a writable QEMUFile and does
not provide any measure to prevent users from using it with a writable
QEMUFile. For safety, it should be improved to avoid some issues.

ram_save_compressed_page can be refined based on the change of
qemu_put_compression_data, very small improvement, but code looks better.

Liang Li (2):
  qemu-file: Fix qemu_put_compression_data flaw
  migration: refine ram_save_compressed_page

 migration/qemu-file.c | 23 +++++++++++++++++++++--
 migration/ram.c       | 20 ++++++++------------
 2 files changed, 29 insertions(+), 14 deletions(-)

-- 
1.9.1

[Qemu-devel] [v2 1/2] qemu-file: Fix qemu_put_compression_data flaw

[Liang Li]

Current qemu_put_compression_data can only work with no writable
QEMUFile, and can't work with the writable QEMUFile. But it does
not provide any measure to prevent users from using it with a
writable QEMUFile.

We should fix this flaw to make it works with writable QEMUFile.

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

diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 0bbd257..b956ab6 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -606,8 +606,14 @@ uint64_t qemu_get_be64(QEMUFile *f)
     return v;
 }
 
-/* compress size bytes of data start at p with specific compression
+/* Compress size bytes of data start at p with specific compression
  * level and store the compressed data to the buffer of f.
+ *
+ * When f is not writable, return 0 if f has no space to save the
+ * compressed data.
+ * When f is wirtable and it has no space to save the compressed data,
+ * do fflush first, if f still has no space to save the compressed
+ * data, return 0.
  */
 
 ssize_t qemu_put_compression_data(QEMUFile *f, const uint8_t *p, size_t size,
@@ -616,7 +622,14 @@ 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 (!qemu_file_is_writable(f)) {
+            return 0;
+        }
+        qemu_fflush(f);
+        blen = IO_BUF_SIZE - sizeof(int32_t);
+        if (blen < compressBound(size)) {
+            return 0;
+        }
     }
     if (compress2(f->buf + f->buf_index + sizeof(int32_t), (uLongf *)&blen,
                   (Bytef *)p, size, level) != Z_OK) {
@@ -624,7 +637,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] [v2 2/2] migration: refine ram_save_compressed_page

[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