From: Alberto Garcia <berto@igalia.com>

At the moment I don't see how to make this function fail after the

dirty bitmap has been created, but if that was possible then we would

hit the assert(QLIST_EMPTY(&bs->dirty_bitmaps)) in bdrv_close().

Signed-off-by: Alberto Garcia <berto@igalia.com>

Signed-off-by: Kevin Wolf <kwolf@redhat.com>

---

block/mirror.c | 3 +++

1 file changed, 3 insertions(+)

diff --git a/block/mirror.c b/block/mirror.c

index XXXXXXX..XXXXXXX 100644

--- a/block/mirror.c

+++ b/block/mirror.c

@@ -XXX,XX +XXX,XX @@ fail:

g_free(s->replaces);

blk_unref(s->target);

bs_opaque->job = NULL;

+ if (s->dirty_bitmap) {

+ bdrv_release_dirty_bitmap(bs, s->dirty_bitmap);

+ }

job_early_fail(&s->common.job);

}

--

2.20.1

From: Alberto Garcia <berto@igalia.com>

The mirror_start_job() function used for the commit-active job blocks

the source, target and all intermediate nodes for the duration of the

job.

target <- intermediate <- source

Since 4ef85a9c2339 this function creates a dummy mirror_top_bs that

goes on top of the source node, and it is this dummy node that gets

blocked instead. The source node is never blocked or added to the

job's list of nodes.

target <- intermediate <- source <- mirror_top

At the moment I don't think it is possible to exploit this problem

because any additional job on 'source' would either be forbidden for

other reasons or it would need to involve an additional node that is

blocked, causing an error.

This can be seen in the error messages, however, because they never

refer to the source node being blocked:

$ qemu-img create -f qcow2 hd0.qcow2 1M

$ qemu-img create -f qcow2 -b hd0.qcow2 hd1.qcow2

$ qemu-io -c 'write 0 1M' hd0.qcow2

$ $QEMU -drive if=none,file=hd1.qcow2,node-name=hd1

{ "execute": "qmp_capabilities" }

{ "execute": "block-commit", "arguments": {"device": "hd1", "speed": 256}}

{ "execute": "block-stream", "arguments": {"device": "hd1"}}

{ "error": {"class": "GenericError",

"desc": "Node 'hd0' is busy: block device is in use by block job: commit"}}

After this patch the error message refers to 'hd1', as it should.

The expected output of iotest 141 also needs to be updated for the

same reason.

Signed-off-by: Alberto Garcia <berto@igalia.com>

Signed-off-by: Kevin Wolf <kwolf@redhat.com>

---

block/mirror.c | 8 ++++++++

tests/qemu-iotests/141.out | 4 ++--

2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c

index XXXXXXX..XXXXXXX 100644

--- a/block/mirror.c

+++ b/block/mirror.c

@@ -XXX,XX +XXX,XX @@ static void mirror_start_job(const char *job_id, BlockDriverState *bs,

goto fail;

}

+ ret = block_job_add_bdrv(&s->common, "source", bs, 0,

+ BLK_PERM_WRITE_UNCHANGED | BLK_PERM_WRITE |

+ BLK_PERM_CONSISTENT_READ,

+ errp);

+ if (ret < 0) {

+ goto fail;

+ }

+

/* Required permissions are already taken with blk_new() */

block_job_add_bdrv(&s->common, "target", target, 0, BLK_PERM_ALL,

&error_abort);

diff --git a/tests/qemu-iotests/141.out b/tests/qemu-iotests/141.out

index XXXXXXX..XXXXXXX 100644

--- a/tests/qemu-iotests/141.out

+++ b/tests/qemu-iotests/141.out

@@ -XXX,XX +XXX,XX @@ Formatting 'TEST_DIR/o.IMGFMT', fmt=IMGFMT size=1048576 backing_file=TEST_DIR/t.

{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "ready", "id": "job0"}}

{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "job0", "len": 0, "offset": 0, "speed": 0, "type": "mirror"}}

{"return": {}}

-{"error": {"class": "GenericError", "desc": "Node 'drv0' is busy: node is used as backing hd of 'NODE_NAME'"}}

+{"error": {"class": "GenericError", "desc": "Node 'drv0' is busy: block device is in use by block job: mirror"}}

{"return": {}}

{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "waiting", "id": "job0"}}

{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "pending", "id": "job0"}}

@@ -XXX,XX +XXX,XX @@ Formatting 'TEST_DIR/o.IMGFMT', fmt=IMGFMT size=1048576 backing_file=TEST_DIR/t.

{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "ready", "id": "job0"}}

{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "job0", "len": 0, "offset": 0, "speed": 0, "type": "commit"}}

{"return": {}}

-{"error": {"class": "GenericError", "desc": "Node 'drv0' is busy: node is used as backing hd of 'NODE_NAME'"}}

+{"error": {"class": "GenericError", "desc": "Node 'drv0' is busy: block device is in use by block job: commit"}}

{"return": {}}

{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "waiting", "id": "job0"}}

{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "pending", "id": "job0"}}

--

2.20.1

From: Alberto Garcia <berto@igalia.com>

Refcount table entries have a field to store the offset of the

refcount block. The rest of the bits of the entry are currently

reserved.

The offset is always taken from the entry using REFT_OFFSET_MASK to

ensure that we only use the bits that belong to that field.

While that mask is used every time we read from the refcount table, it

is never used when we write to it. Due to the other constraints of the

qcow2 format QEMU can never produce refcount block offsets that don't

fit in that field so any such offset when allocating a refcount block

would indicate a bug in QEMU.

Signed-off-by: Alberto Garcia <berto@igalia.com>

Reviewed-by: Eric Blake <eblake@redhat.com>

Signed-off-by: Kevin Wolf <kwolf@redhat.com>

---

block/qcow2-refcount.c | 3 +++

1 file changed, 3 insertions(+)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c

index XXXXXXX..XXXXXXX 100644

--- a/block/qcow2-refcount.c

+++ b/block/qcow2-refcount.c

@@ -XXX,XX +XXX,XX @@ static int alloc_refcount_block(BlockDriverState *bs,

return new_block;

}

+ /* The offset must fit in the offset field of the refcount table entry */

+ assert((new_block & REFT_OFFSET_MASK) == new_block);

