-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 10/06/2014 09:03 PM, Stefan Hajnoczi wrote: > On Fri, Oct 03, 2014 at 02:12:06PM +1000, Alexey Kardashevskiy wrote: >> BDRV_O_INCOMING is only set when QEMU is about to receive migration >> and we do not want QEMU to check the file at opening time as there >> is likely garbage. Is there any other use of BDRV_O_INCOMING? There >> must be some as bdrv_clear_incoming_migration_all() is called at the >> end of live migration and I believe there must be a reason for it >> (cache does not migrate, does it?). > > BDRV_O_INCOMING is just for live migration. The cached data is not > migrated, this is why it must be refreshed upon migration handover. > >> bdrv_invalidate_cache() flushes cache as it could be already >> initialized even if QEMU is receiving migration - QEMU could have >> cached some of real disk data. Is that correct? I do not really >> understand why it would happen if there is BDRV_O_INCOMING set but >> ok. > > .bdrv_open() can load metadata from image files (such as the qcow2 L1 > tables) and it does this even when BDRV_O_INCOMING is set. That data > needs to be re-read at migration handover to prevent the destination > QEMU from running with stale image metadata.
Would not it be easier/more correct if bdrv_open would not load this metadata if BDRV_O_INCOMING is set? > >> diff --git a/nbd.c b/nbd.c index e9b539b..953c378 100644 - --- >> a/nbd.c +++ b/nbd.c @@ -972,6 +972,7 @@ NBDExport >> *nbd_export_new(BlockDriverState *bs, off_t dev_offset, exp->ctx = >> bdrv_get_aio_context(bs); bdrv_ref(bs); >> bdrv_add_aio_context_notifier(bs, bs_aio_attached, bs_aio_detach, >> exp); + bdrv_invalidate_cache(bs, NULL); return exp; } > > Please add a comment to explain why this call is necessary: > > /* NBD exports are used for non-shared storage migration. Make sure * > that BDRV_O_INCOMING is cleared and the image is ready for write * > access since the export could be available before migration handover. > */ > > If someone can come up with a 2- or 3-line comment that explains it > better, great. > > The rest of the patch looks like it will work. I'm not thrilled > about putting BDRV_O_INCOMING logical inside bdrv_invalidate_cache() > because there was no coupling there before, but the code seems correct > now. Ok, will repost today. - -- Alexey -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJUMxvkAAoJEIYTPdgrwSC5+GMP/1sDhIJf3BlGb7FFqi7CeRDk X65p61AKc0BKjchFSGsl62DctcIUFPUN+gO8bGYlfihEdBu7n0pinG0iovg+Dhk3 auExumJGQony7D5K0/7BnR5Ciyvakk8lYs3qUaSE7PD4FrVDtnl8x7RTwP3qet3Q P4TO9yTVCoqnMBSj3ZZzcirwty8+MpFNWD9vhzBsyC3uVkXG/3pPr2LJWWogzosz ewmZQPQ5ydFncpBvT8gZhD+eseRVo6y0iTEac7TGmhDGCSWtyNcZng695lmzbUpB +lIQ5kqXOgGbcn8zgJ2VUwuBy7/sI0TsSIxYO69qwdgOqahSCrd7KgN0t2BoRVtH SOdxKxZhI3ULaNKAvRax93yq+gL8WZSvVmhpfEVh1EA7mZ2TeGMwZ3OsQzvGwi5/ RjsDTruiWg7FWoU6cI2VuPskNkehr0CXTMsheWoR8xvj+WVGz35quropSp8dw1qy NnJ8RmL5sQbtmh3Y8njdP+kwRUilqJAPcrpH6p4a+2cnXH4b3By4gyt0UROl7afs h8Pf/86HFa92kbMZAFs5ajhBj3Dg+SLHdAElzrRuz25/MVREAslaM8Us1q53xx/g P8neTnQIZfbcaH0b4JLxWPN2JPOyYXDV4IaOLEWyoAG0m2ks085+AB1L0o56hn0/ 6RVXGOU8iJKtQ6KGiDy/ =pB8l -----END PGP SIGNATURE-----
