Block patches for 5.0-rc2:

- Fix double QLIST_REMOVE() and potential request object leak in
   xen-block
 - Prevent a potential assertion failure in qcow2's code for compressed
   clusters by rejecting invalid (unaligned) requests with -EIO
 - Prevent discards on qcow2 v2 images from making backing data reappear
 - Make qemu-img convert report I/O error locations by byte offsets
   consistently
 - Fix for potential I/O test errors (accidental globbing due to missing
   quotes)
 -----BEGIN PGP SIGNATURE-----
 
 iQFGBAABCAAwFiEEkb62CjDbPohX0Rgp9AfbAGHVz0AFAl6MckkSHG1yZWl0ekBy
 ZWRoYXQuY29tAAoJEPQH2wBh1c9AGLcH/A8ML6mjaJtwjZG4hL2IuJiA5q+EwswL
 BSM9JBmShFHGkienYhjwMHBgU/QzXMjmsPNLSvdrn09Zd/0C3VjoHVZqfp67o3Cc
 /LmQxnMGfSD8OL/hQmuWeW4S0DKV4rFXFYfbeLFiVpdaS6Sy4BOXiM/ozVJz/IjA
 G+rdr9qk6ICWu940VfQXfl1nNxKw9fUebMH0p2SXbrKDmP4m+Op+Phr9rs506+wj
 Of+RwFQ5mkiou8k5s3ODTzD71gZmWsWP2xOGZ3n5ydVMuwZnblCmaTj2V/tbtBYN
 zQILgnwNYagYWrdOIjJepmC+oHa2tN3tJnMLMYRrB29BJN3AOcB7Etc=
 =0rFv
 -----END PGP SIGNATURE-----

Merge remote-tracking branch 'remotes/maxreitz/tags/pull-block-2020-04-07' into staging

Block patches for 5.0-rc2:
- Fix double QLIST_REMOVE() and potential request object leak in
  xen-block
- Prevent a potential assertion failure in qcow2's code for compressed
  clusters by rejecting invalid (unaligned) requests with -EIO
- Prevent discards on qcow2 v2 images from making backing data reappear
- Make qemu-img convert report I/O error locations by byte offsets
  consistently
- Fix for potential I/O test errors (accidental globbing due to missing
  quotes)

# gpg: Signature made Tue 07 Apr 2020 13:30:01 BST
# gpg:                using RSA key 91BEB60A30DB3E8857D11829F407DB0061D5CF40
# gpg:                issuer "mreitz@redhat.com"
# gpg: Good signature from "Max Reitz <mreitz@redhat.com>" [full]
# Primary key fingerprint: 91BE B60A 30DB 3E88 57D1  1829 F407 DB00 61D5 CF40

* remotes/maxreitz/tags/pull-block-2020-04-07:
  xen-block: Fix double qlist remove and request leak
  iotests/common.pattern: Quote echos
  qcow2: Check request size in qcow2_co_pwritev_compressed_part()
  qemu-img: Report convert errors by bytes, not sectors
  qcow2: Forbid discard in qcow2 v2 images with backing files

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
This commit is contained in:
Peter Maydell 2020-04-07 17:38:47 +01:00
commit 339205e7ef
13 changed files with 225 additions and 68 deletions

View File