+

/* If we're allocating the block at offset 0 then something is wrong */

if (new_block == 0) {

qcow2_signal_corruption(bs, true, -1, -1, "Preventing invalid "

--

2.20.1

From: yuchenlin <npes87184@gmail.com>

Recently, some bugs in dmg file have been fixed. To prevent reading dmg

is broken someday in the future, add a simple test which ensures the

conversion from dmg to raw should not hang or face any I/O error.

Signed-off-by: yuchenlin <npes87184@gmail.com>

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Signed-off-by: Kevin Wolf <kwolf@redhat.com>

---

tests/qemu-iotests/239 | 53 ++++++++++++++++++

tests/qemu-iotests/239.out | 4 ++

tests/qemu-iotests/check | 7 +++

tests/qemu-iotests/group | 1 +

.../sample_images/simple-dmg.dmg.bz2 | Bin 0 -> 3479 bytes

5 files changed, 65 insertions(+)

create mode 100755 tests/qemu-iotests/239

create mode 100644 tests/qemu-iotests/239.out

create mode 100644 tests/qemu-iotests/sample_images/simple-dmg.dmg.bz2

diff --git a/tests/qemu-iotests/239 b/tests/qemu-iotests/239

new file mode 100755

index XXXXXXX..XXXXXXX

--- /dev/null

+++ b/tests/qemu-iotests/239

@@ -XXX,XX +XXX,XX @@

+#!/bin/bash

+#

+# Test case for dmg

+#

+# Copyright (C) 2019 yuchenlin <npes87184@gmail.com>

+#

+# This program is free software; you can redistribute it and/or modify

+# it under the terms of the GNU General Public License as published by

+# the Free Software Foundation; either version 2 of the License, or

+# (at your option) any later version.

+#

+# This program is distributed in the hope that it will be useful,

+# but WITHOUT ANY WARRANTY; without even the implied warranty of

+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the

+# GNU General Public License for more details.

+#

+# You should have received a copy of the GNU General Public License

+# along with this program. If not, see <http://www.gnu.org/licenses/>.

+#

+

+# creator

+owner=npes87184@gmail.com

+

+seq=`basename $0`

+echo "QA output created by $seq"

+

+status=1    # failure is the default!

+

+_cleanup()

+{

+ rm -f "$TEST_IMG.raw"

+ _cleanup_test_img

+}

+trap "_cleanup; exit \$status" 0 1 2 3 15

+

+# get standard environment, filters and checks

+. ./common.rc

+

+_supported_fmt dmg

+_supported_proto file

+_supported_os Linux

+

+echo

+echo "== Testing conversion to raw should success =="

+_use_sample_img simple-dmg.dmg.bz2

+if ! $QEMU_IMG convert -f $IMGFMT -O raw "$TEST_IMG" "$TEST_IMG.raw" ; then

+ exit 1

+fi

+

+# success, all done

+echo "*** done"

+rm -f $seq.full

+status=0

diff --git a/tests/qemu-iotests/239.out b/tests/qemu-iotests/239.out

new file mode 100644

index XXXXXXX..XXXXXXX

--- /dev/null

+++ b/tests/qemu-iotests/239.out

@@ -XXX,XX +XXX,XX @@

+QA output created by 239

+

+== Testing conversion to raw should success ==

+*** done

diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check

index XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/check

+++ b/tests/qemu-iotests/check

@@ -XXX,XX +XXX,XX @@ image format options

-vhdx test vhdx

-vmdk test vmdk

-luks test luks

+ -dmg test dmg

image protocol options

-file test file (default)

@@ -XXX,XX +XXX,XX @@ testlist options

xpand=false

;;

+ -dmg)

+ IMGFMT=dmg

+ IMGFMT_GENERIC=false

+ xpand=false

+ ;;

+

-qed)

IMGFMT=qed

xpand=false

diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group

index XXXXXXX..XXXXXXX 100644

--- a/tests/qemu-iotests/group

+++ b/tests/qemu-iotests/group

@@ -XXX,XX +XXX,XX @@

235 auto quick

236 auto quick

238 auto quick

+239 rw auto quick

diff --git a/tests/qemu-iotests/sample_images/simple-dmg.dmg.bz2 b/tests/qemu-iotests/sample_images/simple-dmg.dmg.bz2

new file mode 100644

index XXXXXXX..XXXXXXX

GIT binary patch

literal 3479

zcmV;I4QTR0T4*^jL0KkKS(36)s{k7pfB*mg|NsC0|NsC0|NsC0|NsC0|Ns5}|NsC0

z|NsC0|Nr0#eji@i`+e|w_jjtUioUNn9=bhi?FYz+8lea^Pt`n6Q`GfM=uI^|rccqR

z#)^4QQ(&j0^vx-<6U8+BMo&{rPgMO9KU6<bdYee{L&A*HX-%dfJg1{mX^f|mdZ(%S

