From a3120deee5fc1d702ba5da98fd9c845ad1c8f301 Mon Sep 17 00:00:00 2001 From: Liu Yuan Date: Mon, 9 Dec 2013 00:11:20 +0800 Subject: [PATCH 1/6] sheepdog: check if '-o redundancy' is passed from user 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 Cc: Stefan Hajnoczi Signed-off-by: Liu Yuan Signed-off-by: Kevin Wolf --- block/sheepdog.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/block/sheepdog.c b/block/sheepdog.c index b4ae50f44d..d1c812df3d 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++; From 01443e1388971999514511a26f9d36a7cdaa2cc2 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Tue, 10 Dec 2013 14:01:27 +0100 Subject: [PATCH 2/6] qapi-schema.json: Change 1.8 reference to 2.0 Reviewed-by: Eric Blake Signed-off-by: Kevin Wolf --- qapi-schema.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qapi-schema.json b/qapi-schema.json index d6f8615942..c3c939c8c3 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 From f671d173c7e1da555b693e8b14f3ed0852601809 Mon Sep 17 00:00:00 2001 From: Stefan Weil Date: Wed, 11 Dec 2013 21:37:11 +0100 Subject: [PATCH 3/6] block/vvfat: Fix compiler warnings for OpenBSD 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 Reviewed-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- block/vvfat.c | 43 ++++++++++++++++++++++--------------------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/block/vvfat.c b/block/vvfat.c index 3ddaa0bcce..1abb8ad8e4 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'; From 3d94ce60ae7ad7c31dc143fdd9da95c61b4e529e Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Thu, 12 Dec 2013 13:57:05 +0100 Subject: [PATCH 4/6] block: expect get_block_status errors in bdrv_make_zero 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 Reviewed-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- block.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/block.c b/block.c index 13f001ad69..64e7d220c6 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; From 802c3d4ccc9853ee11c742bc206f284f04259426 Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Thu, 5 Dec 2013 15:54:53 +0100 Subject: [PATCH 5/6] qemu-img: make progress output more accurate during convert 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 Signed-off-by: Kevin Wolf --- qemu-img.c | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/qemu-img.c b/qemu-img.c index 7dfe982b0c..a4b3931174 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: From c547e5640d5b0993cdfb252331065c1a1d813bd8 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Fri, 13 Dec 2013 15:25:12 +0800 Subject: [PATCH 6/6] blkdebug: Use QLIST_FOREACH_SAFE to resume IO 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 Signed-off-by: Kevin Wolf --- block/blkdebug.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/block/blkdebug.c b/block/blkdebug.c index 37cf028545..957be2c76b 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;