[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/2] parallels: create parallels_inactivate() ca
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH 1/2] parallels: create parallels_inactivate() callback of block driver state |
Date: |
Thu, 6 Apr 2017 16:14:15 +0100 |
User-agent: |
Mutt/1.8.0 (2017-02-23) |
On Wed, Apr 05, 2017 at 06:12:05PM +0300, Denis V. Lunev wrote:
> We should avoid to image file at migration end when BDRV_O_INACTIVE
s/avoid to image/avoid writing to the image/ ?
> is set on the block driver state. All modifications should be done
> in advance, i.e. in bdrv_inactivate.
>
> The patch also adds final bdrv_flush() to be sure that changes have
> reached disk finally before other side will access the image.
If migration fails/cancels on the source after .bdrv_inactivate() we
need to return to the "activated" state. .bdrv_invalidate_cache() will
be called so I think it needs to be implemented by parallels to set
s->header->inuse again if BDRV_O_RDWR.
.bdrv_invalidate_cache() is also called on the destination at live
migration handover. That's okay - it makes sense for the destination to
set the inuse field on success.
> -static void parallels_close(BlockDriverState *bs)
> +static int parallels_inactivate(BlockDriverState *bs)
> {
> + int err;
> BDRVParallelsState *s = bs->opaque;
>
> - if (bs->open_flags & BDRV_O_RDWR) {
> - s->header->inuse = 0;
> - parallels_update_header(bs);
> + if (!(bs->open_flags & BDRV_O_RDWR)) {
> + return 0;
> + }
> +
> + s->header->inuse = 0;
> + err = parallels_update_header(bs);
> + if (err < 0) {
Should we set s->header->inuse again in all error paths in case
something calls parallels_hupdate_header() again later?
signature.asc
Description: PGP signature