zYHDfZWgn$BPgL<R4Jqj~+M0TvOq0bPDe8Glf++<WdYU|=X+1Rn(<i9%Lp2zh4IZY3

zjXgCzA?gg9Q$~!`&}h@t^qy1H^iL_WjXgCzK-!HSQ%^=sO&L8COn}g7>K>tx+G-x7

zKxhH7gFqfr)EbhWqfmN7Xqi0_(|U$T%^~Te(<9O!s(P57)gFnX(HMqCnW^apCLkWB

zfB?c`7=Y8$P|yP*p{9YL8X6e}jSLZ{Kmat*Y9I=F1s<p9DB4Cq^#`bE4Ff^74GlCj

z05tUmngG$D8fXFP0MPXVMoj}i&;S4$8UdgN)B`|zgFpae5-JoYqMnhSqz_5zX!RaX

zN@?n8p`%8PJdIB%dW@MKrbdHIo}+1?O{io&O{DTNL5K|i44$J2gHSf0dW_JG15ZRS

zO#>jv(V%IjnFNxA1T<(7k~Y;aPez3F!fB&IWZEW%)byHqn^PtYN0c*2G<pJPF%2|&

zjXg~oX{V%Vqb8bZplqPgw1%2w9-*VtQR*}r00xGQ4G24kA+jG=o`xW*WTQp80#*@D

zGI^@e0Fq218g7gWKQ0-!!d?<+0Zg$VQi&#k#L$@->*ZllN2a;wYb5Z@K!FpiLKS04

zAXF_RNbYzNTvjprs6=NSa4=(xVBCm>1#>twBnc>5NRuJv+UXVcw)$K?45Ra5gMtys

z5#=tTi{6o_Dolm>AW0*9z$7i33Gt*7^pYSUXUfIy=|9=(Y5F2D)W#);#I!)dBq)W~

zrFlJ*pf<%<p!?|?K4A)WKmn>vEyY-2j;_z6FzSi8mQn!dfdGLU#z+u}O%{xk@FlI`

z^B@Tz!-52v0~!WxwY>yM5j2L!wDW07lb>SjKuGF1Bv7*3#(>;&j=WH?%<2(%C(%mi

z3SK&{tsN6|F+h=$fRF|SAT&YV;=$CM8UPyf&KPGXD@;hw?Ie+ur$Ms`<R|DnF>?n=

z=TVuCRg<`%CTS#rC3Rqi{8+-#G7{}$If&w>{u-}tqnXQGZ1vrDn(W+-ydtwD_5(m9

zUlJm4YF$|a547Ok)VNY_1&|O<rL!z~wcz}tQxE~Dj4cXB5;0mM5(lt%*IA;POn}RT

z;6QRRmFhId9Am|k#g{Q@hX6cXxi&WKM_|eXL97PF^DAB*Vbz~zQKTvZ??`1m>?|T)

z42v8J{j@}JBoR7501!b0TQLa}5=%fGiZ01vc2PsfW3IL_R4y29^38$G<bvvm5nFd0

zCwO?LU{=bViG0S0Zh^;{@n#!Kdu|V}*6A@n-bN2myQ=@_`vaVL_~0ObLdRRH{)&n2

zWjQqZq-v&|Sm)GU$5#3E-aM@MWEDTSJ=WXyvX!Ka^MIpH)wR_6>N@}eFEu8;#~W3O

zhYo*p2)1qo0FWCdNC$P6JWDI)lRL3RsCq~;CddNVfDeUAK|}zQbl`V}JlvEcUSn8M

zpVIf%kFZ>u>OO0J!nRehCj34b&G%X*%F`iNsN=6_pV&CdsRu0CM-x>qqFshZ5Cm<o

