[Qemu-devel] qcow2 autoloading bitmaps

[Qemu-devel] qcow2 autoloading bitmaps

[Vladimir Sementsov-Ogievskiy]

Hi all!

I've just noted that there is an unfortunate contradiction between qcow2 
spec and qapi.

In qcow2 we have:
1: auto
  The bitmap must reflect all changes of the virtual
  disk by any application that would write to this qcow2
  file (including writes, snapshot switching, etc.). The
  type of this bitmap must be 'dirty tracking bitmap'.


so auto means enabled bitmaps, which must track dirtiness. It's logical 
to auto-load them
on Qemu start and make them enabled dirty bitmaps.

In Qapi we have:

# @autoload: the bitmap will be automatically loaded when the image it 
is stored
#            in is opened. This flag may only be specified for persistent
#            bitmaps. Default is false for block-dirty-bitmap-add. 
(Since: 2.10)

so, if we consider only enabled bitmaps it is ok to direct map autoload 
<=> auto.

But now we've faced into necessity of load/store disabled bitmaps.

Current behavior is definitely wrong: user sets autoload flag for 
disabled bitmap, but on next
Qemu start the bitmap will be autoloaded and enabled.


Proposed solution:
  - deprecate @autoload flag for bitmap creation, ignore it
  - save persistent enabled bitmaps with "auto" flag
  - save persistent disabled bitmaps without "auto" flag
  - on Qemu start load all bitmaps, mapping "auto" flag state to 
"enabled" state.


Note: we may store a lot of disabled bitmaps in qcow2 image, but loading 
them all into RAM may be
inefficient. Actually such bitmap will be needed only on demand (for 
export through nbd or
making some kind of backup). So in future it may be optimized by "lazy 
load" of disabled bitmaps,
postponing their actual load up to first read or enabling. This 
optimization doesn't need changing
of qapi or qcow2 format (at first sight).


Note2: now there is no way to disable/enable bitmaps, but there is a
   [PATCH for-2.12 0/4] qmp dirty bitmap API
with big conversation, but I hope, I'll post a new version with a small 
fix soon and it will be merged.

-- 
Best regards,
Vladimir

Re: [Qemu-devel] qcow2 autoloading bitmaps

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2765 bytes --]

On 01/11/2018 08:26 AM, Vladimir Sementsov-Ogievskiy wrote:

> # @autoload: the bitmap will be automatically loaded when the image it
> is stored
> #            in is opened. This flag may only be specified for persistent
> #            bitmaps. Default is false for block-dirty-bitmap-add.
> (Since: 2.10)
> 
> so, if we consider only enabled bitmaps it is ok to direct map autoload
> <=> auto.
> 
> But now we've faced into necessity of load/store disabled bitmaps.
> 
> Current behavior is definitely wrong: user sets autoload flag for
> disabled bitmap, but on next
> Qemu start the bitmap will be autoloaded and enabled.

Can we make it an error to set autoload to true when requesting a
disabled bitmap? Or can we not even request a disabled bitmap?

> 
> 
> Proposed solution:
>  - deprecate @autoload flag for bitmap creation, ignore it

Is there ever a case where you'd create an enabled persistent
dirty-tracking bitmap, but not want it to be re-enabled the next time
the image is created?  Your argument for ignoring/deleting the parameter
is that you can decide whether to set the "auto" flag solely based on
whether the bitmap is both persistent and enabled, without needing any
additional user input?

>  - save persistent enabled bitmaps with "auto" flag
>  - save persistent disabled bitmaps without "auto" flag
>  - on Qemu start load all bitmaps, mapping "auto" flag state to
> "enabled" state.

So if I follow the argument, the state of the 'auto' flag stored into
the file on BDS close (often at qemu exit) is based on whether the
bitmap was currently enabled, and the user can then control whether to
enable/disable the bitmap on the fly to control whether the 'auto' flag
is stored; thus, having the flag at creation time is redundant.

