Re: [Qemu-devel] [PATCH bugfix] snapshot: add bdrv_drain_all() to bdrv_snapshot_delete() to avoid concurrency problem

2014-10-28 Thread Stefan Hajnoczi
On Tue, Oct 21, 2014 at 04:38:01PM +0800, Zhang Haoyu wrote:
 If there are still pending i/o while deleting snapshot,
 because deleting snapshot is done in non-coroutine context, and
 the pending i/o read/write (bdrv_co_do_rw) is done in coroutine context,
 so it's possible to cause concurrency problem between above two operations.
 Add bdrv_drain_all() to bdrv_snapshot_delete() to avoid this problem.
 
 Signed-off-by: Zhang Haoyu zhan...@sangfor.com
 ---
  block/snapshot.c | 4 
  1 file changed, 4 insertions(+)

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan


pgpgDgd6RD0fe.pgp
Description: PGP signature


Re: [Qemu-devel] [PATCH bugfix] snapshot: add bdrv_drain_all() to bdrv_snapshot_delete() to avoid concurrency problem

2014-10-21 Thread Paolo Bonzini



On 10/21/2014 10:38 AM, Zhang Haoyu wrote:

If there are still pending i/o while deleting snapshot,
because deleting snapshot is done in non-coroutine context, and
the pending i/o read/write (bdrv_co_do_rw) is done in coroutine context,
so it's possible to cause concurrency problem between above two operations.
Add bdrv_drain_all() to bdrv_snapshot_delete() to avoid this problem.

Signed-off-by: Zhang Haoyu zhan...@sangfor.com
---
  block/snapshot.c | 4 
  1 file changed, 4 insertions(+)

diff --git a/block/snapshot.c b/block/snapshot.c
index 85c52ff..ebc386a 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -236,6 +236,10 @@ int bdrv_snapshot_delete(BlockDriverState *bs,
  error_setg(errp, snapshot_id and name are both NULL);
  return -EINVAL;
  }
+
+/* drain all pending i/o before deleting snapshot */
+bdrv_drain_all();
+
  if (drv-bdrv_snapshot_delete) {
  return drv-bdrv_snapshot_delete(bs, snapshot_id, name, errp);
  }



Reviewed-by: Paolo Bonzini pbonz...@redhat.com