zjS+4gAk-oc*IeI*Cm4uELVya|XwQV9;aJZ7Mzx<=On?m(0@tY~UQVHbY{wZn;D)7@

z(k#N&X0YCEm2rAN!X56ASS+#z7ao9>#cE(tAoIY<O8RkL;gL@lE`rM`9JnB2CEx^z

z6c8i@0n%FhZje3Zxg#iF@R@T1Isa_wb*LjCgn+!DWDcYQh#WxZLepa#PQjgE{rMz?

zBq1<j2VVpX$Xd;K6V+q_hhHJtdWTbjuPi#R=(f&lykmQl$pDZgP%wm~nI*Ni#z^FO

zE-eT!OaQHD1y1DHB8`9}p<+T&reFk;4s}MH%)0IXH1auiL(e%0z#SY_Q4bA>;2;n;

z>6lWK8VC|iQ`TBT@eh_haUvW?s0F~|jWOEDBrNGaBoJh|LX9lsM4vqVlOYszX7pdA

z{D&2t%ysWP&xa%2Q6LEk#Gf-=1UdvZ5a1%=kY@b_PJ?B`u>CaLI&B7em_7b|;n=Za

zp~fR)pUI5+fvC#Km(lVCNpb=XW#mzCYrKf$7gM(TqYzYBKqz54qqz-xcv$fGQrdYy

zRD%pqsY(=-5JXlPYst2Xv=#sccTUb6A-KFq8XNZ`G)4wExi9zHxBO=zO;AbFM}`M!

zOk?+aXaO;H02oI0Bn>Mv09F(z2D>5BS#O#$P6Hjd6boVGU|fj(_>6Wj@RrKIn;8EB

zble1^sy2Z1#DvynHzELF0f+`WC;nB6;l*sYRsG&{OT)PkEN{C-fC@>voA`)pYKBGR

zy0qd5hS(-IOAKl(iXY}a|K}p1?TmbyuKT-Qeer)gFKD88^Tpxr*kmLy;0FL}C&d#`

zQ|v00fv3#W*mSyZs|-K<h+d6pNO}MT^KN@JOgvk`m2@p1X~U$!#uswfANXB%a*2}d

zwEVT6lZMv@K`D!LB_I^fo)latwZp8vy|3yBt&R41ACiQf!d~t-ftA7uBb30|`l#aR

zSAZd~7y#5rmLOyX5*DtKB=1^~pw$7YjAzU-OkBe`h(K#GCV;6Q6}-9*#6ZQEesARr

zwQh_HsY@9IS)I<~d1n7@4@41T%tXY@5h1NOcGB0n$$P2^q!Iwg96-Ypa~Me&Mkg03

zx4>{zsMn+fmSxpq9<;PGzUke*7{Vc;ba?~~Fp@>uleN{!WoS8VmG%z%nuX9{w6QC^

z>cY}_BnH5wp)m6ur<d4axiRctR|XVPR-dd0=6w1P3>Z5+Eu`$-#pj&flXy9V9wq!p

zo^)}9fQDYjW6Ab;cYA9~3Cm@szFUBpm=2bWB#w0?id*4t_2g7y|0*g1ylSPW9s834

ztjELX^xWJGpL*3Qc>%uRKQna>8~ayaq=0g~k?u90E|+R&pb}xi6sAxgk{0I9Tdk7g

zC&mCT7%%O`$bs$IGS?S?7$DVck$e==@QS_=nF(o3(InF7Dn=3k=Ls-^Fw9D7{(U3k

zK*%{w#OE;-n}5J@QnW~k0W^>q6Frr!p|88#+Wy;ioe~<*h=i%&J;)%alO@Pegg+7j

zs``QTeEyD)x7p(92)N`E@8T8H#s^c%*24OW;aGvPuVQef*wBsRpU@6A_g9zdDt9$k

z2zRm#rdLi0A^}Fb?sXZ{X@%WaZzShTNF-5=Fu)3i@t^qa`TwT<VRADXi9j*Cww!Bk

z$ofZn`qc*}gV@O>ucE88qh>EN`Jdw+s*<*u-@$9+o1#_6XJ989lqI=az!E}P5riY*

zHPwYH$BT`e?rVGc3nGpD(|bD_E!3{ti_*;NZZ0isXL2=SVRX$Ti~X!dedY#W1K<$#

zh_IvD035p;=Ek~DC$}ZJd(G3-dd#k6CG#xgT;mmS6+SLid1tQO-9<Sl@O^GuLUz-u

z417&euHAlM4ypAfeUp|&``-9llf#`5`nX06abKLfN@~l%nAq{U{u*UfSI+fFc$+Nq

zjSkQ*mer2Rs5nuStK%0w;knBzMKh|{QO!^DPwP)t^4~w3qnO*hZDo{4^>~P#9iv%f

zd!`qvbczBF)AAe0yE+WI*}DYOL2J~*ZCc}JWiu0LShw&6`-5`^SkNJY!gsi|e-MHy

zZlN9Lb5NbuT7S&4oY6v}13BpeQ|?u0c*7n)Ku*p%ZDF?$7zjKXj(9#K?tO1dMW=8&

z;MDsGo0=CLWp{b5$Kk0kxAl*^l^ziX<28c4->Ed%#N16IrySH$q`4%gn}5ksr>_0H

zs-;Y4Bb7U_pNGMK075wJ%DahB2ArR=jC`2UWMe`ID;mPgA25l6GVc9>Rqybi7N6(i

z3BcjbfprP7Ivcy%+Vs~Z6Hw>JB!kJcD>mdTThTpO$(fg`Ds7pY<YTYWgmqU8=Xlve

zCriGy+iG;z+{{27v*;>N4BN<|%(+#T+h1sdgPjUAWN73ki+$-%j_&6PZ$I2$HnSEa

zQ?n{BB*A3zmlLHvH@o+ZJm1NSC9(?ka<7OppIcAi{V`Nj+^8SL!ywYLI$v(ivC_){

z5anb)=MQS%<PN>hUHR@_HKDhdDPf*Hm`w^E{GT!Yq<}Hg8Ia{?PyA70;yx8nD^~B|

zjuGESS<A^!S2=(y_Dz2Z3)U+>*z1(bcT1?|;LuVOg&xLebEk(1uB(fZJN&W3!xADj

zQD;ugVYYpOt=NBu$Bi07WcepgHm6q}1y>vxs^oJetK;=ix$9i9M#>O`?6X2pJoXi;

z#0mlFG2eK~z-yqErVC2`l5B&)s}jKZMRZxCyQ0^?cp$L5J>4{eoCysx+#X0J?r^+C

z#R8d5A5=5V#(QMr1?kT}qh|eQ8{a94K6_Sq=vcWz&WH@!TREj$F3%=NwMQ3YR-$or

zlXmr#8-D_(N;#e;g#CE1WYh8byF-ojSdtGh>P8pbz39Ew5qWHa^1-M77ji{7P>_<c

FP^-7hd6WPE

literal 0

HcmV?d00001

--

2.20.1

From: Markus Armbruster <armbru@redhat.com>

Patch created mechanically by rerunning:

$ spatch --sp-file scripts/coccinelle/qobject.cocci \

     --macro-file scripts/cocci-macro-file.h \

     --dir block --in-place

Signed-off-by: Markus Armbruster <armbru@redhat.com>

Reviewed-by: Alberto Garcia <berto@igalia.com>

Reviewed-by: Eric Blake <eblake@redhat.com>

Signed-off-by: Kevin Wolf <kwolf@redhat.com>

---

block/blklogwrites.c | 5 ++---

1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/block/blklogwrites.c b/block/blklogwrites.c

index XXXXXXX..XXXXXXX 100644

--- a/block/blklogwrites.c

+++ b/block/blklogwrites.c

@@ -XXX,XX +XXX,XX @@ static void blk_log_writes_refresh_filename(BlockDriverState *bs,

qdict_put_str(opts, "driver", "blklogwrites");

qobject_ref(bs->file->bs->full_open_options);

- qdict_put_obj(opts, "file", QOBJECT(bs->file->bs->full_open_options));

+ qdict_put(opts, "file", bs->file->bs->full_open_options);

qobject_ref(s->log_file->bs->full_open_options);

- qdict_put_obj(opts, "log",

- QOBJECT(s->log_file->bs->full_open_options));

+ qdict_put(opts, "log", s->log_file->bs->full_open_options);

qdict_put_int(opts, "log-sector-size", s->sectorsize);

bs->full_open_options = opts;

--

2.20.1

In the block layer, synchronous APIs are often implemented by creating a

coroutine that calls the asynchronous coroutine-based implementation and

then waiting for completion with BDRV_POLL_WHILE().

For this to work with iothreads (more specifically, when the synchronous

API is called in a thread that is not the home thread of the block

device, so that the coroutine will run in a different thread), we must

make sure to call aio_wait_kick() at the end of the operation. Many

places are missing this, so that BDRV_POLL_WHILE() keeps hanging even if

the condition has long become false.

Note that bdrv_dec_in_flight() involves an aio_wait_kick() call. This

corresponds to the BDRV_POLL_WHILE() in the drain functions, but it is

generally not enough for most other operations because they haven't set

the return value in the coroutine entry stub yet. To avoid race

conditions there, we need to kick after setting the return value.

The race window is small enough that the problem doesn't usually surface

in the common path. However, it does surface and causes easily

reproducible hangs if the operation can return early before even calling

bdrv_inc/dec_in_flight, which many of them do (trivial error or no-op

success paths).

The bug in bdrv_truncate(), bdrv_check() and bdrv_invalidate_cache() is

slightly different: These functions even neglected to schedule the

coroutine in the home thread of the node. This avoids the hang, but is

obviously wrong, too. Fix those to schedule the coroutine in the right

AioContext in addition to adding aio_wait_kick() calls.

Cc: qemu-stable@nongnu.org

Signed-off-by: Kevin Wolf <kwolf@redhat.com>

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

---

block.c | 6 +-

block/block-backend.c | 5 +

block/io.c | 8 +-

block/nbd-client.c | 1 +

block/nvme.c | 1 +

block/qcow2.c | 1 +

block/qed.c | 1 +

tests/test-block-iothread.c | 372 ++++++++++++++++++++++++++++++++++++

tests/Makefile.include | 2 +

9 files changed, 394 insertions(+), 3 deletions(-)

create mode 100644 tests/test-block-iothread.c

diff --git a/block.c b/block.c

index XXXXXXX..XXXXXXX 100644

--- a/block.c

+++ b/block.c

@@ -XXX,XX +XXX,XX @@ static void bdrv_check_co_entry(void *opaque)

{

CheckCo *cco = opaque;

cco->ret = bdrv_co_check(cco->bs, cco->res, cco->fix);

+ aio_wait_kick();

}

int bdrv_check(BlockDriverState *bs,

@@ -XXX,XX +XXX,XX @@ int bdrv_check(BlockDriverState *bs,

bdrv_check_co_entry(&cco);

} else {

co = qemu_coroutine_create(bdrv_check_co_entry, &cco);

- qemu_coroutine_enter(co);

+ bdrv_coroutine_enter(bs, co);

BDRV_POLL_WHILE(bs, cco.ret == -EINPROGRESS);

}

@@ -XXX,XX +XXX,XX @@ static void coroutine_fn bdrv_invalidate_cache_co_entry(void *opaque)

InvalidateCacheCo *ico = opaque;

bdrv_co_invalidate_cache(ico->bs, ico->errp);

ico->done = true;

+ aio_wait_kick();

}

void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp)

@@ -XXX,XX +XXX,XX @@ void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp)

bdrv_invalidate_cache_co_entry(&ico);

} else {

co = qemu_coroutine_create(bdrv_invalidate_cache_co_entry, &ico);

- qemu_coroutine_enter(co);

+ bdrv_coroutine_enter(bs, co);

BDRV_POLL_WHILE(bs, !ico.done);

}

}

diff --git a/block/block-backend.c b/block/block-backend.c

index XXXXXXX..XXXXXXX 100644

--- a/block/block-backend.c

+++ b/block/block-backend.c

@@ -XXX,XX +XXX,XX @@ static void blk_read_entry(void *opaque)

rwco->ret = blk_co_preadv(rwco->blk, rwco->offset, qiov->size,

qiov, rwco->flags);

+ aio_wait_kick();

}

static void blk_write_entry(void *opaque)

@@ -XXX,XX +XXX,XX @@ static void blk_write_entry(void *opaque)

rwco->ret = blk_co_pwritev(rwco->blk, rwco->offset, qiov->size,

qiov, rwco->flags);

+ aio_wait_kick();

}

static int blk_prw(BlockBackend *blk, int64_t offset, uint8_t *buf,

@@ -XXX,XX +XXX,XX @@ static void blk_ioctl_entry(void *opaque)

rwco->ret = blk_co_ioctl(rwco->blk, rwco->offset,

qiov->iov[0].iov_base);

+ aio_wait_kick();

}

int blk_ioctl(BlockBackend *blk, unsigned long int req, void *buf)

@@ -XXX,XX +XXX,XX @@ static void blk_flush_entry(void *opaque)

{

BlkRwCo *rwco = opaque;

rwco->ret = blk_co_flush(rwco->blk);

+ aio_wait_kick();

}

int blk_flush(BlockBackend *blk)

@@ -XXX,XX +XXX,XX @@ static void blk_pdiscard_entry(void *opaque)

QEMUIOVector *qiov = rwco->iobuf;

rwco->ret = blk_co_pdiscard(rwco->blk, rwco->offset, qiov->size);

+ aio_wait_kick();

}

