qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
To: Peter Krempa <pkrempa@redhat.com>
Cc: John Snow <jsnow@redhat.com>,
	qemu-devel@nongnu.org, qemu-block@nongnu.org,
	Markus Armbruster <armbru@redhat.com>
Subject: Re: [PATCH 2/2] migration: dirty-bitmap: Allow control of bitmap persistence on destination
Date: Wed, 3 Feb 2021 17:14:19 +0300	[thread overview]
Message-ID: <a096a6ed-dcd6-77e3-b3ab-c5dce31bec1d@virtuozzo.com> (raw)
In-Reply-To: <20210203133932.GF54538@angien.pipo.sk>

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


  reply	other threads:[~2021-02-03 14:16 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-03 12:59 [PATCH 0/2] migration: dirty-bitmap: Allow control of bitmap persistence on destination Peter Krempa
2021-02-03 12:59 ` [PATCH 1/2] migration: dirty-bitmap: Convert alias map inner members to a struct Peter Krempa
2021-02-04 19:12   ` Eric Blake
2021-02-05  7:04   ` Vladimir Sementsov-Ogievskiy
2021-02-03 13:00 ` [PATCH 2/2] migration: dirty-bitmap: Allow control of bitmap persistence on destination Peter Krempa
2021-02-03 13:23   ` Vladimir Sementsov-Ogievskiy
2021-02-03 13:27     ` Peter Krempa
2021-02-03 13:39       ` Peter Krempa
2021-02-03 14:14         ` Vladimir Sementsov-Ogievskiy [this message]
2021-02-04 19:15   ` Eric Blake
2021-02-05  8:01   ` Vladimir Sementsov-Ogievskiy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a096a6ed-dcd6-77e3-b3ab-c5dce31bec1d@virtuozzo.com \
    --to=vsementsov@virtuozzo.com \
    --cc=armbru@redhat.com \
    --cc=jsnow@redhat.com \
    --cc=pkrempa@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).