[Qemu-devel] [PULL 0/6] Block patches

[Qemu-devel] [PULL 0/6] Block patches

[Kevin Wolf]

The following changes since commit 47acdd63a33a5966bf4fc94a6ac835d72a70c555:

  target-microblaze: Use the new qemu_ld/st opcodes (2013-12-13 10:00:42 +1000)

are available in the git repository at:

  git://repo.or.cz/qemu/kevin.git tags/for-anthony

for you to fetch changes up to c547e5640d5b0993cdfb252331065c1a1d813bd8:

  blkdebug: Use QLIST_FOREACH_SAFE to resume IO (2013-12-13 17:11:19 +0100)

----------------------------------------------------------------
Block patches

----------------------------------------------------------------
Fam Zheng (1):
      blkdebug: Use QLIST_FOREACH_SAFE to resume IO

Kevin Wolf (1):
      qapi-schema.json: Change 1.8 reference to 2.0

Liu Yuan (1):
      sheepdog: check if '-o redundancy' is passed from user

Peter Lieven (2):
      block: expect get_block_status errors in bdrv_make_zero
      qemu-img: make progress output more accurate during convert

Stefan Weil (1):
      block/vvfat: Fix compiler warnings for OpenBSD

 block.c          |  5 +++++
 block/blkdebug.c |  8 ++++----
 block/sheepdog.c |  8 +++++---
 block/vvfat.c    | 43 ++++++++++++++++++++++---------------------
 qapi-schema.json |  2 +-
 qemu-img.c       | 26 +++++++++++++++++++++-----
 6 files changed, 58 insertions(+), 34 deletions(-)

[Qemu-devel] [PULL 1/6] sheepdog: check if '-o redundancy' is passed from user

[Kevin Wolf]

From: Liu Yuan <namei.unix@gmail.com>

This fix a segfault (that is caused by b3af018f3) of following command:

$ qemu-img convert some_img sheepdog:some_img