int blk_pdiscard(BlockBackend *blk, int64_t offset, int bytes)

diff --git a/block/io.c b/block/io.c

index XXXXXXX..XXXXXXX 100644

--- a/block/io.c

+++ b/block/io.c

@@ -XXX,XX +XXX,XX @@ static void coroutine_fn bdrv_rw_co_entry(void *opaque)

rwco->qiov->size, rwco->qiov,

rwco->flags);

}

+ aio_wait_kick();

}

/*

@@ -XXX,XX +XXX,XX @@ static void coroutine_fn bdrv_block_status_above_co_entry(void *opaque)

data->offset, data->bytes,

data->pnum, data->map, data->file);

data->done = true;

+ aio_wait_kick();

}

/*

@@ -XXX,XX +XXX,XX @@ static void coroutine_fn bdrv_co_rw_vmstate_entry(void *opaque)

{

BdrvVmstateCo *co = opaque;

co->ret = bdrv_co_rw_vmstate(co->bs, co->qiov, co->pos, co->is_read);

+ aio_wait_kick();

}

static inline int

@@ -XXX,XX +XXX,XX @@ static void coroutine_fn bdrv_flush_co_entry(void *opaque)

FlushCo *rwco = opaque;

rwco->ret = bdrv_co_flush(rwco->bs);

+ aio_wait_kick();

}

int coroutine_fn bdrv_co_flush(BlockDriverState *bs)

@@ -XXX,XX +XXX,XX @@ static void coroutine_fn bdrv_pdiscard_co_entry(void *opaque)

DiscardCo *rwco = opaque;

rwco->ret = bdrv_co_pdiscard(rwco->child, rwco->offset, rwco->bytes);

+ aio_wait_kick();

}

int coroutine_fn bdrv_co_pdiscard(BdrvChild *child, int64_t offset, int bytes)

@@ -XXX,XX +XXX,XX @@ static void coroutine_fn bdrv_truncate_co_entry(void *opaque)

TruncateCo *tco = opaque;

tco->ret = bdrv_co_truncate(tco->child, tco->offset, tco->prealloc,

tco->errp);

+ aio_wait_kick();

}

int bdrv_truncate(BdrvChild *child, int64_t offset, PreallocMode prealloc,

@@ -XXX,XX +XXX,XX @@ int bdrv_truncate(BdrvChild *child, int64_t offset, PreallocMode prealloc,

bdrv_truncate_co_entry(&tco);

} else {

co = qemu_coroutine_create(bdrv_truncate_co_entry, &tco);

- qemu_coroutine_enter(co);

+ bdrv_coroutine_enter(child->bs, co);

BDRV_POLL_WHILE(child->bs, tco.ret == NOT_DONE);

}

diff --git a/block/nbd-client.c b/block/nbd-client.c

index XXXXXXX..XXXXXXX 100644

--- a/block/nbd-client.c

+++ b/block/nbd-client.c

@@ -XXX,XX +XXX,XX @@ static coroutine_fn void nbd_read_reply_entry(void *opaque)

s->quit = true;

nbd_recv_coroutines_wake_all(s);

s->read_reply_co = NULL;

+ aio_wait_kick();

}

static int nbd_co_send_request(BlockDriverState *bs,

diff --git a/block/nvme.c b/block/nvme.c

index XXXXXXX..XXXXXXX 100644

--- a/block/nvme.c

+++ b/block/nvme.c

@@ -XXX,XX +XXX,XX @@ static void nvme_cmd_sync_cb(void *opaque, int ret)

{

int *pret = opaque;

*pret = ret;

+ aio_wait_kick();

}

static int nvme_cmd_sync(BlockDriverState *bs, NVMeQueuePair *q,

diff --git a/block/qcow2.c b/block/qcow2.c

index XXXXXXX..XXXXXXX 100644

--- a/block/qcow2.c

+++ b/block/qcow2.c

@@ -XXX,XX +XXX,XX @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,

/* From bdrv_co_create. */