> 
> 
> Note: we may store a lot of disabled bitmaps in qcow2 image, but loading
> them all into RAM may be
> inefficient. Actually such bitmap will be needed only on demand (for
> export through nbd or
> making some kind of backup). So in future it may be optimized by "lazy
> load" of disabled bitmaps,
> postponing their actual load up to first read or enabling. This
> optimization doesn't need changing
> of qapi or qcow2 format (at first sight).
> 
> 
> Note2: now there is no way to disable/enable bitmaps, but there is a
>   [PATCH for-2.12 0/4] qmp dirty bitmap API
> with big conversation, but I hope, I'll post a new version with a small
> fix soon and it will be merged.

In other words, you can incorporate your QAPI tweaks proposed here into
your respin of that series.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: [Qemu-devel] qcow2 autoloading bitmaps

[Vladimir Sementsov-Ogievskiy]

11.01.2018 17:43, Eric Blake wrote:
> On 01/11/2018 08:26 AM, Vladimir Sementsov-Ogievskiy wrote:
>
>> # @autoload: the bitmap will be automatically loaded when the image it
>> is stored
>> #            in is opened. This flag may only be specified for persistent
>> #            bitmaps. Default is false for block-dirty-bitmap-add.
>> (Since: 2.10)
>>
>> so, if we consider only enabled bitmaps it is ok to direct map autoload
>> <=> auto.
>>
>> But now we've faced into necessity of load/store disabled bitmaps.
>>
>> Current behavior is definitely wrong: user sets autoload flag for
>> disabled bitmap, but on next
>> Qemu start the bitmap will be autoloaded and enabled.
> Can we make it an error to set autoload to true when requesting a
> disabled bitmap? Or can we not even request a disabled bitmap?

currently, autoload is set on bitmap creation, so it is always enabled 
at this time.

>
>>
>> Proposed solution:
>>   - deprecate @autoload flag for bitmap creation, ignore it
> Is there ever a case where you'd create an enabled persistent
> dirty-tracking bitmap, but not want it to be re-enabled the next time
> the image is created?

looks like we do not. For now we use only persistent+autoload.

> Your argument for ignoring/deleting the parameter
> is that you can decide whether to set the "auto" flag solely based on
> whether the bitmap is both persistent and enabled, without needing any
> additional user input?

The main point is that, when we want to load/store disabled bitmaps this
parameter is confusing: In user's point of view it's ok to have autoloading
disabled bitmap, which he wants to be autoloaded and disabled on next Qemu
start. So, to maintain this flag (autoload) for disabled bitmaps, we 
will need
to add some additional flag "disabled" to Qcow2 spec, to make it possible to
have "auto" but "disabled" bitmaps. And we will have to change qcow2 "auto"
specification, to consider "disabled" bit set in parallel. But all this 
looks
superfluous for now. We already have type "dirty tracking bitmaps" in qcow2
spec for dirty bitmaps. So, for now, "auto" flag set on "dirty tracking
bitmaps" means that it is enabled. It is logical to assume that all other
"dirty tracking bitmaps" are disabled.

On the other hand, "autoloading" is a bad property for disabled bitmap 
at all.
Disabled bitmap is simpler than enabled (or "auto") bitmap, it doesn't 
require
any care like "auto" bitmap. So, it is more natural to let Qemu decide 
to load
or not any disabled bitmaps.



>
>>   - save persistent enabled bitmaps with "auto" flag
>>   - save persistent disabled bitmaps without "auto" flag
>>   - on Qemu start load all bitmaps, mapping "auto" flag state to
>> "enabled" state.
> So if I follow the argument, the state of the 'auto' flag stored into
> the file on BDS close (often at qemu exit) is based on whether the
> bitmap was currently enabled, and the user can then control whether to
> enable/disable the bitmap on the fly to control whether the 'auto' flag
> is stored; thus, having the flag at creation time is redundant.

Yes.

>
>>
>> Note: we may store a lot of disabled bitmaps in qcow2 image, but loading
>> them all into RAM may be
>> inefficient. Actually such bitmap will be needed only on demand (for
>> export through nbd or
>> making some kind of backup). So in future it may be optimized by "lazy
>> load" of disabled bitmaps,
>> postponing their actual load up to first read or enabling. This
>> optimization doesn't need changing
>> of qapi or qcow2 format (at first sight).
>>
>>
>> Note2: now there is no way to disable/enable bitmaps, but there is a
>>    [PATCH for-2.12 0/4] qmp dirty bitmap API
>> with big conversation, but I hope, I'll post a new version with a small
>> fix soon and it will be merged.
> In other words, you can incorporate your QAPI tweaks proposed here into
> your respin of that series.
>

