On %D, %SN wrote: %Q
%C Liang > -----Original Message----- > From: qemu-devel-bounces+liang.z.li=intel....@nongnu.org [mailto:qemu- > devel-bounces+liang.z.li=intel....@nongnu.org] On Behalf Of Juan Quintela > Sent: Tuesday, February 23, 2016 5:57 PM > To: Li, Liang Z > Cc: amit.s...@redhat.com; zhang.zhanghaili...@huawei.com; qemu- > de...@nongnu.org; dgilb...@redhat.com > Subject: Re: [Qemu-devel] [PATCH RESEND v2 1/2] qemu-file: Fix > qemu_put_compression_data flaw > > "Li, Liang Z" <liang.z...@intel.com> wrote: > > Ping ... > > > > Liang > > Hi > > >> We should fix this flaw to make it works with writable QEMUFile. > >> > >> Signed-off-by: Liang Li <liang.z...@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. > >> */ > > Ok, I still don't understand _why_ f can be writable on compression code, but > well. > > We return r when we are not able to write, right? > static int do_compress_ram_page(CompressParam *param) { > int bytes_sent, blen; > uint8_t *p; > RAMBlock *block = param->block; > ram_addr_t offset = param->offset; > > p = block->host + (offset & TARGET_PAGE_MASK); > > bytes_sent = save_page_header(param->file, block, offset | > RAM_SAVE_FLAG_COMPRESS_PAGE); > blen = qemu_put_compression_data(param->file, p, TARGET_PAGE_SIZE, > migrate_compress_level()); > bytes_sent += blen; > > return bytes_sent; > } > > But we don't check for zero when doing the compression, shouldn't this give > give an error? > > (yes, caller of do_co_compress_ram_page() don't check for zero either). > > I guess you are trying to do something different with the compression code > (otherwise this qemu_fflush() or add_to_iovec() don't make sense), but I > don't know what. > > With current code, the only thing that we miss is the check for errors, right? > And if we want to do something else with it, could you explain? Thanks. > > Later, Juan. I think these two threads will help to explain why I do that. https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg02675.html https://lists.gnu.org/archive/html/qemu-devel/2016-01/msg02674.html I just want to refine the code in the function ram_save_compressed_page(), so as to reduce some data copy. In the other hand, qemu_put_compression_data() is a common function, it maybe used by other code, but it's current implementation has some limitation, we should make it robust. Liang