qcow2_open_entry(&qoc);

} else {

+ assert(qemu_get_current_aio_context() == qemu_get_aio_context());

qemu_coroutine_enter(qemu_coroutine_create(qcow2_open_entry, &qoc));

BDRV_POLL_WHILE(bs, qoc.ret == -EINPROGRESS);

}

diff --git a/block/qed.c b/block/qed.c

index XXXXXXX..XXXXXXX 100644

--- a/block/qed.c

+++ b/block/qed.c

@@ -XXX,XX +XXX,XX @@ static int bdrv_qed_open(BlockDriverState *bs, QDict *options, int flags,

if (qemu_in_coroutine()) {

bdrv_qed_open_entry(&qoc);

} else {

+ assert(qemu_get_current_aio_context() == qemu_get_aio_context());

qemu_coroutine_enter(qemu_coroutine_create(bdrv_qed_open_entry, &qoc));

BDRV_POLL_WHILE(bs, qoc.ret == -EINPROGRESS);

}

diff --git a/tests/test-block-iothread.c b/tests/test-block-iothread.c

new file mode 100644

index XXXXXXX..XXXXXXX

--- /dev/null

+++ b/tests/test-block-iothread.c

@@ -XXX,XX +XXX,XX @@

+/*

+ * Block tests for iothreads

+ *

+ * Copyright (c) 2018 Kevin Wolf <kwolf@redhat.com>

+ *

+ * Permission is hereby granted, free of charge, to any person obtaining a copy

+ * of this software and associated documentation files (the "Software"), to deal

+ * in the Software without restriction, including without limitation the rights

+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell

+ * copies of the Software, and to permit persons to whom the Software is

+ * furnished to do so, subject to the following conditions:

+ *

+ * The above copyright notice and this permission notice shall be included in

+ * all copies or substantial portions of the Software.

+ *

+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR

+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,

+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL

+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER

+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,

+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN

+ * THE SOFTWARE.

+ */

+

+#include "qemu/osdep.h"

+#include "block/block.h"

+#include "block/blockjob_int.h"

+#include "sysemu/block-backend.h"

+#include "qapi/error.h"

+#include "iothread.h"

+

+static int coroutine_fn bdrv_test_co_prwv(BlockDriverState *bs,

+ uint64_t offset, uint64_t bytes,

+ QEMUIOVector *qiov, int flags)

+{

+ return 0;

+}

+

+static int coroutine_fn bdrv_test_co_pdiscard(BlockDriverState *bs,

+ int64_t offset, int bytes)

+{

+ return 0;

+}

+

+static int coroutine_fn

+bdrv_test_co_truncate(BlockDriverState *bs, int64_t offset,

+ PreallocMode prealloc, Error **errp)

+{

+ return 0;

+}

+

+static int coroutine_fn bdrv_test_co_block_status(BlockDriverState *bs,

+ bool want_zero,

+ int64_t offset, int64_t count,

+ int64_t *pnum, int64_t *map,

+ BlockDriverState **file)

+{

+ *pnum = count;

+ return 0;

+}

+

+static BlockDriver bdrv_test = {

+ .format_name = "test",

+ .instance_size = 1,

+

+ .bdrv_co_preadv = bdrv_test_co_prwv,

+ .bdrv_co_pwritev = bdrv_test_co_prwv,

+ .bdrv_co_pdiscard = bdrv_test_co_pdiscard,

+ .bdrv_co_truncate = bdrv_test_co_truncate,

+ .bdrv_co_block_status = bdrv_test_co_block_status,

+};

+

+static void test_sync_op_pread(BdrvChild *c)

+{

+ uint8_t buf[512];

+ int ret;

+

+ /* Success */

+ ret = bdrv_pread(c, 0, buf, sizeof(buf));

+ g_assert_cmpint(ret, ==, 512);

+

+ /* Early error: Negative offset */

+ ret = bdrv_pread(c, -2, buf, sizeof(buf));

+ g_assert_cmpint(ret, ==, -EIO);

+}

+

+static void test_sync_op_pwrite(BdrvChild *c)

+{

+ uint8_t buf[512];

+ int ret;

+

+ /* Success */

+ ret = bdrv_pwrite(c, 0, buf, sizeof(buf));

+ g_assert_cmpint(ret, ==, 512);

+

+ /* Early error: Negative offset */

+ ret = bdrv_pwrite(c, -2, buf, sizeof(buf));

+ g_assert_cmpint(ret, ==, -EIO);

+}

+

+static void test_sync_op_blk_pread(BlockBackend *blk)

+{

+ uint8_t buf[512];

+ int ret;

+

+ /* Success */

+ ret = blk_pread(blk, 0, buf, sizeof(buf));

+ g_assert_cmpint(ret, ==, 512);

+

+ /* Early error: Negative offset */

+ ret = blk_pread(blk, -2, buf, sizeof(buf));

+ g_assert_cmpint(ret, ==, -EIO);

+}

+

+static void test_sync_op_blk_pwrite(BlockBackend *blk)

+{

+ uint8_t buf[512];

+ int ret;

+

+ /* Success */

+ ret = blk_pwrite(blk, 0, buf, sizeof(buf), 0);

+ g_assert_cmpint(ret, ==, 512);

+

+ /* Early error: Negative offset */

+ ret = blk_pwrite(blk, -2, buf, sizeof(buf), 0);

+ g_assert_cmpint(ret, ==, -EIO);

+}

+

+static void test_sync_op_load_vmstate(BdrvChild *c)

+{

+ uint8_t buf[512];

+ int ret;

+

+ /* Error: Driver does not support snapshots */

+ ret = bdrv_load_vmstate(c->bs, buf, 0, sizeof(buf));

+ g_assert_cmpint(ret, ==, -ENOTSUP);

+}

+

+static void test_sync_op_save_vmstate(BdrvChild *c)

+{

+ uint8_t buf[512];

+ int ret;

+

+ /* Error: Driver does not support snapshots */

+ ret = bdrv_save_vmstate(c->bs, buf, 0, sizeof(buf));

+ g_assert_cmpint(ret, ==, -ENOTSUP);

+}

+

+static void test_sync_op_pdiscard(BdrvChild *c)

+{

+ int ret;

+

+ /* Normal success path */

+ c->bs->open_flags |= BDRV_O_UNMAP;

+ ret = bdrv_pdiscard(c, 0, 512);

+ g_assert_cmpint(ret, ==, 0);

+

+ /* Early success: UNMAP not supported */

+ c->bs->open_flags &= ~BDRV_O_UNMAP;

+ ret = bdrv_pdiscard(c, 0, 512);

+ g_assert_cmpint(ret, ==, 0);

+

+ /* Early error: Negative offset */

+ ret = bdrv_pdiscard(c, -2, 512);

+ g_assert_cmpint(ret, ==, -EIO);

+}

+

+static void test_sync_op_blk_pdiscard(BlockBackend *blk)

+{

+ int ret;

+

+ /* Early success: UNMAP not supported */

+ ret = blk_pdiscard(blk, 0, 512);

+ g_assert_cmpint(ret, ==, 0);

+

+ /* Early error: Negative offset */

+ ret = blk_pdiscard(blk, -2, 512);

+ g_assert_cmpint(ret, ==, -EIO);

+}

+

+static void test_sync_op_truncate(BdrvChild *c)

+{

+ int ret;

+

+ /* Normal success path */

+ ret = bdrv_truncate(c, 65536, PREALLOC_MODE_OFF, NULL);

+ g_assert_cmpint(ret, ==, 0);

+

+ /* Early error: Negative offset */

+ ret = bdrv_truncate(c, -2, PREALLOC_MODE_OFF, NULL);

+ g_assert_cmpint(ret, ==, -EINVAL);

+

+ /* Error: Read-only image */

+ c->bs->read_only = true;

+ c->bs->open_flags &= ~BDRV_O_RDWR;

+

+ ret = bdrv_truncate(c, 65536, PREALLOC_MODE_OFF, NULL);

+ g_assert_cmpint(ret, ==, -EACCES);

+

+ c->bs->read_only = false;

+ c->bs->open_flags |= BDRV_O_RDWR;

+}

+

+static void test_sync_op_block_status(BdrvChild *c)

+{

+ int ret;

+ int64_t n;

+

+ /* Normal success path */

+ ret = bdrv_is_allocated(c->bs, 0, 65536, &n);

+ g_assert_cmpint(ret, ==, 0);

+

+ /* Early success: No driver support */

+ bdrv_test.bdrv_co_block_status = NULL;

+ ret = bdrv_is_allocated(c->bs, 0, 65536, &n);

+ g_assert_cmpint(ret, ==, 1);

+

+ /* Early success: bytes = 0 */

+ ret = bdrv_is_allocated(c->bs, 0, 0, &n);

+ g_assert_cmpint(ret, ==, 0);

+

+ /* Early success: Offset > image size*/

+ ret = bdrv_is_allocated(c->bs, 0x1000000, 0x1000000, &n);

+ g_assert_cmpint(ret, ==, 0);

+}

+

+static void test_sync_op_flush(BdrvChild *c)

+{

+ int ret;

+

+ /* Normal success path */

+ ret = bdrv_flush(c->bs);

+ g_assert_cmpint(ret, ==, 0);

+

+ /* Early success: Read-only image */

+ c->bs->read_only = true;

+ c->bs->open_flags &= ~BDRV_O_RDWR;

+

+ ret = bdrv_flush(c->bs);

+ g_assert_cmpint(ret, ==, 0);

+

+ c->bs->read_only = false;

+ c->bs->open_flags |= BDRV_O_RDWR;

+}

+

+static void test_sync_op_blk_flush(BlockBackend *blk)

+{

+ BlockDriverState *bs = blk_bs(blk);

+ int ret;

+

+ /* Normal success path */

+ ret = blk_flush(blk);

+ g_assert_cmpint(ret, ==, 0);

+

+ /* Early success: Read-only image */

+ bs->read_only = true;

+ bs->open_flags &= ~BDRV_O_RDWR;

+

+ ret = blk_flush(blk);

+ g_assert_cmpint(ret, ==, 0);

+

+ bs->read_only = false;

+ bs->open_flags |= BDRV_O_RDWR;

+}

+

+static void test_sync_op_check(BdrvChild *c)

+{

+ BdrvCheckResult result;

+ int ret;

+

+ /* Error: Driver does not implement check */

+ ret = bdrv_check(c->bs, &result, 0);

+ g_assert_cmpint(ret, ==, -ENOTSUP);

+}

+

+static void test_sync_op_invalidate_cache(BdrvChild *c)

+{

+ /* Early success: Image is not inactive */

+ bdrv_invalidate_cache(c->bs, NULL);

+}

+

+

+typedef struct SyncOpTest {

+ const char *name;

+ void (*fn)(BdrvChild *c);

+ void (*blkfn)(BlockBackend *blk);

+} SyncOpTest;

+

+const SyncOpTest sync_op_tests[] = {

+ {

+ .name = "/sync-op/pread",

+ .fn = test_sync_op_pread,

+ .blkfn = test_sync_op_blk_pread,

+ }, {

+ .name = "/sync-op/pwrite",

+ .fn = test_sync_op_pwrite,

+ .blkfn = test_sync_op_blk_pwrite,

+ }, {

+ .name = "/sync-op/load_vmstate",

+ .fn = test_sync_op_load_vmstate,

+ }, {

+ .name = "/sync-op/save_vmstate",

+ .fn = test_sync_op_save_vmstate,

+ }, {

+ .name = "/sync-op/pdiscard",

+ .fn = test_sync_op_pdiscard,

+ .blkfn = test_sync_op_blk_pdiscard,

+ }, {

+ .name = "/sync-op/truncate",

+ .fn = test_sync_op_truncate,

+ }, {

+ .name = "/sync-op/block_status",

+ .fn = test_sync_op_block_status,

+ }, {

+ .name = "/sync-op/flush",

+ .fn = test_sync_op_flush,

+ .blkfn = test_sync_op_blk_flush,

+ }, {

+ .name = "/sync-op/check",

+ .fn = test_sync_op_check,

+ }, {

+ .name = "/sync-op/invalidate_cache",

+ .fn = test_sync_op_invalidate_cache,

+ },

+};

+

+/* Test synchronous operations that run in a different iothread, so we have to

+ * poll for the coroutine there to return. */

+static void test_sync_op(const void *opaque)

+{

+ const SyncOpTest *t = opaque;

+ IOThread *iothread = iothread_new();

+ AioContext *ctx = iothread_get_aio_context(iothread);

+ BlockBackend *blk;

+ BlockDriverState *bs;

+ BdrvChild *c;

+

+ blk = blk_new(BLK_PERM_ALL, BLK_PERM_ALL);

+ bs = bdrv_new_open_driver(&bdrv_test, "base", BDRV_O_RDWR, &error_abort);

+ bs->total_sectors = 65536 / BDRV_SECTOR_SIZE;

+ blk_insert_bs(blk, bs, &error_abort);

+ c = QLIST_FIRST(&bs->parents);

+

+ blk_set_aio_context(blk, ctx);

+ aio_context_acquire(ctx);

+ t->fn(c);

+ if (t->blkfn) {

+ t->blkfn(blk);

+ }

+ aio_context_release(ctx);

+ blk_set_aio_context(blk, qemu_get_aio_context());

+

+ bdrv_unref(bs);

+ blk_unref(blk);

+}

+

+int main(int argc, char **argv)

+{

+ int i;

+

+ bdrv_init();

+ qemu_init_main_loop(&error_abort);

+

+ g_test_init(&argc, &argv, NULL);

+

+ for (i = 0; i < ARRAY_SIZE(sync_op_tests); i++) {

+ const SyncOpTest *t = &sync_op_tests[i];

+ g_test_add_data_func(t->name, t, test_sync_op);

+ }

+

+ return g_test_run();

+}

diff --git a/tests/Makefile.include b/tests/Makefile.include

index XXXXXXX..XXXXXXX 100644

--- a/tests/Makefile.include

+++ b/tests/Makefile.include

@@ -XXX,XX +XXX,XX @@ check-unit-y += tests/test-bdrv-drain$(EXESUF)

check-unit-y += tests/test-blockjob$(EXESUF)

check-unit-y += tests/test-blockjob-txn$(EXESUF)

check-unit-y += tests/test-block-backend$(EXESUF)

+check-unit-y += tests/test-block-iothread$(EXESUF)

check-unit-y += tests/test-image-locking$(EXESUF)

check-unit-y += tests/test-x86-cpuid$(EXESUF)

# all code tested by test-x86-cpuid is inside topology.h

@@ -XXX,XX +XXX,XX @@ tests/test-bdrv-drain$(EXESUF): tests/test-bdrv-drain.o $(test-block-obj-y) $(te

tests/test-blockjob$(EXESUF): tests/test-blockjob.o $(test-block-obj-y) $(test-util-obj-y)

tests/test-blockjob-txn$(EXESUF): tests/test-blockjob-txn.o $(test-block-obj-y) $(test-util-obj-y)

tests/test-block-backend$(EXESUF): tests/test-block-backend.o $(test-block-obj-y) $(test-util-obj-y)

+tests/test-block-iothread$(EXESUF): tests/test-block-iothread.o $(test-block-obj-y) $(test-util-obj-y)

tests/test-image-locking$(EXESUF): tests/test-image-locking.o $(test-block-obj-y) $(test-util-obj-y)

tests/test-thread-pool$(EXESUF): tests/test-thread-pool.o $(test-block-obj-y)

tests/test-iov$(EXESUF): tests/test-iov.o $(test-util-obj-y)

--

2.20.1

From: Max Reitz <mreitz@redhat.com>

This test waits for a MIGRATION event with status=completed on the

source VM before querying the migration status on both source and

destination. However, just because the source says migration has

completed does not mean the destination thinks the same. Therefore, in

some cases, the destination VM may still report "active" instead of

"completed" when asked for its migration status.

Fix this by enabling migration events on both VMs and waiting until both

source and destination emit a status=completed MIGRATION event.

Signed-off-by: Max Reitz <mreitz@redhat.com>

Signed-off-by: Kevin Wolf <kwolf@redhat.com>

---

tests/qemu-iotests/234 | 56 ++++++++++++++++++++------------------

tests/qemu-iotests/234.out | 10 +++++--

2 files changed, 38 insertions(+), 28 deletions(-)

diff --git a/tests/qemu-iotests/234 b/tests/qemu-iotests/234

index XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/234

+++ b/tests/qemu-iotests/234

@@ -XXX,XX +XXX,XX @@ import os

iotests.verify_image_format(supported_fmts=['qcow2'])

iotests.verify_platform(['linux'])

+def enable_migration_events(vm, name):

+ iotests.log('Enabling migration QMP events on %s...' % name)

+ iotests.log(vm.qmp('migrate-set-capabilities', capabilities=[

+ {

+ 'capability': 'events',

+ 'state': True

+ }

+ ]))

+

+def wait_migration(vm):

+ while True:

+ event = vm.event_wait('MIGRATION')

+ iotests.log(event, filters=[iotests.filter_qmp_event])

+ if event['data']['status'] == 'completed':

+ break

+

with iotests.FilePath('img') as img_path, \

iotests.FilePath('backing') as backing_path, \