@ -3784,6 +3784,12 @@ static coroutine_fn int qcow2_co_pdiscard(BlockDriverState *bs,
int ret;
BDRVQcow2State *s = bs->opaque;
/* If the image does not support QCOW_OFLAG_ZERO then discarding
* clusters could expose stale data from the backing file. */
if (s->qcow_version < 3 && bs->backing) {
return -ENOTSUP;
}
if (!QEMU_IS_ALIGNED(offset | bytes, s->cluster_size)) {
assert(bytes < s->cluster_size);
/* Ignore partial clusters, except for the special case of the
@ -4349,6 +4355,11 @@ qcow2_co_pwritev_compressed_part(BlockDriverState *bs,
return -EINVAL;
}
if (offset_into_cluster(s, bytes) &&
(offset + bytes) != (bs->total_sectors << BDRV_SECTOR_BITS)) {
return -EINVAL;
}
while (bytes && aio_task_pool_status(aio) == 0) {
uint64_t chunk_size = MIN(bytes, s->cluster_size);

View File

@ -64,6 +64,8 @@ struct XenBlockDataPlane {
AioContext *ctx;
};
static int xen_block_send_response(XenBlockRequest *request);
static void reset_request(XenBlockRequest *request)
{
memset(&request->req, 0, sizeof(request->req));
@ -115,23 +117,26 @@ out:
return request;
}
static void xen_block_finish_request(XenBlockRequest *request)
static void xen_block_complete_request(XenBlockRequest *request)
{
XenBlockDataPlane *dataplane = request->dataplane;
if (xen_block_send_response(request)) {
Error *local_err = NULL;
xen_device_notify_event_channel(dataplane->xendev,
dataplane->event_channel,
&local_err);
if (local_err) {
error_report_err(local_err);
}
}
QLIST_REMOVE(request, list);
dataplane->requests_inflight--;
}
static void xen_block_release_request(XenBlockRequest *request)
{
XenBlockDataPlane *dataplane = request->dataplane;
QLIST_REMOVE(request, list);
reset_request(request);
request->dataplane = dataplane;
QLIST_INSERT_HEAD(&dataplane->freelist, request, list);
dataplane->requests_inflight--;
}
/*
@ -246,7 +251,6 @@ static int xen_block_copy_request(XenBlockRequest *request)
}
static int xen_block_do_aio(XenBlockRequest *request);
static int xen_block_send_response(XenBlockRequest *request);
static void xen_block_complete_aio(void *opaque, int ret)
{
@ -286,7 +290,6 @@ static void xen_block_complete_aio(void *opaque, int ret)
}
request->status = request->aio_errors ? BLKIF_RSP_ERROR : BLKIF_RSP_OKAY;
xen_block_finish_request(request);
switch (request->req.operation) {
case BLKIF_OP_WRITE:
@ -306,17 +309,8 @@ static void xen_block_complete_aio(void *opaque, int ret)
default:
break;
}
if (xen_block_send_response(request)) {
Error *local_err = NULL;
xen_device_notify_event_channel(dataplane->xendev,
dataplane->event_channel,
&local_err);
if (local_err) {
error_report_err(local_err);
}
}
xen_block_release_request(request);
xen_block_complete_request(request);
if (dataplane->more_work) {
qemu_bh_schedule(dataplane->bh);
@ -420,8 +414,8 @@ static int xen_block_do_aio(XenBlockRequest *request)
return 0;
err:
xen_block_finish_request(request);
request->status = BLKIF_RSP_ERROR;
xen_block_complete_request(request);
return -1;
}
@ -575,17 +569,7 @@ static bool xen_block_handle_requests(XenBlockDataPlane *dataplane)
break;
};
if (xen_block_send_response(request)) {
Error *local_err = NULL;
xen_device_notify_event_channel(dataplane->xendev,
dataplane->event_channel,
&local_err);
if (local_err) {
error_report_err(local_err);
}
}
xen_block_release_request(request);
xen_block_complete_request(request);
continue;
}

View File

@ -1924,8 +1924,8 @@ retry:
if (status == BLK_DATA && !copy_range) {
ret = convert_co_read(s, sector_num, n, buf);
if (ret < 0) {
error_report("error while reading sector %" PRId64
": %s", sector_num, strerror(-ret));
error_report("error while reading at byte %lld: %s",
sector_num * BDRV_SECTOR_SIZE, strerror(-ret));
s->ret = ret;
}
} else if (!s->min_sparse && status == BLK_ZERO) {
@ -1953,8 +1953,8 @@ retry:
ret = convert_co_write(s, sector_num, n, buf, status);
}
if (ret < 0) {
error_report("error while writing sector %" PRId64
": %s", sector_num, strerror(-ret));
error_report("error while writing at byte %lld: %s",
sector_num * BDRV_SECTOR_SIZE, strerror(-ret));
s->ret = ret;
}
}

View File

@ -193,8 +193,8 @@ echo "== Verify image content =="
verify_io()
{
if ($QEMU_IMG info -U -f "$IMGFMT" "$TEST_IMG" | grep "compat: 0.10" > /dev/null); then
# For v2 images, discarded clusters are read from the backing file
# Keep the variable empty so that the backing file value can be used as
# In v2 images clusters are not discarded when there is a backing file.
# Keep the variable empty so that the previous value can be used as
# the default below
discarded=
else
@ -230,14 +230,16 @@ verify_io()
echo read -P 70 0x78000 0x6000
echo read -P 7 0x7e000 0x2000
echo read -P ${discarded:-8} 0x80000 0x6000
echo read -P ${discarded:-89} 0x80000 0x1000
echo read -P ${discarded:-8} 0x81000 0x5000
echo read -P 80 0x86000 0x2000
echo read -P ${discarded:-8} 0x88000 0x2000
echo read -P 81 0x8a000 0xe000
echo read -P 90 0x98000 0x6000
echo read -P 9 0x9e000 0x2000
echo read -P ${discarded:-10} 0xa0000 0x6000
echo read -P ${discarded:-109} 0xa0000 0x1000
echo read -P ${discarded:-10} 0xa1000 0x5000
echo read -P 100 0xa6000 0x2000
echo read -P ${discarded:-10} 0xa8000 0x2000
echo read -P 101 0xaa000 0xe000

View File

@ -187,8 +187,10 @@ read 24576/24576 bytes at offset 491520
24 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 8192/8192 bytes at offset 516096
8 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 24576/24576 bytes at offset 524288
24 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 4096/4096 bytes at offset 524288
4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 20480/20480 bytes at offset 528384
20 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 8192/8192 bytes at offset 548864
8 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 8192/8192 bytes at offset 557056
@ -199,8 +201,10 @@ read 24576/24576 bytes at offset 622592
24 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 8192/8192 bytes at offset 647168
8 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 24576/24576 bytes at offset 655360
24 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 4096/4096 bytes at offset 655360
4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 20480/20480 bytes at offset 659456
20 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 8192/8192 bytes at offset 679936
8 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
read 8192/8192 bytes at offset 688128

View File

@ -160,18 +160,16 @@ TEST_IMG=$BACKING_IMG _make_test_img 1G
$QEMU_IO -c 'write 0k 64k' "$BACKING_IMG" | _filter_qemu_io
# compat=0.10 is required in order to make the following discard actually
# unallocate the sector rather than make it a zero sector - we want COW, after
# all.
_make_test_img -o 'compat=0.10' -b "$BACKING_IMG" 1G
_make_test_img -b "$BACKING_IMG" 1G
# Write two clusters, the second one enforces creation of an L2 table after
# the first data cluster.
$QEMU_IO -c 'write 0k 64k' -c 'write 512M 64k' "$TEST_IMG" | _filter_qemu_io
# Discard the first cluster. This cluster will soon enough be reallocated and
# Free the first cluster. This cluster will soon enough be reallocated and
# used for COW.
$QEMU_IO -c 'discard 0k 64k' "$TEST_IMG" | _filter_qemu_io
poke_file "$TEST_IMG" "$l2_offset" "\x00\x00\x00\x00\x00\x00\x00\x00"
poke_file "$TEST_IMG" "$(($rb_offset+10))" "\x00\x00"
# Now, corrupt the image by marking the second L2 table cluster as free.
poke_file "$TEST_IMG" '131084' "\x00\x00" # 0x2000c
poke_file "$TEST_IMG" "$(($rb_offset+12))" "\x00\x00"
# Start a write operation requiring COW on the image stopping it right before
# doing the read; then, trigger the corruption prevention by writing anything to
# any unallocated cluster, leading to an attempt to overwrite the second L2

View File

@ -105,8 +105,6 @@ wrote 65536/65536 bytes at offset 0
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
wrote 65536/65536 bytes at offset 536870912
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
discard 65536/65536 bytes at offset 0
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
qcow2: Marking image as corrupt: Preventing invalid write on metadata (overlaps with active L2 table); further corruption events will be suppressed
blkdebug: Suspended request '0'
write failed: Input/output error

View File

@ -89,8 +89,9 @@ verify_io()
{
if ($QEMU_IMG info -f "$IMGFMT" "$TEST_IMG" |
grep "compat: 0.10" > /dev/null); then
# For v2 images, discarded clusters are read from the backing file
discarded=11
# In v2 images clusters are not discarded when there is a backing file
# so the previous value is read
discarded=22
else
# Discarded clusters are zeroed for v3 or later
discarded=0

View File

@ -33,7 +33,7 @@ Convert to compressed target with data file:
Formatting 'TEST_DIR/t.IMGFMT.src', fmt=IMGFMT size=67108864
wrote 1048576/1048576 bytes at offset 0
1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
qemu-img: error while writing sector 0: Operation not supported
qemu-img: error while writing at byte 0: Operation not supported
Convert uncompressed, then write compressed data manually:
Images are identical.

97
tests/qemu-iotests/290 Executable file
View File

@ -0,0 +1,97 @@
#!/usr/bin/env bash
#
# Test how 'qemu-io -c discard' behaves on v2 and v3 qcow2 images
#
# Copyright (C) 2020 Igalia, S.L.
# Author: Alberto Garcia <berto@igalia.com>
#
# 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 <http://www.gnu.org/licenses/>.
#
# creator
owner=berto@igalia.com
seq=`basename $0`
echo "QA output created by $seq"
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
_supported_fmt qcow2
_supported_proto file
_supported_os Linux
_unsupported_imgopts 'compat=0.10' refcount_bits data_file
echo
echo "### Test 'qemu-io -c discard' on a QCOW2 image without a backing file"
echo
for qcow2_compat in 0.10 1.1; do
echo "# Create an image with compat=$qcow2_compat without a backing file"
_make_test_img -o "compat=$qcow2_compat" 128k
echo "# Fill all clusters with data and then discard them"
$QEMU_IO -c 'write -P 0x01 0 128k' "$TEST_IMG" | _filter_qemu_io
$QEMU_IO -c 'discard 0 128k' "$TEST_IMG" | _filter_qemu_io
echo "# Read the data from the discarded clusters"
$QEMU_IO -c 'read -P 0x00 0 128k' "$TEST_IMG" | _filter_qemu_io
echo "# Output of qemu-img map"
$QEMU_IMG map "$TEST_IMG" | _filter_testdir
done
echo
echo "### Test 'qemu-io -c discard' on a QCOW2 image with a backing file"
echo
echo "# Create a backing image and fill it with data"
BACKING_IMG="$TEST_IMG.base"
TEST_IMG="$BACKING_IMG" _make_test_img 128k
$QEMU_IO -c 'write -P 0xff 0 128k' "$BACKING_IMG" | _filter_qemu_io
for qcow2_compat in 0.10 1.1; do
echo "# Create an image with compat=$qcow2_compat and a backing file"
_make_test_img -o "compat=$qcow2_compat" -b "$BACKING_IMG"
echo "# Fill all clusters with data and then discard them"
$QEMU_IO -c 'write -P 0x01 0 128k' "$TEST_IMG" | _filter_qemu_io
$QEMU_IO -c 'discard 0 128k' "$TEST_IMG" | _filter_qemu_io
echo "# Read the data from the discarded clusters"
if [ "$qcow2_compat" = "1.1" ]; then
# In qcow2 v3 clusters are zeroed (with QCOW_OFLAG_ZERO)
$QEMU_IO -c 'read -P 0x00 0 128k' "$TEST_IMG" | _filter_qemu_io
else
# In qcow2 v2 if there's a backing image we cannot zero the clusters
# without exposing the backing file data so discard does nothing
$QEMU_IO -c 'read -P 0x01 0 128k' "$TEST_IMG" | _filter_qemu_io
fi
echo "# Output of qemu-img map"
$QEMU_IMG map "$TEST_IMG" | _filter_testdir
done
# success, all done
echo "*** done"
rm -f $seq.full
status=0

View File

@ -0,0 +1,61 @@
QA output created by 290
### Test 'qemu-io -c discard' on a QCOW2 image without a backing file
# Create an image with compat=0.10 without a backing file
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=131072
# Fill all clusters with data and then discard them
wrote 131072/131072 bytes at offset 0
128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
discard 131072/131072 bytes at offset 0
128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
# Read the data from the discarded clusters
read 131072/131072 bytes at offset 0
128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
# Output of qemu-img map
Offset Length Mapped to File
# Create an image with compat=1.1 without a backing file
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=131072
# Fill all clusters with data and then discard them
wrote 131072/131072 bytes at offset 0
128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
discard 131072/131072 bytes at offset 0
128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
# Read the data from the discarded clusters
read 131072/131072 bytes at offset 0
128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
# Output of qemu-img map
Offset Length Mapped to File
### Test 'qemu-io -c discard' on a QCOW2 image with a backing file
# Create a backing image and fill it with data
Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=131072
wrote 131072/131072 bytes at offset 0
128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
# Create an image with compat=0.10 and a backing file
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=131072 backing_file=TEST_DIR/t.IMGFMT.base
# Fill all clusters with data and then discard them
wrote 131072/131072 bytes at offset 0
128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
discard 131072/131072 bytes at offset 0
128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
# Read the data from the discarded clusters
read 131072/131072 bytes at offset 0
128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
# Output of qemu-img map
Offset Length Mapped to File
0 0x20000 0x50000 TEST_DIR/t.qcow2
# Create an image with compat=1.1 and a backing file
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=131072 backing_file=TEST_DIR/t.IMGFMT.base
# Fill all clusters with data and then discard them
wrote 131072/131072 bytes at offset 0
128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
discard 131072/131072 bytes at offset 0
128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
# Read the data from the discarded clusters
read 131072/131072 bytes at offset 0
128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
# Output of qemu-img map
Offset Length Mapped to File
*** done

View File

@ -23,7 +23,7 @@ do_is_allocated() {
local count=$4
for ((i=1;i<=$count;i++)); do
echo alloc $(( start + (i - 1) * step )) $size
echo "alloc $(( start + (i - 1) * step )) $size"
done
}
@ -39,9 +39,9 @@ do_io() {
local count=$5
local pattern=$6
echo === IO: pattern $pattern >&2
echo "=== IO: pattern $pattern" >&2
for ((i=1;i<=$count;i++)); do
echo $op -P $pattern $(( start + (i - 1) * step )) $size
echo "$op -P $pattern $(( start + (i - 1) * step )) $size"
done
}
@ -110,31 +110,31 @@ io_test2() {
# free - free - compressed
# Write the clusters to be compressed
echo === Clusters to be compressed [1]
echo '=== Clusters to be compressed [1]'
io_pattern writev $((offset + 4 * $cluster_size)) $cluster_size $((9 * $cluster_size)) $num 165
echo === Clusters to be compressed [2]
echo '=== Clusters to be compressed [2]'
io_pattern writev $((offset + 5 * $cluster_size)) $cluster_size $((9 * $cluster_size)) $num 165
echo === Clusters to be compressed [3]
echo '=== Clusters to be compressed [3]'
io_pattern writev $((offset + 8 * $cluster_size)) $cluster_size $((9 * $cluster_size)) $num 165
mv "$TEST_IMG" "$TEST_IMG.orig"
$QEMU_IMG convert -f $IMGFMT -O $IMGFMT -c "$TEST_IMG.orig" "$TEST_IMG"
# Write the used clusters
echo === Used clusters [1]
echo '=== Used clusters [1]'
io_pattern writev $((offset + 0 * $cluster_size)) $cluster_size $((9 * $cluster_size)) $num 165
echo === Used clusters [2]
echo '=== Used clusters [2]'
io_pattern writev $((offset + 1 * $cluster_size)) $cluster_size $((9 * $cluster_size)) $num 165
echo === Used clusters [3]
echo '=== Used clusters [3]'
io_pattern writev $((offset + 3 * $cluster_size)) $cluster_size $((9 * $cluster_size)) $num 165
# Read them
echo === Read used/compressed clusters
echo '=== Read used/compressed clusters'
io_pattern readv $((offset + 0 * $cluster_size)) $((2 * $cluster_size)) $((9 * $cluster_size)) $num 165
io_pattern readv $((offset + 3 * $cluster_size)) $((3 * $cluster_size)) $((9 * $cluster_size)) $num 165
io_pattern readv $((offset + 8 * $cluster_size)) $((1 * $cluster_size)) $((9 * $cluster_size)) $num 165
echo === Read zeros
echo '=== Read zeros'
io_zero readv $((offset + 2 * $cluster_size)) $((1 * $cluster_size)) $((9 * $cluster_size)) $num
io_zero readv $((offset + 6 * $cluster_size)) $((2 * $cluster_size)) $((9 * $cluster_size)) $num
}

View File

@ -296,3 +296,4 @@
286 rw quick
288 quick
289 rw quick
290 rw auto quick