Re: [PATCH 2/2] migration: dirty-bitmap: Allow control of bitmap persistence on destination

[Vladimir Sementsov-Ogievskiy]

03.02.2021 16:39, Peter Krempa wrote:
> On Wed, Feb 03, 2021 at 14:27:49 +0100, Peter Krempa wrote:
> > On Wed, Feb 03, 2021 at 16:23:21 +0300, Vladimir Sementsov-Ogievskiy wrote:
> > > 03.02.2021 16:00, Peter Krempa wrote:
> > > > Bitmap's source persistence is transported over the migration stream and
> > > > the destination mirrors it. In some cases the destination might want to
> > > > persist bitmaps which are not persistent on the source (e.g. the result
> > > > of merge of bitmaps from a number of layers on the source when migrating
> > > > into a squashed image)
> > >
> > > Why not make merge target on source be persistent itself? Then it will be 
> > > persistent on migration destination.
> >
> > Because they are temporary on the source. I don't want to make it
> > persistent in case of a failure so that it doesn't get written to the
> > disk e.g. in case of VM shutdown.
>
> To be a bit more specific, I don't want the bitmaps to stay in the
> image, that means that I'd have to also delete them on the source after
> a successfull migration before qemu is terminated, which might not even
> be possible since source deactivates storage after migration.
>
> So making them persistent on source is impossible.

Actually on success path, persistent bitmaps are not stored on source.

Normally persistent bitmaps are stored on image inactivation. But bitmaps
involved into migration are an exclusion, they are not stored (otherwise,
stoing will influence downtime of migration). And of-course, we can't
store bitmaps after disks inactivation.

So, on success path, the only way to store bitmaps on source is to do
qmp 'cont' command on source after migration..

I'm not so sure about error path. Of course, if something breaks between
merge target creation and migration start, bitmaps will be stored.

So, I agree that in general it's bad idea to make temporary bitmap 'persistent'.

> > > > but currently it would need to create another set
> > > > of persistent bitmaps and merge them.
> > > >
> > > > This adds 'dest-persistent' optional property to
> > > > 'BitmapMigrationBitmapAlias' which when present overrides the bitmap
> > > > presence state from the source.
> > >
> > > It's seems simpler to make a separate qmp command 
> > > block-dirty-bitmap-make-persistent.. Didn't you consider this way?
> >
> > I'm not sure how the internals work entirely. In my case it's way
> > simpler to do this setup when generating the mapping which I need to do
> > anyways rather than calling separate commands.
>
> Similarly here, after a successful migration I'd have to go and make all
> the bitmaps persistent, which is an extra step, and also a vector for
> possible failures after migration which also doesn't seem appealing.

OK, that's reasonable, thanks for explanation


--
Best regards,
Vladimir