[PATCH] block/qcow2-threads: fix qcow2_decompress

[PATCH] block/qcow2-threads: fix qcow2_decompress

[Vladimir Sementsov-Ogievskiy]

On success path we return what inflate() returns instead of 0. And it
most probably works for Z_STREAM_END as it is positive, but is
definitely broken for Z_BUF_ERROR.

While being here, switch to errno return code, to be closer to
qcow2_compress API (and usual expectations).

Revert condition in if to be more positive. Drop dead initialization of
ret.

Cc: qemu-stable@nongnu.org # v4.0
Fixes: 341926ab83e2b
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---

Hi!

Reviewing Den's series about zstd in qcow2 support, I found an existing
bug. Let's fix it. This is to be a new base of zstd series.

 block/qcow2-threads.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/block/qcow2-threads.c b/block/qcow2-threads.c
index 8f5a0d1ebe..0d193d1614 100644
--- a/block/qcow2-threads.c
+++ b/block/qcow2-threads.c
@@ -128,12 +128,12 @@ static ssize_t qcow2_compress(void *dest, size_t dest_size,
  * @src - source buffer, @src_size bytes
  *
  * Returns: 0 on success
- *          -1 on fail
+ *          -EIO on fail
  */
 static ssize_t qcow2_decompress(void *dest, size_t dest_size,
                                 const void *src, size_t src_size)
 {
-    int ret = 0;
+    int ret;
     z_stream strm;
 
     memset(&strm, 0, sizeof(strm));
@@ -144,17 +144,19 @@ static ssize_t qcow2_decompress(void *dest, size_t dest_size,
 
     ret = inflateInit2(&strm, -12);
     if (ret != Z_OK) {
-        return -1;
+        return -EIO;
     }
 
     ret = inflate(&strm, Z_FINISH);
-    if ((ret != Z_STREAM_END && ret != Z_BUF_ERROR) || strm.avail_out != 0) {
+    if ((ret == Z_STREAM_END || ret == Z_BUF_ERROR) && strm.avail_out == 0) {
         /*
          * We approve Z_BUF_ERROR because we need @dest buffer to be filled, but
          * @src buffer may be processed partly (because in qcow2 we know size of
          * compressed data with precision of one sector)
          */
-        ret = -1;
+        ret = 0;
+    } else {
+        ret = -EIO;
     }
 
     inflateEnd(&strm);
-- 
2.21.0

Re: [PATCH] block/qcow2-threads: fix qcow2_decompress

[Alberto Garcia]

On Mon 02 Mar 2020 04:09:30 PM CET, Vladimir Sementsov-Ogievskiy wrote:
> On success path we return what inflate() returns instead of 0. And it
> most probably works for Z_STREAM_END as it is positive, but is
> definitely broken for Z_BUF_ERROR.
>
> While being here, switch to errno return code, to be closer to
> qcow2_compress API (and usual expectations).
>
> Revert condition in if to be more positive. Drop dead initialization of
> ret.
>
> Cc: qemu-stable@nongnu.org # v4.0
> Fixes: 341926ab83e2b
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Reviewed-by: Alberto Garcia <berto@igalia.com>

Berto

Re: [PATCH] block/qcow2-threads: fix qcow2_decompress

[Ján Tomko]

[-- Attachment #1: Type: text/plain, Size: 874 bytes --]

On a Monday in 2020, Vladimir Sementsov-Ogievskiy wrote:
>On success path we return what inflate() returns instead of 0. And it
>most probably works for Z_STREAM_END as it is positive, but is
>definitely broken for Z_BUF_ERROR.
>
>While being here, switch to errno return code, to be closer to
>qcow2_compress API (and usual expectations).
>
>Revert condition in if to be more positive. Drop dead initialization of
>ret.
>
>Cc: qemu-stable@nongnu.org # v4.0
>Fixes: 341926ab83e2b
>Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>---
>
>Hi!
>
>Reviewing Den's series about zstd in qcow2 support, I found an existing
>bug. Let's fix it. This is to be a new base of zstd series.
>
> block/qcow2-threads.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] block/qcow2-threads: fix qcow2_decompress

[Max Reitz]

[-- Attachment #1.1: Type: text/plain, Size: 946 bytes --]

On 02.03.20 16:09, Vladimir Sementsov-Ogievskiy wrote:
> On success path we return what inflate() returns instead of 0. And it
> most probably works for Z_STREAM_END as it is positive, but is
> definitely broken for Z_BUF_ERROR.
> 
> While being here, switch to errno return code, to be closer to
> qcow2_compress API (and usual expectations).
> 
> Revert condition in if to be more positive. Drop dead initialization of
> ret.
> 
> Cc: qemu-stable@nongnu.org # v4.0
> Fixes: 341926ab83e2b
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> 
> Hi!
> 
> Reviewing Den's series about zstd in qcow2 support, I found an existing
> bug. Let's fix it. This is to be a new base of zstd series.
> 
>  block/qcow2-threads.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)

Thanks, applied to my block branch:

https://git.xanclic.moe/XanClic/qemu/commits/branch/block

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]