Re: [Qemu-devel] [PATCH v5 08/11] blockdev: allow BdrvActionOps-commit() to be NULL
Am 30.05.2013 um 14:34 hat Stefan Hajnoczi geschrieben: Some QMP 'transaction' types don't need to do anything on .commit(). Make .commit() optional just like .abort(). The drive-backup action will take advantage of this, it only needs to cancel the block job on .abort(). Other block job actions will probably follow the same pattern, so allow .commit() to be NULL. Suggested-by: Eric Blake ebl...@redhat.com Signed-off-by: Stefan Hajnoczi stefa...@redhat.com Reviewed-by: Kevin Wolf kw...@redhat.com
Re: [Qemu-devel] [PATCH v5 08/11] blockdev: allow BdrvActionOps-commit() to be NULL
On Thu, May 30, 2013 at 04:57:21PM -0600, Eric Blake wrote: On 05/30/2013 06:34 AM, Stefan Hajnoczi wrote: Some QMP 'transaction' types don't need to do anything on .commit(). Make .commit() optional just like .abort(). The drive-backup action will take advantage of this, it only needs to cancel the block job on .abort(). Other block job actions will probably follow the same pattern, so allow .commit() to be NULL. Suggested-by: Eric Blake ebl...@redhat.com Signed-off-by: Stefan Hajnoczi stefa...@redhat.com --- blockdev.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) Reviewed-by: Eric Blake ebl...@redhat.com Is it worth enforcing that at least one of commit or abort is supplied (that is, assert if the user codes up an action that has neither callback)? Or is that just overkill? I left it out because it seems like overkill. The action types are small in number, statically defined in an array, and we test both the commit and abort code paths. So there's no convenience place to put a compile-time check and developers would stumble across their mistake when running their tests. Stefan
[Qemu-devel] [PATCH v5 08/11] blockdev: allow BdrvActionOps-commit() to be NULL
Some QMP 'transaction' types don't need to do anything on .commit().
Make .commit() optional just like .abort().
The drive-backup action will take advantage of this, it only needs to
cancel the block job on .abort(). Other block job actions will probably
follow the same pattern, so allow .commit() to be NULL.
Suggested-by: Eric Blake ebl...@redhat.com
Signed-off-by: Stefan Hajnoczi stefa...@redhat.com
---
blockdev.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 94ad829..0e649df 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -789,7 +789,7 @@ typedef struct BdrvActionOps {
size_t instance_size;
/* Prepare the work, must NOT be NULL. */
void (*prepare)(BlkTransactionState *common, Error **errp);
-/* Commit the changes, must NOT be NULL. */
+/* Commit the changes, can be NULL. */
void (*commit)(BlkTransactionState *common);
/* Abort the changes on fail, can be NULL. */
void (*abort)(BlkTransactionState *common);
@@ -969,7 +969,9 @@ void qmp_transaction(TransactionActionList *dev_list, Error
**errp)
}
QSIMPLEQ_FOREACH(state, snap_bdrv_states, entry) {
-state-ops-commit(state);
+if (state-ops-commit) {
+state-ops-commit(state);
+}
}
/* success */
--
1.8.1.4
Re: [Qemu-devel] [PATCH v5 08/11] blockdev: allow BdrvActionOps-commit() to be NULL
On 05/30/2013 06:34 AM, Stefan Hajnoczi wrote: Some QMP 'transaction' types don't need to do anything on .commit(). Make .commit() optional just like .abort(). The drive-backup action will take advantage of this, it only needs to cancel the block job on .abort(). Other block job actions will probably follow the same pattern, so allow .commit() to be NULL. Suggested-by: Eric Blake ebl...@redhat.com Signed-off-by: Stefan Hajnoczi stefa...@redhat.com --- blockdev.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) Reviewed-by: Eric Blake ebl...@redhat.com Is it worth enforcing that at least one of commit or abort is supplied (that is, assert if the user codes up an action that has neither callback)? Or is that just overkill? -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
