| Message ID | 1369917299-5725-11-git-send-email-stefanha@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 05/30/2013 06:34 AM, Stefan Hajnoczi wrote: > The Abort action can be used to test QMP 'transaction' failure. Add it > as the last action to exercise the .abort() and .cleanup() code paths > for all previous actions. Another thread questioned whether we should name this action __org.qemu-debug-abort, if only to be clear that it is not a normal interface.
On Thu, May 30, 2013 at 07:11:25AM -0600, Eric Blake wrote: > On 05/30/2013 06:34 AM, Stefan Hajnoczi wrote: > > The Abort action can be used to test QMP 'transaction' failure. Add it > > as the last action to exercise the .abort() and .cleanup() code paths > > for all previous actions. > > Another thread questioned whether we should name this action > __org.qemu-debug-abort, if only to be clear that it is not a normal > interface. kwolf: Do you want Abort to be namespaced as a debug-only action? I think it's perfectly supportable so there's no need to hide it. Granted it's rare that anyone would want to use this action. Stefan
Am 03.06.2013 um 11:21 hat Stefan Hajnoczi geschrieben: > On Thu, May 30, 2013 at 07:11:25AM -0600, Eric Blake wrote: > > On 05/30/2013 06:34 AM, Stefan Hajnoczi wrote: > > > The Abort action can be used to test QMP 'transaction' failure. Add it > > > as the last action to exercise the .abort() and .cleanup() code paths > > > for all previous actions. > > > > Another thread questioned whether we should name this action > > __org.qemu-debug-abort, if only to be clear that it is not a normal > > interface. > > kwolf: Do you want Abort to be namespaced as a debug-only action? > > I think it's perfectly supportable so there's no need to hide it. > Granted it's rare that anyone would want to use this action. I don't have a strong opinion on this. On the one hand it's probably useless outside testing and debugging, on the other hand it won't be a pain to support and the interface looks like it could be stable for the next few years... It's your patch, so you get to decide. Reviewed-by: Kevin Wolf <kwolf@redhat.com>
On 06/19/2013 12:21 PM, Kevin Wolf wrote: > Am 03.06.2013 um 11:21 hat Stefan Hajnoczi geschrieben: >> On Thu, May 30, 2013 at 07:11:25AM -0600, Eric Blake wrote: >>> On 05/30/2013 06:34 AM, Stefan Hajnoczi wrote: >>>> The Abort action can be used to test QMP 'transaction' failure. Add it >>>> as the last action to exercise the .abort() and .cleanup() code paths >>>> for all previous actions. >>> >>> Another thread questioned whether we should name this action >>> __org.qemu-debug-abort, if only to be clear that it is not a normal >>> interface. >> >> kwolf: Do you want Abort to be namespaced as a debug-only action? >> >> I think it's perfectly supportable so there's no need to hide it. >> Granted it's rare that anyone would want to use this action. > > I don't have a strong opinion on this. On the one hand it's probably > useless outside testing and debugging, on the other hand it won't be a > pain to support and the interface looks like it could be stable for > the next few years... > > It's your patch, so you get to decide. I don't have a strong opinion, either - it doesn't matter what it is named if no one outside of qtest uses that name :) When developing libvirt interfaces around new actions, I might use an abort to intentionally test behavior of libvirt error-handling code, but that would still be something I do by hand (by any name) and not something coded into libvirt itself.
diff --git a/blockdev.c b/blockdev.c index 18a2012..e3393d2 100644 --- a/blockdev.c +++ b/blockdev.c @@ -963,6 +963,16 @@ static void drive_backup_abort(BlkTransactionState *common) } } +static void abort_prepare(BlkTransactionState *common, Error **errp) +{ + error_setg(errp, "Transaction aborted using Abort action"); +} + +static void abort_commit(BlkTransactionState *common) +{ + assert(false); /* this action never succeeds */ +} + static const BdrvActionOps actions[] = { [TRANSACTION_ACTION_KIND_BLOCKDEV_SNAPSHOT_SYNC] = { .instance_size = sizeof(ExternalSnapshotState), @@ -975,6 +985,11 @@ static const BdrvActionOps actions[] = { .prepare = drive_backup_prepare, .abort = drive_backup_abort, }, + [TRANSACTION_ACTION_KIND_ABORT] = { + .instance_size = sizeof(BlkTransactionState), + .prepare = abort_prepare, + .commit = abort_commit, + }, }; /* diff --git a/qapi-schema.json b/qapi-schema.json index 6bf96aa..0da1b98 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -1646,6 +1646,16 @@ '*on-target-error': 'BlockdevOnError' } } ## +# @Abort +# +# This action can be used to test transaction failure. +# +# Since: 1.6 +### +{ 'type': 'Abort', + 'data': { } } + +## # @TransactionAction # # A discriminated record of operations that can be performed with @@ -1654,7 +1664,8 @@ { 'union': 'TransactionAction', 'data': { 'blockdev-snapshot-sync': 'BlockdevSnapshot', - 'drive-backup': 'DriveBackup' + 'drive-backup': 'DriveBackup', + 'abort': 'Abort' } } ##
The Abort action can be used to test QMP 'transaction' failure. Add it as the last action to exercise the .abort() and .cleanup() code paths for all previous actions. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- blockdev.c | 15 +++++++++++++++ qapi-schema.json | 13 ++++++++++++- 2 files changed, 27 insertions(+), 1 deletion(-)