John, your MUA turned the QMP examples to mush. You may want to teach
it manners.
John Snow <js...@redhat.com> writes:
> On 09/22/2015 06:34 PM, Eric Blake wrote:
>> On 09/22/2015 03:08 PM, John Snow wrote:
>>> Eric, Markus: I've CC'd you for some crazy what-if QAPI questions
>>> after my R-B on this patch.
>>>
>>
>>> The current design of this patch is such that the blockdev-backup
>>> and drive-backup QMP commands are extended for use in the
>>> transaction action. This means that as part of the arguments for
>>> the action, we can specify "transactional-cancel" as on/off,
>>> defaulting to off. This maintains backwards compatible behavior.
>>>
>>>
>>> As an example of a lone command (for simplicity...)
>>>
>>> { "execute": "transaction",
>>> "arguments": {
>>> "actions": [
>>> {"type": "drive-backup",
>>> "data": {"device": "drive0",
>>> "target": "/path/to/new_full_backup.img",
>>> "sync": "full", "format": "qcow2",
>>> "transactional-cancel": true
>>> }
>>> }
>>> ]
>>> }
>>> }
>>>
>>> This means that this command should fail if any of the other
>>> block jobs in the transaction (that have also set
>>> transactional-cancel(!)) fail.
>>
>> Just wondering - is there ever a case where someone would want to
>> create a transaction with multiple sub-groups? That is, I want
>> actions A, B, C, D to all occur at the same point in time, but with
>> grouping [A,B] and [C, D] (so that if A fails B gets cancelled, but
>> C and D can still
You make my head hurt.
> Only two sub-groups:
>
> (A) actions that live and die together
> (B) actions that proceed no matter what.
>
> The only reason we even allow these two is because the default
> behavior has been B historically, so we need to allow that to continue on.
>
> I don't think we need to architect multiple subgroups of "live and die
> together" semantics.
>
>> continue)? Worse, is there a series of actions to accomplish
>> something that cannot happen unless it is interleaved across
>> multiple such subgroups?
>>
>
> Not sure we'll need to address this.
>
>> Here's hoping that, for design simplicity, we only ever need one
>> subgroup per 'transaction' (auto-cancel semantics applies to all
>> commands in the group that opted in, with no way to further refine
>> into sub-groups of commands).
>>
>
> I think this is correct.
Puh!
>>>
>>> This is nice because it allows us to specify per-action which
>>> actions should live or die on the success of the transaction as a
>>> whole.
>>>
>>> What I was wondering is if we wanted to pidgeon-hole ourselves
>>> like this by adding arguments per-action and instead opt for a
>>> more generic, transaction-wide setting.
>>>
>>> In my mind, the transactional cancel has nothing to do with each
>>> /specific/ action, but has more to do with a property of
>>> transactions and actions in general.
>>>
>>>
>>> So I was thinking of two things:
>>>
>>> (1) Transactional cancel, while false by default, could be
>>> toggled to true by default for an entire transaction all-at-once
>>>
>>> { "execute": "transaction",
>>> "arguments": {
>>> "actions": [ ... ],
>>> "transactional-cancel": true
>>> }
>>> }
>>>
>>> This would toggle the default state for all actions to "fail if
>>> anything else in the transaction does."
>>
>> Or worded in another way - what is the use case for having a batch
>> of actions where some commands are grouped-cancel, but the
>> remaining actions are independent? Is there ever a case where you
>> would supply "transactional-cancel":true to one action, but not all
>> of them? If not, then this idea of hoisting the bool to the
>> transaction arguments level makes more sense (it's an all-or-none
>> switch, not a per-action switch).
>>
>> Qapi-wise, here's how you would do this:
>>
>> { 'command': 'transaction',
>> 'data': { 'actions': [ 'TransactionAction' ],
>> '*transactional-cancel': 'bool' } }
>>
>
> Right. If there's no need for us to specify per-action settings at
> all, this becomes the obvious "correct" solution.
>
> If we do want to be able to specify this sub-grouping per-action (for
> whatever reason), this might still be nice as a convenience feature.
A common flag is semantically simpler than a per-action flag. As
always, the more complex solution needs to be justified with an actual
use case.
A common @transactional-cancel defaulting to false suffices for backward
compatibility.
We think users will generally want to set it to true for all actions.
Again, a common flag suffices.
Unless someone comes up with actual use case for a per-action flag,
let's stick to the simpler common flag.
>>> Of course, as of right now, not all actions actually support this
>>>feature, but they might need to in the future -- and as more and more
>>>actions use this feature, it might be nice to have the global
>>>setting.
Uh-oh, you mean the flag is currently per-action because only some kinds
of actions actually implement it being set?
>>> If "transactional-cancel" was specified and is true (i.e. not
>>> the default) and actions are provided that do not support the
>>> argument explicitly, the transaction command itself should fail
>>> up-front.
>>
>> This actually makes sense to me, and might be simpler to
>> implement.
Yes, that's the stupidest solution that could possibly work, and
therefore the one that should be tried first.
> I'm not sure how to implement this, per-se, but in my mind it's
> something like:
>
> - A property of the transaction gets set (transactional-cancel) in
> qmp_transaction.
> - Each action has to prove that it has retrieved this value
> - drive-backup of course actually uses it,
> - other commands might fetch it to intentionally ignore it
> (i.e. if cancel y/n would have no effect)
> - If an action does not fetch this property, the transactional setup
> flags an error and aborts the transaction with an error
> - e.g. "Attempted to use property unsupported by action ..."
>
> With perhaps an allowance that, as long as the property is set to
> default, actions aren't required to check it.
Do we really need code to detect commands that don't know about the
flag? As long as the number of transaction-capable commands is small,
why not simple make them all aware of the flag? Make the ones that
don't implement it fail, which nicely fails the whole transaction.
[...]
