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

[Eric Blake]

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

(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)

> 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.

> +++ 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.

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

signature.asc
Description: OpenPGP digital signature