Re: [Qemu-block] [Qemu-devel] [PATCH v10 12/14] block: add transactional properties

[Stefan Hajnoczi]

On Tue, Nov 03, 2015 at 12:27:19PM -0500, John Snow wrote:
> 
> 
> On 11/03/2015 10:17 AM, Stefan Hajnoczi wrote:
> > On Fri, Oct 23, 2015 at 07:56:50PM -0400, John Snow wrote:
> >> @@ -1732,6 +1757,10 @@ static void 
> >> block_dirty_bitmap_add_prepare(BlkActionState *common,
> >>      BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
> >>                                               common, common);
> >>  
> >> +    if (action_check_cancel_mode(common, errp) < 0) {
> >> +        return;
> >> +    }
> >> +
> >>      action = common->action->block_dirty_bitmap_add;
> >>      /* AIO context taken and released within qmp_block_dirty_bitmap_add */
> >>      qmp_block_dirty_bitmap_add(action->node, action->name,
> >> @@ -1767,6 +1796,10 @@ static void 
> >> block_dirty_bitmap_clear_prepare(BlkActionState *common,
> >>                                               common, common);
> >>      BlockDirtyBitmap *action;
> >>  
> >> +    if (action_check_cancel_mode(common, errp) < 0) {
> >> +        return;
> >> +    }
> >> +
> >>      action = common->action->block_dirty_bitmap_clear;
> >>      state->bitmap = block_dirty_bitmap_lookup(action->node,
> >>                                                action->name,
> > 
> > Why do the bitmap add/clear actions not support err-cancel=all?
> > 
> > I understand why other block jobs don't support it, but it's not clear
> > why these non-block job actions cannot.
> > 
> 
> Because they don't have a callback to invoke if the rest of the job fails.
> 
> I could create a BlockJob for them complete with a callback to invoke,
> but basically it's just because there's no interface to unwind them, or
> an interface to join them with the transaction.
> 
> They're small, synchronous non-job actions. Which makes them weird.

Funny, we've been looking at the same picture while seeing different
things:
https://en.wikipedia.org/wiki/Rabbit%E2%80%93duck_illusion

I think I understand your idea: the transaction should include both
immediate actions as well as block jobs.

My mental model was different: immediate actions commit/abort along with
the 'transaction' command.  Block jobs are separate and complete/cancel
together in a group.

In practice I think the two end up being similar because we won't be
able to implement immediate action commit/abort together with
long-running block jobs because the immediate actions rely on
quiescing/pausing the guest for atomic commit/abort.

So with your mental model the QMP client has to submit 2 'transaction'
commands: 1 for the immediate actions, 1 for the block jobs.

In my mental model the QMP client submits 1 command but the immediate
actions and block jobs are two separate transaction scopes.  This means
if the block jobs fail, the client needs to be aware of the immediate
actions that have committed.  Because of this, it becomes just as much
client effort as submitting two separate 'transaction' commands in your
model.

Can anyone see a practical difference?  I think I'm happy with John's
model.

Stefan

signature.asc
Description: PGP signature