right idea, I'll do so, just wait a bit for others to comment here.

-- 
Best regards,
Vladimir

Re: [Qemu-devel] qcow2 autoloading bitmaps

[John Snow]

On 01/11/2018 10:15 AM, Vladimir Sementsov-Ogievskiy wrote:
> 11.01.2018 17:43, Eric Blake wrote:
>> On 01/11/2018 08:26 AM, Vladimir Sementsov-Ogievskiy wrote:
>>
>>> # @autoload: the bitmap will be automatically loaded when the image it
>>> is stored
>>> #            in is opened. This flag may only be specified for persistent
>>> #            bitmaps. Default is false for block-dirty-bitmap-add.
>>> (Since: 2.10)
>>>
>>> so, if we consider only enabled bitmaps it is ok to direct map autoload
>>> <=> auto.
>>>
>>> But now we've faced into necessity of load/store disabled bitmaps.
>>>
>>> Current behavior is definitely wrong: user sets autoload flag for
>>> disabled bitmap, but on next
>>> Qemu start the bitmap will be autoloaded and enabled.
>> Can we make it an error to set autoload to true when requesting a
>> disabled bitmap? Or can we not even request a disabled bitmap?
> 
> currently, autoload is set on bitmap creation, so it is always enabled
> at this time.
> 
>>
>>>
>>> Proposed solution:
>>>  - deprecate @autoload flag for bitmap creation, ignore it
>> Is there ever a case where you'd create an enabled persistent
>> dirty-tracking bitmap, but not want it to be re-enabled the next time
>> the image is created?
> 
> looks like we do not. For now we use only persistent+autoload.
> 
>> Your argument for ignoring/deleting the parameter
>> is that you can decide whether to set the "auto" flag solely based on
>> whether the bitmap is both persistent and enabled, without needing any
>> additional user input?
> 
> The main point is that, when we want to load/store disabled bitmaps this
> parameter is confusing: In user's point of view it's ok to have autoloading
> disabled bitmap, which he wants to be autoloaded and disabled on next Qemu
> start. So, to maintain this flag (autoload) for disabled bitmaps, we
> will need
> to add some additional flag "disabled" to Qcow2 spec, to make it possible to
> have "auto" but "disabled" bitmaps. And we will have to change qcow2 "auto"
> specification, to consider "disabled" bit set in parallel. But all this
> looks
> superfluous for now. We already have type "dirty tracking bitmaps" in qcow2
> spec for dirty bitmaps. So, for now, "auto" flag set on "dirty tracking
> bitmaps" means that it is enabled. It is logical to assume that all other
> "dirty tracking bitmaps" are disabled.
> 

Oh, so you want to be able to "autoload" a disabled bitmap, is that right?

Is it important to distinguish these two cases?

(1) A bitmap is saved to the qcow2, but autoload is set to false
(2) A bitmap is saved to the qcow2, autoload is true, but a (new)
disabled bit is set.

Is it okay, basically, to always autoload bitmaps in a disabled state?
Is it imperative we add the new bit?

