[libvirt] [PATCH RFC 15/40] qemu: checkpoint: Fix rollback and access to unlocked 'vm' when deleting checkpoints

[Ján Tomko]

On Fri, Oct 18, 2019 at 05:29:49PM -0500, Eric Blake wrote:
>On 10/18/19 11:11 AM, Peter Krempa wrote:
>>Delete/merge bitmaps when deleting checkpoints using a 'transaction' so
>>that we don't have to deal with halfway-failed scenarios and also fix
>>access to 'vm' while in the monitor lock.
>>

The whole if (!metadata_only) block looks like a separate function and
it has some variables declared in unecessarily big scopes.

But the change itself looks good.

Reviewed-by: Ján Tomko <jtomko at redhat.com>

>>Signed-off-by: Peter Krempa <pkrempa at redhat.com>
>>---
>>  src/qemu/qemu_checkpoint.c | 47 +++++++++++++++++++-------------------
>>  1 file changed, 24 insertions(+), 23 deletions(-)
>>
>
>>-            if (qemuMonitorDeleteBitmap(priv->mon, node, disk->bitmap) < 0) {
>>-                success = false;
>>-                break;
>>-            }
>>+
>>+            if (qemuMonitorTransactionBitmapRemove(actions, node, disk->bitmap) < 0)
>>+                return -1;
>
>Transactional bitmap remove depends on a newer qemu than what the 
>pre-patch state required; is this properly gated on capabilities so 
>that you can't get in a scenario where you can create bitmaps but not 
>delete them?

As of this series, yes :)
qemuCheckpointCreateXML requires QEMU_CAPS_INCREMENTAL_BACKUP which is
not set even after this series.

Jano

>
>Otherwise, the conversion makes sense.
>
>-- 
>Eric Blake, Principal Software Engineer
>Red Hat, Inc.           +1-919-301-3226
>Virtualization:  qemu.org | libvirt.org
>
>--
>libvir-list mailing list
>libvir-list at redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20191024/25dd5aa5/attachment-0001.sig>