* [Qemu-devel] [PATCH v2 0/9] raw: Prohibit dangerous writes for probed images
@ 2014-11-07 19:39 Kevin Wolf
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 1/9] qemu-io: Allow explicitly specifying format Kevin Wolf
` (11 more replies)
0 siblings, 12 replies; 37+ messages in thread
From: Kevin Wolf @ 2014-11-07 19:39 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, jcody, armbru, mreitz, stefanha
See the commit message of patch 7 for the why and how. This series
will probably be only part of the solution and doesn't mean that we
should stop looking for other patches which improve different parts of
the problem.
See the mailing list thread "Image probing: how it can be insecure, and
what we could do about it" for the complete context.
v2:
- Fixed offset in qemu_iovec_concat [Kevin]
- Added paragraph to patch 7 explaining that we're not breaking
additional cases, but only change the failure mode of already
broken scenarios [Max]
- Added a warning when opening an image in "restricted raw" mode,
which required a few more patches to make the test cases avoid
this warning [Markus]
Kevin Wolf (8):
qemu-io: Allow explicitly specifying format
qemu-iotests: Use qemu-io -f $IMGFMT
qemu-iotests: Add qemu-io format option in Python tests
qtests: Specify image format explicitly
block: Read only one sector for format probing
raw: Prohibit dangerous writes for probed images
qemu-iotests: Fix stderr handling in common.qemu
qemu-iotests: Test writing non-raw image headers to raw image
Markus Armbruster (1):
block: Factor bdrv_probe_all() out of find_image_format()
block.c | 48 +++++++++----
block/raw_bsd.c | 57 +++++++++++++++-
include/block/block_int.h | 5 ++
qemu-io.c | 28 +++++---
tests/ahci-test.c | 3 +-
tests/bios-tables-test.c | 2 +-
tests/drive_del-test.c | 2 +-
tests/fdc-test.c | 2 +-
tests/hd-geo-test.c | 2 +-
tests/i440fx-test.c | 5 +-
tests/ide-test.c | 9 +--
tests/nvme-test.c | 2 +-
tests/qemu-iotests/016 | 11 +--
tests/qemu-iotests/030 | 22 +++---
tests/qemu-iotests/040 | 32 ++++-----
tests/qemu-iotests/048 | 2 +-
tests/qemu-iotests/055 | 18 ++---
tests/qemu-iotests/058 | 11 +--
tests/qemu-iotests/071 | 10 +--
tests/qemu-iotests/071.out | 6 +-
tests/qemu-iotests/077 | 2 +-
tests/qemu-iotests/081 | 8 ++-
tests/qemu-iotests/081.out | 2 +-
tests/qemu-iotests/089 | 6 +-
tests/qemu-iotests/109 | 100 +++++++++++++++++++++++++++
tests/qemu-iotests/109.out | 149 +++++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/common | 2 +-
tests/qemu-iotests/common.qemu | 3 +-
tests/qemu-iotests/group | 1 +
tests/usb-hcd-uhci-test.c | 2 +-
tests/usb-hcd-xhci-test.c | 2 +-
tests/virtio-blk-test.c | 4 +-
tests/virtio-scsi-test.c | 4 +-
33 files changed, 460 insertions(+), 102 deletions(-)
create mode 100755 tests/qemu-iotests/109
create mode 100644 tests/qemu-iotests/109.out
--
1.8.3.1
^ permalink raw reply [flat|nested] 37+ messages in thread
* [Qemu-devel] [PATCH v2 1/9] qemu-io: Allow explicitly specifying format
2014-11-07 19:39 [Qemu-devel] [PATCH v2 0/9] raw: Prohibit dangerous writes for probed images Kevin Wolf
@ 2014-11-07 19:39 ` Kevin Wolf
2014-11-10 14:07 ` Max Reitz
` (2 more replies)
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 2/9] qemu-iotests: Use qemu-io -f $IMGFMT Kevin Wolf
` (10 subsequent siblings)
11 siblings, 3 replies; 37+ messages in thread
From: Kevin Wolf @ 2014-11-07 19:39 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, jcody, armbru, mreitz, stefanha
This adds a -f option to qemu-io which allows to explicitly specify the
block driver to use for the given image.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
qemu-io.c | 28 ++++++++++++++++++++--------
1 file changed, 20 insertions(+), 8 deletions(-)
diff --git a/qemu-io.c b/qemu-io.c
index 60f84dd..91a445a 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -51,7 +51,8 @@ static const cmdinfo_t close_cmd = {
.oneline = "close the current open file",
};
-static int openfile(char *name, int flags, int growable, QDict *opts)
+static int openfile(char *name, BlockDriver *drv, int flags, int growable,
+ QDict *opts)
{
Error *local_err = NULL;
@@ -68,7 +69,7 @@ static int openfile(char *name, int flags, int growable, QDict *opts)
flags |= BDRV_O_PROTOCOL;
}
- if (bdrv_open(&qemuio_bs, name, NULL, opts, flags, NULL, &local_err) < 0) {
+ if (bdrv_open(&qemuio_bs, name, NULL, opts, flags, drv, &local_err) < 0) {
fprintf(stderr, "%s: can't open%s%s: %s\n", progname,
name ? " device " : "", name ?: "",
error_get_pretty(local_err));
@@ -169,9 +170,9 @@ static int open_f(BlockDriverState *bs, int argc, char **argv)
qemu_opts_reset(&empty_opts);
if (optind == argc - 1) {
- return openfile(argv[optind], flags, growable, opts);
+ return openfile(argv[optind], NULL, flags, growable, opts);
} else if (optind == argc) {
- return openfile(NULL, flags, growable, opts);
+ return openfile(NULL, NULL, flags, growable, opts);
} else {
QDECREF(opts);
return qemuio_command_usage(&open_cmd);
@@ -196,11 +197,12 @@ static const cmdinfo_t quit_cmd = {
static void usage(const char *name)
{
printf(
-"Usage: %s [-h] [-V] [-rsnm] [-c STRING] ... [file]\n"
+"Usage: %s [-h] [-V] [-rsnm] [-f FMT] [-c STRING] ... [file]\n"
"QEMU Disk exerciser\n"
"\n"
" -c, --cmd STRING execute command with its arguments\n"
" from the given string\n"
+" -f, --format FMT specifies the block driver to use\n"
" -r, --read-only export read-only\n"
" -s, --snapshot use snapshot file\n"
" -n, --nocache disable host cache\n"
@@ -364,12 +366,13 @@ int main(int argc, char **argv)
{
int readonly = 0;
int growable = 0;
- const char *sopt = "hVc:d:rsnmgkt:T:";
+ const char *sopt = "hVc:d:f:rsnmgkt:T:";
const struct option lopt[] = {
{ "help", 0, NULL, 'h' },
{ "version", 0, NULL, 'V' },
{ "offset", 1, NULL, 'o' },
{ "cmd", 1, NULL, 'c' },
+ { "format", 1, NULL, 'f' },
{ "read-only", 0, NULL, 'r' },
{ "snapshot", 0, NULL, 's' },
{ "nocache", 0, NULL, 'n' },
@@ -384,6 +387,7 @@ int main(int argc, char **argv)
int c;
int opt_index = 0;
int flags = BDRV_O_UNMAP;
+ BlockDriver *drv = NULL;
Error *local_error = NULL;
#ifdef CONFIG_POSIX
@@ -393,6 +397,8 @@ int main(int argc, char **argv)
progname = basename(argv[0]);
qemu_init_exec_dir(argv[0]);
+ bdrv_init();
+
while ((c = getopt_long(argc, argv, sopt, lopt, &opt_index)) != -1) {
switch (c) {
case 's':
@@ -407,6 +413,13 @@ int main(int argc, char **argv)
exit(1);
}
break;
+ case 'f':
+ drv = bdrv_find_format(optarg);
+ if (!drv) {
+ error_report("Invalid format '%s'", optarg);
+ exit(EXIT_FAILURE);
+ }
+ break;
case 'c':
add_user_command(optarg);
break;
@@ -455,7 +468,6 @@ int main(int argc, char **argv)
error_free(local_error);
exit(1);
}
- bdrv_init();
/* initialize commands */
qemuio_add_command(&quit_cmd);
@@ -477,7 +489,7 @@ int main(int argc, char **argv)
}
if ((argc - optind) == 1) {
- openfile(argv[optind], flags, growable, NULL);
+ openfile(argv[optind], drv, flags, growable, NULL);
}
command_loop();
--
1.8.3.1
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [Qemu-devel] [PATCH v2 2/9] qemu-iotests: Use qemu-io -f $IMGFMT
2014-11-07 19:39 [Qemu-devel] [PATCH v2 0/9] raw: Prohibit dangerous writes for probed images Kevin Wolf
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 1/9] qemu-io: Allow explicitly specifying format Kevin Wolf
@ 2014-11-07 19:39 ` Kevin Wolf
2014-11-10 14:21 ` Max Reitz
2014-11-13 10:47 ` Stefan Hajnoczi
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 3/9] qemu-iotests: Add qemu-io format option in Python tests Kevin Wolf
` (9 subsequent siblings)
11 siblings, 2 replies; 37+ messages in thread
From: Kevin Wolf @ 2014-11-07 19:39 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, jcody, armbru, mreitz, stefanha
This patch changes $QEMU_IO so that all tests by default pass a format
argument to qemu-io.
There are a few cases where -f $IMGFMT is not wanted because it selects
the wrong driver or json: filenames including a driver are used. They
are changed to use $QEMU_IO_PROG, which doesn't include any options.
Tests 071 and 081 have output changes because now the actual request
fails instead of reading the 2k probing buffer.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
tests/qemu-iotests/016 | 11 +++++++----
tests/qemu-iotests/048 | 2 +-
tests/qemu-iotests/058 | 11 +++++++----
tests/qemu-iotests/071 | 10 +++++-----
tests/qemu-iotests/071.out | 6 +++---
tests/qemu-iotests/077 | 2 +-
tests/qemu-iotests/081 | 8 ++++++--
tests/qemu-iotests/081.out | 2 +-
tests/qemu-iotests/089 | 6 ++++--
tests/qemu-iotests/common | 2 +-
10 files changed, 36 insertions(+), 24 deletions(-)
diff --git a/tests/qemu-iotests/016 b/tests/qemu-iotests/016
index 7ea9e94..52397aa 100755
--- a/tests/qemu-iotests/016
+++ b/tests/qemu-iotests/016
@@ -43,25 +43,28 @@ _supported_proto file sheepdog nfs
_supported_os Linux
+# No -f, use probing for the protocol driver
+QEMU_IO_PROTO="$QEMU_IO_PROG -g --cache $CACHEMODE"
+
size=128M
_make_test_img $size
echo
echo "== reading at EOF =="
-$QEMU_IO -g -c "read -P 0 $size 512" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO_PROTO -c "read -P 0 $size 512" "$TEST_IMG" | _filter_qemu_io
echo
echo "== reading far past EOF =="
-$QEMU_IO -g -c "read -P 0 256M 512" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO_PROTO -c "read -P 0 256M 512" "$TEST_IMG" | _filter_qemu_io
echo
echo "== writing at EOF =="
-$QEMU_IO -g -c "write -P 66 $size 512" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO_PROTO -c "write -P 66 $size 512" "$TEST_IMG" | _filter_qemu_io
$QEMU_IO -c "read -P 66 $size 512" "$TEST_IMG" | _filter_qemu_io
echo
echo "== writing far past EOF =="
-$QEMU_IO -g -c "write -P 66 256M 512" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO_PROTO -c "write -P 66 256M 512" "$TEST_IMG" | _filter_qemu_io
$QEMU_IO -c "read -P 66 256M 512" "$TEST_IMG" | _filter_qemu_io
# success, all done
diff --git a/tests/qemu-iotests/048 b/tests/qemu-iotests/048
index 65da46d..e1eeac2 100755
--- a/tests/qemu-iotests/048
+++ b/tests/qemu-iotests/048
@@ -64,7 +64,7 @@ _compare
_compare -q
# Compare images with different size
-$QEMU_IMG resize "$TEST_IMG" +512M
+$QEMU_IMG resize -f $IMGFMT "$TEST_IMG" +512M
_compare
_compare -s
diff --git a/tests/qemu-iotests/058 b/tests/qemu-iotests/058
index 14584cd..2d5ca85 100755
--- a/tests/qemu-iotests/058
+++ b/tests/qemu-iotests/058
@@ -89,6 +89,9 @@ _supported_fmt qcow2
_supported_proto file
_require_command QEMU_NBD
+# Use -f raw instead of -f $IMGFMT for the NBD connection
+QEMU_IO_NBD="$QEMU_IO -f raw --cache=$CACHEMODE"
+
echo
echo "== preparing image =="
_make_test_img 64M
@@ -108,15 +111,15 @@ _export_nbd_snapshot sn1
echo
echo "== verifying the exported snapshot with patterns, method 1 =="
-$QEMU_IO -c 'read -P 0xa 0x1000 0x1000' "$nbd_snapshot_img" | _filter_qemu_io
-$QEMU_IO -c 'read -P 0xb 0x2000 0x1000' "$nbd_snapshot_img" | _filter_qemu_io
+$QEMU_IO_NBD -c 'read -P 0xa 0x1000 0x1000' "$nbd_snapshot_img" | _filter_qemu_io
+$QEMU_IO_NBD -c 'read -P 0xb 0x2000 0x1000' "$nbd_snapshot_img" | _filter_qemu_io
_export_nbd_snapshot1 sn1
echo
echo "== verifying the exported snapshot with patterns, method 2 =="
-$QEMU_IO -c 'read -P 0xa 0x1000 0x1000' "$nbd_snapshot_img" | _filter_qemu_io
-$QEMU_IO -c 'read -P 0xb 0x2000 0x1000' "$nbd_snapshot_img" | _filter_qemu_io
+$QEMU_IO_NBD -c 'read -P 0xa 0x1000 0x1000' "$nbd_snapshot_img" | _filter_qemu_io
+$QEMU_IO_NBD -c 'read -P 0xb 0x2000 0x1000' "$nbd_snapshot_img" | _filter_qemu_io
$QEMU_IMG convert "$TEST_IMG" -l sn1 -O qcow2 "$converted_image"
diff --git a/tests/qemu-iotests/071 b/tests/qemu-iotests/071
index 3924e51..5d61ef6 100755
--- a/tests/qemu-iotests/071
+++ b/tests/qemu-iotests/071
@@ -63,12 +63,12 @@ echo
TEST_IMG="$TEST_IMG.base" IMGOPTS="" IMGFMT="raw" _make_test_img $IMG_SIZE |\
_filter_imgfmt
_make_test_img $IMG_SIZE
-$QEMU_IO -c "open -o file.driver=blkverify,file.raw.filename=$TEST_IMG.base $TEST_IMG" \
+$QEMU_IO -c "open -o driver=raw,file.driver=blkverify,file.raw.filename=$TEST_IMG.base $TEST_IMG" \
-c 'read 0 512' -c 'write -P 42 0x38000 512' -c 'read -P 42 0x38000 512' | _filter_qemu_io
$QEMU_IO -c 'write -P 42 0 512' "$TEST_IMG" | _filter_qemu_io
-$QEMU_IO -c "open -o file.driver=blkverify,file.raw.filename=$TEST_IMG.base $TEST_IMG" \
+$QEMU_IO -c "open -o driver=raw,file.driver=blkverify,file.raw.filename=$TEST_IMG.base $TEST_IMG" \
-c 'read -P 42 0 512' | _filter_qemu_io
echo
@@ -78,12 +78,12 @@ echo
TEST_IMG="$TEST_IMG.base" IMGOPTS="" IMGFMT="raw" _make_test_img $IMG_SIZE |\
_filter_imgfmt
_make_test_img $IMG_SIZE
-$QEMU_IO -c "open -o file.driver=blkverify,file.raw.filename=$TEST_IMG.base,file.test.driver=$IMGFMT,file.test.file.filename=$TEST_IMG" \
+$QEMU_IO -c "open -o driver=raw,file.driver=blkverify,file.raw.filename=$TEST_IMG.base,file.test.driver=$IMGFMT,file.test.file.filename=$TEST_IMG" \
-c 'read 0 512' -c 'write -P 42 0x38000 512' -c 'read -P 42 0x38000 512' | _filter_qemu_io
$QEMU_IO -c 'write -P 42 0 512' "$TEST_IMG" | _filter_qemu_io
-$QEMU_IO -c "open -o file.driver=blkverify,file.raw.filename=$TEST_IMG.base $TEST_IMG" \
+$QEMU_IO -c "open -o driver=raw,file.driver=blkverify,file.raw.filename=$TEST_IMG.base $TEST_IMG" \
-c 'read -P 42 0 512' | _filter_qemu_io
echo
@@ -163,7 +163,7 @@ echo
echo "=== Testing blkverify on existing raw block device ==="
echo
-run_qemu -drive "file=$TEST_IMG.base,if=none,id=drive0" <<EOF
+run_qemu -drive "file=$TEST_IMG.base,format=raw,if=none,id=drive0" <<EOF
{ "execute": "qmp_capabilities" }
{ "execute": "blockdev-add",
"arguments": {
diff --git a/tests/qemu-iotests/071.out b/tests/qemu-iotests/071.out
index 5f840a9..f36b898 100644
--- a/tests/qemu-iotests/071.out
+++ b/tests/qemu-iotests/071.out
@@ -12,7 +12,7 @@ read 512/512 bytes at offset 229376
512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
wrote 512/512 bytes at offset 0
512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-blkverify: read sector_num=0 nb_sectors=4 contents mismatch in sector 0
+blkverify: read sector_num=0 nb_sectors=1 contents mismatch in sector 0
=== Testing blkverify through file blockref ===
@@ -26,7 +26,7 @@ read 512/512 bytes at offset 229376
512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
wrote 512/512 bytes at offset 0
512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-blkverify: read sector_num=0 nb_sectors=4 contents mismatch in sector 0
+blkverify: read sector_num=0 nb_sectors=1 contents mismatch in sector 0
=== Testing blkdebug through filename ===
@@ -61,7 +61,7 @@ blkverify: read sector_num=0 nb_sectors=1 contents mismatch in sector 0
=== Testing blkverify on existing raw block device ===
-Testing: -drive file=TEST_DIR/t.IMGFMT.base,if=none,id=drive0
+Testing: -drive file=TEST_DIR/t.IMGFMT.base,format=raw,if=none,id=drive0
QMP_VERSION
{"return": {}}
{"return": {}}
diff --git a/tests/qemu-iotests/077 b/tests/qemu-iotests/077
index 4dd1bdd..42a9085 100755
--- a/tests/qemu-iotests/077
+++ b/tests/qemu-iotests/077
@@ -52,7 +52,7 @@ echo "== Some concurrent requests involving RMW =="
function test_io()
{
-echo "open -o file.align=4k blkdebug::$TEST_IMG"
+echo "open -o driver=$IMGFMT,file.align=4k blkdebug::$TEST_IMG"
# A simple RMW request
cat <<EOF
aio_write -P 10 0x200 0x200
diff --git a/tests/qemu-iotests/081 b/tests/qemu-iotests/081
index ed3c29e..9ab93ff 100755
--- a/tests/qemu-iotests/081
+++ b/tests/qemu-iotests/081
@@ -59,9 +59,13 @@ function run_qemu()
test_quorum=$($QEMU_IMG --help|grep quorum)
[ "$test_quorum" = "" ] && _supported_fmt quorum
-quorum="file.driver=quorum,file.children.0.file.filename=$TEST_DIR/1.raw"
+quorum="driver=raw,file.driver=quorum,file.vote-threshold=2"
+quorum="$quorum,file.children.0.file.filename=$TEST_DIR/1.raw"
quorum="$quorum,file.children.1.file.filename=$TEST_DIR/2.raw"
-quorum="$quorum,file.children.2.file.filename=$TEST_DIR/3.raw,file.vote-threshold=2"
+quorum="$quorum,file.children.2.file.filename=$TEST_DIR/3.raw"
+quorum="$quorum,file.children.0.driver=raw"
+quorum="$quorum,file.children.1.driver=raw"
+quorum="$quorum,file.children.2.driver=raw"
echo
echo "== creating quorum files =="
diff --git a/tests/qemu-iotests/081.out b/tests/qemu-iotests/081.out
index 073515e..190905a 100644
--- a/tests/qemu-iotests/081.out
+++ b/tests/qemu-iotests/081.out
@@ -55,5 +55,5 @@ wrote 10485760/10485760 bytes at offset 0
10 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
== checking that quorum is broken ==
-qemu-io: can't open: Could not read image for determining its format: Input/output error
+read failed: Input/output error
*** done
diff --git a/tests/qemu-iotests/089 b/tests/qemu-iotests/089
index dffc977..6f74cec 100755
--- a/tests/qemu-iotests/089
+++ b/tests/qemu-iotests/089
@@ -65,7 +65,8 @@ $QEMU_IO -c 'write -P 42 0 512' -c 'write -P 23 512 512' \
$QEMU_IMG convert -f raw -O $IMGFMT "$TEST_IMG.base" "$TEST_IMG"
-$QEMU_IO -c 'read -P 42 0 512' -c 'read -P 23 512 512' \
+$QEMU_IO_PROG --cache $CACHEMODE \
+ -c 'read -P 42 0 512' -c 'read -P 23 512 512' \
-c 'read -P 66 1024 512' "json:{
\"driver\": \"$IMGFMT\",
\"file\": {
@@ -91,7 +92,8 @@ $QEMU_IO -c 'write -P 42 0x38000 512' "$TEST_IMG" | _filter_qemu_io
# The "image.filename" part tests whether "a": { "b": "c" } and "a.b": "c" do
# the same (which they should).
-$QEMU_IO -c 'read -P 42 0x38000 512' "json:{
+$QEMU_IO_PROG --cache $CACHEMODE \
+ -c 'read -P 42 0x38000 512' "json:{
\"driver\": \"$IMGFMT\",
\"file\": {
\"driver\": \"blkdebug\",
diff --git a/tests/qemu-iotests/common b/tests/qemu-iotests/common
index 9e12bec..b923cd2 100644
--- a/tests/qemu-iotests/common
+++ b/tests/qemu-iotests/common
@@ -391,7 +391,7 @@ BEGIN { for (t='$start'; t<='$end'; t++) printf "%03d\n",t }' \
done
# Set qemu-io cache mode with $CACHEMODE we have
-QEMU_IO_OPTIONS="$QEMU_IO_OPTIONS --cache $CACHEMODE"
+QEMU_IO_OPTIONS="$QEMU_IO_OPTIONS -f $IMGFMT --cache $CACHEMODE"
# Set default options for qemu-img create -o if they were not specified
_set_default_imgopts
--
1.8.3.1
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [Qemu-devel] [PATCH v2 3/9] qemu-iotests: Add qemu-io format option in Python tests
2014-11-07 19:39 [Qemu-devel] [PATCH v2 0/9] raw: Prohibit dangerous writes for probed images Kevin Wolf
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 1/9] qemu-io: Allow explicitly specifying format Kevin Wolf
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 2/9] qemu-iotests: Use qemu-io -f $IMGFMT Kevin Wolf
@ 2014-11-07 19:39 ` Kevin Wolf
2014-11-10 14:29 ` Max Reitz
2014-11-13 10:47 ` Stefan Hajnoczi
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 4/9] qtests: Specify image format explicitly Kevin Wolf
` (8 subsequent siblings)
11 siblings, 2 replies; 37+ messages in thread
From: Kevin Wolf @ 2014-11-07 19:39 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, jcody, armbru, mreitz, stefanha
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
tests/qemu-iotests/030 | 22 +++++++++++-----------
tests/qemu-iotests/040 | 32 ++++++++++++++++----------------
tests/qemu-iotests/055 | 18 +++++++++---------
3 files changed, 36 insertions(+), 36 deletions(-)
diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030
index 8ce2373..952a524 100755
--- a/tests/qemu-iotests/030
+++ b/tests/qemu-iotests/030
@@ -34,7 +34,7 @@ class TestSingleDrive(iotests.QMPTestCase):
iotests.create_image(backing_img, TestSingleDrive.image_len)
qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % backing_img, mid_img)
qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % mid_img, test_img)
- qemu_io('-c', 'write -P 0x1 0 512', backing_img)
+ qemu_io('-f', 'raw', '-c', 'write -P 0x1 0 512', backing_img)
self.vm = iotests.VM().add_drive("blkdebug::" + test_img)
self.vm.launch()
@@ -55,8 +55,8 @@ class TestSingleDrive(iotests.QMPTestCase):
self.assert_no_active_block_jobs()
self.vm.shutdown()
- self.assertEqual(qemu_io('-c', 'map', backing_img),
- qemu_io('-c', 'map', test_img),
+ self.assertEqual(qemu_io('-f', 'raw', '-c', 'map', backing_img),
+ qemu_io('-f', iotests.imgfmt, '-c', 'map', test_img),
'image file map does not match backing file after streaming')
def test_stream_pause(self):
@@ -86,8 +86,8 @@ class TestSingleDrive(iotests.QMPTestCase):
self.assert_no_active_block_jobs()
self.vm.shutdown()
- self.assertEqual(qemu_io('-c', 'map', backing_img),
- qemu_io('-c', 'map', test_img),
+ self.assertEqual(qemu_io('-f', 'raw', '-c', 'map', backing_img),
+ qemu_io('-f', iotests.imgfmt, '-c', 'map', test_img),
'image file map does not match backing file after streaming')
def test_stream_partial(self):
@@ -101,8 +101,8 @@ class TestSingleDrive(iotests.QMPTestCase):
self.assert_no_active_block_jobs()
self.vm.shutdown()
- self.assertEqual(qemu_io('-c', 'map', mid_img),
- qemu_io('-c', 'map', test_img),
+ self.assertEqual(qemu_io('-f', iotests.imgfmt, '-c', 'map', mid_img),
+ qemu_io('-f', iotests.imgfmt, '-c', 'map', test_img),
'image file map does not match backing file after streaming')
def test_device_not_found(self):
@@ -359,9 +359,9 @@ class TestStreamStop(iotests.QMPTestCase):
def setUp(self):
qemu_img('create', backing_img, str(TestStreamStop.image_len))
- qemu_io('-c', 'write -P 0x1 0 32M', backing_img)
+ qemu_io('-f', 'raw', '-c', 'write -P 0x1 0 32M', backing_img)
qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % backing_img, test_img)
- qemu_io('-c', 'write -P 0x1 32M 32M', test_img)
+ qemu_io('-f', iotests.imgfmt, '-c', 'write -P 0x1 32M 32M', test_img)
self.vm = iotests.VM().add_drive("blkdebug::" + test_img)
self.vm.launch()
@@ -388,9 +388,9 @@ class TestSetSpeed(iotests.QMPTestCase):
def setUp(self):
qemu_img('create', backing_img, str(TestSetSpeed.image_len))
- qemu_io('-c', 'write -P 0x1 0 32M', backing_img)
+ qemu_io('-f', 'raw', '-c', 'write -P 0x1 0 32M', backing_img)
qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % backing_img, test_img)
- qemu_io('-c', 'write -P 0x1 32M 32M', test_img)
+ qemu_io('-f', iotests.imgfmt, '-c', 'write -P 0x1 32M 32M', test_img)
self.vm = iotests.VM().add_drive('blkdebug::' + test_img)
self.vm.launch()
diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040
index 2b432ad..ea2f98e 100755
--- a/tests/qemu-iotests/040
+++ b/tests/qemu-iotests/040
@@ -76,8 +76,8 @@ class TestSingleDrive(ImageCommitTestCase):
iotests.create_image(backing_img, self.image_len)
qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % backing_img, mid_img)
qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % mid_img, test_img)
- qemu_io('-c', 'write -P 0xab 0 524288', backing_img)
- qemu_io('-c', 'write -P 0xef 524288 524288', mid_img)
+ qemu_io('-f', 'raw', '-c', 'write -P 0xab 0 524288', backing_img)
+ qemu_io('-f', iotests.imgfmt, '-c', 'write -P 0xef 524288 524288', mid_img)
self.vm = iotests.VM().add_drive(test_img)
self.vm.launch()
@@ -89,8 +89,8 @@ class TestSingleDrive(ImageCommitTestCase):
def test_commit(self):
self.run_commit_test(mid_img, backing_img)
- self.assertEqual(-1, qemu_io('-c', 'read -P 0xab 0 524288', backing_img).find("verification failed"))
- self.assertEqual(-1, qemu_io('-c', 'read -P 0xef 524288 524288', backing_img).find("verification failed"))
+ self.assertEqual(-1, qemu_io('-f', 'raw', '-c', 'read -P 0xab 0 524288', backing_img).find("verification failed"))
+ self.assertEqual(-1, qemu_io('-f', 'raw', '-c', 'read -P 0xef 524288 524288', backing_img).find("verification failed"))
def test_device_not_found(self):
result = self.vm.qmp('block-commit', device='nonexistent', top='%s' % mid_img)
@@ -116,13 +116,13 @@ class TestSingleDrive(ImageCommitTestCase):
def test_top_is_active(self):
self.run_commit_test(test_img, backing_img, need_ready=True)
- self.assertEqual(-1, qemu_io('-c', 'read -P 0xab 0 524288', backing_img).find("verification failed"))
- self.assertEqual(-1, qemu_io('-c', 'read -P 0xef 524288 524288', backing_img).find("verification failed"))
+ self.assertEqual(-1, qemu_io('-f', 'raw', '-c', 'read -P 0xab 0 524288', backing_img).find("verification failed"))
+ self.assertEqual(-1, qemu_io('-f', 'raw', '-c', 'read -P 0xef 524288 524288', backing_img).find("verification failed"))
def test_top_is_default_active(self):
self.run_default_commit_test()
- self.assertEqual(-1, qemu_io('-c', 'read -P 0xab 0 524288', backing_img).find("verification failed"))
- self.assertEqual(-1, qemu_io('-c', 'read -P 0xef 524288 524288', backing_img).find("verification failed"))
+ self.assertEqual(-1, qemu_io('-f', 'raw', '-c', 'read -P 0xab 0 524288', backing_img).find("verification failed"))
+ self.assertEqual(-1, qemu_io('-f', 'raw', '-c', 'read -P 0xef 524288 524288', backing_img).find("verification failed"))
def test_top_and_base_reversed(self):
self.assert_no_active_block_jobs()
@@ -159,8 +159,8 @@ class TestRelativePaths(ImageCommitTestCase):
qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % self.mid_img_abs, self.test_img)
qemu_img('rebase', '-u', '-b', self.backing_img, self.mid_img_abs)
qemu_img('rebase', '-u', '-b', self.mid_img, self.test_img)
- qemu_io('-c', 'write -P 0xab 0 524288', self.backing_img_abs)
- qemu_io('-c', 'write -P 0xef 524288 524288', self.mid_img_abs)
+ qemu_io('-f', 'raw', '-c', 'write -P 0xab 0 524288', self.backing_img_abs)
+ qemu_io('-f', iotests.imgfmt, '-c', 'write -P 0xef 524288 524288', self.mid_img_abs)
self.vm = iotests.VM().add_drive(self.test_img)
self.vm.launch()
@@ -179,8 +179,8 @@ class TestRelativePaths(ImageCommitTestCase):
def test_commit(self):
self.run_commit_test(self.mid_img, self.backing_img)
- self.assertEqual(-1, qemu_io('-c', 'read -P 0xab 0 524288', self.backing_img_abs).find("verification failed"))
- self.assertEqual(-1, qemu_io('-c', 'read -P 0xef 524288 524288', self.backing_img_abs).find("verification failed"))
+ self.assertEqual(-1, qemu_io('-f', 'raw', '-c', 'read -P 0xab 0 524288', self.backing_img_abs).find("verification failed"))
+ self.assertEqual(-1, qemu_io('-f', 'raw', '-c', 'read -P 0xef 524288 524288', self.backing_img_abs).find("verification failed"))
def test_device_not_found(self):
result = self.vm.qmp('block-commit', device='nonexistent', top='%s' % self.mid_img)
@@ -206,8 +206,8 @@ class TestRelativePaths(ImageCommitTestCase):
def test_top_is_active(self):
self.run_commit_test(self.test_img, self.backing_img)
- self.assertEqual(-1, qemu_io('-c', 'read -P 0xab 0 524288', self.backing_img_abs).find("verification failed"))
- self.assertEqual(-1, qemu_io('-c', 'read -P 0xef 524288 524288', self.backing_img_abs).find("verification failed"))
+ self.assertEqual(-1, qemu_io('-f', 'raw', '-c', 'read -P 0xab 0 524288', self.backing_img_abs).find("verification failed"))
+ self.assertEqual(-1, qemu_io('-f', 'raw', '-c', 'read -P 0xef 524288 524288', self.backing_img_abs).find("verification failed"))
def test_top_and_base_reversed(self):
self.assert_no_active_block_jobs()
@@ -223,8 +223,8 @@ class TestSetSpeed(ImageCommitTestCase):
qemu_img('create', backing_img, str(TestSetSpeed.image_len))
qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % backing_img, mid_img)
qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % mid_img, test_img)
- qemu_io('-c', 'write -P 0x1 0 512', test_img)
- qemu_io('-c', 'write -P 0xef 524288 524288', mid_img)
+ qemu_io('-f', iotests.imgfmt, '-c', 'write -P 0x1 0 512', test_img)
+ qemu_io('-f', iotests.imgfmt, '-c', 'write -P 0xef 524288 524288', mid_img)
self.vm = iotests.VM().add_drive(test_img)
self.vm.launch()
diff --git a/tests/qemu-iotests/055 b/tests/qemu-iotests/055
index 451b67d..0872444 100755
--- a/tests/qemu-iotests/055
+++ b/tests/qemu-iotests/055
@@ -34,10 +34,10 @@ class TestSingleDrive(iotests.QMPTestCase):
def setUp(self):
# Write data to the image so we can compare later
qemu_img('create', '-f', iotests.imgfmt, test_img, str(TestSingleDrive.image_len))
- qemu_io('-c', 'write -P0x5d 0 64k', test_img)
- qemu_io('-c', 'write -P0xd5 1M 32k', test_img)
- qemu_io('-c', 'write -P0xdc 32M 124k', test_img)
- qemu_io('-c', 'write -P0xdc 67043328 64k', test_img)
+ qemu_io('-f', iotests.imgfmt, '-c', 'write -P0x5d 0 64k', test_img)
+ qemu_io('-f', iotests.imgfmt, '-c', 'write -P0xd5 1M 32k', test_img)
+ qemu_io('-f', iotests.imgfmt, '-c', 'write -P0xdc 32M 124k', test_img)
+ qemu_io('-f', iotests.imgfmt, '-c', 'write -P0xdc 67043328 64k', test_img)
self.vm = iotests.VM().add_drive(test_img)
self.vm.launch()
@@ -115,7 +115,7 @@ class TestSetSpeed(iotests.QMPTestCase):
def setUp(self):
qemu_img('create', '-f', iotests.imgfmt, test_img, str(TestSetSpeed.image_len))
- qemu_io('-c', 'write -P1 0 512', test_img)
+ qemu_io('-f', iotests.imgfmt, '-c', 'write -P1 0 512', test_img)
self.vm = iotests.VM().add_drive(test_img)
self.vm.launch()
@@ -186,10 +186,10 @@ class TestSingleTransaction(iotests.QMPTestCase):
def setUp(self):
qemu_img('create', '-f', iotests.imgfmt, test_img, str(TestSingleTransaction.image_len))
- qemu_io('-c', 'write -P0x5d 0 64k', test_img)
- qemu_io('-c', 'write -P0xd5 1M 32k', test_img)
- qemu_io('-c', 'write -P0xdc 32M 124k', test_img)
- qemu_io('-c', 'write -P0xdc 67043328 64k', test_img)
+ qemu_io('-f', iotests.imgfmt, '-c', 'write -P0x5d 0 64k', test_img)
+ qemu_io('-f', iotests.imgfmt, '-c', 'write -P0xd5 1M 32k', test_img)
+ qemu_io('-f', iotests.imgfmt, '-c', 'write -P0xdc 32M 124k', test_img)
+ qemu_io('-f', iotests.imgfmt, '-c', 'write -P0xdc 67043328 64k', test_img)
self.vm = iotests.VM().add_drive(test_img)
self.vm.launch()
--
1.8.3.1
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [Qemu-devel] [PATCH v2 4/9] qtests: Specify image format explicitly
2014-11-07 19:39 [Qemu-devel] [PATCH v2 0/9] raw: Prohibit dangerous writes for probed images Kevin Wolf
` (2 preceding siblings ...)
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 3/9] qemu-iotests: Add qemu-io format option in Python tests Kevin Wolf
@ 2014-11-07 19:39 ` Kevin Wolf
2014-11-10 14:39 ` Max Reitz
2014-11-13 10:47 ` Stefan Hajnoczi
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 5/9] block: Factor bdrv_probe_all() out of find_image_format() Kevin Wolf
` (7 subsequent siblings)
11 siblings, 2 replies; 37+ messages in thread
From: Kevin Wolf @ 2014-11-07 19:39 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, jcody, armbru, mreitz, stefanha
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
tests/ahci-test.c | 3 ++-
tests/bios-tables-test.c | 2 +-
tests/drive_del-test.c | 2 +-
tests/fdc-test.c | 2 +-
tests/hd-geo-test.c | 2 +-
tests/i440fx-test.c | 5 +++--
tests/ide-test.c | 9 +++++----
tests/nvme-test.c | 2 +-
tests/usb-hcd-uhci-test.c | 2 +-
tests/usb-hcd-xhci-test.c | 2 +-
tests/virtio-blk-test.c | 4 ++--
tests/virtio-scsi-test.c | 4 ++--
12 files changed, 21 insertions(+), 18 deletions(-)
diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 4c77ebe..e77fa3a 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -487,7 +487,8 @@ static void qtest_shutdown(void)
*/
static QPCIDevice *ahci_boot(void)
{
- qtest_boot("-drive if=none,id=drive0,file=%s,cache=writeback,serial=%s"
+ qtest_boot("-drive if=none,id=drive0,file=%s,cache=writeback,serial=%s,"
+ "format=raw"
" -M q35 "
"-device ide-hd,drive=drive0 "
"-global ide-hd.ver=%s",
diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index 9e4d205..2519f7d 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -716,7 +716,7 @@ static void test_acpi_one(const char *params, test_data *data)
int i;
args = g_strdup_printf("-net none -display none %s "
- "-drive id=hd0,if=none,file=%s "
+ "-drive id=hd0,if=none,file=%s,format=raw "
"-device ide-hd,drive=hd0 ",
params ? params : "", disk);
diff --git a/tests/drive_del-test.c b/tests/drive_del-test.c
index 53fa969..8951f6f 100644
--- a/tests/drive_del-test.c
+++ b/tests/drive_del-test.c
@@ -103,7 +103,7 @@ static void test_after_failed_device_add(void)
static void test_drive_del_device_del(void)
{
/* Start with a drive used by a device that unplugs instantaneously */
- qtest_start("-drive if=none,id=drive0,file=/dev/null"
+ qtest_start("-drive if=none,id=drive0,file=/dev/null,format=raw"
" -device virtio-scsi-pci"
" -device scsi-hd,drive=drive0,id=dev0");
diff --git a/tests/fdc-test.c b/tests/fdc-test.c
index 203074c..3c6c83c 100644
--- a/tests/fdc-test.c
+++ b/tests/fdc-test.c
@@ -291,7 +291,7 @@ static void test_media_insert(void)
/* Insert media in drive. DSKCHK should not be reset until a step pulse
* is sent. */
qmp_discard_response("{'execute':'change', 'arguments':{"
- " 'device':'floppy0', 'target': %s }}",
+ " 'device':'floppy0', 'target': %s, 'arg': 'raw' }}",
test_image);
qmp_discard_response(""); /* ignore event
(FIXME open -> open transition?!) */
diff --git a/tests/hd-geo-test.c b/tests/hd-geo-test.c
index c84d1e7..7cc8dff 100644
--- a/tests/hd-geo-test.c
+++ b/tests/hd-geo-test.c
@@ -208,7 +208,7 @@ static int setup_ide(int argc, char *argv[], int argv_sz,
{
char *s1, *s2, *s3;
- s1 = g_strdup_printf("-drive id=drive%d,if=%s",
+ s1 = g_strdup_printf("-drive id=drive%d,if=%s,format=raw",
ide_idx, dev ? "none" : "ide");
s2 = dev ? g_strdup("") : g_strdup_printf(",index=%d", ide_idx);
diff --git a/tests/i440fx-test.c b/tests/i440fx-test.c
index ad232b5..a3f7279 100644
--- a/tests/i440fx-test.c
+++ b/tests/i440fx-test.c
@@ -342,8 +342,9 @@ static void test_i440fx_firmware(FirmwareTestFixture *fixture,
g_assert(fw_pathname != NULL);
/* Better hope the user didn't put metacharacters in TMPDIR and co. */
- cmdline = g_strdup_printf("-S %s %s",
- fixture->is_bios ? "-bios" : "-pflash",
+ cmdline = g_strdup_printf("-S %s%s", fixture->is_bios
+ ? "-bios "
+ : "-drive if=pflash,format=raw,file=",
fw_pathname);
g_test_message("qemu cmdline: %s", cmdline);
qtest_start(cmdline);
diff --git a/tests/ide-test.c b/tests/ide-test.c
index b7a97e9..29f4039 100644
--- a/tests/ide-test.c
+++ b/tests/ide-test.c
@@ -385,7 +385,7 @@ static void test_bmdma_no_busmaster(void)
static void test_bmdma_setup(void)
{
ide_test_start(
- "-drive file=%s,if=ide,serial=%s,cache=writeback "
+ "-drive file=%s,if=ide,serial=%s,cache=writeback,format=raw "
"-global ide-hd.ver=%s",
tmp_path, "testdisk", "version");
}
@@ -414,7 +414,7 @@ static void test_identify(void)
int ret;
ide_test_start(
- "-drive file=%s,if=ide,serial=%s,cache=writeback "
+ "-drive file=%s,if=ide,serial=%s,cache=writeback,format=raw "
"-global ide-hd.ver=%s",
tmp_path, "testdisk", "version");
@@ -458,7 +458,7 @@ static void test_flush(void)
uint8_t data;
ide_test_start(
- "-drive file=blkdebug::%s,if=ide,cache=writeback",
+ "-drive file=blkdebug::%s,if=ide,cache=writeback,format=raw",
tmp_path);
/* Delay the completion of the flush request until we explicitly do it */
@@ -526,7 +526,8 @@ static void test_retry_flush(void)
ide_test_start(
"-vnc none "
- "-drive file=blkdebug:%s:%s,if=ide,cache=writeback,rerror=stop,werror=stop",
+ "-drive file=blkdebug:%s:%s,if=ide,cache=writeback,format=raw,"
+ "rerror=stop,werror=stop",
debug_path, tmp_path);
/* FLUSH CACHE command on device 0*/
diff --git a/tests/nvme-test.c b/tests/nvme-test.c
index 85768e8..ff38b5e 100644
--- a/tests/nvme-test.c
+++ b/tests/nvme-test.c
@@ -24,7 +24,7 @@ int main(int argc, char **argv)
g_test_init(&argc, &argv, NULL);
qtest_add_func("/nvme/nop", nop);
- qtest_start("-drive id=drv0,if=none,file=/dev/null "
+ qtest_start("-drive id=drv0,if=none,file=/dev/null,format=raw "
"-device nvme,drive=drv0,serial=foo");
ret = g_test_run();
diff --git a/tests/usb-hcd-uhci-test.c b/tests/usb-hcd-uhci-test.c
index 8cf2c5b..6d631cf 100644
--- a/tests/usb-hcd-uhci-test.c
+++ b/tests/usb-hcd-uhci-test.c
@@ -87,7 +87,7 @@ int main(int argc, char **argv)
qtest_add_func("/uhci/pci/hotplug/usb-storage", test_usb_storage_hotplug);
qtest_start("-device piix3-usb-uhci,id=uhci,addr=1d.0"
- " -drive id=drive0,if=none,file=/dev/null"
+ " -drive id=drive0,if=none,file=/dev/null,format=raw"
" -device usb-tablet,bus=uhci.0,port=1");
ret = g_test_run();
qtest_end();
diff --git a/tests/usb-hcd-xhci-test.c b/tests/usb-hcd-xhci-test.c
index b1a7dec..adf261e 100644
--- a/tests/usb-hcd-xhci-test.c
+++ b/tests/usb-hcd-xhci-test.c
@@ -91,7 +91,7 @@ int main(int argc, char **argv)
qtest_add_func("/xhci/pci/hotplug/usb-uas", test_usb_uas_hotplug);
qtest_start("-device nec-usb-xhci,id=xhci"
- " -drive id=drive0,if=none,file=/dev/null");
+ " -drive id=drive0,if=none,file=/dev/null,format=raw");
ret = g_test_run();
qtest_end();
diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index ead3911..89d7cbf 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -68,8 +68,8 @@ static QPCIBus *test_start(void)
g_assert_cmpint(ret, ==, 0);
close(fd);
- cmdline = g_strdup_printf("-drive if=none,id=drive0,file=%s "
- "-drive if=none,id=drive1,file=/dev/null "
+ cmdline = g_strdup_printf("-drive if=none,id=drive0,file=%s,format=raw "
+ "-drive if=none,id=drive1,file=/dev/null,format=raw "
"-device virtio-blk-pci,id=drv0,drive=drive0,"
"addr=%x.%x",
tmp_path, PCI_SLOT, PCI_FN);
diff --git a/tests/virtio-scsi-test.c b/tests/virtio-scsi-test.c
index 41f9602..989f825 100644
--- a/tests/virtio-scsi-test.c
+++ b/tests/virtio-scsi-test.c
@@ -52,8 +52,8 @@ int main(int argc, char **argv)
qtest_add_func("/virtio/scsi/pci/nop", pci_nop);
qtest_add_func("/virtio/scsi/pci/hotplug", hotplug);
- qtest_start("-drive id=drv0,if=none,file=/dev/null "
- "-drive id=drv1,if=none,file=/dev/null "
+ qtest_start("-drive id=drv0,if=none,file=/dev/null,format=raw "
+ "-drive id=drv1,if=none,file=/dev/null,format=raw "
"-device virtio-scsi-pci,id=vscsi0 "
"-device scsi-hd,bus=vscsi0.0,drive=drv0");
ret = g_test_run();
--
1.8.3.1
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [Qemu-devel] [PATCH v2 5/9] block: Factor bdrv_probe_all() out of find_image_format()
2014-11-07 19:39 [Qemu-devel] [PATCH v2 0/9] raw: Prohibit dangerous writes for probed images Kevin Wolf
` (3 preceding siblings ...)
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 4/9] qtests: Specify image format explicitly Kevin Wolf
@ 2014-11-07 19:39 ` Kevin Wolf
2014-11-10 14:47 ` Max Reitz
2014-11-13 10:47 ` Stefan Hajnoczi
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 6/9] block: Read only one sector for format probing Kevin Wolf
` (6 subsequent siblings)
11 siblings, 2 replies; 37+ messages in thread
From: Kevin Wolf @ 2014-11-07 19:39 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, jcody, armbru, mreitz, stefanha
From: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block.c | 45 ++++++++++++++++++++++++++++++++-------------
1 file changed, 32 insertions(+), 13 deletions(-)
diff --git a/block.c b/block.c
index 4b5735c..2fdc33f 100644
--- a/block.c
+++ b/block.c
@@ -648,11 +648,40 @@ BlockDriver *bdrv_find_protocol(const char *filename,
return NULL;
}
+/*
+ * Guess image format by probing its contents.
+ * This is not a good idea when your image is raw (CVE-2008-2004), but
+ * we do it anyway for backward compatibility.
+ * @buf contains the image's first @buf_size bytes.
+ * @buf_size is 2048 or the image's size, whatever is smaller.
+ * @filename is its filename.
+ * For all block drivers, call the bdrv_probe() method to get its
+ * probing score.
+ * Return the first block driver with the highest probing score.
+ */
+static BlockDriver *bdrv_probe_all(const uint8_t *buf, int buf_size,
+ const char *filename)
+{
+ int score_max = 0, score;
+ BlockDriver *drv = NULL, *d;
+
+ QLIST_FOREACH(d, &bdrv_drivers, list) {
+ if (d->bdrv_probe) {
+ score = d->bdrv_probe(buf, buf_size, filename);
+ if (score > score_max) {
+ score_max = score;
+ drv = d;
+ }
+ }
+ }
+
+ return drv;
+}
+
static int find_image_format(BlockDriverState *bs, const char *filename,
BlockDriver **pdrv, Error **errp)
{
- int score, score_max;
- BlockDriver *drv1, *drv;
+ BlockDriver *drv;
uint8_t buf[2048];
int ret = 0;
@@ -675,17 +704,7 @@ static int find_image_format(BlockDriverState *bs, const char *filename,
return ret;
}
- score_max = 0;
- drv = NULL;
- QLIST_FOREACH(drv1, &bdrv_drivers, list) {
- if (drv1->bdrv_probe) {
- score = drv1->bdrv_probe(buf, ret, filename);
- if (score > score_max) {
- score_max = score;
- drv = drv1;
- }
- }
- }
+ drv = bdrv_probe_all(buf, ret, filename);
if (!drv) {
error_setg(errp, "Could not determine image format: No compatible "
"driver found");
--
1.8.3.1
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [Qemu-devel] [PATCH v2 6/9] block: Read only one sector for format probing
2014-11-07 19:39 [Qemu-devel] [PATCH v2 0/9] raw: Prohibit dangerous writes for probed images Kevin Wolf
` (4 preceding siblings ...)
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 5/9] block: Factor bdrv_probe_all() out of find_image_format() Kevin Wolf
@ 2014-11-07 19:39 ` Kevin Wolf
2014-11-10 14:48 ` Max Reitz
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 7/9] raw: Prohibit dangerous writes for probed images Kevin Wolf
` (5 subsequent siblings)
11 siblings, 1 reply; 37+ messages in thread
From: Kevin Wolf @ 2014-11-07 19:39 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, jcody, armbru, mreitz, stefanha
The only image format driver that even potentially accesses anything
after 512 bytes in its bdrv_probe() implementation is VMDK, which reads
a plain-text descriptor file. In practice, the field it's looking for
seems to come first and will be well within the first 512 bytes, too.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
block.c | 2 +-
include/block/block_int.h | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/block.c b/block.c
index 2fdc33f..1fd4b8e 100644
--- a/block.c
+++ b/block.c
@@ -682,7 +682,7 @@ static int find_image_format(BlockDriverState *bs, const char *filename,
BlockDriver **pdrv, Error **errp)
{
BlockDriver *drv;
- uint8_t buf[2048];
+ uint8_t buf[BLOCK_PROBE_BUF_SIZE];
int ret = 0;
/* Return the raw BlockDriver * to scsi-generic devices or empty drives */
diff --git a/include/block/block_int.h b/include/block/block_int.h
index a1c17b9..cd94559 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -57,6 +57,8 @@
#define BLOCK_OPT_REDUNDANCY "redundancy"
#define BLOCK_OPT_NOCOW "nocow"
+#define BLOCK_PROBE_BUF_SIZE 512
+
typedef struct BdrvTrackedRequest {
BlockDriverState *bs;
int64_t offset;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [Qemu-devel] [PATCH v2 7/9] raw: Prohibit dangerous writes for probed images
2014-11-07 19:39 [Qemu-devel] [PATCH v2 0/9] raw: Prohibit dangerous writes for probed images Kevin Wolf
` (5 preceding siblings ...)
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 6/9] block: Read only one sector for format probing Kevin Wolf
@ 2014-11-07 19:39 ` Kevin Wolf
2014-11-10 15:03 ` Max Reitz
` (2 more replies)
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 8/9] qemu-iotests: Fix stderr handling in common.qemu Kevin Wolf
` (4 subsequent siblings)
11 siblings, 3 replies; 37+ messages in thread
From: Kevin Wolf @ 2014-11-07 19:39 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, jcody, armbru, mreitz, stefanha
If the user neglects to specify the image format, QEMU probes the
image to guess it automatically, for convenience.
Relying on format probing is insecure for raw images (CVE-2008-2004).
If the guest writes a suitable header to the device, the next probe
will recognize a format chosen by the guest. A malicious guest can
abuse this to gain access to host files, e.g. by crafting a QCOW2
header with backing file /etc/shadow.
Commit 1e72d3b (April 2008) provided -drive parameter format to let
users disable probing. Commit f965509 (March 2009) extended QCOW2 to
optionally store the backing file format, to let users disable backing
file probing. QED has had a flag to suppress probing since the
beginning (2010), set whenever a raw backing file is assigned.
All of these additions that allow to avoid format probing have to be
specified explicitly. The default still allows the attack.
In order to fix this, commit 79368c8 (July 2010) put probed raw images
in a restricted mode, in which they wouldn't be able to overwrite the
first few bytes of the image so that they would identify as a different
image. If a write to the first sector would write one of the signatures
of another driver, qemu would instead zero out the first four bytes.
This patch was later reverted in commit 8b33d9e (September 2010) because
it didn't get the handling of unaligned qiov members right.
Today's block layer that is based on coroutines and has qiov utility
functions makes it much easier to get this functionality right, so this
patch implements it.
The other differences of this patch to the old one are that it doesn't
silently write something different than the guest requested by zeroing
out some bytes (it fails the request instead) and that it doesn't
maintain a list of signatures in the raw driver (it calls the usual
probe function instead).
Note that this change doesn't introduce new breakage for false positive
cases where the guest legitimately writes data into the first sector
that matches the signatures of an image format (e.g. for nested virt):
These cases were broken before, only the failure mode changes from
corruption after the next restart (when the wrong format is probed) to
failing the problematic write request.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block.c | 5 +++--
block/raw_bsd.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++-
include/block/block_int.h | 3 +++
3 files changed, 62 insertions(+), 3 deletions(-)
diff --git a/block.c b/block.c
index 1fd4b8e..4eb2498 100644
--- a/block.c
+++ b/block.c
@@ -659,8 +659,8 @@ BlockDriver *bdrv_find_protocol(const char *filename,
* probing score.
* Return the first block driver with the highest probing score.
*/
-static BlockDriver *bdrv_probe_all(const uint8_t *buf, int buf_size,
- const char *filename)
+BlockDriver *bdrv_probe_all(const uint8_t *buf, int buf_size,
+ const char *filename)
{
int score_max = 0, score;
BlockDriver *drv = NULL, *d;
@@ -1486,6 +1486,7 @@ int bdrv_open(BlockDriverState **pbs, const char *filename,
}
/* Image format probing */
+ bs->probed = !drv;
if (!drv && file) {
ret = find_image_format(file, filename, &drv, &local_err);
if (ret < 0) {
diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index 401b967..462498e 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -58,8 +58,52 @@ static int coroutine_fn raw_co_readv(BlockDriverState *bs, int64_t sector_num,
static int coroutine_fn raw_co_writev(BlockDriverState *bs, int64_t sector_num,
int nb_sectors, QEMUIOVector *qiov)
{
+ void *buf = NULL;
+ BlockDriver *drv;
+ QEMUIOVector local_qiov;
+ int ret;
+
+ if (bs->probed && sector_num == 0) {
+ /* As long as these conditions are true, we can't get partial writes to
+ * the probe buffer and can just directly check the request. */
+ QEMU_BUILD_BUG_ON(BLOCK_PROBE_BUF_SIZE != 512);
+ QEMU_BUILD_BUG_ON(BDRV_SECTOR_SIZE != 512);
+
+ buf = qemu_try_blockalign(bs->file, 512);
+ if (!buf) {
+ ret = -ENOMEM;
+ goto fail;
+ }
+
+ ret = qemu_iovec_to_buf(qiov, 0, buf, 512);
+ if (ret != 512) {
+ ret = -EINVAL;
+ goto fail;
+ }
+
+ drv = bdrv_probe_all(buf, 512, NULL);
+ if (drv != bs->drv) {
+ ret = -EPERM;
+ goto fail;
+ }
+
+ /* Use the checked buffer, a malicious guest might be overwriting its
+ * original buffer in the background. */
+ qemu_iovec_init(&local_qiov, qiov->niov + 1);
+ qemu_iovec_add(&local_qiov, buf, 512);
+ qemu_iovec_concat(&local_qiov, qiov, 512, qiov->size - 512);
+ qiov = &local_qiov;
+ }
+
BLKDBG_EVENT(bs->file, BLKDBG_WRITE_AIO);
- return bdrv_co_writev(bs->file, sector_num, nb_sectors, qiov);
+ ret = bdrv_co_writev(bs->file, sector_num, nb_sectors, qiov);
+
+fail:
+ if (qiov == &local_qiov) {
+ qemu_iovec_destroy(&local_qiov);
+ }
+ qemu_vfree(buf);
+ return ret;
}
static int64_t coroutine_fn raw_co_get_block_status(BlockDriverState *bs,
@@ -158,6 +202,17 @@ static int raw_open(BlockDriverState *bs, QDict *options, int flags,
Error **errp)
{
bs->sg = bs->file->sg;
+
+ if (bs->probed && !bdrv_is_read_only(bs)) {
+ fprintf(stderr,
+ "WARNING: Image format was not specified for '%s'.\n"
+ " Automatically detecting the format is dangerous for "
+ "raw images, write operations on block 0 will be restricted.\n"
+ " Specify the 'raw' format explicitly to remove the "
+ "restrictions.\n",
+ bs->file->filename);
+ }
+
return 0;
}
diff --git a/include/block/block_int.h b/include/block/block_int.h
index cd94559..192fce8 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -326,6 +326,7 @@ struct BlockDriverState {
int sg; /* if true, the device is a /dev/sg* */
int copy_on_read; /* if true, copy read backing sectors into image
note this is a reference count */
+ bool probed;
BlockDriver *drv; /* NULL means no media */
void *opaque;
@@ -414,6 +415,8 @@ struct BlockDriverState {
};
int get_tmp_filename(char *filename, int size);
+BlockDriver *bdrv_probe_all(const uint8_t *buf, int buf_size,
+ const char *filename);
void bdrv_set_io_limits(BlockDriverState *bs,
ThrottleConfig *cfg);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [Qemu-devel] [PATCH v2 8/9] qemu-iotests: Fix stderr handling in common.qemu
2014-11-07 19:39 [Qemu-devel] [PATCH v2 0/9] raw: Prohibit dangerous writes for probed images Kevin Wolf
` (6 preceding siblings ...)
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 7/9] raw: Prohibit dangerous writes for probed images Kevin Wolf
@ 2014-11-07 19:39 ` Kevin Wolf
2014-11-10 15:04 ` Max Reitz
` (2 more replies)
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 9/9] qemu-iotests: Test writing non-raw image headers to raw image Kevin Wolf
` (3 subsequent siblings)
11 siblings, 3 replies; 37+ messages in thread
From: Kevin Wolf @ 2014-11-07 19:39 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, jcody, armbru, mreitz, stefanha
The original intention was to pipe stderr of qemu into $fifo_out.
However, the redirections were specified in the wrong order for this.
This patch fixes it.
Now qemu's output on stderr can be retrieved with _send_qemu_cmd, which
applies several useful filters on the output that were missing before.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
tests/qemu-iotests/common.qemu | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tests/qemu-iotests/common.qemu b/tests/qemu-iotests/common.qemu
index ee7ebb4..8e618b5 100644
--- a/tests/qemu-iotests/common.qemu
+++ b/tests/qemu-iotests/common.qemu
@@ -153,8 +153,9 @@ function _launch_qemu()
mkfifo "${fifo_out}"
mkfifo "${fifo_in}"
- "${QEMU}" -nographic -serial none ${comm} -machine accel=qtest "${@}" 2>&1 \
+ "${QEMU}" -nographic -serial none ${comm} -machine accel=qtest "${@}" \
>"${fifo_out}" \
+ 2>&1 \
<"${fifo_in}" &
QEMU_PID[${_QEMU_HANDLE}]=$!
--
1.8.3.1
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [Qemu-devel] [PATCH v2 9/9] qemu-iotests: Test writing non-raw image headers to raw image
2014-11-07 19:39 [Qemu-devel] [PATCH v2 0/9] raw: Prohibit dangerous writes for probed images Kevin Wolf
` (7 preceding siblings ...)
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 8/9] qemu-iotests: Fix stderr handling in common.qemu Kevin Wolf
@ 2014-11-07 19:39 ` Kevin Wolf
2014-11-10 15:53 ` Max Reitz
` (2 more replies)
2014-11-10 20:02 ` [Qemu-devel] [PATCH v2 0/9] raw: Prohibit dangerous writes for probed images Eric Blake
` (2 subsequent siblings)
11 siblings, 3 replies; 37+ messages in thread
From: Kevin Wolf @ 2014-11-07 19:39 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, jcody, armbru, mreitz, stefanha
This is forbidden if the raw driver was probed.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
tests/qemu-iotests/109 | 100 ++++++++++++++++++++++++++++++
tests/qemu-iotests/109.out | 149 +++++++++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/group | 1 +
3 files changed, 250 insertions(+)
create mode 100755 tests/qemu-iotests/109
create mode 100644 tests/qemu-iotests/109.out
diff --git a/tests/qemu-iotests/109 b/tests/qemu-iotests/109
new file mode 100755
index 0000000..ca7ea43
--- /dev/null
+++ b/tests/qemu-iotests/109
@@ -0,0 +1,100 @@
+#!/bin/bash
+#
+# Test writing image headers of other formats into raw images
+#
+# Copyright (C) 2014 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 <http://www.gnu.org/licenses/>.
+#
+
+# creator
+owner=kwolf@redhat.com
+
+seq="$(basename $0)"
+echo "QA output created by $seq"
+
+here="$PWD"
+tmp=/tmp/$$
+status=1 # failure is the default!
+
+_cleanup()
+{
+ rm -f $TEST_IMG.src
+ _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 raw
+_supported_proto file
+_supported_os Linux
+
+qemu_comm_method=qmp
+
+for fmt in qcow qcow2 qed vdi vhdx vmdk vpc; do
+
+ echo
+ echo "=== Writing a $fmt header into raw ==="
+ echo
+
+ _make_test_img 64M
+ TEST_IMG="$TEST_IMG.src" IMGFMT=$fmt _make_test_img 64M
+
+
+ # This first test should fail: The image format was probed, we may not
+ # write an image header at the start of the image
+
+ _launch_qemu -drive file="${TEST_IMG}.src",format=raw,cache=${CACHEMODE},id=src
+ _send_qemu_cmd $QEMU_HANDLE "{ 'execute': 'qmp_capabilities' }" "return"
+
+ _send_qemu_cmd $QEMU_HANDLE \
+ "{'execute':'drive-mirror', 'arguments':{
+ 'device': 'src', 'target': '$TEST_IMG',
+ 'mode': 'existing', 'sync': 'full'}}" \
+ "return"
+
+ _send_qemu_cmd $QEMU_HANDLE '' "BLOCK_JOB_ERROR"
+ _send_qemu_cmd $QEMU_HANDLE '{"execute":"query-block-jobs"}' "return"
+ _cleanup_qemu
+
+ $QEMU_IO -c 'read -P 0 0 64k' "$TEST_IMG" | _filter_qemu_io
+
+
+ # When raw was explicitly specified, the same must succeed
+
+ _launch_qemu -drive file="${TEST_IMG}.src",format=raw,cache=${CACHEMODE},id=src
+ _send_qemu_cmd $QEMU_HANDLE "{ 'execute': 'qmp_capabilities' }" "return"
+
+ _send_qemu_cmd $QEMU_HANDLE \
+ "{'execute':'drive-mirror', 'arguments':{
+ 'device': 'src', 'target': '$TEST_IMG', 'format': 'raw',
+ 'mode': 'existing', 'sync': 'full'}}" \
+ "return"
+
+ _send_qemu_cmd $QEMU_HANDLE '' "BLOCK_JOB_READY"
+ _send_qemu_cmd $QEMU_HANDLE '{"execute":"query-block-jobs"}' "return"
+ _cleanup_qemu
+
+ $QEMU_IMG compare -f raw -F raw "$TEST_IMG" "$TEST_IMG.src"
+
+done
+
+# success, all done
+echo '*** done'
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/109.out b/tests/qemu-iotests/109.out
new file mode 100644
index 0000000..fde64a8
--- /dev/null
+++ b/tests/qemu-iotests/109.out
@@ -0,0 +1,149 @@
+QA output created by 109
+
+=== Writing a qcow header into raw ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+Formatting 'TEST_DIR/t.raw.src', fmt=IMGFMT size=67108864
+{"return": {}}
+WARNING: Image format was not specified for 'TEST_DIR/t.raw'.
+Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted.
+Specify the 'raw' format explicitly to remove the restrictions.
+{"return": {}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_ERROR", "data": {"device": "src", "operation": "write", "action": "report"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 1024, "offset": 0, "speed": 0, "type": "mirror", "error": "Operation not permitted"}}
+{"return": []}
+read 65536/65536 bytes at offset 0
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+{"return": {}}
+{"return": {}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "src", "len": 1024, "offset": 1024, "speed": 0, "type": "mirror"}}
+{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 1024, "offset": 1024, "paused": false, "speed": 0, "ready": true, "type": "mirror"}]}
+Warning: Image size mismatch!
+Images are identical.
+
+=== Writing a qcow2 header into raw ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+Formatting 'TEST_DIR/t.raw.src', fmt=IMGFMT size=67108864
+{"return": {}}
+WARNING: Image format was not specified for 'TEST_DIR/t.raw'.
+Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted.
+Specify the 'raw' format explicitly to remove the restrictions.
+{"return": {}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_ERROR", "data": {"device": "src", "operation": "write", "action": "report"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 197120, "offset": 0, "speed": 0, "type": "mirror", "error": "Operation not permitted"}}
+{"return": []}
+read 65536/65536 bytes at offset 0
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+{"return": {}}
+{"return": {}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "src", "len": 197120, "offset": 197120, "speed": 0, "type": "mirror"}}
+{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 197120, "offset": 197120, "paused": false, "speed": 0, "ready": true, "type": "mirror"}]}
+Warning: Image size mismatch!
+Images are identical.
+
+=== Writing a qed header into raw ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+Formatting 'TEST_DIR/t.raw.src', fmt=IMGFMT size=67108864
+{"return": {}}
+WARNING: Image format was not specified for 'TEST_DIR/t.raw'.
+Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted.
+Specify the 'raw' format explicitly to remove the restrictions.
+{"return": {}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_ERROR", "data": {"device": "src", "operation": "write", "action": "report"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 327680, "offset": 0, "speed": 0, "type": "mirror", "error": "Operation not permitted"}}
+{"return": []}
+read 65536/65536 bytes at offset 0
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+{"return": {}}
+{"return": {}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "src", "len": 327680, "offset": 327680, "speed": 0, "type": "mirror"}}
+{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 327680, "offset": 327680, "paused": false, "speed": 0, "ready": true, "type": "mirror"}]}
+Warning: Image size mismatch!
+Images are identical.
+
+=== Writing a vdi header into raw ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+Formatting 'TEST_DIR/t.raw.src', fmt=IMGFMT size=67108864
+{"return": {}}
+WARNING: Image format was not specified for 'TEST_DIR/t.raw'.
+Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted.
+Specify the 'raw' format explicitly to remove the restrictions.
+{"return": {}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_ERROR", "data": {"device": "src", "operation": "write", "action": "report"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 1024, "offset": 0, "speed": 0, "type": "mirror", "error": "Operation not permitted"}}
+{"return": []}
+read 65536/65536 bytes at offset 0
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+{"return": {}}
+{"return": {}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "src", "len": 1024, "offset": 1024, "speed": 0, "type": "mirror"}}
+{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 1024, "offset": 1024, "paused": false, "speed": 0, "ready": true, "type": "mirror"}]}
+Warning: Image size mismatch!
+Images are identical.
+
+=== Writing a vhdx header into raw ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+Formatting 'TEST_DIR/t.raw.src', fmt=IMGFMT size=67108864
+{"return": {}}
+WARNING: Image format was not specified for 'TEST_DIR/t.raw'.
+Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted.
+Specify the 'raw' format explicitly to remove the restrictions.
+{"return": {}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_ERROR", "data": {"device": "src", "operation": "write", "action": "report"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 8388608, "offset": 0, "speed": 0, "type": "mirror", "error": "Operation not permitted"}}
+{"return": []}
+read 65536/65536 bytes at offset 0
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+{"return": {}}
+{"return": {}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "src", "len": 8388608, "offset": 8388608, "speed": 0, "type": "mirror"}}
+{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 8388608, "offset": 8388608, "paused": false, "speed": 0, "ready": true, "type": "mirror"}]}
+Warning: Image size mismatch!
+Images are identical.
+
+=== Writing a vmdk header into raw ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+Formatting 'TEST_DIR/t.raw.src', fmt=IMGFMT size=67108864
+{"return": {}}
+WARNING: Image format was not specified for 'TEST_DIR/t.raw'.
+Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted.
+Specify the 'raw' format explicitly to remove the restrictions.
+{"return": {}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_ERROR", "data": {"device": "src", "operation": "write", "action": "report"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 65536, "offset": 0, "speed": 0, "type": "mirror", "error": "Operation not permitted"}}
+{"return": []}
+read 65536/65536 bytes at offset 0
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+{"return": {}}
+{"return": {}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "src", "len": 65536, "offset": 65536, "speed": 0, "type": "mirror"}}
+{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 65536, "offset": 65536, "paused": false, "speed": 0, "ready": true, "type": "mirror"}]}
+Warning: Image size mismatch!
+Images are identical.
+
+=== Writing a vpc header into raw ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+Formatting 'TEST_DIR/t.raw.src', fmt=IMGFMT size=67108864
+{"return": {}}
+WARNING: Image format was not specified for 'TEST_DIR/t.raw'.
+Automatically detecting the format is dangerous for raw images, write operations on block 0 will be restricted.
+Specify the 'raw' format explicitly to remove the restrictions.
+{"return": {}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_ERROR", "data": {"device": "src", "operation": "write", "action": "report"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "src", "len": 2560, "offset": 0, "speed": 0, "type": "mirror", "error": "Operation not permitted"}}
+{"return": []}
+read 65536/65536 bytes at offset 0
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+{"return": {}}
+{"return": {}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "src", "len": 2560, "offset": 2560, "speed": 0, "type": "mirror"}}
+{"return": [{"io-status": "ok", "device": "src", "busy": false, "len": 2560, "offset": 2560, "paused": false, "speed": 0, "ready": true, "type": "mirror"}]}
+Warning: Image size mismatch!
+Images are identical.
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 7dfe469..ce57592 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -111,4 +111,5 @@
105 rw auto quick
107 rw auto quick
108 rw auto quick
+109 rw auto quick
111 rw auto quick
--
1.8.3.1
^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/9] qemu-io: Allow explicitly specifying format
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 1/9] qemu-io: Allow explicitly specifying format Kevin Wolf
@ 2014-11-10 14:07 ` Max Reitz
2014-11-10 14:18 ` Max Reitz
2014-11-10 19:24 ` Eric Blake
2014-11-13 10:47 ` Stefan Hajnoczi
2 siblings, 1 reply; 37+ messages in thread
From: Max Reitz @ 2014-11-10 14:07 UTC (permalink / raw)
To: Kevin Wolf, qemu-devel; +Cc: jcody, armbru, stefanha
On 2014-11-07 at 20:39, Kevin Wolf wrote:
> This adds a -f option to qemu-io which allows to explicitly specify the
> block driver to use for the given image.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> qemu-io.c | 28 ++++++++++++++++++++--------
> 1 file changed, 20 insertions(+), 8 deletions(-)
I would have liked the same option for the "open" command, but well.
And maybe also an error if a format but no filename has been specified,
but qemu-io already does not emit an error if some other image option
has been specified, so it's fine.
Reviewed-by: Max Reitz <mreitz@redhat.com>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/9] qemu-io: Allow explicitly specifying format
2014-11-10 14:07 ` Max Reitz
@ 2014-11-10 14:18 ` Max Reitz
0 siblings, 0 replies; 37+ messages in thread
From: Max Reitz @ 2014-11-10 14:18 UTC (permalink / raw)
To: Kevin Wolf, qemu-devel; +Cc: jcody, armbru, stefanha
On 2014-11-10 at 15:07, Max Reitz wrote:
> On 2014-11-07 at 20:39, Kevin Wolf wrote:
>> This adds a -f option to qemu-io which allows to explicitly specify the
>> block driver to use for the given image.
>>
>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>> ---
>> qemu-io.c | 28 ++++++++++++++++++++--------
>> 1 file changed, 20 insertions(+), 8 deletions(-)
>
> I would have liked the same option for the "open" command, but well.
Reviewing patch 2, yes, I totally forgot about -o driver=foo. Very well
then.
Max
> And maybe also an error if a format but no filename has been
> specified, but qemu-io already does not emit an error if some other
> image option has been specified, so it's fine.
>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/9] qemu-iotests: Use qemu-io -f $IMGFMT
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 2/9] qemu-iotests: Use qemu-io -f $IMGFMT Kevin Wolf
@ 2014-11-10 14:21 ` Max Reitz
2014-11-13 10:47 ` Stefan Hajnoczi
1 sibling, 0 replies; 37+ messages in thread
From: Max Reitz @ 2014-11-10 14:21 UTC (permalink / raw)
To: Kevin Wolf, qemu-devel; +Cc: jcody, armbru, stefanha
On 2014-11-07 at 20:39, Kevin Wolf wrote:
> This patch changes $QEMU_IO so that all tests by default pass a format
> argument to qemu-io.
>
> There are a few cases where -f $IMGFMT is not wanted because it selects
> the wrong driver or json: filenames including a driver are used. They
> are changed to use $QEMU_IO_PROG, which doesn't include any options.
>
> Tests 071 and 081 have output changes because now the actual request
> fails instead of reading the 2k probing buffer.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> tests/qemu-iotests/016 | 11 +++++++----
> tests/qemu-iotests/048 | 2 +-
> tests/qemu-iotests/058 | 11 +++++++----
> tests/qemu-iotests/071 | 10 +++++-----
> tests/qemu-iotests/071.out | 6 +++---
> tests/qemu-iotests/077 | 2 +-
> tests/qemu-iotests/081 | 8 ++++++--
> tests/qemu-iotests/081.out | 2 +-
> tests/qemu-iotests/089 | 6 ++++--
> tests/qemu-iotests/common | 2 +-
> 10 files changed, 36 insertions(+), 24 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/9] qemu-iotests: Add qemu-io format option in Python tests
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 3/9] qemu-iotests: Add qemu-io format option in Python tests Kevin Wolf
@ 2014-11-10 14:29 ` Max Reitz
2014-11-10 14:33 ` Kevin Wolf
2014-11-13 10:47 ` Stefan Hajnoczi
1 sibling, 1 reply; 37+ messages in thread
From: Max Reitz @ 2014-11-10 14:29 UTC (permalink / raw)
To: Kevin Wolf, qemu-devel; +Cc: jcody, armbru, stefanha
On 2014-11-07 at 20:39, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> tests/qemu-iotests/030 | 22 +++++++++++-----------
> tests/qemu-iotests/040 | 32 ++++++++++++++++----------------
> tests/qemu-iotests/055 | 18 +++++++++---------
> 3 files changed, 36 insertions(+), 36 deletions(-)
>
> diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030
> index 8ce2373..952a524 100755
> --- a/tests/qemu-iotests/030
> +++ b/tests/qemu-iotests/030
> @@ -34,7 +34,7 @@ class TestSingleDrive(iotests.QMPTestCase):
> iotests.create_image(backing_img, TestSingleDrive.image_len)
> qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % backing_img, mid_img)
> qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % mid_img, test_img)
> - qemu_io('-c', 'write -P 0x1 0 512', backing_img)
> + qemu_io('-f', 'raw', '-c', 'write -P 0x1 0 512', backing_img)
> self.vm = iotests.VM().add_drive("blkdebug::" + test_img)
> self.vm.launch()
>
> @@ -55,8 +55,8 @@ class TestSingleDrive(iotests.QMPTestCase):
> self.assert_no_active_block_jobs()
> self.vm.shutdown()
>
> - self.assertEqual(qemu_io('-c', 'map', backing_img),
> - qemu_io('-c', 'map', test_img),
> + self.assertEqual(qemu_io('-f', 'raw', '-c', 'map', backing_img),
> + qemu_io('-f', iotests.imgfmt, '-c', 'map', test_img),
> 'image file map does not match backing file after streaming')
>
> def test_stream_pause(self):
> @@ -86,8 +86,8 @@ class TestSingleDrive(iotests.QMPTestCase):
> self.assert_no_active_block_jobs()
> self.vm.shutdown()
>
> - self.assertEqual(qemu_io('-c', 'map', backing_img),
> - qemu_io('-c', 'map', test_img),
> + self.assertEqual(qemu_io('-f', 'raw', '-c', 'map', backing_img),
> + qemu_io('-f', iotests.imgfmt, '-c', 'map', test_img),
> 'image file map does not match backing file after streaming')
>
> def test_stream_partial(self):
> @@ -101,8 +101,8 @@ class TestSingleDrive(iotests.QMPTestCase):
> self.assert_no_active_block_jobs()
> self.vm.shutdown()
>
> - self.assertEqual(qemu_io('-c', 'map', mid_img),
> - qemu_io('-c', 'map', test_img),
> + self.assertEqual(qemu_io('-f', iotests.imgfmt, '-c', 'map', mid_img),
> + qemu_io('-f', iotests.imgfmt, '-c', 'map', test_img),
> 'image file map does not match backing file after streaming')
>
> def test_device_not_found(self):
> @@ -359,9 +359,9 @@ class TestStreamStop(iotests.QMPTestCase):
>
> def setUp(self):
> qemu_img('create', backing_img, str(TestStreamStop.image_len))
I would have most certainly not opposed to adding -f raw here as well.
> - qemu_io('-c', 'write -P 0x1 0 32M', backing_img)
> + qemu_io('-f', 'raw', '-c', 'write -P 0x1 0 32M', backing_img)
> qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % backing_img, test_img)
> - qemu_io('-c', 'write -P 0x1 32M 32M', test_img)
> + qemu_io('-f', iotests.imgfmt, '-c', 'write -P 0x1 32M 32M', test_img)
> self.vm = iotests.VM().add_drive("blkdebug::" + test_img)
> self.vm.launch()
>
> @@ -388,9 +388,9 @@ class TestSetSpeed(iotests.QMPTestCase):
>
> def setUp(self):
> qemu_img('create', backing_img, str(TestSetSpeed.image_len))
Or here.
> - qemu_io('-c', 'write -P 0x1 0 32M', backing_img)
> + qemu_io('-f', 'raw', '-c', 'write -P 0x1 0 32M', backing_img)
> qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % backing_img, test_img)
> - qemu_io('-c', 'write -P 0x1 32M 32M', test_img)
> + qemu_io('-f', iotests.imgfmt, '-c', 'write -P 0x1 32M 32M', test_img)
> self.vm = iotests.VM().add_drive('blkdebug::' + test_img)
> self.vm.launch()
>
> diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040
> index 2b432ad..ea2f98e 100755
> --- a/tests/qemu-iotests/040
> +++ b/tests/qemu-iotests/040
> @@ -76,8 +76,8 @@ class TestSingleDrive(ImageCommitTestCase):
> iotests.create_image(backing_img, self.image_len)
> qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % backing_img, mid_img)
> qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % mid_img, test_img)
> - qemu_io('-c', 'write -P 0xab 0 524288', backing_img)
> - qemu_io('-c', 'write -P 0xef 524288 524288', mid_img)
> + qemu_io('-f', 'raw', '-c', 'write -P 0xab 0 524288', backing_img)
> + qemu_io('-f', iotests.imgfmt, '-c', 'write -P 0xef 524288 524288', mid_img)
> self.vm = iotests.VM().add_drive(test_img)
> self.vm.launch()
>
> @@ -89,8 +89,8 @@ class TestSingleDrive(ImageCommitTestCase):
>
> def test_commit(self):
> self.run_commit_test(mid_img, backing_img)
> - self.assertEqual(-1, qemu_io('-c', 'read -P 0xab 0 524288', backing_img).find("verification failed"))
> - self.assertEqual(-1, qemu_io('-c', 'read -P 0xef 524288 524288', backing_img).find("verification failed"))
> + self.assertEqual(-1, qemu_io('-f', 'raw', '-c', 'read -P 0xab 0 524288', backing_img).find("verification failed"))
> + self.assertEqual(-1, qemu_io('-f', 'raw', '-c', 'read -P 0xef 524288 524288', backing_img).find("verification failed"))
>
> def test_device_not_found(self):
> result = self.vm.qmp('block-commit', device='nonexistent', top='%s' % mid_img)
> @@ -116,13 +116,13 @@ class TestSingleDrive(ImageCommitTestCase):
>
> def test_top_is_active(self):
> self.run_commit_test(test_img, backing_img, need_ready=True)
> - self.assertEqual(-1, qemu_io('-c', 'read -P 0xab 0 524288', backing_img).find("verification failed"))
> - self.assertEqual(-1, qemu_io('-c', 'read -P 0xef 524288 524288', backing_img).find("verification failed"))
> + self.assertEqual(-1, qemu_io('-f', 'raw', '-c', 'read -P 0xab 0 524288', backing_img).find("verification failed"))
> + self.assertEqual(-1, qemu_io('-f', 'raw', '-c', 'read -P 0xef 524288 524288', backing_img).find("verification failed"))
>
> def test_top_is_default_active(self):
> self.run_default_commit_test()
> - self.assertEqual(-1, qemu_io('-c', 'read -P 0xab 0 524288', backing_img).find("verification failed"))
> - self.assertEqual(-1, qemu_io('-c', 'read -P 0xef 524288 524288', backing_img).find("verification failed"))
> + self.assertEqual(-1, qemu_io('-f', 'raw', '-c', 'read -P 0xab 0 524288', backing_img).find("verification failed"))
> + self.assertEqual(-1, qemu_io('-f', 'raw', '-c', 'read -P 0xef 524288 524288', backing_img).find("verification failed"))
>
> def test_top_and_base_reversed(self):
> self.assert_no_active_block_jobs()
> @@ -159,8 +159,8 @@ class TestRelativePaths(ImageCommitTestCase):
> qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % self.mid_img_abs, self.test_img)
> qemu_img('rebase', '-u', '-b', self.backing_img, self.mid_img_abs)
> qemu_img('rebase', '-u', '-b', self.mid_img, self.test_img)
> - qemu_io('-c', 'write -P 0xab 0 524288', self.backing_img_abs)
> - qemu_io('-c', 'write -P 0xef 524288 524288', self.mid_img_abs)
> + qemu_io('-f', 'raw', '-c', 'write -P 0xab 0 524288', self.backing_img_abs)
> + qemu_io('-f', iotests.imgfmt, '-c', 'write -P 0xef 524288 524288', self.mid_img_abs)
> self.vm = iotests.VM().add_drive(self.test_img)
> self.vm.launch()
>
> @@ -179,8 +179,8 @@ class TestRelativePaths(ImageCommitTestCase):
>
> def test_commit(self):
> self.run_commit_test(self.mid_img, self.backing_img)
> - self.assertEqual(-1, qemu_io('-c', 'read -P 0xab 0 524288', self.backing_img_abs).find("verification failed"))
> - self.assertEqual(-1, qemu_io('-c', 'read -P 0xef 524288 524288', self.backing_img_abs).find("verification failed"))
> + self.assertEqual(-1, qemu_io('-f', 'raw', '-c', 'read -P 0xab 0 524288', self.backing_img_abs).find("verification failed"))
> + self.assertEqual(-1, qemu_io('-f', 'raw', '-c', 'read -P 0xef 524288 524288', self.backing_img_abs).find("verification failed"))
>
> def test_device_not_found(self):
> result = self.vm.qmp('block-commit', device='nonexistent', top='%s' % self.mid_img)
> @@ -206,8 +206,8 @@ class TestRelativePaths(ImageCommitTestCase):
>
> def test_top_is_active(self):
> self.run_commit_test(self.test_img, self.backing_img)
> - self.assertEqual(-1, qemu_io('-c', 'read -P 0xab 0 524288', self.backing_img_abs).find("verification failed"))
> - self.assertEqual(-1, qemu_io('-c', 'read -P 0xef 524288 524288', self.backing_img_abs).find("verification failed"))
> + self.assertEqual(-1, qemu_io('-f', 'raw', '-c', 'read -P 0xab 0 524288', self.backing_img_abs).find("verification failed"))
> + self.assertEqual(-1, qemu_io('-f', 'raw', '-c', 'read -P 0xef 524288 524288', self.backing_img_abs).find("verification failed"))
>
> def test_top_and_base_reversed(self):
> self.assert_no_active_block_jobs()
> @@ -223,8 +223,8 @@ class TestSetSpeed(ImageCommitTestCase):
> qemu_img('create', backing_img, str(TestSetSpeed.image_len))
Or here.
> qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % backing_img, mid_img)
> qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % mid_img, test_img)
> - qemu_io('-c', 'write -P 0x1 0 512', test_img)
> - qemu_io('-c', 'write -P 0xef 524288 524288', mid_img)
> + qemu_io('-f', iotests.imgfmt, '-c', 'write -P 0x1 0 512', test_img)
> + qemu_io('-f', iotests.imgfmt, '-c', 'write -P 0xef 524288 524288', mid_img)
> self.vm = iotests.VM().add_drive(test_img)
> self.vm.launch()
>
> diff --git a/tests/qemu-iotests/055 b/tests/qemu-iotests/055
> index 451b67d..0872444 100755
> --- a/tests/qemu-iotests/055
> +++ b/tests/qemu-iotests/055
> @@ -34,10 +34,10 @@ class TestSingleDrive(iotests.QMPTestCase):
> def setUp(self):
> # Write data to the image so we can compare later
> qemu_img('create', '-f', iotests.imgfmt, test_img, str(TestSingleDrive.image_len))
> - qemu_io('-c', 'write -P0x5d 0 64k', test_img)
> - qemu_io('-c', 'write -P0xd5 1M 32k', test_img)
> - qemu_io('-c', 'write -P0xdc 32M 124k', test_img)
> - qemu_io('-c', 'write -P0xdc 67043328 64k', test_img)
> + qemu_io('-f', iotests.imgfmt, '-c', 'write -P0x5d 0 64k', test_img)
> + qemu_io('-f', iotests.imgfmt, '-c', 'write -P0xd5 1M 32k', test_img)
> + qemu_io('-f', iotests.imgfmt, '-c', 'write -P0xdc 32M 124k', test_img)
> + qemu_io('-f', iotests.imgfmt, '-c', 'write -P0xdc 67043328 64k', test_img)
>
> self.vm = iotests.VM().add_drive(test_img)
> self.vm.launch()
> @@ -115,7 +115,7 @@ class TestSetSpeed(iotests.QMPTestCase):
>
> def setUp(self):
> qemu_img('create', '-f', iotests.imgfmt, test_img, str(TestSetSpeed.image_len))
> - qemu_io('-c', 'write -P1 0 512', test_img)
> + qemu_io('-f', iotests.imgfmt, '-c', 'write -P1 0 512', test_img)
> self.vm = iotests.VM().add_drive(test_img)
> self.vm.launch()
>
> @@ -186,10 +186,10 @@ class TestSingleTransaction(iotests.QMPTestCase):
>
> def setUp(self):
> qemu_img('create', '-f', iotests.imgfmt, test_img, str(TestSingleTransaction.image_len))
> - qemu_io('-c', 'write -P0x5d 0 64k', test_img)
> - qemu_io('-c', 'write -P0xd5 1M 32k', test_img)
> - qemu_io('-c', 'write -P0xdc 32M 124k', test_img)
> - qemu_io('-c', 'write -P0xdc 67043328 64k', test_img)
> + qemu_io('-f', iotests.imgfmt, '-c', 'write -P0x5d 0 64k', test_img)
> + qemu_io('-f', iotests.imgfmt, '-c', 'write -P0xd5 1M 32k', test_img)
> + qemu_io('-f', iotests.imgfmt, '-c', 'write -P0xdc 32M 124k', test_img)
> + qemu_io('-f', iotests.imgfmt, '-c', 'write -P0xdc 67043328 64k', test_img)
>
> self.vm = iotests.VM().add_drive(test_img)
> self.vm.launch()
Reviewed-by: Max Reitz <mreitz@redhat.com>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/9] qemu-iotests: Add qemu-io format option in Python tests
2014-11-10 14:29 ` Max Reitz
@ 2014-11-10 14:33 ` Kevin Wolf
0 siblings, 0 replies; 37+ messages in thread
From: Kevin Wolf @ 2014-11-10 14:33 UTC (permalink / raw)
To: Max Reitz; +Cc: jcody, qemu-devel, stefanha, armbru
Am 10.11.2014 um 15:29 hat Max Reitz geschrieben:
> On 2014-11-07 at 20:39, Kevin Wolf wrote:
> >Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> >---
> > tests/qemu-iotests/030 | 22 +++++++++++-----------
> > tests/qemu-iotests/040 | 32 ++++++++++++++++----------------
> > tests/qemu-iotests/055 | 18 +++++++++---------
> > 3 files changed, 36 insertions(+), 36 deletions(-)
> >
> >diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030
> >index 8ce2373..952a524 100755
> >--- a/tests/qemu-iotests/030
> >+++ b/tests/qemu-iotests/030
> >@@ -34,7 +34,7 @@ class TestSingleDrive(iotests.QMPTestCase):
> > iotests.create_image(backing_img, TestSingleDrive.image_len)
> > qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % backing_img, mid_img)
> > qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % mid_img, test_img)
> >- qemu_io('-c', 'write -P 0x1 0 512', backing_img)
> >+ qemu_io('-f', 'raw', '-c', 'write -P 0x1 0 512', backing_img)
> > self.vm = iotests.VM().add_drive("blkdebug::" + test_img)
> > self.vm.launch()
> >@@ -55,8 +55,8 @@ class TestSingleDrive(iotests.QMPTestCase):
> > self.assert_no_active_block_jobs()
> > self.vm.shutdown()
> >- self.assertEqual(qemu_io('-c', 'map', backing_img),
> >- qemu_io('-c', 'map', test_img),
> >+ self.assertEqual(qemu_io('-f', 'raw', '-c', 'map', backing_img),
> >+ qemu_io('-f', iotests.imgfmt, '-c', 'map', test_img),
> > 'image file map does not match backing file after streaming')
> > def test_stream_pause(self):
> >@@ -86,8 +86,8 @@ class TestSingleDrive(iotests.QMPTestCase):
> > self.assert_no_active_block_jobs()
> > self.vm.shutdown()
> >- self.assertEqual(qemu_io('-c', 'map', backing_img),
> >- qemu_io('-c', 'map', test_img),
> >+ self.assertEqual(qemu_io('-f', 'raw', '-c', 'map', backing_img),
> >+ qemu_io('-f', iotests.imgfmt, '-c', 'map', test_img),
> > 'image file map does not match backing file after streaming')
> > def test_stream_partial(self):
> >@@ -101,8 +101,8 @@ class TestSingleDrive(iotests.QMPTestCase):
> > self.assert_no_active_block_jobs()
> > self.vm.shutdown()
> >- self.assertEqual(qemu_io('-c', 'map', mid_img),
> >- qemu_io('-c', 'map', test_img),
> >+ self.assertEqual(qemu_io('-f', iotests.imgfmt, '-c', 'map', mid_img),
> >+ qemu_io('-f', iotests.imgfmt, '-c', 'map', test_img),
> > 'image file map does not match backing file after streaming')
> > def test_device_not_found(self):
> >@@ -359,9 +359,9 @@ class TestStreamStop(iotests.QMPTestCase):
> > def setUp(self):
> > qemu_img('create', backing_img, str(TestStreamStop.image_len))
>
> I would have most certainly not opposed to adding -f raw here as well.
qemu-img is a different story. Would probably be nice to add -f raw
there, but then, there's no real reason to do it either.
The only reason why I'm adding it to the qemu-io calls is that after
patch 7 each of the fixed calls would print a warning.
Kevin
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH v2 4/9] qtests: Specify image format explicitly
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 4/9] qtests: Specify image format explicitly Kevin Wolf
@ 2014-11-10 14:39 ` Max Reitz
2014-11-13 10:47 ` Stefan Hajnoczi
1 sibling, 0 replies; 37+ messages in thread
From: Max Reitz @ 2014-11-10 14:39 UTC (permalink / raw)
To: Kevin Wolf, qemu-devel; +Cc: jcody, armbru, stefanha
On 2014-11-07 at 20:39, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> tests/ahci-test.c | 3 ++-
> tests/bios-tables-test.c | 2 +-
> tests/drive_del-test.c | 2 +-
> tests/fdc-test.c | 2 +-
> tests/hd-geo-test.c | 2 +-
> tests/i440fx-test.c | 5 +++--
> tests/ide-test.c | 9 +++++----
> tests/nvme-test.c | 2 +-
> tests/usb-hcd-uhci-test.c | 2 +-
> tests/usb-hcd-xhci-test.c | 2 +-
> tests/virtio-blk-test.c | 4 ++--
> tests/virtio-scsi-test.c | 4 ++--
> 12 files changed, 21 insertions(+), 18 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH v2 5/9] block: Factor bdrv_probe_all() out of find_image_format()
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 5/9] block: Factor bdrv_probe_all() out of find_image_format() Kevin Wolf
@ 2014-11-10 14:47 ` Max Reitz
2014-11-13 10:47 ` Stefan Hajnoczi
1 sibling, 0 replies; 37+ messages in thread
From: Max Reitz @ 2014-11-10 14:47 UTC (permalink / raw)
To: Kevin Wolf, qemu-devel; +Cc: jcody, armbru, stefanha
On 2014-11-07 at 20:39, Kevin Wolf wrote:
> From: Markus Armbruster <armbru@redhat.com>
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> block.c | 45 ++++++++++++++++++++++++++++++++-------------
> 1 file changed, 32 insertions(+), 13 deletions(-)
>
> diff --git a/block.c b/block.c
> index 4b5735c..2fdc33f 100644
> --- a/block.c
> +++ b/block.c
> @@ -648,11 +648,40 @@ BlockDriver *bdrv_find_protocol(const char *filename,
> return NULL;
> }
>
> +/*
> + * Guess image format by probing its contents.
> + * This is not a good idea when your image is raw (CVE-2008-2004), but
> + * we do it anyway for backward compatibility.
> + * @buf contains the image's first @buf_size bytes.
> + * @buf_size is 2048 or the image's size, whatever is smaller.
That is a funny parameter description. You should fix it in patch 6 (or
directly here).
Anyway, as for this patch (or with the comment saying "@buf_size should
be at least 512" or something like that):
Reviewed-by: Max Reitz <mreitz@redhat.com>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH v2 6/9] block: Read only one sector for format probing
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 6/9] block: Read only one sector for format probing Kevin Wolf
@ 2014-11-10 14:48 ` Max Reitz
2014-11-13 10:36 ` Stefan Hajnoczi
0 siblings, 1 reply; 37+ messages in thread
From: Max Reitz @ 2014-11-10 14:48 UTC (permalink / raw)
To: Kevin Wolf, qemu-devel; +Cc: jcody, armbru, stefanha
On 2014-11-07 at 20:39, Kevin Wolf wrote:
> The only image format driver that even potentially accesses anything
> after 512 bytes in its bdrv_probe() implementation is VMDK, which reads
> a plain-text descriptor file. In practice, the field it's looking for
> seems to come first and will be well within the first 512 bytes, too.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> block.c | 2 +-
> include/block/block_int.h | 2 ++
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/block.c b/block.c
> index 2fdc33f..1fd4b8e 100644
> --- a/block.c
> +++ b/block.c
> @@ -682,7 +682,7 @@ static int find_image_format(BlockDriverState *bs, const char *filename,
> BlockDriver **pdrv, Error **errp)
> {
> BlockDriver *drv;
> - uint8_t buf[2048];
> + uint8_t buf[BLOCK_PROBE_BUF_SIZE];
> int ret = 0;
>
> /* Return the raw BlockDriver * to scsi-generic devices or empty drives */
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index a1c17b9..cd94559 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -57,6 +57,8 @@
> #define BLOCK_OPT_REDUNDANCY "redundancy"
> #define BLOCK_OPT_NOCOW "nocow"
>
> +#define BLOCK_PROBE_BUF_SIZE 512
> +
> typedef struct BdrvTrackedRequest {
> BlockDriverState *bs;
> int64_t offset;
You should change the description of the buf_size parameter for
bdrv_probe_all, as I wrote in my review for patch 5.
With that description fixed (either here or in patch 5):
Reviewed-by: Max Reitz <mreitz@redhat.com>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH v2 7/9] raw: Prohibit dangerous writes for probed images
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 7/9] raw: Prohibit dangerous writes for probed images Kevin Wolf
@ 2014-11-10 15:03 ` Max Reitz
2014-11-10 19:51 ` Eric Blake
2014-11-13 10:46 ` Stefan Hajnoczi
2 siblings, 0 replies; 37+ messages in thread
From: Max Reitz @ 2014-11-10 15:03 UTC (permalink / raw)
To: Kevin Wolf, qemu-devel; +Cc: jcody, armbru, stefanha
On 2014-11-07 at 20:39, Kevin Wolf wrote:
> If the user neglects to specify the image format, QEMU probes the
> image to guess it automatically, for convenience.
>
> Relying on format probing is insecure for raw images (CVE-2008-2004).
> If the guest writes a suitable header to the device, the next probe
> will recognize a format chosen by the guest. A malicious guest can
> abuse this to gain access to host files, e.g. by crafting a QCOW2
> header with backing file /etc/shadow.
>
> Commit 1e72d3b (April 2008) provided -drive parameter format to let
> users disable probing. Commit f965509 (March 2009) extended QCOW2 to
> optionally store the backing file format, to let users disable backing
> file probing. QED has had a flag to suppress probing since the
> beginning (2010), set whenever a raw backing file is assigned.
>
> All of these additions that allow to avoid format probing have to be
> specified explicitly. The default still allows the attack.
>
> In order to fix this, commit 79368c8 (July 2010) put probed raw images
> in a restricted mode, in which they wouldn't be able to overwrite the
> first few bytes of the image so that they would identify as a different
> image. If a write to the first sector would write one of the signatures
> of another driver, qemu would instead zero out the first four bytes.
> This patch was later reverted in commit 8b33d9e (September 2010) because
> it didn't get the handling of unaligned qiov members right.
>
> Today's block layer that is based on coroutines and has qiov utility
> functions makes it much easier to get this functionality right, so this
> patch implements it.
>
> The other differences of this patch to the old one are that it doesn't
> silently write something different than the guest requested by zeroing
> out some bytes (it fails the request instead) and that it doesn't
> maintain a list of signatures in the raw driver (it calls the usual
> probe function instead).
>
> Note that this change doesn't introduce new breakage for false positive
> cases where the guest legitimately writes data into the first sector
> that matches the signatures of an image format (e.g. for nested virt):
> These cases were broken before, only the failure mode changes from
> corruption after the next restart (when the wrong format is probed) to
> failing the problematic write request.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> block.c | 5 +++--
> block/raw_bsd.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++-
> include/block/block_int.h | 3 +++
> 3 files changed, 62 insertions(+), 3 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH v2 8/9] qemu-iotests: Fix stderr handling in common.qemu
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 8/9] qemu-iotests: Fix stderr handling in common.qemu Kevin Wolf
@ 2014-11-10 15:04 ` Max Reitz
2014-11-10 19:55 ` Eric Blake
2014-11-13 10:48 ` Stefan Hajnoczi
2 siblings, 0 replies; 37+ messages in thread
From: Max Reitz @ 2014-11-10 15:04 UTC (permalink / raw)
To: Kevin Wolf, qemu-devel; +Cc: jcody, armbru, stefanha
On 2014-11-07 at 20:39, Kevin Wolf wrote:
> The original intention was to pipe stderr of qemu into $fifo_out.
> However, the redirections were specified in the wrong order for this.
> This patch fixes it.
>
> Now qemu's output on stderr can be retrieved with _send_qemu_cmd, which
> applies several useful filters on the output that were missing before.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> tests/qemu-iotests/common.qemu | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH v2 9/9] qemu-iotests: Test writing non-raw image headers to raw image
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 9/9] qemu-iotests: Test writing non-raw image headers to raw image Kevin Wolf
@ 2014-11-10 15:53 ` Max Reitz
2014-11-10 20:00 ` Eric Blake
2014-11-13 10:48 ` Stefan Hajnoczi
2 siblings, 0 replies; 37+ messages in thread
From: Max Reitz @ 2014-11-10 15:53 UTC (permalink / raw)
To: Kevin Wolf, qemu-devel; +Cc: jcody, armbru, stefanha
On 2014-11-07 at 20:39, Kevin Wolf wrote:
> This is forbidden if the raw driver was probed.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> tests/qemu-iotests/109 | 100 ++++++++++++++++++++++++++++++
> tests/qemu-iotests/109.out | 149 +++++++++++++++++++++++++++++++++++++++++++++
> tests/qemu-iotests/group | 1 +
> 3 files changed, 250 insertions(+)
> create mode 100755 tests/qemu-iotests/109
> create mode 100644 tests/qemu-iotests/109.out
>
> diff --git a/tests/qemu-iotests/109 b/tests/qemu-iotests/109
> new file mode 100755
> index 0000000..ca7ea43
> --- /dev/null
> +++ b/tests/qemu-iotests/109
> @@ -0,0 +1,100 @@
> +#!/bin/bash
> +#
> +# Test writing image headers of other formats into raw images
> +#
> +# Copyright (C) 2014 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 <http://www.gnu.org/licenses/>.
> +#
> +
> +# creator
> +owner=kwolf@redhat.com
> +
> +seq="$(basename $0)"
> +echo "QA output created by $seq"
> +
> +here="$PWD"
> +tmp=/tmp/$$
> +status=1 # failure is the default!
> +
> +_cleanup()
> +{
> + rm -f $TEST_IMG.src
> + _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 raw
> +_supported_proto file
> +_supported_os Linux
> +
> +qemu_comm_method=qmp
> +
> +for fmt in qcow qcow2 qed vdi vhdx vmdk vpc; do
> +
> + echo
> + echo "=== Writing a $fmt header into raw ==="
> + echo
> +
> + _make_test_img 64M
> + TEST_IMG="$TEST_IMG.src" IMGFMT=$fmt _make_test_img 64M
Sometimes you're using "$TEST_IMG.src" and sometimes "${TEST_IMG}.src".
I don't know why, but it's not wrong. I guess.
Note that vhdx support is optional. For another optional block driver,
quorum, we decided to skip the test if it was not enabled. The easiest
way to do this here would be to drop vhdx from the list. I'm not sure
whether we want to do that, but I don't feel so good about having this
test fail (for raw!) when vhdx is not enabled, either.
With vhdx removed or somehow that case circumvented if it isn't enabled:
Reviewed-by: Max Reitz <mreitz@redhat.com>
This doesn't mean I strongly oppose leaving it in but I at least want
you to acknowledge that this test fails without having vhdx enabled.
Max
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/9] qemu-io: Allow explicitly specifying format
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 1/9] qemu-io: Allow explicitly specifying format Kevin Wolf
2014-11-10 14:07 ` Max Reitz
@ 2014-11-10 19:24 ` Eric Blake
2014-11-13 10:47 ` Stefan Hajnoczi
2 siblings, 0 replies; 37+ messages in thread
From: Eric Blake @ 2014-11-10 19:24 UTC (permalink / raw)
To: Kevin Wolf, qemu-devel; +Cc: jcody, armbru, stefanha, mreitz
[-- Attachment #1: Type: text/plain, Size: 2168 bytes --]
On 11/07/2014 12:39 PM, Kevin Wolf wrote:
> This adds a -f option to qemu-io which allows to explicitly specify the
> block driver to use for the given image.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> qemu-io.c | 28 ++++++++++++++++++++--------
> 1 file changed, 20 insertions(+), 8 deletions(-)
>
> "\n"
> " -c, --cmd STRING execute command with its arguments\n"
> " from the given string\n"
> +" -f, --format FMT specifies the block driver to use\n"
> " -r, --read-only export read-only\n"
Documented in one order...
> " -s, --snapshot use snapshot file\n"
> " -n, --nocache disable host cache\n"
> @@ -364,12 +366,13 @@ int main(int argc, char **argv)
> {
> int readonly = 0;
> int growable = 0;
> - const char *sopt = "hVc:d:rsnmgkt:T:";
> + const char *sopt = "hVc:d:f:rsnmgkt:T:";
inserted into the longopt in a different order...
> const struct option lopt[] = {
> { "help", 0, NULL, 'h' },
> { "version", 0, NULL, 'V' },
> { "offset", 1, NULL, 'o' },
> { "cmd", 1, NULL, 'c' },
> + { "format", 1, NULL, 'f' },
> { "read-only", 0, NULL, 'r' },
then spelled out in the first order...
> @@ -407,6 +413,13 @@ int main(int argc, char **argv)
> exit(1);
> }
> break;
> + case 'f':
> + drv = bdrv_find_format(optarg);
> + if (!drv) {
> + error_report("Invalid format '%s'", optarg);
> + exit(EXIT_FAILURE);
> + }
> + break;
> case 'c':
...and finally implemented in yet another order. While it doesn't
impact code correctness, it does impact maintainability (it would be
nice to use one common order among all four locations, rather than 3
different orders). Arguably, some of it is pre-existing, so I won't
require a respin, but it is worth considering.
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH v2 7/9] raw: Prohibit dangerous writes for probed images
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 7/9] raw: Prohibit dangerous writes for probed images Kevin Wolf
2014-11-10 15:03 ` Max Reitz
@ 2014-11-10 19:51 ` Eric Blake
2014-11-13 10:46 ` Stefan Hajnoczi
2 siblings, 0 replies; 37+ messages in thread
From: Eric Blake @ 2014-11-10 19:51 UTC (permalink / raw)
To: Kevin Wolf, qemu-devel; +Cc: jcody, armbru, stefanha, mreitz
[-- Attachment #1: Type: text/plain, Size: 2916 bytes --]
On 11/07/2014 12:39 PM, Kevin Wolf wrote:
> If the user neglects to specify the image format, QEMU probes the
> image to guess it automatically, for convenience.
>
[for those patches in 1-6 where I did not leave comments, I'm happy with
them, and saw that Max already gave R-b so I didn't spend thorough
review time on them]
>
> The other differences of this patch to the old one are that it doesn't
> silently write something different than the guest requested by zeroing
> out some bytes (it fails the request instead) and that it doesn't
> maintain a list of signatures in the raw driver (it calls the usual
> probe function instead).
>
> Note that this change doesn't introduce new breakage for false positive
> cases where the guest legitimately writes data into the first sector
> that matches the signatures of an image format (e.g. for nested virt):
> These cases were broken before, only the failure mode changes from
> corruption after the next restart (when the wrong format is probed) to
> failing the problematic write request.
I would feel better if this commit message explicitly mentioned that the
failed write can ONLY occur when probing occurs; therefore, a user can
ensure that guests can legitimately write anything to the first sector
by explicitly providing a format. But at least the error message does it.
I'm still not 100% convinced this is the patch we want, but am happy
enough that it won't break libvirt (which strives to always pass a
format), so I'm comfortable leaving a review.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> block.c | 5 +++--
> block/raw_bsd.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++-
> include/block/block_int.h | 3 +++
> 3 files changed, 62 insertions(+), 3 deletions(-)
> @@ -158,6 +202,17 @@ static int raw_open(BlockDriverState *bs, QDict *options, int flags,
> Error **errp)
> {
> bs->sg = bs->file->sg;
> +
> + if (bs->probed && !bdrv_is_read_only(bs)) {
> + fprintf(stderr,
> + "WARNING: Image format was not specified for '%s'.\n"
> + " Automatically detecting the format is dangerous for "
> + "raw images, write operations on block 0 will be restricted.\n"
> + " Specify the 'raw' format explicitly to remove the "
> + "restrictions.\n",
This error message works fairly well for me. Maybe the first line could
be a bit longer:
WARNING: Image format was not specified for '%s', and raw was assumed.\n
or maybe:
WARNING: Image format was not specified for '%s', and probing guessed raw.\n
but even with your original shorter wording,
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH v2 8/9] qemu-iotests: Fix stderr handling in common.qemu
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 8/9] qemu-iotests: Fix stderr handling in common.qemu Kevin Wolf
2014-11-10 15:04 ` Max Reitz
@ 2014-11-10 19:55 ` Eric Blake
2014-11-13 10:48 ` Stefan Hajnoczi
2 siblings, 0 replies; 37+ messages in thread
From: Eric Blake @ 2014-11-10 19:55 UTC (permalink / raw)
To: Kevin Wolf, qemu-devel; +Cc: jcody, armbru, stefanha, mreitz
[-- Attachment #1: Type: text/plain, Size: 673 bytes --]
On 11/07/2014 12:39 PM, Kevin Wolf wrote:
> The original intention was to pipe stderr of qemu into $fifo_out.
> However, the redirections were specified in the wrong order for this.
> This patch fixes it.
>
> Now qemu's output on stderr can be retrieved with _send_qemu_cmd, which
> applies several useful filters on the output that were missing before.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> tests/qemu-iotests/common.qemu | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH v2 9/9] qemu-iotests: Test writing non-raw image headers to raw image
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 9/9] qemu-iotests: Test writing non-raw image headers to raw image Kevin Wolf
2014-11-10 15:53 ` Max Reitz
@ 2014-11-10 20:00 ` Eric Blake
2014-11-13 10:48 ` Stefan Hajnoczi
2 siblings, 0 replies; 37+ messages in thread
From: Eric Blake @ 2014-11-10 20:00 UTC (permalink / raw)
To: Kevin Wolf, qemu-devel; +Cc: jcody, armbru, stefanha, mreitz
[-- Attachment #1: Type: text/plain, Size: 1123 bytes --]
On 11/07/2014 12:39 PM, Kevin Wolf wrote:
> This is forbidden if the raw driver was probed.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> tests/qemu-iotests/109 | 100 ++++++++++++++++++++++++++++++
> tests/qemu-iotests/109.out | 149 +++++++++++++++++++++++++++++++++++++++++++++
> tests/qemu-iotests/group | 1 +
> 3 files changed, 250 insertions(+)
> create mode 100755 tests/qemu-iotests/109
> create mode 100644 tests/qemu-iotests/109.out
Could you also add a test for writing an MBR/GRUB stage 1 sector into
the image? Since that is a very common guest action on initial install
of an OS into a blank disk, and also should NEVER match one of our
probes (assuming we never add support for a file format that resembles
an MBR master sector), it would be nice to ensure that our restrictions
against writing to sector 0 only fire when actually hitting a colliding
probe, and not for all writes. Likewise, maybe add a test for writing a
sector of all 0 or all FF.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/9] raw: Prohibit dangerous writes for probed images
2014-11-07 19:39 [Qemu-devel] [PATCH v2 0/9] raw: Prohibit dangerous writes for probed images Kevin Wolf
` (8 preceding siblings ...)
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 9/9] qemu-iotests: Test writing non-raw image headers to raw image Kevin Wolf
@ 2014-11-10 20:02 ` Eric Blake
2014-11-11 10:03 ` Markus Armbruster
2014-11-13 10:49 ` Stefan Hajnoczi
11 siblings, 0 replies; 37+ messages in thread
From: Eric Blake @ 2014-11-10 20:02 UTC (permalink / raw)
To: Kevin Wolf, qemu-devel; +Cc: jcody, armbru, stefanha, mreitz
[-- Attachment #1: Type: text/plain, Size: 717 bytes --]
On 11/07/2014 12:39 PM, Kevin Wolf wrote:
> See the commit message of patch 7 for the why and how. This series
> will probably be only part of the solution and doesn't mean that we
> should stop looking for other patches which improve different parts of
> the problem.
I definitely agree that tackling multiple aspects of the problem will
give us an overall better solution than sticking to one approach in
isolation.
>
> See the mailing list thread "Image probing: how it can be insecure, and
> what we could do about it" for the complete context.
Thanks again for writing that one up.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/9] raw: Prohibit dangerous writes for probed images
2014-11-07 19:39 [Qemu-devel] [PATCH v2 0/9] raw: Prohibit dangerous writes for probed images Kevin Wolf
` (9 preceding siblings ...)
2014-11-10 20:02 ` [Qemu-devel] [PATCH v2 0/9] raw: Prohibit dangerous writes for probed images Eric Blake
@ 2014-11-11 10:03 ` Markus Armbruster
2014-11-13 10:49 ` Stefan Hajnoczi
11 siblings, 0 replies; 37+ messages in thread
From: Markus Armbruster @ 2014-11-11 10:03 UTC (permalink / raw)
To: Kevin Wolf; +Cc: jcody, qemu-devel, stefanha, mreitz
Kevin Wolf <kwolf@redhat.com> writes:
> See the commit message of patch 7 for the why and how. This series
> will probably be only part of the solution and doesn't mean that we
> should stop looking for other patches which improve different parts of
> the problem.
>
> See the mailing list thread "Image probing: how it can be insecure, and
> what we could do about it" for the complete context.
Not a review, just to update the record of my opinion on this approach:
* This is not a full solution to the problem I want solved, but that's
okay, it's not sold as one.
* It helps in other scenarios I personally find less interesting, but
that's okay, others find them interesting enough.
* It changes failure modes subtly. I figure the failures are
sufficiently rare and sufficiently catastrophic for me not to worry
about changing them.
Therefore, I don't object to the general idea.
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH v2 6/9] block: Read only one sector for format probing
2014-11-10 14:48 ` Max Reitz
@ 2014-11-13 10:36 ` Stefan Hajnoczi
0 siblings, 0 replies; 37+ messages in thread
From: Stefan Hajnoczi @ 2014-11-13 10:36 UTC (permalink / raw)
To: Max Reitz; +Cc: Kevin Wolf, jcody, qemu-devel, stefanha, armbru
[-- Attachment #1: Type: text/plain, Size: 1857 bytes --]
On Mon, Nov 10, 2014 at 03:48:52PM +0100, Max Reitz wrote:
> On 2014-11-07 at 20:39, Kevin Wolf wrote:
> >The only image format driver that even potentially accesses anything
> >after 512 bytes in its bdrv_probe() implementation is VMDK, which reads
> >a plain-text descriptor file. In practice, the field it's looking for
> >seems to come first and will be well within the first 512 bytes, too.
> >
> >Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> >Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> >---
> > block.c | 2 +-
> > include/block/block_int.h | 2 ++
> > 2 files changed, 3 insertions(+), 1 deletion(-)
> >
> >diff --git a/block.c b/block.c
> >index 2fdc33f..1fd4b8e 100644
> >--- a/block.c
> >+++ b/block.c
> >@@ -682,7 +682,7 @@ static int find_image_format(BlockDriverState *bs, const char *filename,
> > BlockDriver **pdrv, Error **errp)
> > {
> > BlockDriver *drv;
> >- uint8_t buf[2048];
> >+ uint8_t buf[BLOCK_PROBE_BUF_SIZE];
> > int ret = 0;
> > /* Return the raw BlockDriver * to scsi-generic devices or empty drives */
> >diff --git a/include/block/block_int.h b/include/block/block_int.h
> >index a1c17b9..cd94559 100644
> >--- a/include/block/block_int.h
> >+++ b/include/block/block_int.h
> >@@ -57,6 +57,8 @@
> > #define BLOCK_OPT_REDUNDANCY "redundancy"
> > #define BLOCK_OPT_NOCOW "nocow"
> >+#define BLOCK_PROBE_BUF_SIZE 512
> >+
> > typedef struct BdrvTrackedRequest {
> > BlockDriverState *bs;
> > int64_t offset;
>
> You should change the description of the buf_size parameter for
> bdrv_probe_all, as I wrote in my review for patch 5.
>
> With that description fixed (either here or in patch 5):
Here please, since the value is still 2048 in Patch 5.
Stefan
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH v2 7/9] raw: Prohibit dangerous writes for probed images
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 7/9] raw: Prohibit dangerous writes for probed images Kevin Wolf
2014-11-10 15:03 ` Max Reitz
2014-11-10 19:51 ` Eric Blake
@ 2014-11-13 10:46 ` Stefan Hajnoczi
2 siblings, 0 replies; 37+ messages in thread
From: Stefan Hajnoczi @ 2014-11-13 10:46 UTC (permalink / raw)
To: Kevin Wolf; +Cc: jcody, mreitz, qemu-devel, stefanha, armbru
[-- Attachment #1: Type: text/plain, Size: 1367 bytes --]
On Fri, Nov 07, 2014 at 08:39:23PM +0100, Kevin Wolf wrote:
> diff --git a/block/raw_bsd.c b/block/raw_bsd.c
> index 401b967..462498e 100644
> --- a/block/raw_bsd.c
> +++ b/block/raw_bsd.c
> @@ -58,8 +58,52 @@ static int coroutine_fn raw_co_readv(BlockDriverState *bs, int64_t sector_num,
> static int coroutine_fn raw_co_writev(BlockDriverState *bs, int64_t sector_num,
> int nb_sectors, QEMUIOVector *qiov)
> {
> + void *buf = NULL;
> + BlockDriver *drv;
> + QEMUIOVector local_qiov;
> + int ret;
> +
> + if (bs->probed && sector_num == 0) {
> + /* As long as these conditions are true, we can't get partial writes to
> + * the probe buffer and can just directly check the request. */
> + QEMU_BUILD_BUG_ON(BLOCK_PROBE_BUF_SIZE != 512);
> + QEMU_BUILD_BUG_ON(BDRV_SECTOR_SIZE != 512);
> +
> + buf = qemu_try_blockalign(bs->file, 512);
> + if (!buf) {
> + ret = -ENOMEM;
> + goto fail;
> + }
> +
> + ret = qemu_iovec_to_buf(qiov, 0, buf, 512);
> + if (ret != 512) {
> + ret = -EINVAL;
> + goto fail;
> + }
Does this change the return value when nb_sectors == 0?
I couldn't find anything that prevents the nb_sectors edge case and I
guess we'd return 0 when nb_sectors == 0 && sector_num != 0.
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/9] qemu-io: Allow explicitly specifying format
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 1/9] qemu-io: Allow explicitly specifying format Kevin Wolf
2014-11-10 14:07 ` Max Reitz
2014-11-10 19:24 ` Eric Blake
@ 2014-11-13 10:47 ` Stefan Hajnoczi
2 siblings, 0 replies; 37+ messages in thread
From: Stefan Hajnoczi @ 2014-11-13 10:47 UTC (permalink / raw)
To: Kevin Wolf; +Cc: jcody, mreitz, qemu-devel, stefanha, armbru
[-- Attachment #1: Type: text/plain, Size: 394 bytes --]
On Fri, Nov 07, 2014 at 08:39:17PM +0100, Kevin Wolf wrote:
> This adds a -f option to qemu-io which allows to explicitly specify the
> block driver to use for the given image.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> qemu-io.c | 28 ++++++++++++++++++++--------
> 1 file changed, 20 insertions(+), 8 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/9] qemu-iotests: Use qemu-io -f $IMGFMT
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 2/9] qemu-iotests: Use qemu-io -f $IMGFMT Kevin Wolf
2014-11-10 14:21 ` Max Reitz
@ 2014-11-13 10:47 ` Stefan Hajnoczi
1 sibling, 0 replies; 37+ messages in thread
From: Stefan Hajnoczi @ 2014-11-13 10:47 UTC (permalink / raw)
To: Kevin Wolf; +Cc: jcody, mreitz, qemu-devel, stefanha, armbru
[-- Attachment #1: Type: text/plain, Size: 1109 bytes --]
On Fri, Nov 07, 2014 at 08:39:18PM +0100, Kevin Wolf wrote:
> This patch changes $QEMU_IO so that all tests by default pass a format
> argument to qemu-io.
>
> There are a few cases where -f $IMGFMT is not wanted because it selects
> the wrong driver or json: filenames including a driver are used. They
> are changed to use $QEMU_IO_PROG, which doesn't include any options.
>
> Tests 071 and 081 have output changes because now the actual request
> fails instead of reading the 2k probing buffer.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> tests/qemu-iotests/016 | 11 +++++++----
> tests/qemu-iotests/048 | 2 +-
> tests/qemu-iotests/058 | 11 +++++++----
> tests/qemu-iotests/071 | 10 +++++-----
> tests/qemu-iotests/071.out | 6 +++---
> tests/qemu-iotests/077 | 2 +-
> tests/qemu-iotests/081 | 8 ++++++--
> tests/qemu-iotests/081.out | 2 +-
> tests/qemu-iotests/089 | 6 ++++--
> tests/qemu-iotests/common | 2 +-
> 10 files changed, 36 insertions(+), 24 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/9] qemu-iotests: Add qemu-io format option in Python tests
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 3/9] qemu-iotests: Add qemu-io format option in Python tests Kevin Wolf
2014-11-10 14:29 ` Max Reitz
@ 2014-11-13 10:47 ` Stefan Hajnoczi
1 sibling, 0 replies; 37+ messages in thread
From: Stefan Hajnoczi @ 2014-11-13 10:47 UTC (permalink / raw)
To: Kevin Wolf; +Cc: jcody, mreitz, qemu-devel, stefanha, armbru
[-- Attachment #1: Type: text/plain, Size: 387 bytes --]
On Fri, Nov 07, 2014 at 08:39:19PM +0100, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> tests/qemu-iotests/030 | 22 +++++++++++-----------
> tests/qemu-iotests/040 | 32 ++++++++++++++++----------------
> tests/qemu-iotests/055 | 18 +++++++++---------
> 3 files changed, 36 insertions(+), 36 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH v2 4/9] qtests: Specify image format explicitly
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 4/9] qtests: Specify image format explicitly Kevin Wolf
2014-11-10 14:39 ` Max Reitz
@ 2014-11-13 10:47 ` Stefan Hajnoczi
1 sibling, 0 replies; 37+ messages in thread
From: Stefan Hajnoczi @ 2014-11-13 10:47 UTC (permalink / raw)
To: Kevin Wolf; +Cc: jcody, mreitz, qemu-devel, stefanha, armbru
[-- Attachment #1: Type: text/plain, Size: 667 bytes --]
On Fri, Nov 07, 2014 at 08:39:20PM +0100, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> tests/ahci-test.c | 3 ++-
> tests/bios-tables-test.c | 2 +-
> tests/drive_del-test.c | 2 +-
> tests/fdc-test.c | 2 +-
> tests/hd-geo-test.c | 2 +-
> tests/i440fx-test.c | 5 +++--
> tests/ide-test.c | 9 +++++----
> tests/nvme-test.c | 2 +-
> tests/usb-hcd-uhci-test.c | 2 +-
> tests/usb-hcd-xhci-test.c | 2 +-
> tests/virtio-blk-test.c | 4 ++--
> tests/virtio-scsi-test.c | 4 ++--
> 12 files changed, 21 insertions(+), 18 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH v2 5/9] block: Factor bdrv_probe_all() out of find_image_format()
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 5/9] block: Factor bdrv_probe_all() out of find_image_format() Kevin Wolf
2014-11-10 14:47 ` Max Reitz
@ 2014-11-13 10:47 ` Stefan Hajnoczi
1 sibling, 0 replies; 37+ messages in thread
From: Stefan Hajnoczi @ 2014-11-13 10:47 UTC (permalink / raw)
To: Kevin Wolf; +Cc: jcody, mreitz, qemu-devel, stefanha, armbru
[-- Attachment #1: Type: text/plain, Size: 394 bytes --]
On Fri, Nov 07, 2014 at 08:39:21PM +0100, Kevin Wolf wrote:
> From: Markus Armbruster <armbru@redhat.com>
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> block.c | 45 ++++++++++++++++++++++++++++++++-------------
> 1 file changed, 32 insertions(+), 13 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH v2 8/9] qemu-iotests: Fix stderr handling in common.qemu
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 8/9] qemu-iotests: Fix stderr handling in common.qemu Kevin Wolf
2014-11-10 15:04 ` Max Reitz
2014-11-10 19:55 ` Eric Blake
@ 2014-11-13 10:48 ` Stefan Hajnoczi
2 siblings, 0 replies; 37+ messages in thread
From: Stefan Hajnoczi @ 2014-11-13 10:48 UTC (permalink / raw)
To: Kevin Wolf; +Cc: jcody, mreitz, qemu-devel, stefanha, armbru
[-- Attachment #1: Type: text/plain, Size: 587 bytes --]
On Fri, Nov 07, 2014 at 08:39:24PM +0100, Kevin Wolf wrote:
> The original intention was to pipe stderr of qemu into $fifo_out.
> However, the redirections were specified in the wrong order for this.
> This patch fixes it.
>
> Now qemu's output on stderr can be retrieved with _send_qemu_cmd, which
> applies several useful filters on the output that were missing before.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> tests/qemu-iotests/common.qemu | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH v2 9/9] qemu-iotests: Test writing non-raw image headers to raw image
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 9/9] qemu-iotests: Test writing non-raw image headers to raw image Kevin Wolf
2014-11-10 15:53 ` Max Reitz
2014-11-10 20:00 ` Eric Blake
@ 2014-11-13 10:48 ` Stefan Hajnoczi
2 siblings, 0 replies; 37+ messages in thread
From: Stefan Hajnoczi @ 2014-11-13 10:48 UTC (permalink / raw)
To: Kevin Wolf; +Cc: jcody, mreitz, qemu-devel, stefanha, armbru
[-- Attachment #1: Type: text/plain, Size: 550 bytes --]
On Fri, Nov 07, 2014 at 08:39:25PM +0100, Kevin Wolf wrote:
> This is forbidden if the raw driver was probed.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> tests/qemu-iotests/109 | 100 ++++++++++++++++++++++++++++++
> tests/qemu-iotests/109.out | 149 +++++++++++++++++++++++++++++++++++++++++++++
> tests/qemu-iotests/group | 1 +
> 3 files changed, 250 insertions(+)
> create mode 100755 tests/qemu-iotests/109
> create mode 100644 tests/qemu-iotests/109.out
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/9] raw: Prohibit dangerous writes for probed images
2014-11-07 19:39 [Qemu-devel] [PATCH v2 0/9] raw: Prohibit dangerous writes for probed images Kevin Wolf
` (10 preceding siblings ...)
2014-11-11 10:03 ` Markus Armbruster
@ 2014-11-13 10:49 ` Stefan Hajnoczi
11 siblings, 0 replies; 37+ messages in thread
From: Stefan Hajnoczi @ 2014-11-13 10:49 UTC (permalink / raw)
To: Kevin Wolf; +Cc: jcody, mreitz, qemu-devel, stefanha, armbru
[-- Attachment #1: Type: text/plain, Size: 3224 bytes --]
On Fri, Nov 07, 2014 at 08:39:16PM +0100, Kevin Wolf wrote:
> See the commit message of patch 7 for the why and how. This series
> will probably be only part of the solution and doesn't mean that we
> should stop looking for other patches which improve different parts of
> the problem.
>
> See the mailing list thread "Image probing: how it can be insecure, and
> what we could do about it" for the complete context.
>
> v2:
> - Fixed offset in qemu_iovec_concat [Kevin]
> - Added paragraph to patch 7 explaining that we're not breaking
> additional cases, but only change the failure mode of already
> broken scenarios [Max]
> - Added a warning when opening an image in "restricted raw" mode,
> which required a few more patches to make the test cases avoid
> this warning [Markus]
>
>
> Kevin Wolf (8):
> qemu-io: Allow explicitly specifying format
> qemu-iotests: Use qemu-io -f $IMGFMT
> qemu-iotests: Add qemu-io format option in Python tests
> qtests: Specify image format explicitly
> block: Read only one sector for format probing
> raw: Prohibit dangerous writes for probed images
> qemu-iotests: Fix stderr handling in common.qemu
> qemu-iotests: Test writing non-raw image headers to raw image
>
> Markus Armbruster (1):
> block: Factor bdrv_probe_all() out of find_image_format()
>
> block.c | 48 +++++++++----
> block/raw_bsd.c | 57 +++++++++++++++-
> include/block/block_int.h | 5 ++
> qemu-io.c | 28 +++++---
> tests/ahci-test.c | 3 +-
> tests/bios-tables-test.c | 2 +-
> tests/drive_del-test.c | 2 +-
> tests/fdc-test.c | 2 +-
> tests/hd-geo-test.c | 2 +-
> tests/i440fx-test.c | 5 +-
> tests/ide-test.c | 9 +--
> tests/nvme-test.c | 2 +-
> tests/qemu-iotests/016 | 11 +--
> tests/qemu-iotests/030 | 22 +++---
> tests/qemu-iotests/040 | 32 ++++-----
> tests/qemu-iotests/048 | 2 +-
> tests/qemu-iotests/055 | 18 ++---
> tests/qemu-iotests/058 | 11 +--
> tests/qemu-iotests/071 | 10 +--
> tests/qemu-iotests/071.out | 6 +-
> tests/qemu-iotests/077 | 2 +-
> tests/qemu-iotests/081 | 8 ++-
> tests/qemu-iotests/081.out | 2 +-
> tests/qemu-iotests/089 | 6 +-
> tests/qemu-iotests/109 | 100 +++++++++++++++++++++++++++
> tests/qemu-iotests/109.out | 149 +++++++++++++++++++++++++++++++++++++++++
> tests/qemu-iotests/common | 2 +-
> tests/qemu-iotests/common.qemu | 3 +-
> tests/qemu-iotests/group | 1 +
> tests/usb-hcd-uhci-test.c | 2 +-
> tests/usb-hcd-xhci-test.c | 2 +-
> tests/virtio-blk-test.c | 4 +-
> tests/virtio-scsi-test.c | 4 +-
> 33 files changed, 460 insertions(+), 102 deletions(-)
> create mode 100755 tests/qemu-iotests/109
> create mode 100644 tests/qemu-iotests/109.out
QEMU 2.3 material but looks pretty close. Eric and Max had comments, I
also posted a question about the nb_sectors == 0 edge-case.
Stefan
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 37+ messages in thread
end of thread, other threads:[~2014-11-13 10:49 UTC | newest]
Thread overview: 37+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-07 19:39 [Qemu-devel] [PATCH v2 0/9] raw: Prohibit dangerous writes for probed images Kevin Wolf
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 1/9] qemu-io: Allow explicitly specifying format Kevin Wolf
2014-11-10 14:07 ` Max Reitz
2014-11-10 14:18 ` Max Reitz
2014-11-10 19:24 ` Eric Blake
2014-11-13 10:47 ` Stefan Hajnoczi
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 2/9] qemu-iotests: Use qemu-io -f $IMGFMT Kevin Wolf
2014-11-10 14:21 ` Max Reitz
2014-11-13 10:47 ` Stefan Hajnoczi
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 3/9] qemu-iotests: Add qemu-io format option in Python tests Kevin Wolf
2014-11-10 14:29 ` Max Reitz
2014-11-10 14:33 ` Kevin Wolf
2014-11-13 10:47 ` Stefan Hajnoczi
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 4/9] qtests: Specify image format explicitly Kevin Wolf
2014-11-10 14:39 ` Max Reitz
2014-11-13 10:47 ` Stefan Hajnoczi
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 5/9] block: Factor bdrv_probe_all() out of find_image_format() Kevin Wolf
2014-11-10 14:47 ` Max Reitz
2014-11-13 10:47 ` Stefan Hajnoczi
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 6/9] block: Read only one sector for format probing Kevin Wolf
2014-11-10 14:48 ` Max Reitz
2014-11-13 10:36 ` Stefan Hajnoczi
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 7/9] raw: Prohibit dangerous writes for probed images Kevin Wolf
2014-11-10 15:03 ` Max Reitz
2014-11-10 19:51 ` Eric Blake
2014-11-13 10:46 ` Stefan Hajnoczi
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 8/9] qemu-iotests: Fix stderr handling in common.qemu Kevin Wolf
2014-11-10 15:04 ` Max Reitz
2014-11-10 19:55 ` Eric Blake
2014-11-13 10:48 ` Stefan Hajnoczi
2014-11-07 19:39 ` [Qemu-devel] [PATCH v2 9/9] qemu-iotests: Test writing non-raw image headers to raw image Kevin Wolf
2014-11-10 15:53 ` Max Reitz
2014-11-10 20:00 ` Eric Blake
2014-11-13 10:48 ` Stefan Hajnoczi
2014-11-10 20:02 ` [Qemu-devel] [PATCH v2 0/9] raw: Prohibit dangerous writes for probed images Eric Blake
2014-11-11 10:03 ` Markus Armbruster
2014-11-13 10:49 ` Stefan Hajnoczi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).