Re: [Qemu-devel] [PATCH 2/8] migration: stop allocating and freeingmemory frequently

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 03/19/2018 09:49 AM, jiang.biao2@xxxxxxxxxx wrote:
Hi, guangrong

+/* return the size after compression, or negative value on error */
+static int qemu_compress_data(z_stream *stream, uint8_t *dest, size_t dest_len,
+                              const uint8_t *source, size_t source_len)
+{
+    int err;
+
+    err = deflateReset(stream);
+    if (err != Z_OK) {
+        return -1;
+    }
+
+    stream->avail_in = source_len;
+    stream->next_in = (uint8_t *)source;
+    stream->avail_out = dest_len;
+    stream->next_out = dest;
+
duplicated code with qemu_uncompress(), would initializing stream outside
of qemu_compress_data() be better? In that case, we could pass much less
parameters down, and avoid the duplicated code. Or could we encapsulate
some struct to ease the case?

There are multiple places to do compression/uncompression in QEMU,
i am going to introduce common functions to cleanup these places,
that can be another patchset later...

+    err = deflate(stream, Z_FINISH);
+    if (err != Z_STREAM_END) {
+        return -1;
+    }
+
+    return stream->next_out - dest;
+}
+

@@ -683,8 +707,10 @@ ssize_t qemu_put_compression_data(QEMUFile *f, const uint8_t *p, size_t size,
return -1;
}
}
-    if (compress2(f->buf + f->buf_index + sizeof(int32_t), (uLongf *)&blen,
-                  (Bytef *)p, size, level) != Z_OK) {
+
+    blen = qemu_compress_data(stream, f->buf + f->buf_index + sizeof(int32_t),
+                              blen, p, size);
The "level" parameter is never used after the patch, could we just removed it?
On the other hand, deflate() of zlib supports compression level too(by
deflateInit(stream, level)), should we just reuse the level properly?  If not, the
*migrate parameter compress_level* will be useless.

The 'level' has been pushed to @stream:
+        if (deflateInit(&comp_param[i].stream,
+                           migrate_compress_level()) != Z_OK) {
+            goto exit;
+        }

+    if (blen < 0) {
error_report("Compress Failed!");
return 0;
}

+/* return the size after decompression, or negative value on error */
+static int qemu_uncompress(z_stream *stream, uint8_t *dest, size_t dest_len,
+                           uint8_t *source, size_t source_len)
The name of *qemu_uncompress* does not quite match *qemu_compress_data*,
would *qemu_uncompress_data* be better?

It's good to me. will rename it.

Besides, the prototype is not consistent with  *qemu_compress_data* either,
should the -*source- be -const- also here?

Okay.

Thanks!



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux