From b135233b0da2b20af4cabc7663e0b65ae896b75f Mon Sep 17 00:00:00 2001 From: QingFeng Hao Date: Fri, 16 Dec 2016 06:47:23 +0100 Subject: [PATCH 01/21] iotests: Fix a problem in common.filter If TEST_DIR is set to /tmp, test case 144 will fail. The reason is that TEST_DIR resembles 144's test image name tmp.qcow2. When 144 is testing $TEST_DIR/tmp.qcow2, it wants to replace $TEST_DIR/tmp.qcow2 to TEST_DIR/tmp.qcow2, but actually it will fail and get TEST_DIRTEST_DIR.qcow2 in this case. The fix is just to modify the code to replace $TEST_DIR/ with TEST_DIR/. Signed-off-by: QingFeng Hao Message-id: 20161216054723.96055-2-haoqf@linux.vnet.ibm.com Reviewed-by: Eric Blake [mreitz: Fixed commit message and dropped superfluous escaping] Signed-off-by: Max Reitz --- tests/qemu-iotests/common.filter | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter index 240ed0697a..4befd865f4 100644 --- a/tests/qemu-iotests/common.filter +++ b/tests/qemu-iotests/common.filter @@ -35,7 +35,7 @@ _filter_generated_node_ids() # replace occurrences of the actual TEST_DIR value with TEST_DIR _filter_testdir() { - sed -e "s#$TEST_DIR#TEST_DIR#g" + sed -e "s#$TEST_DIR/#TEST_DIR/#g" } # replace occurrences of the actual IMGFMT value with IMGFMT From 6b33f3ae8b79726ef0812597b8a83c3e82d31514 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Thu, 1 Dec 2016 03:05:08 +0100 Subject: [PATCH 02/21] qemu-img: Improve commit invalid base message When trying to invoke qemu-img commit with a base image file name that is not part of the top image's backing chain, the user receives a rather plain "Base not found" error message. This is not really helpful because it does not explain what "not found" means, potentially leaving the user wondering why qemu cannot find a file despite it clearly existing in the file system. Improve the error message by clarifying that "not found" means "not found in the top image's backing chain". Reported-by: Ala Hino Signed-off-by: Max Reitz Message-id: 20161201020508.24417-1-mreitz@redhat.com Reviewed-by: Eric Blake Signed-off-by: Max Reitz --- qemu-img.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/qemu-img.c b/qemu-img.c index 74e3362653..933876cfe1 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -912,7 +912,9 @@ static int img_commit(int argc, char **argv) if (base) { base_bs = bdrv_find_backing_image(bs, base); if (!base_bs) { - error_setg(&local_err, QERR_BASE_NOT_FOUND, base); + error_setg(&local_err, + "Did not find '%s' in the backing chain of '%s'", + base, filename); goto done; } } else { From 9adceb02133f856fc69bf105848849f409e805b9 Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Thu, 19 Jan 2017 21:07:58 +0800 Subject: [PATCH 03/21] qapi: Tweak error message of bdrv_query_image_info @bs doesn't always have a device name, such as when it comes from "qemu-img info". Report file name instead. Signed-off-by: Fam Zheng Message-id: 20170119130759.28319-2-famz@redhat.com Reviewed-by: Eric Blake Signed-off-by: Max Reitz --- block/qapi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/block/qapi.c b/block/qapi.c index a62e862f3c..63297353f7 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -237,8 +237,8 @@ void bdrv_query_image_info(BlockDriverState *bs, size = bdrv_getlength(bs); if (size < 0) { - error_setg_errno(errp, -size, "Can't get size of device '%s'", - bdrv_get_device_name(bs)); + error_setg_errno(errp, -size, "Can't get image size '%s'", + bs->exact_filename); goto out; } From 53b63460f61906340bfe73d263c47896f68f7b9d Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Thu, 19 Jan 2017 21:07:59 +0800 Subject: [PATCH 04/21] iotests: Fix reference output for 059 It was broken by efaa7c4eeb7 when it dropped the device name "image" from BB API. Now this error message text is updated again, sync it up. Signed-off-by: Fam Zheng Message-id: 20170119130759.28319-3-famz@redhat.com Reviewed-by: Eric Blake Signed-off-by: Max Reitz --- tests/qemu-iotests/059.out | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out index 678adb4379..19bd50de1e 100644 --- a/tests/qemu-iotests/059.out +++ b/tests/qemu-iotests/059.out @@ -2361,5 +2361,5 @@ Offset Length Mapped to File 0x140000000 0x10000 0x50000 TEST_DIR/iotest-version3-s003.vmdk === Testing afl image with a very large capacity === -qemu-img: Can't get size of device 'image': File too large +qemu-img: Can't get image size 'TEST_DIR/afl9.IMGFMT': File too large *** done From 36bd4228126139a382a4fdbc49c96798a9894626 Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Tue, 3 Jan 2017 16:05:56 +0000 Subject: [PATCH 05/21] iotests: record separate timings per format,protocol pair The 'check' program records timings for each test that is run. These timings are only valid, however, for a particular format/protocol combination. So if frequently running 'check' with a variety of different formats or protocols, the times printed can be very misleading. Instead of having a single 'check.time' file, maintain multiple 'check.time-$IMGPROTO-$IMGFMT' files. Signed-off-by: Daniel P. Berrange Message-id: 20170103160556.9895-1-berrange@redhat.com Reviewed-by: Eric Blake Signed-off-by: Max Reitz --- tests/qemu-iotests/.gitignore | 2 +- tests/qemu-iotests/Makefile | 2 +- tests/qemu-iotests/check | 12 +++++++----- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/tests/qemu-iotests/.gitignore b/tests/qemu-iotests/.gitignore index 0711cbdbf3..da62054000 100644 --- a/tests/qemu-iotests/.gitignore +++ b/tests/qemu-iotests/.gitignore @@ -1,5 +1,5 @@ check.log -check.time +check.time* common.env *.out.bad *.notrun diff --git a/tests/qemu-iotests/Makefile b/tests/qemu-iotests/Makefile index 2fb527c5b5..27380e60c1 100644 --- a/tests/qemu-iotests/Makefile +++ b/tests/qemu-iotests/Makefile @@ -1,5 +1,5 @@ -CLEANFILES= *.out.bad *.notrun check.log check.time +CLEANFILES= *.out.bad *.notrun check.log check.time* # no default target default: diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check index 4cba2151e4..4b1c6749b7 100755 --- a/tests/qemu-iotests/check +++ b/tests/qemu-iotests/check @@ -129,6 +129,8 @@ fi # exit 1 #fi +TIMESTAMP_FILE=check.time-$IMGPROTO-$IMGFMT + tmp="${TEST_DIR}"/$$ _wallclock() @@ -155,9 +157,9 @@ _wrapup() : elif $needwrap then - if [ -f check.time -a -f $tmp.time ] + if [ -f $TIMESTAMP_FILE -a -f $tmp.time ] then - cat check.time $tmp.time \ + cat $TIMESTAMP_FILE $tmp.time \ | $AWK_PROG ' { t[$1] = $2 } END { if (NR > 0) { @@ -165,7 +167,7 @@ END { if (NR > 0) { } }' \ | sort -n >$tmp.out - mv $tmp.out check.time + mv $tmp.out $TIMESTAMP_FILE fi if [ -f $tmp.expunged ] @@ -223,7 +225,7 @@ echo "preamble" > "${TEST_DIR}"/check.sts # don't leave old full output behind on a clean run rm -f check.full -[ -f check.time ] || touch check.time +[ -f $TIMESTAMP_FILE ] || touch $TIMESTAMP_FILE FULL_IMGFMT_DETAILS=`_full_imgfmt_details` FULL_IMGPROTO_DETAILS=`_full_imgproto_details` @@ -277,7 +279,7 @@ do # really going to try and run this one # rm -f $seq.out.bad - lasttime=`sed -n -e "/^$seq /s/.* //p" Date: Fri, 16 Dec 2016 06:20:40 +0100 Subject: [PATCH 06/21] block/vmdk: Fix the endian problem of buf_len and lba The problem was triggered by qemu-iotests case 055. It failed when it was comparing the compressed vmdk image with original test.img. The cause is that buf_len in vmdk_write_extent wasn't converted to little-endian before it was stored to disk. But later vmdk_read_extent read it and converted it from little-endian to cpu endian. If the cpu is big-endian like s390, the problem will happen and the data length read by vmdk_read_extent will become invalid! The fix is to add the conversion in vmdk_write_extent, meanwhile, repair the endianness problem of lba field which shall also be converted to little-endian before storing to disk. Cc: qemu-stable@nongnu.org Signed-off-by: QingFeng Hao Signed-off-by: Jing Liu Signed-off-by: Kevin Wolf Reviewed-by: Fam Zheng Message-id: 20161216052040.53067-2-haoqf@linux.vnet.ibm.com Signed-off-by: Max Reitz --- block/vmdk.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/block/vmdk.c b/block/vmdk.c index 7750212969..393c84d8b1 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -1361,8 +1361,8 @@ static int vmdk_write_extent(VmdkExtent *extent, int64_t cluster_offset, goto out; } - data->lba = offset >> BDRV_SECTOR_BITS; - data->size = buf_len; + data->lba = cpu_to_le64(offset >> BDRV_SECTOR_BITS); + data->size = cpu_to_le32(buf_len); n_bytes = buf_len + sizeof(VmdkGrainMarker); iov = (struct iovec) { From 418661e0324c1c419552cf24bd4447292e884bdd Mon Sep 17 00:00:00 2001 From: Jeff Cody Date: Wed, 25 Jan 2017 20:08:20 -0500 Subject: [PATCH 07/21] block: check full backing filename when searching protocol filenames In bdrv_find_backing_image(), if we are searching an image for a backing file that contains a protocol, we currently only compare unmodified paths. However, some management software will change the backing filename to be a relative filename in a path. QEMU is able to handle this fine, because internally it will use path_combine to put together the full protocol URI. However, this can lead to an inability to match an image during a QAPI command that needs to use bdrv_find_backing_image() to find the image, when it is searched by the full URI. When searching for a protocol filename, if the straight comparison fails, this patch will also compare against the full backing filename to see if that is a match. Signed-off-by: Jeff Cody Message-id: c2d025adca8a2b665189e6f4cf080f44126d0b6b.1485392617.git.jcody@redhat.com Reviewed-by: Max Reitz Signed-off-by: Max Reitz --- block.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/block.c b/block.c index 1dbc060c3f..b8bc2a1c68 100644 --- a/block.c +++ b/block.c @@ -3145,6 +3145,7 @@ BlockDriverState *bdrv_find_backing_image(BlockDriverState *bs, int is_protocol = 0; BlockDriverState *curr_bs = NULL; BlockDriverState *retval = NULL; + Error *local_error = NULL; if (!bs || !bs->drv || !backing_file) { return NULL; @@ -3165,6 +3166,18 @@ BlockDriverState *bdrv_find_backing_image(BlockDriverState *bs, retval = curr_bs->backing->bs; break; } + /* Also check against the full backing filename for the image */ + bdrv_get_full_backing_filename(curr_bs, backing_file_full, PATH_MAX, + &local_error); + if (local_error == NULL) { + if (strcmp(backing_file, backing_file_full) == 0) { + retval = curr_bs->backing->bs; + break; + } + } else { + error_free(local_error); + local_error = NULL; + } } else { /* If not an absolute filename path, make it relative to the current * image's filename path */ From 846a1d118e2e7bce8640452e181c53f902a40dbf Mon Sep 17 00:00:00 2001 From: Jeff Cody Date: Wed, 25 Jan 2017 20:08:21 -0500 Subject: [PATCH 08/21] qemu-iotests: Don't create fifos / pidfiles with protocol paths Trying to create, use, and remove fifos and pidfiles on protocol paths (e.g. nfs://localhost/scratch/qemu-nbd.pid) is obviously broken. Use the local $TEST_DIR path before it is 'protocolized' for these files. Signed-off-by: Jeff Cody Message-id: bb4a731a35bc4ac81fe3db17479dd686315317c7.1485392617.git.jcody@redhat.com Reviewed-by: Max Reitz Signed-off-by: Max Reitz --- tests/qemu-iotests/common.config | 6 ++++-- tests/qemu-iotests/common.qemu | 10 +++++----- tests/qemu-iotests/common.rc | 6 +++--- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/common.config index f6384fbae7..55527aac87 100644 --- a/tests/qemu-iotests/common.config +++ b/tests/qemu-iotests/common.config @@ -109,7 +109,7 @@ _qemu_wrapper() { ( if [ -n "${QEMU_NEED_PID}" ]; then - echo $BASHPID > "${TEST_DIR}/qemu-${_QEMU_HANDLE}.pid" + echo $BASHPID > "${QEMU_TEST_DIR}/qemu-${_QEMU_HANDLE}.pid" fi exec "$QEMU_PROG" $QEMU_OPTIONS "$@" ) @@ -151,7 +151,7 @@ _qemu_io_wrapper() _qemu_nbd_wrapper() { ( - echo $BASHPID > "${TEST_DIR}/qemu-nbd.pid" + echo $BASHPID > "${QEMU_TEST_DIR}/qemu-nbd.pid" exec "$QEMU_NBD_PROG" $QEMU_NBD_OPTIONS "$@" ) } @@ -186,6 +186,8 @@ if [ -z "$TEST_DIR" ]; then TEST_DIR=`pwd`/scratch fi +QEMU_TEST_DIR="${TEST_DIR}" + if [ ! -e "$TEST_DIR" ]; then mkdir "$TEST_DIR" fi diff --git a/tests/qemu-iotests/common.qemu b/tests/qemu-iotests/common.qemu index e657361790..42787896af 100644 --- a/tests/qemu-iotests/common.qemu +++ b/tests/qemu-iotests/common.qemu @@ -27,8 +27,8 @@ QEMU_COMM_TIMEOUT=10 -QEMU_FIFO_IN="${TEST_DIR}/qmp-in-$$" -QEMU_FIFO_OUT="${TEST_DIR}/qmp-out-$$" +QEMU_FIFO_IN="${QEMU_TEST_DIR}/qmp-in-$$" +QEMU_FIFO_OUT="${QEMU_TEST_DIR}/qmp-out-$$" QEMU_HANDLE=0 @@ -204,9 +204,9 @@ function _cleanup_qemu() for i in "${!QEMU_OUT[@]}" do local QEMU_PID - if [ -f "${TEST_DIR}/qemu-${i}.pid" ]; then - read QEMU_PID < "${TEST_DIR}/qemu-${i}.pid" - rm -f "${TEST_DIR}/qemu-${i}.pid" + if [ -f "${QEMU_TEST_DIR}/qemu-${i}.pid" ]; then + read QEMU_PID < "${QEMU_TEST_DIR}/qemu-${i}.pid" + rm -f "${QEMU_TEST_DIR}/qemu-${i}.pid" if [ -z "${wait}" ] && [ -n "${QEMU_PID}" ]; then kill -KILL ${QEMU_PID} 2>/dev/null fi diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc index 3213765f4e..4bb9b77807 100644 --- a/tests/qemu-iotests/common.rc +++ b/tests/qemu-iotests/common.rc @@ -193,11 +193,11 @@ _cleanup_test_img() case "$IMGPROTO" in nbd) - if [ -f "${TEST_DIR}/qemu-nbd.pid" ]; then + if [ -f "${QEMU_TEST_DIR}/qemu-nbd.pid" ]; then local QEMU_NBD_PID - read QEMU_NBD_PID < "${TEST_DIR}/qemu-nbd.pid" + read QEMU_NBD_PID < "${QEMU_TEST_DIR}/qemu-nbd.pid" kill ${QEMU_NBD_PID} - rm -f "${TEST_DIR}/qemu-nbd.pid" + rm -f "${QEMU_TEST_DIR}/qemu-nbd.pid" fi rm -f "$TEST_IMG_FILE" ;; From 256e3b6387c91113a39cdfaa5d3b020ccd4b1a33 Mon Sep 17 00:00:00 2001 From: Jeff Cody Date: Wed, 25 Jan 2017 20:08:22 -0500 Subject: [PATCH 09/21] qemu-iotest: test to lookup protocol-based image with relative backing This test uses NFS and block-stream to force a lookup of a backing image that has a relative filename, but a full backing image name with the protocol path intact. Signed-off-by: Jeff Cody Message-id: 1a7a3d6e6d8af36cd5b47ed6ea93b5a9ededf81b.1485392617.git.jcody@redhat.com Reviewed-by: Max Reitz Signed-off-by: Max Reitz --- tests/qemu-iotests/173 | 97 ++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/173.out | 12 +++++ tests/qemu-iotests/group | 1 + 3 files changed, 110 insertions(+) create mode 100755 tests/qemu-iotests/173 create mode 100644 tests/qemu-iotests/173.out diff --git a/tests/qemu-iotests/173 b/tests/qemu-iotests/173 new file mode 100755 index 0000000000..bdaa092979 --- /dev/null +++ b/tests/qemu-iotests/173 @@ -0,0 +1,97 @@ +#!/bin/bash +# +# Test QAPI commands looking up protocol based images with relative +# filename backing strings +# +# Copyright (C) 2017 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# +# creator +owner=jcody@redhat.com + +seq=`basename $0` +echo "QA output created by $seq" + +here=`pwd` +status=1 # failure is the default! + +_cleanup() +{ + _cleanup_qemu + rm -f "${QEMU_TEST_DIR}/image.base" "${QEMU_TEST_DIR}/image.snp1" + _cleanup_test_img +} +trap "_cleanup; exit \$status" 0 1 2 3 15 + +# get standard environment, filters and checks +. ./common.rc +. ./common.filter +. ./common.qemu + +_supported_fmt qcow2 +_supported_proto nfs +_supported_os Linux + +size=100M + +BASE_IMG="${TEST_DIR}/image.base" +TOP_IMG="${TEST_DIR}/image.snp1" + +TEST_IMG="${BASE_IMG}" _make_test_img $size + +TEST_IMG="${TOP_IMG}" _make_test_img $size + +echo +echo === Running QEMU, using block-stream to find backing image === +echo + +qemu_comm_method="qmp" +_launch_qemu -drive file="${BASE_IMG}",if=virtio,id=disk2 +h=$QEMU_HANDLE + +_send_qemu_cmd $h "{ 'execute': 'qmp_capabilities' }" "return" + +_send_qemu_cmd $h "{ 'arguments': { + 'device': 'disk2', + 'format': '${IMGFMT}', + 'mode': 'existing', + 'snapshot-file': '${TOP_IMG}', + 'snapshot-node-name': 'snp1' + }, + 'execute': 'blockdev-snapshot-sync' + }" "return" + + +_send_qemu_cmd $h "{ 'arguments': { + 'backing-file': 'image.base', + 'device': 'disk2', + 'image-node-name': 'snp1' + }, + 'execute': 'change-backing-file' + }" "return" + +_send_qemu_cmd $h "{ 'arguments': { + 'base': '${BASE_IMG}', + 'device': 'disk2' + }, + 'execute': 'block-stream' + }" "BLOCK_JOB_COMPLETED" + +_cleanup_qemu + +# success, all done +echo "*** done" +rm -f $seq.full +status=0 diff --git a/tests/qemu-iotests/173.out b/tests/qemu-iotests/173.out new file mode 100644 index 0000000000..f477a0099a --- /dev/null +++ b/tests/qemu-iotests/173.out @@ -0,0 +1,12 @@ +QA output created by 173 +Formatting 'TEST_DIR/image.base', fmt=IMGFMT size=104857600 +Formatting 'TEST_DIR/image.snp1', fmt=IMGFMT size=104857600 + +=== Running QEMU, using block-stream to find backing image === + +{"return": {}} +{"return": {}} +{"return": {}} +{"return": {}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "disk2", "len": 104857600, "offset": 104857600, "speed": 0, "type": "stream"}} +*** done diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group index 866c1a032d..5a93ba9af9 100644 --- a/tests/qemu-iotests/group +++ b/tests/qemu-iotests/group @@ -165,3 +165,4 @@ 170 rw auto quick 171 rw auto quick 172 auto +173 rw auto From 20a6d768f5c1a590d8a42c343aa862f89966cb23 Mon Sep 17 00:00:00 2001 From: Dou Liyang Date: Sun, 15 Jan 2017 16:01:14 +0800 Subject: [PATCH 10/21] block/qapi: reduce the coupling between the bdrv_query_stats and bdrv_query_bds_stats The bdrv_query_stats and bdrv_query_bds_stats functions need to call each other, that increases the coupling. it also makes the program complicated and makes some unnecessary tests. Remove the call from bdrv_query_bds_stats to bdrv_query_stats, just take some recursion to make it clearly. Avoid testing whether the blk is NULL during querying the bds stats. It is unnecessary. Signed-off-by: Dou Liyang Message-id: 1484467275-27919-2-git-send-email-douly.fnst@cn.fujitsu.com Reviewed-by: Markus Armbruster Signed-off-by: Max Reitz --- block/qapi.c | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/block/qapi.c b/block/qapi.c index 63297353f7..48105ed277 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -357,10 +357,6 @@ static void bdrv_query_info(BlockBackend *blk, BlockInfo **p_info, qapi_free_BlockInfo(info); } -static BlockStats *bdrv_query_stats(BlockBackend *blk, - const BlockDriverState *bs, - bool query_backing); - static void bdrv_query_blk_stats(BlockDeviceStats *ds, BlockBackend *blk) { BlockAcctStats *stats = blk_get_stats(blk); @@ -428,9 +424,18 @@ static void bdrv_query_blk_stats(BlockDeviceStats *ds, BlockBackend *blk) } } -static void bdrv_query_bds_stats(BlockStats *s, const BlockDriverState *bs, +static BlockStats *bdrv_query_bds_stats(const BlockDriverState *bs, bool query_backing) { + BlockStats *s = NULL; + + s = g_malloc0(sizeof(*s)); + s->stats = g_malloc0(sizeof(*s->stats)); + + if (!bs) { + return s; + } + if (bdrv_get_node_name(bs)[0]) { s->has_node_name = true; s->node_name = g_strdup(bdrv_get_node_name(bs)); @@ -440,14 +445,15 @@ static void bdrv_query_bds_stats(BlockStats *s, const BlockDriverState *bs, if (bs->file) { s->has_parent = true; - s->parent = bdrv_query_stats(NULL, bs->file->bs, query_backing); + s->parent = bdrv_query_bds_stats(bs->file->bs, query_backing); } if (query_backing && bs->backing) { s->has_backing = true; - s->backing = bdrv_query_stats(NULL, bs->backing->bs, query_backing); + s->backing = bdrv_query_bds_stats(bs->backing->bs, query_backing); } + return s; } static BlockStats *bdrv_query_stats(BlockBackend *blk, @@ -456,17 +462,13 @@ static BlockStats *bdrv_query_stats(BlockBackend *blk, { BlockStats *s; - s = g_malloc0(sizeof(*s)); - s->stats = g_malloc0(sizeof(*s->stats)); + s = bdrv_query_bds_stats(bs, query_backing); if (blk) { s->has_device = true; s->device = g_strdup(blk_name(blk)); bdrv_query_blk_stats(s->stats, blk); } - if (bs) { - bdrv_query_bds_stats(s, bs, query_backing); - } return s; } From a6baa60807f88ba7d97b1787797fb58882ccbfb9 Mon Sep 17 00:00:00 2001 From: Dou Liyang Date: Sun, 15 Jan 2017 16:01:15 +0800 Subject: [PATCH 11/21] block/qapi: reduce the execution time of qmp_query_blockstats In order to reduce the execution time, this patch optimize the qmp_query_blockstats(): Remove the next_query_bds function. Remove the bdrv_query_stats function. Remove some judgement sentence. The original qmp_query_blockstats calls next_query_bds to get the next objects in each loops. In the next_query_bds, it checks the query_nodes and blk. It also call bdrv_query_stats to get the stats, In the bdrv_query_stats, it checks blk and bs each times. This waste more times, which may stall the main loop a bit. And if the disk is too many and donot use the dataplane feature, this may affect the performance in main loop thread. This patch removes that two functions, and makes the structure clearly. Signed-off-by: Dou Liyang Message-id: 1484467275-27919-3-git-send-email-douly.fnst@cn.fujitsu.com Reviewed-by: Markus Armbruster [mreitz: Removed duplicate info->value assignment] Signed-off-by: Max Reitz --- block/qapi.c | 71 +++++++++++++++++++++------------------------------- 1 file changed, 28 insertions(+), 43 deletions(-) diff --git a/block/qapi.c b/block/qapi.c index 48105ed277..ac480aa93c 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -456,23 +456,6 @@ static BlockStats *bdrv_query_bds_stats(const BlockDriverState *bs, return s; } -static BlockStats *bdrv_query_stats(BlockBackend *blk, - const BlockDriverState *bs, - bool query_backing) -{ - BlockStats *s; - - s = bdrv_query_bds_stats(bs, query_backing); - - if (blk) { - s->has_device = true; - s->device = g_strdup(blk_name(blk)); - bdrv_query_blk_stats(s->stats, blk); - } - - return s; -} - BlockInfoList *qmp_query_block(Error **errp) { BlockInfoList *head = NULL, **p_next = &head; @@ -496,42 +479,44 @@ BlockInfoList *qmp_query_block(Error **errp) return head; } -static bool next_query_bds(BlockBackend **blk, BlockDriverState **bs, - bool query_nodes) -{ - if (query_nodes) { - *bs = bdrv_next_node(*bs); - return !!*bs; - } - - *blk = blk_next(*blk); - *bs = *blk ? blk_bs(*blk) : NULL; - - return !!*blk; -} - BlockStatsList *qmp_query_blockstats(bool has_query_nodes, bool query_nodes, Error **errp) { BlockStatsList *head = NULL, **p_next = &head; - BlockBackend *blk = NULL; - BlockDriverState *bs = NULL; + BlockBackend *blk; + BlockDriverState *bs; /* Just to be safe if query_nodes is not always initialized */ - query_nodes = has_query_nodes && query_nodes; + if (has_query_nodes && query_nodes) { + for (bs = bdrv_next_node(NULL); bs; bs = bdrv_next_node(bs)) { + BlockStatsList *info = g_malloc0(sizeof(*info)); + AioContext *ctx = bdrv_get_aio_context(bs); - while (next_query_bds(&blk, &bs, query_nodes)) { - BlockStatsList *info = g_malloc0(sizeof(*info)); - AioContext *ctx = blk ? blk_get_aio_context(blk) - : bdrv_get_aio_context(bs); + aio_context_acquire(ctx); + info->value = bdrv_query_bds_stats(bs, false); + aio_context_release(ctx); - aio_context_acquire(ctx); - info->value = bdrv_query_stats(blk, bs, !query_nodes); - aio_context_release(ctx); + *p_next = info; + p_next = &info->next; + } + } else { + for (blk = blk_next(NULL); blk; blk = blk_next(blk)) { + BlockStatsList *info = g_malloc0(sizeof(*info)); + AioContext *ctx = blk_get_aio_context(blk); + BlockStats *s; - *p_next = info; - p_next = &info->next; + aio_context_acquire(ctx); + s = bdrv_query_bds_stats(blk_bs(blk), true); + s->has_device = true; + s->device = g_strdup(blk_name(blk)); + bdrv_query_blk_stats(s->stats, blk); + aio_context_release(ctx); + + info->value = s; + *p_next = info; + p_next = &info->next; + } } return head; From 16e977d506bcc2d9f7daa4a9f7cc2b48536d9da6 Mon Sep 17 00:00:00 2001 From: Vladimir Sementsov-Ogievskiy Date: Tue, 31 Jan 2017 14:23:08 +0300 Subject: [PATCH 12/21] block: bdrv_invalidate_cache: invalidate children first Current implementation invalidates firstly parent bds and then its children. This leads to the following bug: after incoming migration, in bdrv_invalidate_cache_all: 1. invalidate parent bds - reopen it with BDRV_O_INACTIVE cleared 2. child is not yet invalidated 3. parent check that its BDRV_O_INACTIVE is cleared 4. parent writes to child 5. assert in bdrv_co_pwritev, as BDRV_O_INACTIVE is set for child This patch fixes it by just changing invalidate sequence: invalidate children first. Signed-off-by: Vladimir Sementsov-Ogievskiy Message-id: 20170131112308.54189-1-vsementsov@virtuozzo.com Reviewed-by: Max Reitz Reviewed-by: Stefan Hajnoczi Signed-off-by: Max Reitz --- block.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/block.c b/block.c index b8bc2a1c68..743c349100 100644 --- a/block.c +++ b/block.c @@ -3248,19 +3248,18 @@ void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp) if (!(bs->open_flags & BDRV_O_INACTIVE)) { return; } - bs->open_flags &= ~BDRV_O_INACTIVE; - if (bs->drv->bdrv_invalidate_cache) { - bs->drv->bdrv_invalidate_cache(bs, &local_err); + QLIST_FOREACH(child, &bs->children, next) { + bdrv_invalidate_cache(child->bs, &local_err); if (local_err) { - bs->open_flags |= BDRV_O_INACTIVE; error_propagate(errp, local_err); return; } } - QLIST_FOREACH(child, &bs->children, next) { - bdrv_invalidate_cache(child->bs, &local_err); + bs->open_flags &= ~BDRV_O_INACTIVE; + if (bs->drv->bdrv_invalidate_cache) { + bs->drv->bdrv_invalidate_cache(bs, &local_err); if (local_err) { bs->open_flags |= BDRV_O_INACTIVE; error_propagate(errp, local_err); From 8d20abe87afa735cd0ae6688bd105c7a27390343 Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Wed, 1 Feb 2017 10:53:48 +0100 Subject: [PATCH 13/21] block/nfs: fix NULL pointer dereference in URI parsing parse_uint_full wants to put the parsed value into the variable passed via its second argument which is NULL. Fixes: 94d6a7a76e9df9919629428f6c598e2b97d9426c Cc: qemu-stable@nongnu.org Signed-off-by: Peter Lieven Reviewed-by: Eric Blake Message-id: 1485942829-10756-2-git-send-email-pl@kamp.de Signed-off-by: Max Reitz --- block/nfs.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/block/nfs.c b/block/nfs.c index a564340d15..baaecff3fd 100644 --- a/block/nfs.c +++ b/block/nfs.c @@ -108,12 +108,13 @@ static int nfs_parse_uri(const char *filename, QDict *options, Error **errp) qdict_put(options, "path", qstring_from_str(uri->path)); for (i = 0; i < qp->n; i++) { + unsigned long long val; if (!qp->p[i].value) { error_setg(errp, "Value for NFS parameter expected: %s", qp->p[i].name); goto out; } - if (parse_uint_full(qp->p[i].value, NULL, 0)) { + if (parse_uint_full(qp->p[i].value, &val, 0)) { error_setg(errp, "Illegal value for NFS parameter: %s", qp->p[i].name); goto out; From f67409a5bb43ebe74401fa8e187267eb0f139293 Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Wed, 1 Feb 2017 10:53:49 +0100 Subject: [PATCH 14/21] block/nfs: fix naming of runtime opts commit 94d6a7a accidentally left the naming of runtime opts and QAPI scheme inconsistent. As one consequence passing of parameters in the URI is broken. Sync the naming of the runtime opts to the QAPI scheme. Please note that this is technically backwards incompatible with the 2.8 release, but the 2.8 release is the only version that had the wrong naming. Furthermore release 2.8 suffered from a NULL pointer dereference during URI parsing. Fixes: 94d6a7a76e9df9919629428f6c598e2b97d9426c Cc: qemu-stable@nongnu.org Signed-off-by: Peter Lieven Message-id: 1485942829-10756-3-git-send-email-pl@kamp.de [mreitz: Fixed commit message] Reviewed-by: Eric Blake Signed-off-by: Max Reitz --- block/nfs.c | 46 +++++++++++++++++++++++----------------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/block/nfs.c b/block/nfs.c index baaecff3fd..689eaa792e 100644 --- a/block/nfs.c +++ b/block/nfs.c @@ -359,27 +359,27 @@ static QemuOptsList runtime_opts = { .help = "Path of the image on the host", }, { - .name = "uid", + .name = "user", .type = QEMU_OPT_NUMBER, .help = "UID value to use when talking to the server", }, { - .name = "gid", + .name = "group", .type = QEMU_OPT_NUMBER, .help = "GID value to use when talking to the server", }, { - .name = "tcp-syncnt", + .name = "tcp-syn-count", .type = QEMU_OPT_NUMBER, .help = "Number of SYNs to send during the session establish", }, { - .name = "readahead", + .name = "readahead-size", .type = QEMU_OPT_NUMBER, .help = "Set the readahead size in bytes", }, { - .name = "pagecache", + .name = "page-cache-size", .type = QEMU_OPT_NUMBER, .help = "Set the pagecache size in bytes", }, @@ -508,29 +508,29 @@ static int64_t nfs_client_open(NFSClient *client, QDict *options, goto fail; } - if (qemu_opt_get(opts, "uid")) { - client->uid = qemu_opt_get_number(opts, "uid", 0); + if (qemu_opt_get(opts, "user")) { + client->uid = qemu_opt_get_number(opts, "user", 0); nfs_set_uid(client->context, client->uid); } - if (qemu_opt_get(opts, "gid")) { - client->gid = qemu_opt_get_number(opts, "gid", 0); + if (qemu_opt_get(opts, "group")) { + client->gid = qemu_opt_get_number(opts, "group", 0); nfs_set_gid(client->context, client->gid); } - if (qemu_opt_get(opts, "tcp-syncnt")) { - client->tcp_syncnt = qemu_opt_get_number(opts, "tcp-syncnt", 0); + if (qemu_opt_get(opts, "tcp-syn-count")) { + client->tcp_syncnt = qemu_opt_get_number(opts, "tcp-syn-count", 0); nfs_set_tcp_syncnt(client->context, client->tcp_syncnt); } #ifdef LIBNFS_FEATURE_READAHEAD - if (qemu_opt_get(opts, "readahead")) { + if (qemu_opt_get(opts, "readahead-size")) { if (open_flags & BDRV_O_NOCACHE) { error_setg(errp, "Cannot enable NFS readahead " "if cache.direct = on"); goto fail; } - client->readahead = qemu_opt_get_number(opts, "readahead", 0); + client->readahead = qemu_opt_get_number(opts, "readahead-size", 0); if (client->readahead > QEMU_NFS_MAX_READAHEAD_SIZE) { error_report("NFS Warning: Truncating NFS readahead " "size to %d", QEMU_NFS_MAX_READAHEAD_SIZE); @@ -545,13 +545,13 @@ static int64_t nfs_client_open(NFSClient *client, QDict *options, #endif #ifdef LIBNFS_FEATURE_PAGECACHE - if (qemu_opt_get(opts, "pagecache")) { + if (qemu_opt_get(opts, "page-cache-size")) { if (open_flags & BDRV_O_NOCACHE) { error_setg(errp, "Cannot enable NFS pagecache " "if cache.direct = on"); goto fail; } - client->pagecache = qemu_opt_get_number(opts, "pagecache", 0); + client->pagecache = qemu_opt_get_number(opts, "page-cache-size", 0); if (client->pagecache > QEMU_NFS_MAX_PAGECACHE_SIZE) { error_report("NFS Warning: Truncating NFS pagecache " "size to %d pages", QEMU_NFS_MAX_PAGECACHE_SIZE); @@ -804,22 +804,22 @@ static void nfs_refresh_filename(BlockDriverState *bs, QDict *options) qdict_put(opts, "path", qstring_from_str(client->path)); if (client->uid) { - qdict_put(opts, "uid", qint_from_int(client->uid)); + qdict_put(opts, "user", qint_from_int(client->uid)); } if (client->gid) { - qdict_put(opts, "gid", qint_from_int(client->gid)); + qdict_put(opts, "group", qint_from_int(client->gid)); } if (client->tcp_syncnt) { - qdict_put(opts, "tcp-syncnt", - qint_from_int(client->tcp_syncnt)); + qdict_put(opts, "tcp-syn-cnt", + qint_from_int(client->tcp_syncnt)); } if (client->readahead) { - qdict_put(opts, "readahead", - qint_from_int(client->readahead)); + qdict_put(opts, "readahead-size", + qint_from_int(client->readahead)); } if (client->pagecache) { - qdict_put(opts, "pagecache", - qint_from_int(client->pagecache)); + qdict_put(opts, "page-cache-size", + qint_from_int(client->pagecache)); } if (client->debug) { qdict_put(opts, "debug", qint_from_int(client->debug)); From b7aa1315198de1bd2c5f457d2e2c6cd007b3f430 Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Wed, 1 Feb 2017 02:31:18 +0200 Subject: [PATCH 15/21] qemu-io: Return non-zero exit code on failure The result of openfile was not checked, leading to failure deep in the actual command with confusing error message, and exiting with exit code 0. Here is a simple example - trying to read with the wrong format: $ touch file $ qemu-io -f qcow2 -c 'read -P 1 0 1024' file; echo $? can't open device file: Image is not in qcow2 format no file open, try 'help open' 0 With this patch, we fail earlier with exit code 1: $ ./qemu-io -f qcow2 -c 'read -P 1 0 1024' file; echo $? can't open device file: Image is not in qcow2 format 1 Failing earlier, we don't log this error now: no file open, try 'help open' But some tests expected it; the line was removed from the test output. Signed-off-by: Nir Soffer Reviewed-by: Eric Blake Message-id: 20170201003120.23378-2-nirsof@gmail.com Reviewed-by: Max Reitz Signed-off-by: Max Reitz --- qemu-io.c | 8 ++++++-- tests/qemu-iotests/059.out | 3 --- tests/qemu-iotests/070.out | 1 - tests/qemu-iotests/075.out | 7 ------- tests/qemu-iotests/076.out | 3 --- tests/qemu-iotests/078.out | 6 ------ tests/qemu-iotests/080.out | 18 ------------------ tests/qemu-iotests/083.out | 17 ----------------- tests/qemu-iotests/088.out | 6 ------ tests/qemu-iotests/092.out | 12 ------------ tests/qemu-iotests/116.out | 7 ------- tests/qemu-iotests/131.out | 1 - tests/qemu-iotests/140.out | 1 - 13 files changed, 6 insertions(+), 84 deletions(-) diff --git a/qemu-io.c b/qemu-io.c index 23a229f880..427cbaef57 100644 --- a/qemu-io.c +++ b/qemu-io.c @@ -595,13 +595,17 @@ int main(int argc, char **argv) exit(1); } opts = qemu_opts_to_qdict(qopts, NULL); - openfile(NULL, flags, writethrough, opts); + if (openfile(NULL, flags, writethrough, opts)) { + exit(1); + } } else { if (format) { opts = qdict_new(); qdict_put(opts, "driver", qstring_from_str(format)); } - openfile(argv[optind], flags, writethrough, opts); + if (openfile(argv[optind], flags, writethrough, opts)) { + exit(1); + } } } command_loop(); diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out index 19bd50de1e..6154509bc3 100644 --- a/tests/qemu-iotests/059.out +++ b/tests/qemu-iotests/059.out @@ -3,17 +3,14 @@ QA output created by 059 === Testing invalid granularity === Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 can't open device TEST_DIR/t.vmdk: Invalid granularity, image may be corrupt -no file open, try 'help open' === Testing too big L2 table size === Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 can't open device TEST_DIR/t.vmdk: L2 table size too big -no file open, try 'help open' === Testing too big L1 table size === Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 can't open device TEST_DIR/t.vmdk: L1 size too big -no file open, try 'help open' === Testing monolithicFlat creation and opening === Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2147483648 subformat=monolithicFlat diff --git a/tests/qemu-iotests/070.out b/tests/qemu-iotests/070.out index 131a5b17dc..c269d99483 100644 --- a/tests/qemu-iotests/070.out +++ b/tests/qemu-iotests/070.out @@ -4,7 +4,6 @@ QA output created by 070 can't open device TEST_DIR/iotest-dirtylog-10G-4M.vhdx: VHDX image file 'TEST_DIR/iotest-dirtylog-10G-4M.vhdx' opened read-only, but contains a log that needs to be replayed To replay the log, run: qemu-img check -r all 'TEST_DIR/iotest-dirtylog-10G-4M.vhdx' - no file open, try 'help open' === Verify open image replays log === read 18874368/18874368 bytes at offset 0 18 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) diff --git a/tests/qemu-iotests/075.out b/tests/qemu-iotests/075.out index 87beae4e3c..b234b758e0 100644 --- a/tests/qemu-iotests/075.out +++ b/tests/qemu-iotests/075.out @@ -10,29 +10,22 @@ read 512/512 bytes at offset 1048064 == block_size must be a multiple of 512 == can't open device TEST_DIR/simple-pattern.cloop: block_size 513 must be a multiple of 512 -no file open, try 'help open' == block_size cannot be zero == can't open device TEST_DIR/simple-pattern.cloop: block_size cannot be zero -no file open, try 'help open' == huge block_size === can't open device TEST_DIR/simple-pattern.cloop: block_size 4294966784 must be 64 MB or less -no file open, try 'help open' == offsets_size overflow === can't open device TEST_DIR/simple-pattern.cloop: n_blocks 4294967295 must be 536870911 or less -no file open, try 'help open' == refuse images that require too many offsets === can't open device TEST_DIR/simple-pattern.cloop: image requires too many offsets, try increasing block size -no file open, try 'help open' == refuse images with non-monotonically increasing offsets == can't open device TEST_DIR/simple-pattern.cloop: offsets not monotonically increasing at index 1, image file is corrupt -no file open, try 'help open' == refuse images with invalid compressed block size == can't open device TEST_DIR/simple-pattern.cloop: invalid compressed block size at index 1, image file is corrupt -no file open, try 'help open' *** done diff --git a/tests/qemu-iotests/076.out b/tests/qemu-iotests/076.out index 72645b2522..9c66c5fb46 100644 --- a/tests/qemu-iotests/076.out +++ b/tests/qemu-iotests/076.out @@ -6,15 +6,12 @@ read 65536/65536 bytes at offset 0 == Negative catalog size == can't open device TEST_DIR/parallels-v1: Catalog too large -no file open, try 'help open' == Overflow in catalog allocation == can't open device TEST_DIR/parallels-v1: Catalog too large -no file open, try 'help open' == Zero sectors per track == can't open device TEST_DIR/parallels-v1: Invalid image: Zero sectors per track -no file open, try 'help open' == Read from a valid v2 image == read 65536/65536 bytes at offset 0 diff --git a/tests/qemu-iotests/078.out b/tests/qemu-iotests/078.out index 42b8a83015..c3d6aa4fe4 100644 --- a/tests/qemu-iotests/078.out +++ b/tests/qemu-iotests/078.out @@ -6,23 +6,17 @@ read 512/512 bytes at offset 0 == Negative catalog size == can't open device TEST_DIR/empty.bochs: Catalog size is too large -no file open, try 'help open' == Overflow for catalog size * sizeof(uint32_t) == can't open device TEST_DIR/empty.bochs: Catalog size is too large -no file open, try 'help open' == Too small catalog bitmap for image size == can't open device TEST_DIR/empty.bochs: Catalog size is too small for this disk size -no file open, try 'help open' can't open device TEST_DIR/empty.bochs: Catalog size is too small for this disk size -no file open, try 'help open' == Negative extent size == can't open device TEST_DIR/empty.bochs: Extent size 2147483648 is too large -no file open, try 'help open' == Zero extent size == can't open device TEST_DIR/empty.bochs: Extent size must be at least 512 -no file open, try 'help open' *** done diff --git a/tests/qemu-iotests/080.out b/tests/qemu-iotests/080.out index 0daac48b12..6a7fda1356 100644 --- a/tests/qemu-iotests/080.out +++ b/tests/qemu-iotests/080.out @@ -3,46 +3,33 @@ QA output created by 080 == Huge header size == Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 can't open device TEST_DIR/t.qcow2: qcow2 header exceeds cluster size -no file open, try 'help open' can't open device TEST_DIR/t.qcow2: qcow2 header exceeds cluster size -no file open, try 'help open' == Huge unknown header extension == Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 can't open device TEST_DIR/t.qcow2: Invalid backing file offset -no file open, try 'help open' can't open device TEST_DIR/t.qcow2: Header extension too large -no file open, try 'help open' can't open device TEST_DIR/t.qcow2: Header extension too large -no file open, try 'help open' == Huge refcount table size == Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 can't open device TEST_DIR/t.qcow2: Reference count table too large -no file open, try 'help open' can't open device TEST_DIR/t.qcow2: Reference count table too large -no file open, try 'help open' == Misaligned refcount table == Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 can't open device TEST_DIR/t.qcow2: Invalid reference count table offset -no file open, try 'help open' == Huge refcount offset == Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 can't open device TEST_DIR/t.qcow2: Invalid reference count table offset -no file open, try 'help open' == Invalid snapshot table == Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 can't open device TEST_DIR/t.qcow2: Too many snapshots -no file open, try 'help open' can't open device TEST_DIR/t.qcow2: Too many snapshots -no file open, try 'help open' can't open device TEST_DIR/t.qcow2: Invalid snapshot table offset -no file open, try 'help open' can't open device TEST_DIR/t.qcow2: Invalid snapshot table offset -no file open, try 'help open' == Hitting snapshot table size limit == Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 @@ -53,13 +40,9 @@ read 512/512 bytes at offset 0 == Invalid L1 table == Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 can't open device TEST_DIR/t.qcow2: Active L1 table too large -no file open, try 'help open' can't open device TEST_DIR/t.qcow2: Active L1 table too large -no file open, try 'help open' can't open device TEST_DIR/t.qcow2: Invalid L1 table offset -no file open, try 'help open' can't open device TEST_DIR/t.qcow2: Invalid L1 table offset -no file open, try 'help open' == Invalid L1 table (with internal snapshot in the image) == Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 @@ -68,7 +51,6 @@ qemu-img: Could not open 'TEST_DIR/t.IMGFMT': L1 table is too small == Invalid backing file size == Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 can't open device TEST_DIR/t.qcow2: Backing file name too long -no file open, try 'help open' == Invalid L2 entry (huge physical offset) == Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 diff --git a/tests/qemu-iotests/083.out b/tests/qemu-iotests/083.out index ef3d1e32a5..0c13888ba1 100644 --- a/tests/qemu-iotests/083.out +++ b/tests/qemu-iotests/083.out @@ -2,52 +2,42 @@ QA output created by 083 === Check disconnect before neg1 === can't open device nbd:127.0.0.1:PORT:exportname=foo -no file open, try 'help open' === Check disconnect after neg1 === can't open device nbd:127.0.0.1:PORT:exportname=foo -no file open, try 'help open' === Check disconnect 8 neg1 === can't open device nbd:127.0.0.1:PORT:exportname=foo -no file open, try 'help open' === Check disconnect 16 neg1 === can't open device nbd:127.0.0.1:PORT:exportname=foo -no file open, try 'help open' === Check disconnect before export === can't open device nbd:127.0.0.1:PORT:exportname=foo -no file open, try 'help open' === Check disconnect after export === can't open device nbd:127.0.0.1:PORT:exportname=foo -no file open, try 'help open' === Check disconnect 4 export === can't open device nbd:127.0.0.1:PORT:exportname=foo -no file open, try 'help open' === Check disconnect 12 export === can't open device nbd:127.0.0.1:PORT:exportname=foo -no file open, try 'help open' === Check disconnect 16 export === can't open device nbd:127.0.0.1:PORT:exportname=foo -no file open, try 'help open' === Check disconnect before neg2 === can't open device nbd:127.0.0.1:PORT:exportname=foo -no file open, try 'help open' === Check disconnect after neg2 === @@ -56,12 +46,10 @@ read failed: Input/output error === Check disconnect 8 neg2 === can't open device nbd:127.0.0.1:PORT:exportname=foo -no file open, try 'help open' === Check disconnect 10 neg2 === can't open device nbd:127.0.0.1:PORT:exportname=foo -no file open, try 'help open' === Check disconnect before request === @@ -99,27 +87,22 @@ read 512/512 bytes at offset 0 === Check disconnect before neg-classic === can't open device nbd:127.0.0.1:PORT -no file open, try 'help open' === Check disconnect 8 neg-classic === can't open device nbd:127.0.0.1:PORT -no file open, try 'help open' === Check disconnect 16 neg-classic === can't open device nbd:127.0.0.1:PORT -no file open, try 'help open' === Check disconnect 24 neg-classic === can't open device nbd:127.0.0.1:PORT -no file open, try 'help open' === Check disconnect 28 neg-classic === can't open device nbd:127.0.0.1:PORT -no file open, try 'help open' === Check disconnect after neg-classic === diff --git a/tests/qemu-iotests/088.out b/tests/qemu-iotests/088.out index a2a83b8a1c..1f6bcf0abc 100644 --- a/tests/qemu-iotests/088.out +++ b/tests/qemu-iotests/088.out @@ -3,15 +3,9 @@ QA output created by 088 == Invalid block size == Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 can't open device TEST_DIR/t.vpc: Invalid block size 0 -no file open, try 'help open' can't open device TEST_DIR/t.vpc: Invalid block size 0 -no file open, try 'help open' can't open device TEST_DIR/t.vpc: Invalid block size 128 -no file open, try 'help open' can't open device TEST_DIR/t.vpc: Invalid block size 128 -no file open, try 'help open' can't open device TEST_DIR/t.vpc: Invalid block size 305419896 -no file open, try 'help open' can't open device TEST_DIR/t.vpc: Invalid block size 305419896 -no file open, try 'help open' *** done diff --git a/tests/qemu-iotests/092.out b/tests/qemu-iotests/092.out index e18f54c200..6eda321fc6 100644 --- a/tests/qemu-iotests/092.out +++ b/tests/qemu-iotests/092.out @@ -3,36 +3,24 @@ QA output created by 092 == Invalid cluster size == Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 can't open device TEST_DIR/t.qcow: Cluster size must be between 512 and 64k -no file open, try 'help open' can't open device TEST_DIR/t.qcow: Cluster size must be between 512 and 64k -no file open, try 'help open' can't open device TEST_DIR/t.qcow: Cluster size must be between 512 and 64k -no file open, try 'help open' can't open device TEST_DIR/t.qcow: Cluster size must be between 512 and 64k -no file open, try 'help open' == Invalid L2 table size == Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 can't open device TEST_DIR/t.qcow: L2 table size must be between 512 and 64k -no file open, try 'help open' can't open device TEST_DIR/t.qcow: L2 table size must be between 512 and 64k -no file open, try 'help open' can't open device TEST_DIR/t.qcow: L2 table size must be between 512 and 64k -no file open, try 'help open' can't open device TEST_DIR/t.qcow: L2 table size must be between 512 and 64k -no file open, try 'help open' == Invalid size == Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 can't open device TEST_DIR/t.qcow: Image too large -no file open, try 'help open' can't open device TEST_DIR/t.qcow: Image too large -no file open, try 'help open' == Invalid backing file length == Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 can't open device TEST_DIR/t.qcow: Backing file name too long -no file open, try 'help open' can't open device TEST_DIR/t.qcow: Backing file name too long -no file open, try 'help open' *** done diff --git a/tests/qemu-iotests/116.out b/tests/qemu-iotests/116.out index 1f11d4446d..24bee57783 100644 --- a/tests/qemu-iotests/116.out +++ b/tests/qemu-iotests/116.out @@ -3,35 +3,28 @@ QA output created by 116 == truncated header cluster == Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 can't open device TEST_DIR/t.qed: Could not open 'TEST_DIR/t.qed': Invalid argument -no file open, try 'help open' == invalid header magic == Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 can't open device TEST_DIR/t.qed: Image not in QED format -no file open, try 'help open' == invalid cluster size == Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 can't open device TEST_DIR/t.qed: Could not open 'TEST_DIR/t.qed': Invalid argument -no file open, try 'help open' == invalid table size == Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 can't open device TEST_DIR/t.qed: Could not open 'TEST_DIR/t.qed': Invalid argument -no file open, try 'help open' == invalid header size == Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 can't open device TEST_DIR/t.qed: Could not open 'TEST_DIR/t.qed': Invalid argument -no file open, try 'help open' == invalid L1 table offset == Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 can't open device TEST_DIR/t.qed: Could not open 'TEST_DIR/t.qed': Invalid argument -no file open, try 'help open' == invalid image size == Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 can't open device TEST_DIR/t.qed: Could not open 'TEST_DIR/t.qed': Invalid argument -no file open, try 'help open' *** done diff --git a/tests/qemu-iotests/131.out b/tests/qemu-iotests/131.out index ae2412ebf7..27c2c5389b 100644 --- a/tests/qemu-iotests/131.out +++ b/tests/qemu-iotests/131.out @@ -23,7 +23,6 @@ read 32768/32768 bytes at offset 0 32 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) == Corrupt image == can't open device TEST_DIR/t.parallels: parallels: Image was not closed correctly; cannot be opened read/write -no file open, try 'help open' ERROR image was not closed correctly 1 errors were found on the image. diff --git a/tests/qemu-iotests/140.out b/tests/qemu-iotests/140.out index 0409cd0174..6c0445603a 100644 --- a/tests/qemu-iotests/140.out +++ b/tests/qemu-iotests/140.out @@ -9,7 +9,6 @@ read 65536/65536 bytes at offset 0 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) {"return": {}} can't open device nbd+unix:///drv?socket=TEST_DIR/nbd: No export with name 'drv' available -no file open, try 'help open' {"return": {}} {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN"} *** done From b4a2caa4bd5ad4e3cd6dd7eb1c0067d9205280cb Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Wed, 1 Feb 2017 02:31:19 +0200 Subject: [PATCH 16/21] qemu-iotests: Add _unsupported_fmt helper This helper allows adding tests supporting any format expect the specified formats. This may be useful to test that many formats behave in a common way. Signed-off-by: Nir Soffer Message-id: 20170201003120.23378-3-nirsof@gmail.com Reviewed-by: Max Reitz Signed-off-by: Max Reitz --- tests/qemu-iotests/common.rc | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc index 4bb9b77807..a3d904fc22 100644 --- a/tests/qemu-iotests/common.rc +++ b/tests/qemu-iotests/common.rc @@ -355,6 +355,17 @@ _supported_fmt() _notrun "not suitable for this image format: $IMGFMT" } +# tests whether $IMGFMT is one of the unsupported image format for a test +# +_unsupported_fmt() +{ + for f; do + if [ "$f" = "$IMGFMT" ]; then + _notrun "not suitable for this image format: $IMGFMT" + fi + done +} + # tests whether $IMGPROTO is one of the supported image protocols for a test # _supported_proto() From bf68bcb18e65e2fc6aaa55079740feee4c8b474c Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Wed, 1 Feb 2017 02:31:20 +0200 Subject: [PATCH 17/21] qemu-io: Add failure regression tests Add regression tests checking that qemu-io fails with non-zero exit code when reading non-existing file or using the wrong image format. Signed-off-by: Nir Soffer Message-id: 20170201003120.23378-4-nirsof@gmail.com Reviewed-by: Max Reitz Signed-off-by: Max Reitz --- tests/qemu-iotests/174 | 59 ++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/174.out | 7 +++++ tests/qemu-iotests/group | 1 + 3 files changed, 67 insertions(+) create mode 100755 tests/qemu-iotests/174 create mode 100644 tests/qemu-iotests/174.out diff --git a/tests/qemu-iotests/174 b/tests/qemu-iotests/174 new file mode 100755 index 0000000000..c1c20a1a57 --- /dev/null +++ b/tests/qemu-iotests/174 @@ -0,0 +1,59 @@ +#!/bin/bash +# +# Test that qemu-io fail with non-zero exit code +# +# Copyright (C) 2017 Nir Soffer +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# + +# creator +owner=nirsof@gmail.com + +seq=`basename $0` +echo "QA output created by $seq" + +here=`pwd` +status=1 # failure is the default! + +_cleanup() +{ + _cleanup_test_img +} +trap "_cleanup; exit \$status" 0 1 2 3 15 + +# get standard environment, filters and checks +. ./common.rc +. ./common.filter + +_unsupported_fmt raw + + +size=256K +IMGFMT=raw IMGOPTS= _make_test_img $size | _filter_imgfmt + +echo +echo "== reading wrong format should fail ==" +$QEMU_IO -f $IMGFMT -c "read 0 $size" "$TEST_IMG" 2>/dev/null +test $? -eq 1 || _fail "did not fail" + +echo +echo "== reading missing file should fail ==" +$QEMU_IO -c "read 0 $size" "$TEST_DIR/missing" 2>/dev/null +test $? -eq 1 || _fail "did not fail" + +# success, all done +echo "*** done" +rm -f $seq.full +status=0 diff --git a/tests/qemu-iotests/174.out b/tests/qemu-iotests/174.out new file mode 100644 index 0000000000..a06d23792e --- /dev/null +++ b/tests/qemu-iotests/174.out @@ -0,0 +1,7 @@ +QA output created by 174 +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=262144 + +== reading wrong format should fail == + +== reading missing file should fail == +*** done diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group index 5a93ba9af9..985b9a6a36 100644 --- a/tests/qemu-iotests/group +++ b/tests/qemu-iotests/group @@ -166,3 +166,4 @@ 171 rw auto quick 172 auto 173 rw auto +174 auto From 7061a078984ba7d08b8b80686ad98c5162e56fbd Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Wed, 1 Feb 2017 14:38:28 +0200 Subject: [PATCH 18/21] qcow2: Optimize the refcount-block overlap check The metadata overlap checks introduced in a40f1c2add help detect corruption in the qcow2 image by verifying that data writes don't overlap with existing metadata sections. The 'refcount-block' check in particular iterates over the refcount table in order to get the addresses of all refcount blocks and check that none of them overlap with the region where we want to write. The problem with the refcount table is that since it always occupies complete clusters its size is usually very big. With the default values of cluster_size=64KB and refcount_bits=16 this table holds 8192 entries, each one of them enough to map 2GB worth of host clusters. So unless we're using images with several TB of allocated data this table is going to be mostly empty, and iterating over it is a waste of CPU. If the storage backend is fast enough this can have an effect on I/O performance. This patch keeps the index of the last used (i.e. non-zero) entry in the refcount table and updates it every time the table changes. The refcount-block overlap check then uses that index instead of reading the whole table. In my tests with a 4GB qcow2 file stored in RAM this doubles the amount of write IOPS. Signed-off-by: Alberto Garcia Message-id: 20170201123828.4815-1-berto@igalia.com Reviewed-by: Max Reitz Signed-off-by: Max Reitz --- block/qcow2-refcount.c | 24 +++++++++++++++++++++++- block/qcow2.c | 1 + block/qcow2.h | 1 + 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index cbfb3fe064..3dbde18612 100644 --- a/block/qcow2-refcount.c +++ b/block/qcow2-refcount.c @@ -83,6 +83,16 @@ static Qcow2SetRefcountFunc *const set_refcount_funcs[] = { /*********************************************************/ /* refcount handling */ +static void update_max_refcount_table_index(BDRVQcow2State *s) +{ + unsigned i = s->refcount_table_size - 1; + while (i > 0 && (s->refcount_table[i] & REFT_OFFSET_MASK) == 0) { + i--; + } + /* Set s->max_refcount_table_index to the index of the last used entry */ + s->max_refcount_table_index = i; +} + int qcow2_refcount_init(BlockDriverState *bs) { BDRVQcow2State *s = bs->opaque; @@ -111,6 +121,7 @@ int qcow2_refcount_init(BlockDriverState *bs) } for(i = 0; i < s->refcount_table_size; i++) be64_to_cpus(&s->refcount_table[i]); + update_max_refcount_table_index(s); } return 0; fail: @@ -439,6 +450,10 @@ static int alloc_refcount_block(BlockDriverState *bs, } s->refcount_table[refcount_table_index] = new_block; + /* If there's a hole in s->refcount_table then it can happen + * that refcount_table_index < s->max_refcount_table_index */ + s->max_refcount_table_index = + MAX(s->max_refcount_table_index, refcount_table_index); /* The new refcount block may be where the caller intended to put its * data, so let it restart the search. */ @@ -580,6 +595,7 @@ static int alloc_refcount_block(BlockDriverState *bs, s->refcount_table = new_table; s->refcount_table_size = table_size; s->refcount_table_offset = table_offset; + update_max_refcount_table_index(s); /* Free old table. */ qcow2_free_clusters(bs, old_table_offset, old_table_size * sizeof(uint64_t), @@ -2171,6 +2187,7 @@ write_refblocks: s->refcount_table = on_disk_reftable; s->refcount_table_offset = reftable_offset; s->refcount_table_size = reftable_size; + update_max_refcount_table_index(s); return 0; @@ -2383,7 +2400,11 @@ int qcow2_check_metadata_overlap(BlockDriverState *bs, int ign, int64_t offset, } if ((chk & QCOW2_OL_REFCOUNT_BLOCK) && s->refcount_table) { - for (i = 0; i < s->refcount_table_size; i++) { + unsigned last_entry = s->max_refcount_table_index; + assert(last_entry < s->refcount_table_size); + assert(last_entry + 1 == s->refcount_table_size || + (s->refcount_table[last_entry + 1] & REFT_OFFSET_MASK) == 0); + for (i = 0; i <= last_entry; i++) { if ((s->refcount_table[i] & REFT_OFFSET_MASK) && overlaps_with(s->refcount_table[i] & REFT_OFFSET_MASK, s->cluster_size)) { @@ -2871,6 +2892,7 @@ int qcow2_change_refcount_order(BlockDriverState *bs, int refcount_order, /* Now update the rest of the in-memory information */ old_reftable = s->refcount_table; s->refcount_table = new_reftable; + update_max_refcount_table_index(s); s->refcount_bits = 1 << refcount_order; s->refcount_max = UINT64_C(1) << (s->refcount_bits - 1); diff --git a/block/qcow2.c b/block/qcow2.c index 96fb8a8f16..3e274bd1ba 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -2743,6 +2743,7 @@ static int make_completely_empty(BlockDriverState *bs) s->refcount_table_offset = s->cluster_size; s->refcount_table_size = s->cluster_size / sizeof(uint64_t); + s->max_refcount_table_index = 0; g_free(s->refcount_table); s->refcount_table = new_reftable; diff --git a/block/qcow2.h b/block/qcow2.h index 182341483a..f8aeb08794 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -251,6 +251,7 @@ typedef struct BDRVQcow2State { uint64_t *refcount_table; uint64_t refcount_table_offset; uint32_t refcount_table_size; + uint32_t max_refcount_table_index; /* Last used entry in refcount_table */ uint64_t free_cluster_index; uint64_t free_byte_offset; From 3026c4688ca80d9c5cc1606368c4a1009a6f507d Mon Sep 17 00:00:00 2001 From: Alberto Garcia Date: Tue, 31 Jan 2017 18:09:54 +0200 Subject: [PATCH 19/21] qemu-io: don't allow I/O operations larger than BDRV_REQUEST_MAX_BYTES Passing a request size larger than BDRV_REQUEST_MAX_BYTES to any of the I/O commands results in an error. While 'read' and 'write' handle the error correctly, 'aio_read' and 'aio_write' hit an assertion: blk_aio_read_entry: Assertion `rwco->qiov->size == acb->bytes' failed. The reason is that the QEMU I/O code cannot handle request sizes larger than BDRV_REQUEST_MAX_BYTES, so this patch makes qemu-io check that all values are within range. Signed-off-by: Alberto Garcia Message-id: 79f66648c685929a144396bda24d13a207131dcf.1485878688.git.berto@igalia.com [mreitz: Use BDRV_REQUEST_MAX_BYTES instead of INT_MAX] Signed-off-by: Max Reitz --- qemu-io-cmds.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c index 95bcde1d88..e415b03cd0 100644 --- a/qemu-io-cmds.c +++ b/qemu-io-cmds.c @@ -388,9 +388,15 @@ create_iovec(BlockBackend *blk, QEMUIOVector *qiov, char **argv, int nr_iov, goto fail; } - if (len > SIZE_MAX) { - printf("Argument '%s' exceeds maximum size %llu\n", arg, - (unsigned long long)SIZE_MAX); + if (len > BDRV_REQUEST_MAX_BYTES) { + printf("Argument '%s' exceeds maximum size %" PRIu64 "\n", arg, + (uint64_t)BDRV_REQUEST_MAX_BYTES); + goto fail; + } + + if (count > BDRV_REQUEST_MAX_BYTES - len) { + printf("The total number of bytes exceed the maximum size %" PRIu64 + "\n", (uint64_t)BDRV_REQUEST_MAX_BYTES); goto fail; } @@ -682,9 +688,9 @@ static int read_f(BlockBackend *blk, int argc, char **argv) if (count < 0) { print_cvtnum_err(count, argv[optind]); return 0; - } else if (count > SIZE_MAX) { + } else if (count > BDRV_REQUEST_MAX_BYTES) { printf("length cannot exceed %" PRIu64 ", given %s\n", - (uint64_t) SIZE_MAX, argv[optind]); + (uint64_t)BDRV_REQUEST_MAX_BYTES, argv[optind]); return 0; } @@ -1004,9 +1010,9 @@ static int write_f(BlockBackend *blk, int argc, char **argv) if (count < 0) { print_cvtnum_err(count, argv[optind]); return 0; - } else if (count > SIZE_MAX) { + } else if (count > BDRV_REQUEST_MAX_BYTES) { printf("length cannot exceed %" PRIu64 ", given %s\n", - (uint64_t) SIZE_MAX, argv[optind]); + (uint64_t)BDRV_REQUEST_MAX_BYTES, argv[optind]); return 0; } From 8b3c67922851bee9d5420373bfea010d3648bbc4 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Fri, 10 Feb 2017 16:28:23 +0000 Subject: [PATCH 20/21] qemu-img: Use qemu_strtoul() rather than raw strtoul() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some of the argument parsing in qemu-img uses strtoul() to parse integer arguments. This is tricky to get correct and in fact the code does not get it right, because it assigns the result of strtoul() to an 'int' variable and then tries to check for > INT_MAX. Coverity correctly complains that the comparison is always false. Rewrite to use qemu_strtoul(), which has a saner convention for reporting conversion failures. (Fixes CID 1356421, CID 1356422, CID 1356423.) Signed-off-by: Peter Maydell Message-id: 1486744104-15590-2-git-send-email-peter.maydell@linaro.org Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Max Reitz --- qemu-img.c | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/qemu-img.c b/qemu-img.c index 933876cfe1..38266e56b0 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -3623,24 +3623,24 @@ static int img_bench(int argc, char **argv) break; case 'c': { - char *end; - errno = 0; - count = strtoul(optarg, &end, 0); - if (errno || *end || count > INT_MAX) { + unsigned long res; + + if (qemu_strtoul(optarg, NULL, 0, &res) < 0 || res > INT_MAX) { error_report("Invalid request count specified"); return 1; } + count = res; break; } case 'd': { - char *end; - errno = 0; - depth = strtoul(optarg, &end, 0); - if (errno || *end || depth > INT_MAX) { + unsigned long res; + + if (qemu_strtoul(optarg, NULL, 0, &res) < 0 || res > INT_MAX) { error_report("Invalid queue depth specified"); return 1; } + depth = res; break; } case 'f': @@ -3707,24 +3707,24 @@ static int img_bench(int argc, char **argv) break; case OPTION_PATTERN: { - char *end; - errno = 0; - pattern = strtoul(optarg, &end, 0); - if (errno || *end || pattern > 0xff) { + unsigned long res; + + if (qemu_strtoul(optarg, NULL, 0, &res) < 0 || res > 0xff) { error_report("Invalid pattern byte specified"); return 1; } + pattern = res; break; } case OPTION_FLUSH_INTERVAL: { - char *end; - errno = 0; - flush_interval = strtoul(optarg, &end, 0); - if (errno || *end || flush_interval > INT_MAX) { + unsigned long res; + + if (qemu_strtoul(optarg, NULL, 0, &res) < 0 || res > INT_MAX) { error_report("Invalid flush interval specified"); return 1; } + flush_interval = res; break; } case OPTION_NO_DRAIN: From 10d6eda1926804a09aa0710ca62933087813de0b Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Fri, 10 Feb 2017 16:28:24 +0000 Subject: [PATCH 21/21] qemu-img: Avoid setting ret to unused value in img_convert() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Coverity points out that we assign the return value from bdrv_snapshot_load_tmp() to 'ret' in img_convert(), but then never use that variable. (We check for failure by looking at local_err instead.) Drop the unused assignment, bringing the call into line with the following call to bdrv_snapshot_laod_tmp_by_id_or_name(). (Fixes CID 1247240.) Signed-off-by: Peter Maydell Message-id: 1486744104-15590-3-git-send-email-peter.maydell@linaro.org Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Max Reitz --- qemu-img.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/qemu-img.c b/qemu-img.c index 38266e56b0..cff22e3005 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -1968,10 +1968,10 @@ static int img_convert(int argc, char **argv) } if (sn_opts) { - ret = bdrv_snapshot_load_tmp(bs[0], - qemu_opt_get(sn_opts, SNAPSHOT_OPT_ID), - qemu_opt_get(sn_opts, SNAPSHOT_OPT_NAME), - &local_err); + bdrv_snapshot_load_tmp(bs[0], + qemu_opt_get(sn_opts, SNAPSHOT_OPT_ID), + qemu_opt_get(sn_opts, SNAPSHOT_OPT_NAME), + &local_err); } else if (snapshot_name != NULL) { if (bs_n > 1) { error_report("No support for concatenating multiple snapshot");