Re: [Qemu-block] [Qemu-devel] [PATCH 2/2] qapi: add transaction support for x-block-dirty-bitmap-merge

[Eric Blake]

On 07/06/2018 10:38 AM, John Snow wrote:

> > I think the "discard stashed state, making undo impossible"
> > interpretation is good because .commit() is not allowed to fail.  That
> > function should only do things that never fail.
>
> I think this is probably the correct way to proceed, and we ought to
> formalize this model. I think it's actually nearly impossible to do it
> the other way around, too.
>
> Let's say we've got two actions that both do error-checking in .prepare,
> but perform their actual stateful changes in .commit, but are
> interdependent.
>
> { a, b }
>
> - a.prepare() checks state and confirms we will be able to proceed, but
> changes no state.
> - b.prepare() checks state and confirms it will be able to proceed, but
> can't see what a is planning to do.
> - a.commit() succeeds as expected.
> - b.commit() does not fail as it is not allowed to, but the effects are
> undetermined because we have not checked the state change from a.commit()

I'm also trying to figure out if we guarantee that .abort is always 
called in reverse order of .prepare.  If we have { a, b, c }, where 
'a'.prepare makes one change, 'b'.prepare makes another that 'a' cannot 
directly undo, then 'c' fails, we're still okay if 'b'.abort can roll 
back to the state it had during prepare, so that 'a'.abort can then undo 
state cleanly.  And for consistency, I'd argue that .commit should have 
the same ordering as .abort (that is, every .prepare - .abort/.commit 
pair is a nested stack to track for unwinding purposes, and should be 
unwound in reverse order of setting aside resources up front).

> In general we can get around this, but the more actions we add, the
> harder it is to do proper error checking while considering the
> hypothetical state after some actions have committed.
>
> I think the model where we take effect in .prepare() and undo it if
> necessary in .abort() is actually easier to model in your head, because
> error checking is simpler. That's probably the right model, then.

Yes, I'm also leaning heavily towards strong guarantees.  In addition to 
our examples of which operations depend on which bitmaps, we have this 
additional case:

{ blockdev-snapshot, blockdev-snapshot-internal-sync }

vs.

{ blockdev-snapshot-internal-sync, blockdev-snapshot }

Yes, I know that mixing internal and external snapshots is currently 
VERY dangerous 
(https://lists.gnu.org/archive/html/qemu-devel/2018-04/msg00865.html) if 
you aren't careful - but there are two plausible outcomes that can be 
chosen, where the order matters.  In the first instance, we want to 
create a new external snapshot (going from 'A' to 'A <- B') and THEN 
create an internal snapshot (image 'B' gets an internal snapshot); in 
the second instance we want to create an internal snapshot (image 'A' 
gets an internal snapshot) and THEN create an external snapshot (going 
from 'A' to 'A <- B').  Since the file where the internal snapshot is 
created is indeterminate unless we strictly process the transaction in 
the order that it was given, that argues that we want the strong order 
guarantees.

> At the very least, I think that's correct for 3.0 and the immediate
> future. If you disagree, please speak up because I've long been
> particularly uncertain about this aspect.
>
> --John

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