From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:59146)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <quintela@redhat.com>) id 1a4oDM-0006Y0-Gl
	for qemu-devel@nongnu.org; Fri, 04 Dec 2015 06:07:49 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <quintela@redhat.com>) id 1a4oDI-0006sd-G3
	for qemu-devel@nongnu.org; Fri, 04 Dec 2015 06:07:48 -0500
Received: from mx1.redhat.com ([209.132.183.28]:37947)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <quintela@redhat.com>) id 1a4oDI-0006sX-Az
	for qemu-devel@nongnu.org; Fri, 04 Dec 2015 06:07:44 -0500
From: Juan Quintela <quintela@redhat.com>
In-Reply-To: <1449201128-22779-2-git-send-email-liang.z.li@intel.com> (Liang
	Li's message of "Fri, 4 Dec 2015 11:52:07 +0800")
References: <1449201128-22779-1-git-send-email-liang.z.li@intel.com>
	<1449201128-22779-2-git-send-email-liang.z.li@intel.com>
Date: Fri, 04 Dec 2015 12:07:31 +0100
Message-ID: <87h9jyjwuk.fsf@emacs.mitica>
MIME-Version: 1.0
Content-Type: text/plain
Subject: Re: [Qemu-devel] [PATCH 1/2] qemu-file: fix flaws of
	qemu_put_compression_data
Reply-To: quintela@redhat.com
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Liang Li <liang.z.li@intel.com>
Cc: amit.shah@redhat.com, qemu-devel@nongnu.org, dgilbert@redhat.com

Liang Li <liang.z.li@intel.com> wrote:
> 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);
> +        }
>      }

With your change, when we arrive here:

- blen could still be smaller that compressBound(size), you need to
  recheck
- blen could have changed, but you don't take that in account for the
  following caller.

So, I think code has a bug?

Later, Juan.

>      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);
>  }