Cc: qemu-devel@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Liu Yuan <namei.unix@gmail.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/sheepdog.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index b4ae50f..d1c812d 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1666,9 +1666,11 @@ static int sd_create(const char *filename, QEMUOptionParameter *options,
                 goto out;
             }
         } else if (!strcmp(options->name, BLOCK_OPT_REDUNDANCY)) {
-            ret = parse_redundancy(s, options->value.s);
-            if (ret < 0) {
-                goto out;
+            if (options->value.s) {
+                ret = parse_redundancy(s, options->value.s);
+                if (ret < 0) {
+                    goto out;
+                }
             }
         }
         options++;
-- 
1.8.1.4

[Qemu-devel] [PULL 2/6] qapi-schema.json: Change 1.8 reference to 2.0

[Kevin Wolf]

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qapi-schema.json | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index d6f8615..c3c939c 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -949,7 +949,7 @@
 #             (only present if removable is true)
 #
 # @dirty-bitmaps: #optional dirty bitmaps information (only present if the
-#                 driver has one or more dirty bitmaps) (Since 1.8)
+#                 driver has one or more dirty bitmaps) (Since 2.0)
 #
 # @io-status: #optional @BlockDeviceIoStatus. Only present if the device
 #             supports it and the VM is configured to stop on errors
-- 
1.8.1.4

[Qemu-devel] [PULL 3/6] block/vvfat: Fix compiler warnings for OpenBSD

[Kevin Wolf]

From: Stefan Weil <sw@weilnetz.de>

The buildbot shows these compiler warnings:

block/vvfat.c: In function 'create_short_and_long_name':
block/vvfat.c:620: warning: array size (8) smaller than bound length (11)
block/vvfat.c:620: warning: array size (8) smaller than bound length (11)
block/vvfat.c:635: warning: array size (8) smaller than bound length (11)
block/vvfat.c:635: warning: array size (8) smaller than bound length (11)

They are caused by tricky code where 8 characters for the name are followed
by 3 characters for the extension, and some operations touch both name and
extension.

Using an 11 character name which includes the extension fixes the compiler
warning, satisfies cppcheck, valgrind and maybe other static and dynamic
code checkers, and even simplifies some parts of the code.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/vvfat.c | 43 ++++++++++++++++++++++---------------------
 1 file changed, 22 insertions(+), 21 deletions(-)

diff --git a/block/vvfat.c b/block/vvfat.c
index 3ddaa0b..1abb8ad 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -266,8 +266,7 @@ typedef struct mbr_t {
 } QEMU_PACKED mbr_t;
 
 typedef struct direntry_t {
-    uint8_t name[8];
-    uint8_t extension[3];
+    uint8_t name[8 + 3];
     uint8_t attributes;
     uint8_t reserved[2];
     uint16_t ctime;
@@ -518,11 +517,9 @@ static inline uint8_t fat_chksum(const direntry_t* entry)
     uint8_t chksum=0;
     int i;
 
-    for(i=0;i<11;i++) {
-        unsigned char c;
-
-        c = (i < 8) ? entry->name[i] : entry->extension[i-8];
-        chksum=(((chksum&0xfe)>>1)|((chksum&0x01)?0x80:0)) + c;
+    for (i = 0; i < ARRAY_SIZE(entry->name); i++) {
+        chksum = (((chksum & 0xfe) >> 1) |
+                  ((chksum & 0x01) ? 0x80 : 0)) + entry->name[i];
     }
 
     return chksum;
@@ -617,7 +614,7 @@ static inline direntry_t* create_short_and_long_name(BDRVVVFATState* s,
 
     if(is_dot) {
 	entry=array_get_next(&(s->directory));
-	memset(entry->name,0x20,11);
+        memset(entry->name, 0x20, sizeof(entry->name));
 	memcpy(entry->name,filename,strlen(filename));
 	return entry;
     }
@@ -632,12 +629,14 @@ static inline direntry_t* create_short_and_long_name(BDRVVVFATState* s,
 	i = 8;
 
     entry=array_get_next(&(s->directory));
-    memset(entry->name,0x20,11);
+    memset(entry->name, 0x20, sizeof(entry->name));
     memcpy(entry->name, filename, i);
 
-    if(j > 0)
-	for (i = 0; i < 3 && filename[j+1+i]; i++)
-	    entry->extension[i] = filename[j+1+i];
+    if (j > 0) {
+        for (i = 0; i < 3 && filename[j + 1 + i]; i++) {
+            entry->name[8 + i] = filename[j + 1 + i];
+        }
+    }
 
     /* upcase & remove unwanted characters */
     for(i=10;i>=0;i--) {
@@ -861,8 +860,7 @@ static int init_directories(BDRVVVFATState* s,
     {
 	direntry_t* entry=array_get_next(&(s->directory));
 	entry->attributes=0x28; /* archive | volume label */
-	memcpy(entry->name,"QEMU VVF",8);
-	memcpy(entry->extension,"AT ",3);
+        memcpy(entry->name, "QEMU VVFAT ", sizeof(entry->name));
     }
 
     /* Now build FAT, and write back information into directory */
@@ -1591,17 +1589,20 @@ static int parse_short_name(BDRVVVFATState* s,
 	    lfn->name[i] = direntry->name[i];
     }
 
-    for (j = 2; j >= 0 && direntry->extension[j] == ' '; j--);
+    for (j = 2; j >= 0 && direntry->name[8 + j] == ' '; j--) {
+    }
     if (j >= 0) {
 	lfn->name[i++] = '.';
 	lfn->name[i + j + 1] = '\0';
 	for (;j >= 0; j--) {
-	    if (direntry->extension[j] <= ' ' || direntry->extension[j] > 0x7f)
-		return -2;
-	    else if (s->downcase_short_names)
-		lfn->name[i + j] = qemu_tolower(direntry->extension[j]);
-	    else
-		lfn->name[i + j] = direntry->extension[j];
+            uint8_t c = direntry->name[8 + j];
+            if (c <= ' ' || c > 0x7f) {
+                return -2;
+            } else if (s->downcase_short_names) {
+                lfn->name[i + j] = qemu_tolower(c);
+            } else {
+                lfn->name[i + j] = c;
+            }
 	}
     } else
 	lfn->name[i + j + 1] = '\0';
-- 
1.8.1.4

[Qemu-devel] [PULL 4/6] block: expect get_block_status errors in bdrv_make_zero

[Kevin Wolf]

From: Peter Lieven <pl@kamp.de>

during testing around with 4k LUNs a bad target implementation
triggert an -EIO in iscsi_get_block_status, but it got never caught
resulting in an infinite loop.

CC: qemu-stable@nongnu.org
Signed-off-by: Peter Lieven <pl@kamp.de>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/block.c b/block.c
index 13f001a..64e7d22 100644
--- a/block.c
+++ b/block.c
@@ -2421,6 +2421,11 @@ int bdrv_make_zero(BlockDriverState *bs, BdrvRequestFlags flags)
             nb_sectors = INT_MAX;
         }
         ret = bdrv_get_block_status(bs, sector_num, nb_sectors, &n);
+        if (ret < 0) {
+            error_report("error getting block status at sector %" PRId64 ": %s",
+                         sector_num, strerror(-ret));
+            return ret;
+        }
         if (ret & BDRV_BLOCK_ZERO) {
             sector_num += n;
             continue;
-- 
1.8.1.4

[Qemu-devel] [PULL 5/6] qemu-img: make progress output more accurate during convert

[Kevin Wolf]

From: Peter Lieven <pl@kamp.de>

the progress output is very bumpy if the input images contains
a significant portion of unallocated sectors. This patch
checks how much sectors are allocated a priori if progress
output is selected.

Signed-off-by: Peter Lieven <pl@kamp.de>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qemu-img.c | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 7dfe982..a4b3931 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1135,8 +1135,7 @@ static int img_convert(int argc, char **argv)
     const char *fmt, *out_fmt, *cache, *out_baseimg, *out_filename;
     BlockDriver *drv, *proto_drv;
     BlockDriverState **bs = NULL, *out_bs = NULL;
-    int64_t total_sectors, nb_sectors, sector_num, bs_offset,
-            sector_num_next_status = 0;
+    int64_t total_sectors, nb_sectors, sector_num, bs_offset;
     uint64_t bs_sectors;
     uint8_t * buf = NULL;
     size_t bufsectors = IO_BUF_SIZE / BDRV_SECTOR_SIZE;
@@ -1505,6 +1504,8 @@ static int img_convert(int argc, char **argv)
         /* signal EOF to align */
         bdrv_write_compressed(out_bs, 0, NULL, 0);
     } else {
+        int64_t sectors_to_read, sectors_read, sector_num_next_status;
+        bool count_allocated_sectors;
         int has_zero_init = min_sparse ? bdrv_has_zero_init(out_bs) : 0;
 
         if (!has_zero_init && bdrv_can_write_zeroes_with_unmap(out_bs)) {
@@ -1515,12 +1516,21 @@ static int img_convert(int argc, char **argv)
             has_zero_init = 1;
         }
 
+        sectors_to_read = total_sectors;
+        count_allocated_sectors = progress && (out_baseimg || has_zero_init);
+restart:
         sector_num = 0; // total number of sectors converted so far
-        nb_sectors = total_sectors - sector_num;
+        sectors_read = 0;
+        sector_num_next_status = 0;
 
         for(;;) {
             nb_sectors = total_sectors - sector_num;
             if (nb_sectors <= 0) {
+                if (count_allocated_sectors) {
+                    sectors_to_read = sectors_read;
+                    count_allocated_sectors = false;
+                    goto restart;
+                }
                 ret = 0;
                 break;
             }
@@ -1586,8 +1596,14 @@ static int img_convert(int argc, char **argv)
             }
 
             n = MIN(n, bs_sectors - (sector_num - bs_offset));
-            n1 = n;
 
+            sectors_read += n;
+            if (count_allocated_sectors) {
+                sector_num += n;
+                continue;
+            }
+
+            n1 = n;
             ret = bdrv_read(bs[bs_i], sector_num - bs_offset, buf, n);
             if (ret < 0) {
                 error_report("error while reading sector %" PRId64 ": %s",
@@ -1612,7 +1628,7 @@ static int img_convert(int argc, char **argv)
                 n -= n1;
                 buf1 += n1 * 512;
             }
-            qemu_progress_print(100.0 * sector_num / total_sectors, 0);
+            qemu_progress_print(100.0 * sectors_read / sectors_to_read, 0);
         }
     }
 out:
-- 
1.8.1.4

[Qemu-devel] [PULL 6/6] blkdebug: Use QLIST_FOREACH_SAFE to resume IO

[Kevin Wolf]

From: Fam Zheng <famz@redhat.com>

Qemu-iotest 030 was broken.

When the coroutine runs and finishes, it will remove itself from the req
list, so let's use safe version of foreach to avoid use after free.

Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/blkdebug.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/block/blkdebug.c b/block/blkdebug.c
index 37cf028..957be2c 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -594,9 +594,9 @@ static int blkdebug_debug_breakpoint(BlockDriverState *bs, const char *event,
 static int blkdebug_debug_resume(BlockDriverState *bs, const char *tag)
 {
     BDRVBlkdebugState *s = bs->opaque;
-    BlkdebugSuspendedReq *r;
+    BlkdebugSuspendedReq *r, *next;
 
-    QLIST_FOREACH(r, &s->suspended_reqs, next) {
+    QLIST_FOREACH_SAFE(r, &s->suspended_reqs, next, next) {
         if (!strcmp(r->tag, tag)) {
             qemu_coroutine_enter(r->co, NULL);
             return 0;
@@ -609,7 +609,7 @@ static int blkdebug_debug_remove_breakpoint(BlockDriverState *bs,
                                             const char *tag)
 {
     BDRVBlkdebugState *s = bs->opaque;
-    BlkdebugSuspendedReq *r;
+    BlkdebugSuspendedReq *r, *r_next;
     BlkdebugRule *rule, *next;
     int i, ret = -ENOENT;
 
@@ -622,7 +622,7 @@ static int blkdebug_debug_remove_breakpoint(BlockDriverState *bs,
             }
         }
     }
-    QLIST_FOREACH(r, &s->suspended_reqs, next) {
+    QLIST_FOREACH_SAFE(r, &s->suspended_reqs, next, r_next) {
         if (!strcmp(r->tag, tag)) {
             qemu_coroutine_enter(r->co, NULL);
             ret = 0;
-- 
1.8.1.4