diff mbox

[for-2.0,34/47] dmg: drop broken bdrv_pread() loop

Message ID 1395835569-21193-35-git-send-email-stefanha@redhat.com
State New
Headers show

Commit Message

Stefan Hajnoczi March 26, 2014, 12:05 p.m. UTC
It is not necessary to check errno for EINTR and the block layer does
not produce short reads.  Therefore we can drop the loop that attempts
to read a compressed chunk.

The loop is buggy because it incorrectly adds the transferred bytes
twice:

  do {
      ret = bdrv_pread(...);
      i += ret;
  } while (ret >= 0 && ret + i < s->lengths[chunk]);

Luckily we can drop the loop completely and perform a single
bdrv_pread().

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/dmg.c | 15 ++-------------
 1 file changed, 2 insertions(+), 13 deletions(-)

Comments

Max Reitz March 28, 2014, 11:10 p.m. UTC | #1
On 26.03.2014 13:05, Stefan Hajnoczi wrote:
> It is not necessary to check errno for EINTR and the block layer does
> not produce short reads.  Therefore we can drop the loop that attempts
> to read a compressed chunk.
>
> The loop is buggy because it incorrectly adds the transferred bytes
> twice:
>
>    do {
>        ret = bdrv_pread(...);
>        i += ret;
>    } while (ret >= 0 && ret + i < s->lengths[chunk]);
>
> Luckily we can drop the loop completely and perform a single
> bdrv_pread().
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>   block/dmg.c | 15 ++-------------
>   1 file changed, 2 insertions(+), 13 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>
diff mbox

Patch

diff --git a/block/dmg.c b/block/dmg.c
index f4f3e8e..1cc5426 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -298,21 +298,10 @@  static inline int dmg_read_chunk(BlockDriverState *bs, int sector_num)
         s->current_chunk = s->n_chunks;
         switch (s->types[chunk]) {
         case 0x80000005: { /* zlib compressed */
-            int i;
-
             /* we need to buffer, because only the chunk as whole can be
              * inflated. */
-            i = 0;
-            do {
-                ret = bdrv_pread(bs->file, s->offsets[chunk] + i,
-                                 s->compressed_chunk + i,
-                                 s->lengths[chunk] - i);
-                if (ret < 0 && errno == EINTR) {
-                    ret = 0;
-                }
-                i += ret;
-            } while (ret >= 0 && ret + i < s->lengths[chunk]);
-
+            ret = bdrv_pread(bs->file, s->offsets[chunk],
+                             s->compressed_chunk, s->lengths[chunk]);
             if (ret != s->lengths[chunk]) {
                 return -1;
             }