[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [RFC] dirty-bitmaps: add block-dirty-bitmap-persist com

From: John Snow
Subject: Re: [Qemu-block] [RFC] dirty-bitmaps: add block-dirty-bitmap-persist command
Date: Wed, 14 Aug 2019 14:37:12 -0400
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0

On 8/13/19 8:08 PM, Eric Blake wrote:
> On 8/13/19 5:44 PM, John Snow wrote:
>> This is for the purpose of toggling on/off persistence on a bitmap.
>> This enables you to save a bitmap that was not persistent, but may
>> have already accumulated valuable data.
>> This is simply a QOL enhancement:
>> - Allows user to "upgrade" an existing bitmap to persistent
>> - Allows user to "downgrade" an existing bitmap to transient,
>>   removing it from storage without deleting the bitmap.
> In the meantime, a workaround is:
> create tmp bitmap (non-persistent is fine)
> merge existing bitmap into tmp bitmap
> delete existing bitmap
> recreate original bitmap with desired change in persistence
> merge tmp bitmap into re-created original bitmap
> delete tmp bitmap

Merge really lets us get away with a lot :) It's a powerful command. And
now that merge supports cross-granularities, you can even use it to
change the granularity of a bitmap.

> (I'm not sure how much, if any of that, has to be done with a
> transaction; ideally none, since merging two bitmaps that are both
> enabled is not going to lose any bits.  And since one of the two ends of
> the transaction has a non-persistent bitmap, qemu failing in the narrow
> window where the original bitmap does not exist at all is not that much
> different from failing while the bitmap is transient. If losing data due
> to qemu failure was important, the bitmap should never have been
> transient in the first place)

Yup, quite a lengthy workaround, but it _IS_ possible, it's just not
very nice.

>> Signed-off-by: John Snow <address@hidden>
>> ---
>> This is just an RFC because I'm not sure if I really want to pursue
>> adding this, but it was raised in a discussion I had recently that it
>> was a little annoying as an API design that persistence couldn't be
>> changed after addition, so I wanted to see how much code it would take
>> to address that.
>> (So this patch isn't really tested; just: "Hey, look!")
>> I don't like this patch because it exacerbates my perceived problems
>> with the "check if I can make it persistent, then toggle the flag"
>> model, where I prefer the "Just try to set it persistent and let it fail
>> if it cannot" model, but there were some issues with that patchset that
>> I want to revisit.
> The idea itself makes sense. I don't know if libvirt would ever use it,
> but it does seem like it could make hand-management of bitmaps easier to
> reason about.

Right, this isn't for libvirt as much as it is people doing manual
configuration with e.g. qmp-shell (or heaven forbid, their own QMP tooling.)

>> +++ b/qapi/block-core.json
>> @@ -2001,6 +2001,19 @@
>>    'data': { 'node': 'str', 'name': 'str', '*granularity': 'uint32',
>>              '*persistent': 'bool', '*autoload': 'bool', '*disabled': 'bool' 
>> } }
>> +##
>> +# @BlockDirtyBitmapPersist:
> The QAPI additions look fine to me, regardless of whether you respin the
> code based on review there.

Thanks, just wanted this one out there on the record.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]