On 04/06/2017 06:14 PM, Stefan Hajnoczi wrote:
> 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/ ?
yes
>> 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.
hmm. sounds like a good catch..
>> -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?
yes.
thank you :)