(Genuinely asking. I'm not sure.)

> On the other hand, "autoloading" is a bad property for disabled bitmap
> at all.
> Disabled bitmap is simpler than enabled (or "auto") bitmap, it doesn't
> require
> any care like "auto" bitmap. So, it is more natural to let Qemu decide
> to load
> or not any disabled bitmaps.
> 

If we don't autoload the disabled ones, I imagine you'll eventually want
some way to manually load them into memory to manipulate them for
various reasons.

> 
> 
>>
>>>  - save persistent enabled bitmaps with "auto" flag
>>>  - save persistent disabled bitmaps without "auto" flag
>>>  - on Qemu start load all bitmaps, mapping "auto" flag state to
>>> "enabled" state.
>> So if I follow the argument, the state of the 'auto' flag stored into
>> the file on BDS close (often at qemu exit) is based on whether the
>> bitmap was currently enabled, and the user can then control whether to
>> enable/disable the bitmap on the fly to control whether the 'auto' flag
>> is stored; thus, having the flag at creation time is redundant.
> 
> Yes.
> 
>>
>>>
>>> Note: we may store a lot of disabled bitmaps in qcow2 image, but loading
>>> them all into RAM may be
>>> inefficient. Actually such bitmap will be needed only on demand (for
>>> export through nbd or
>>> making some kind of backup). So in future it may be optimized by "lazy
>>> load" of disabled bitmaps,
>>> postponing their actual load up to first read or enabling. This
>>> optimization doesn't need changing
>>> of qapi or qcow2 format (at first sight).
>>>
>>>
>>> Note2: now there is no way to disable/enable bitmaps, but there is a
>>>   [PATCH for-2.12 0/4] qmp dirty bitmap API
>>> with big conversation, but I hope, I'll post a new version with a small
>>> fix soon and it will be merged.
>> In other words, you can incorporate your QAPI tweaks proposed here into
>> your respin of that series.
>>
> 
> right idea, I'll do so, just wait a bit for others to comment here.
> 

Stay tuned...

> -- 
> Best regards,
> Vladimir
> 

Re: [Qemu-devel] qcow2 autoloading bitmaps

[Vladimir Sementsov-Ogievskiy]

16.01.2018 00:26, John Snow wrote:
>
> On 01/11/2018 10:15 AM, Vladimir Sementsov-Ogievskiy wrote:
>> 11.01.2018 17:43, Eric Blake wrote:
>>> On 01/11/2018 08:26 AM, Vladimir Sementsov-Ogievskiy wrote:
>>>
>>>> # @autoload: the bitmap will be automatically loaded when the image it
>>>> is stored
>>>> #            in is opened. This flag may only be specified for persistent
>>>> #            bitmaps. Default is false for block-dirty-bitmap-add.
>>>> (Since: 2.10)
>>>>
>>>> so, if we consider only enabled bitmaps it is ok to direct map autoload
>>>> <=> auto.
>>>>
>>>> But now we've faced into necessity of load/store disabled bitmaps.
>>>>
>>>> Current behavior is definitely wrong: user sets autoload flag for
>>>> disabled bitmap, but on next
>>>> Qemu start the bitmap will be autoloaded and enabled.
>>> Can we make it an error to set autoload to true when requesting a
>>> disabled bitmap? Or can we not even request a disabled bitmap?
>> currently, autoload is set on bitmap creation, so it is always enabled
>> at this time.
>>
>>>> Proposed solution:
>>>>   - deprecate @autoload flag for bitmap creation, ignore it
>>> Is there ever a case where you'd create an enabled persistent
>>> dirty-tracking bitmap, but not want it to be re-enabled the next time
>>> the image is created?
>> looks like we do not. For now we use only persistent+autoload.
>>
>>> Your argument for ignoring/deleting the parameter
>>> is that you can decide whether to set the "auto" flag solely based on
>>> whether the bitmap is both persistent and enabled, without needing any
>>> additional user input?
>> The main point is that, when we want to load/store disabled bitmaps this
>> parameter is confusing: In user's point of view it's ok to have autoloading
>> disabled bitmap, which he wants to be autoloaded and disabled on next Qemu
>> start. So, to maintain this flag (autoload) for disabled bitmaps, we
>> will need
>> to add some additional flag "disabled" to Qcow2 spec, to make it possible to
>> have "auto" but "disabled" bitmaps. And we will have to change qcow2 "auto"
>> specification, to consider "disabled" bit set in parallel. But all this
>> looks
>> superfluous for now. We already have type "dirty tracking bitmaps" in qcow2
>> spec for dirty bitmaps. So, for now, "auto" flag set on "dirty tracking
>> bitmaps" means that it is enabled. It is logical to assume that all other
>> "dirty tracking bitmaps" are disabled.
>>
> Oh, so you want to be able to "autoload" a disabled bitmap, is that right?

yes.

>
> Is it important to distinguish these two cases?
> (1) A bitmap is saved to the qcow2, but autoload is set to false
> (2) A bitmap is saved to the qcow2, autoload is true, but a (new)
> disabled bit is set.

there no "autoload" bitmaps in qcow2. there are "auto" ones, which has a 
bit different
meaning, see above. And, if we create bitmaps with "auto" and "disabled" 
bits set to true,
it will contradict with current "auto" definition.

actually "auto" definition looks more like "enabled" bit (which requires 
autoloading, if we are
going to write to the drive, to not break this enabled bitmap).

the concept of "autoloading" exists only in qapi, and actually it 
doesn't support disabled bitmaps.
And I think the most simple way to unravel all this mess is to drop this 
concept for now, as it
is one of the following:
1) it is useless (if we don't support persistent disabled bitmaps)
2) it is broken (if we support persistent disabled bitmaps)

