Fam Zheng <f...@redhat.com> writes: > On Thu, 03/13 14:25, Markus Armbruster wrote: >> Fam Zheng <f...@redhat.com> writes: >> >> > On Wed, 03/12 18:00, Markus Armbruster wrote: >> >> Opening an encrypted image takes an additional step: setting the key. >> >> Between open and the key set, the image must not be used. >> >> >> >> We have some protection against accidental use in place: you can't >> >> unpause a guest while we're missing keys. You can, however, hot-plug >> >> block devices lacking keys into a running guest just fine, or insert >> >> media lacking keys. In the latter case, notifying the guest of the >> >> insert is delayed until the key is set, which may suffice to protect >> >> at least some guests in common usage. >> >> >> >> This patch makes the protection apply in more cases, in a rather >> >> heavy-handed way: it doesn't let you open encrypted images unless >> >> we're in a paused state. >> >> >> >> It doesn't extend the protection to users other than the guest (block >> >> jobs?). Use of runstate_check() from block.c is disgusting. Best I >> >> can do right now. >> >> >> >> Signed-off-by: Markus Armbruster <arm...@redhat.com> >> >> --- >> >> block.c | 8 +++++++- >> >> stubs/Makefile.objs | 1 + >> >> stubs/runstate-check.c | 6 ++++++ >> >> 3 files changed, 14 insertions(+), 1 deletion(-) >> >> create mode 100644 stubs/runstate-check.c >> >> >> >> diff --git a/block.c b/block.c >> >> index f1ef4b0..7604881 100644 >> >> --- a/block.c >> >> +++ b/block.c >> >> @@ -1388,12 +1388,18 @@ done: >> >> ret = -EINVAL; >> >> goto close_and_fail; >> >> } >> >> - QDECREF(options); >> >> >> >> if (!bdrv_key_required(bs)) { >> >> bdrv_dev_change_media_cb(bs, true); >> >> + } else if (!runstate_check(RUN_STATE_PRELAUNCH) >> >> + && !runstate_check(RUN_STATE_PAUSED)) { /* HACK */ >> >> + error_setg(errp, >> >> + "Guest must be stopped for opening of encrypted >> >> image"); >> > >> > Changing error message here breaks qemu-iotests 087. >> >> Crap. I'm on vacation until Monday, just checking in to shepherd this >> patch... >> >> On *master*, "make check-block" reports >> >> Not run: 016 052 059 064 070 077 >> Failures: 085 087 >> Failed 2 of 34 tests >> > > I didn't run it from "make check-block" (Stefan sent a patch to remove 085 and > 087 from check-block. > > Running command: TEST_DIR=/tmp/qemu-iotests > QEMU_PROG=../../x86_64-softmmu/qemu-system-x86_64 > QEMU_IMG_PROG=../../qemu-img QEMU_IO_PROG=../../qemu-io > QEMU_NBD_PROG=../../qemu-nbd ./check -qcow2 -o "" 087 > QEMU -- ../../x86_64-softmmu/qemu-system-x86_64 > QEMU_IMG -- ../../qemu-img > QEMU_IO -- ../../qemu-io > QEMU_NBD -- ../../qemu-nbd > IMGFMT -- qcow2 (compat=1.1) > IMGPROTO -- file > PLATFORM -- Linux/x86_64 T430 3.13.6-1-ARCH > SOCKET_SCM_HELPER -- > > 087 0s ... - output mismatch (see 087.out.bad) > --- 087.out 2014-03-12 09:28:04.167060760 +0800 > +++ 087.out.bad 2014-03-14 09:11:59.770326144 +0800 > @@ -31,7 +31,7 @@ > Testing: > QMP_VERSION > {"return": {}} > -{"error": {"class": "GenericError", "desc": "blockdev-add doesn't > support encrypted devices"}} > +{"error": {"class": "GenericError", "desc": "could not open disk > image disk: Guest must be stopped for opening of encrypted image"}} > {"return": {}} > {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, > "event": "SHUTDOWN"} > {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, > "event": "DEVICE_TRAY_MOVED", "data": {"device": "ide1-cd0", > "tray-open": true}} > Failures: 087 > Failed 1 of 1 tests > > Thanks, > Fam
I'll update this test to use -S, and add another one for the new failure mode. Thanks!