| Message ID | 1455897130-27625-1-git-send-email-kwolf@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 19/02/2016 16:52, Kevin Wolf wrote: > The BDRV_O_INACTIVE flag should only be set for images explicitly opened > by the user. snapshot=on needs to create a new qcow2 image and write > some metadata to it. This is not a problem because it can't come from > the source, so there's no reason to mark it as BDRV_O_INACTIVE, even > though it is opened while waiting for the migration to complete. > > This fixes an assertion failure when -incoming and snapshot=on are > combined. > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> Does this also fix "migrate -b"? Paolo > --- > block.c | 4 ---- > blockdev.c | 8 +++++++ > tests/qemu-iotests/145 | 52 ++++++++++++++++++++++++++++++++++++++++++++++ > tests/qemu-iotests/145.out | 5 +++++ > tests/qemu-iotests/group | 1 + > 5 files changed, 66 insertions(+), 4 deletions(-) > create mode 100755 tests/qemu-iotests/145 > create mode 100644 tests/qemu-iotests/145.out > > diff --git a/block.c b/block.c > index efc3c43..ba24b8e 100644 > --- a/block.c > +++ b/block.c > @@ -1191,10 +1191,6 @@ static int bdrv_fill_options(QDict **options, const char *filename, > } > } > > - if (runstate_check(RUN_STATE_INMIGRATE)) { > - *flags |= BDRV_O_INACTIVE; > - } > - > return 0; > } > > diff --git a/blockdev.c b/blockdev.c > index 1f73478..ed97d8a 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -610,6 +610,10 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts, > qdict_put(bs_opts, BDRV_OPT_CACHE_NO_FLUSH, qstring_from_str("on")); > } > > + if (runstate_check(RUN_STATE_INMIGRATE)) { > + bdrv_flags |= BDRV_O_INACTIVE; > + } > + > blk = blk_new_open(qemu_opts_id(opts), file, NULL, bs_opts, bdrv_flags, > errp); > if (!blk) { > @@ -688,6 +692,10 @@ static BlockDriverState *bds_tree_init(QDict *bs_opts, Error **errp) > goto fail; > } > > + if (runstate_check(RUN_STATE_INMIGRATE)) { > + bdrv_flags |= BDRV_O_INACTIVE; > + } > + > bs = NULL; > ret = bdrv_open(&bs, NULL, NULL, bs_opts, bdrv_flags, errp); > if (ret < 0) { > diff --git a/tests/qemu-iotests/145 b/tests/qemu-iotests/145 > new file mode 100755 > index 0000000..7d8febb > --- /dev/null > +++ b/tests/qemu-iotests/145 > @@ -0,0 +1,52 @@ > +#!/bin/bash > +# > +# Test the combination of -incoming and snapshot=on > +# > +# Copyright (C) 2016 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() > +{ > + _cleanup_test_img > + true > +} > +trap "_cleanup; exit \$status" 0 1 2 3 15 > + > +# get standard environment, filters and checks > +. ./common.rc > +. ./common.filter > + > +_supported_fmt generic > +_supported_proto generic > +_supported_os Linux > + > +_make_test_img 1M > +echo quit | $QEMU -nographic -hda "$TEST_IMG" -incoming 'exec:true' -snapshot -serial none -monitor stdio | _filter_qemu > + > +# success, all done > +echo "*** done" > +rm -f $seq.full > +status=0 > diff --git a/tests/qemu-iotests/145.out b/tests/qemu-iotests/145.out > new file mode 100644 > index 0000000..75b5c8a > --- /dev/null > +++ b/tests/qemu-iotests/145.out > @@ -0,0 +1,5 @@ > +QA output created by 145 > +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 > +QEMU X.Y.Z monitor - type 'help' for more information > +(qemu) q[K[Dqu[K[D[Dqui[K[D[D[Dquit[K > +*** done > diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group > index 65df029..47fd40c 100644 > --- a/tests/qemu-iotests/group > +++ b/tests/qemu-iotests/group > @@ -147,3 +147,4 @@ > 142 auto > 143 auto quick > 144 rw auto quick > +145 auto quick >
Am 19.02.2016 um 17:12 hat Paolo Bonzini geschrieben: > > > On 19/02/2016 16:52, Kevin Wolf wrote: > > The BDRV_O_INACTIVE flag should only be set for images explicitly opened > > by the user. snapshot=on needs to create a new qcow2 image and write > > some metadata to it. This is not a problem because it can't come from > > the source, so there's no reason to mark it as BDRV_O_INACTIVE, even > > though it is opened while waiting for the migration to complete. > > > > This fixes an assertion failure when -incoming and snapshot=on are > > combined. > > > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > > Does this also fix "migrate -b"? No, that still fails for me. I think 'migrate -b' needs to clear BDRV_O_INACTIVE before it starts using the image on the destination. I wonder how it ever worked without this, it should have thrown all of the metadata caches away instead of writing them to the image. Kevin
diff --git a/block.c b/block.c index efc3c43..ba24b8e 100644 --- a/block.c +++ b/block.c @@ -1191,10 +1191,6 @@ static int bdrv_fill_options(QDict **options, const char *filename, } } - if (runstate_check(RUN_STATE_INMIGRATE)) { - *flags |= BDRV_O_INACTIVE; - } - return 0; } diff --git a/blockdev.c b/blockdev.c index 1f73478..ed97d8a 100644 --- a/blockdev.c +++ b/blockdev.c @@ -610,6 +610,10 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts, qdict_put(bs_opts, BDRV_OPT_CACHE_NO_FLUSH, qstring_from_str("on")); } + if (runstate_check(RUN_STATE_INMIGRATE)) { + bdrv_flags |= BDRV_O_INACTIVE; + } + blk = blk_new_open(qemu_opts_id(opts), file, NULL, bs_opts, bdrv_flags, errp); if (!blk) { @@ -688,6 +692,10 @@ static BlockDriverState *bds_tree_init(QDict *bs_opts, Error **errp) goto fail; } + if (runstate_check(RUN_STATE_INMIGRATE)) { + bdrv_flags |= BDRV_O_INACTIVE; + } + bs = NULL; ret = bdrv_open(&bs, NULL, NULL, bs_opts, bdrv_flags, errp); if (ret < 0) { diff --git a/tests/qemu-iotests/145 b/tests/qemu-iotests/145 new file mode 100755 index 0000000..7d8febb --- /dev/null +++ b/tests/qemu-iotests/145 @@ -0,0 +1,52 @@ +#!/bin/bash +# +# Test the combination of -incoming and snapshot=on +# +# Copyright (C) 2016 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() +{ + _cleanup_test_img + true +} +trap "_cleanup; exit \$status" 0 1 2 3 15 + +# get standard environment, filters and checks +. ./common.rc +. ./common.filter + +_supported_fmt generic +_supported_proto generic +_supported_os Linux + +_make_test_img 1M +echo quit | $QEMU -nographic -hda "$TEST_IMG" -incoming 'exec:true' -snapshot -serial none -monitor stdio | _filter_qemu + +# success, all done +echo "*** done" +rm -f $seq.full +status=0 diff --git a/tests/qemu-iotests/145.out b/tests/qemu-iotests/145.out new file mode 100644 index 0000000..75b5c8a --- /dev/null +++ b/tests/qemu-iotests/145.out @@ -0,0 +1,5 @@ +QA output created by 145 +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 +QEMU X.Y.Z monitor - type 'help' for more information +(qemu) q[K[Dqu[K[D[Dqui[K[D[D[Dquit[K +*** done diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group index 65df029..47fd40c 100644 --- a/tests/qemu-iotests/group +++ b/tests/qemu-iotests/group @@ -147,3 +147,4 @@ 142 auto 143 auto quick 144 rw auto quick +145 auto quick
The BDRV_O_INACTIVE flag should only be set for images explicitly opened by the user. snapshot=on needs to create a new qcow2 image and write some metadata to it. This is not a problem because it can't come from the source, so there's no reason to mark it as BDRV_O_INACTIVE, even though it is opened while waiting for the migration to complete. This fixes an assertion failure when -incoming and snapshot=on are combined. Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- block.c | 4 ---- blockdev.c | 8 +++++++ tests/qemu-iotests/145 | 52 ++++++++++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/145.out | 5 +++++ tests/qemu-iotests/group | 1 + 5 files changed, 66 insertions(+), 4 deletions(-) create mode 100755 tests/qemu-iotests/145 create mode 100644 tests/qemu-iotests/145.out