hmm, sorry. a lot of words, but I'm not sure that I've directly answered 
your question.
if we have "autoload" instead of "auto" in qcow2, then it should be:
autoload=true, disabled=false - normal autoloading enabled bitmap
autoload=true, disabled=true - normal autoloading disabled bitmap
autoload=false, disabled=true - not-autoloading disabled bitmap. (the 
case, which we can't
realize with my proposed approach, of loading all bitmaps, but I do not 
think that this is bad,
and I think that "autoload" is wrong concep for qcow2)
autoload=false, disabled=false - looks like something wrong. enabled 
bitmap, which should not be
autoloaded? Does it mean that we should open disk read-only, if we do 
not load this bitmap? Anyway,
this doesn't look very useful.


>
> Is it okay, basically, to always autoload bitmaps in a disabled state?

"auto" bitmaps should be enabled to track dirtiness, as described in spec


> Is it imperative we add the new bit?

I think new bit is a bit of redundant information, as, actually, all 
bitmaps in qcow2 with type="dirty tracking bitmap" and with auto=false 
are actually disabled

>
> (Genuinely asking. I'm not sure.)
>
>> On the other hand, "autoloading" is a bad property for disabled bitmap
>> at all.
>> Disabled bitmap is simpler than enabled (or "auto") bitmap, it doesn't
>> require
>> any care like "auto" bitmap. So, it is more natural to let Qemu decide
>> to load
>> or not any disabled bitmaps.
>>
> If we don't autoload the disabled ones, I imagine you'll eventually want
> some way to manually load them into memory to manipulate them for
> various reasons.

Yes, I thought about it. But now for simplicity I propose just to load 
all of them.
And later it may be optimized to "lazy load", so we actually will load 
such bitmaps only on real
request to its data, however it will be in list and in query-block from 
qemu start. It should be
comfortable for user.

However, other strategy (by hand) of loading may be implemented in 
future. It also may be needed to
load bitmap through NBD.

>
>>
>>>>   - save persistent enabled bitmaps with "auto" flag
>>>>   - save persistent disabled bitmaps without "auto" flag
>>>>   - on Qemu start load all bitmaps, mapping "auto" flag state to
>>>> "enabled" state.
>>> So if I follow the argument, the state of the 'auto' flag stored into
>>> the file on BDS close (often at qemu exit) is based on whether the
>>> bitmap was currently enabled, and the user can then control whether to
>>> enable/disable the bitmap on the fly to control whether the 'auto' flag
>>> is stored; thus, having the flag at creation time is redundant.
>> Yes.
>>
>>>> Note: we may store a lot of disabled bitmaps in qcow2 image, but loading
>>>> them all into RAM may be
>>>> inefficient. Actually such bitmap will be needed only on demand (for
>>>> export through nbd or
>>>> making some kind of backup). So in future it may be optimized by "lazy
>>>> load" of disabled bitmaps,
>>>> postponing their actual load up to first read or enabling. This
>>>> optimization doesn't need changing
>>>> of qapi or qcow2 format (at first sight).
>>>>
>>>>
>>>> Note2: now there is no way to disable/enable bitmaps, but there is a
>>>>    [PATCH for-2.12 0/4] qmp dirty bitmap API
>>>> with big conversation, but I hope, I'll post a new version with a small
>>>> fix soon and it will be merged.
>>> In other words, you can incorporate your QAPI tweaks proposed here into
>>> your respin of that series.
>>>
>> right idea, I'll do so, just wait a bit for others to comment here.
>>
> Stay tuned...
>
>> -- 
>> Best regards,
>> Vladimir
>>


-- 
Best regards,
Vladimir