All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] block: use drained section around bdrv_snapshot_delete
@ 2015-12-16 18:33 Paolo Bonzini
  2015-12-17  1:55 ` Fam Zheng
  0 siblings, 1 reply; 3+ messages in thread
From: Paolo Bonzini @ 2015-12-16 18:33 UTC (permalink / raw)
  To: qemu-devel; +Cc: famz

Do not use bdrv_drain, since by itself it does not guarantee
anything.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/snapshot.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/block/snapshot.c b/block/snapshot.c
index 6e9fa8d..2d86b88 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -229,6 +229,8 @@ int bdrv_snapshot_delete(BlockDriverState *bs,
                          Error **errp)
 {
     BlockDriver *drv = bs->drv;
+    int ret;
+
     if (!drv) {
         error_setg(errp, QERR_DEVICE_HAS_NO_MEDIUM, bdrv_get_device_name(bs));
         return -ENOMEDIUM;
@@ -239,18 +241,21 @@ int bdrv_snapshot_delete(BlockDriverState *bs,
     }
 
     /* drain all pending i/o before deleting snapshot */
-    bdrv_drain(bs);
+    bdrv_drained_begin(bs);
 
     if (drv->bdrv_snapshot_delete) {
-        return drv->bdrv_snapshot_delete(bs, snapshot_id, name, errp);
+        ret = drv->bdrv_snapshot_delete(bs, snapshot_id, name, errp);
+    } else if (bs->file) {
+        ret = bdrv_snapshot_delete(bs->file->bs, snapshot_id, name, errp);
+    } else {
+        error_setg(errp, "Block format '%s' used by device '%s' "
+                   "does not support internal snapshot deletion",
+                   drv->format_name, bdrv_get_device_name(bs));
+        ret = -ENOTSUP;
     }
-    if (bs->file) {
-        return bdrv_snapshot_delete(bs->file->bs, snapshot_id, name, errp);
-    }
-    error_setg(errp, "Block format '%s' used by device '%s' "
-               "does not support internal snapshot deletion",
-               drv->format_name, bdrv_get_device_name(bs));
-    return -ENOTSUP;
+
+    bdrv_drained_end(bs);
+    return ret;
 }
 
 int bdrv_snapshot_delete_by_id_or_name(BlockDriverState *bs,
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] block: use drained section around bdrv_snapshot_delete
  2015-12-16 18:33 [Qemu-devel] [PATCH] block: use drained section around bdrv_snapshot_delete Paolo Bonzini
@ 2015-12-17  1:55 ` Fam Zheng
  2015-12-18 12:11   ` Kevin Wolf
  0 siblings, 1 reply; 3+ messages in thread
From: Fam Zheng @ 2015-12-17  1:55 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel

On Wed, 12/16 19:33, Paolo Bonzini wrote:
> Do not use bdrv_drain, since by itself it does not guarantee
> anything.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  block/snapshot.c | 23 ++++++++++++++---------
>  1 file changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/block/snapshot.c b/block/snapshot.c
> index 6e9fa8d..2d86b88 100644
> --- a/block/snapshot.c
> +++ b/block/snapshot.c
> @@ -229,6 +229,8 @@ int bdrv_snapshot_delete(BlockDriverState *bs,
>                           Error **errp)
>  {
>      BlockDriver *drv = bs->drv;
> +    int ret;
> +
>      if (!drv) {
>          error_setg(errp, QERR_DEVICE_HAS_NO_MEDIUM, bdrv_get_device_name(bs));
>          return -ENOMEDIUM;
> @@ -239,18 +241,21 @@ int bdrv_snapshot_delete(BlockDriverState *bs,
>      }
>  
>      /* drain all pending i/o before deleting snapshot */
> -    bdrv_drain(bs);
> +    bdrv_drained_begin(bs);
>  
>      if (drv->bdrv_snapshot_delete) {
> -        return drv->bdrv_snapshot_delete(bs, snapshot_id, name, errp);
> +        ret = drv->bdrv_snapshot_delete(bs, snapshot_id, name, errp);
> +    } else if (bs->file) {
> +        ret = bdrv_snapshot_delete(bs->file->bs, snapshot_id, name, errp);
> +    } else {
> +        error_setg(errp, "Block format '%s' used by device '%s' "
> +                   "does not support internal snapshot deletion",
> +                   drv->format_name, bdrv_get_device_name(bs));
> +        ret = -ENOTSUP;
>      }
> -    if (bs->file) {
> -        return bdrv_snapshot_delete(bs->file->bs, snapshot_id, name, errp);
> -    }
> -    error_setg(errp, "Block format '%s' used by device '%s' "
> -               "does not support internal snapshot deletion",
> -               drv->format_name, bdrv_get_device_name(bs));
> -    return -ENOTSUP;
> +
> +    bdrv_drained_end(bs);
> +    return ret;
>  }
>  
>  int bdrv_snapshot_delete_by_id_or_name(BlockDriverState *bs,
> -- 
> 2.5.0
> 
> 

Reviewed-by: Fam Zheng <famz@redhat.com>

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] block: use drained section around bdrv_snapshot_delete
  2015-12-17  1:55 ` Fam Zheng
@ 2015-12-18 12:11   ` Kevin Wolf
  0 siblings, 0 replies; 3+ messages in thread
From: Kevin Wolf @ 2015-12-18 12:11 UTC (permalink / raw)
  To: Fam Zheng; +Cc: Paolo Bonzini, qemu-devel, qemu-block

[ Cc: qemu-block ]

Am 17.12.2015 um 02:55 hat Fam Zheng geschrieben:
> On Wed, 12/16 19:33, Paolo Bonzini wrote:
> > Do not use bdrv_drain, since by itself it does not guarantee
> > anything.
> > 
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Thanks, applied to the block branch.

Kevin

> >  block/snapshot.c | 23 ++++++++++++++---------
> >  1 file changed, 14 insertions(+), 9 deletions(-)
> > 
> > diff --git a/block/snapshot.c b/block/snapshot.c
> > index 6e9fa8d..2d86b88 100644
> > --- a/block/snapshot.c
> > +++ b/block/snapshot.c
> > @@ -229,6 +229,8 @@ int bdrv_snapshot_delete(BlockDriverState *bs,
> >                           Error **errp)
> >  {
> >      BlockDriver *drv = bs->drv;
> > +    int ret;
> > +
> >      if (!drv) {
> >          error_setg(errp, QERR_DEVICE_HAS_NO_MEDIUM, bdrv_get_device_name(bs));
> >          return -ENOMEDIUM;
> > @@ -239,18 +241,21 @@ int bdrv_snapshot_delete(BlockDriverState *bs,
> >      }
> >  
> >      /* drain all pending i/o before deleting snapshot */
> > -    bdrv_drain(bs);
> > +    bdrv_drained_begin(bs);
> >  
> >      if (drv->bdrv_snapshot_delete) {
> > -        return drv->bdrv_snapshot_delete(bs, snapshot_id, name, errp);
> > +        ret = drv->bdrv_snapshot_delete(bs, snapshot_id, name, errp);
> > +    } else if (bs->file) {
> > +        ret = bdrv_snapshot_delete(bs->file->bs, snapshot_id, name, errp);
> > +    } else {
> > +        error_setg(errp, "Block format '%s' used by device '%s' "
> > +                   "does not support internal snapshot deletion",
> > +                   drv->format_name, bdrv_get_device_name(bs));
> > +        ret = -ENOTSUP;
> >      }
> > -    if (bs->file) {
> > -        return bdrv_snapshot_delete(bs->file->bs, snapshot_id, name, errp);
> > -    }
> > -    error_setg(errp, "Block format '%s' used by device '%s' "
> > -               "does not support internal snapshot deletion",
> > -               drv->format_name, bdrv_get_device_name(bs));
> > -    return -ENOTSUP;
> > +
> > +    bdrv_drained_end(bs);
> > +    return ret;
> >  }
> >  
> >  int bdrv_snapshot_delete_by_id_or_name(BlockDriverState *bs,
> > -- 
> > 2.5.0
> > 
> > 
> 
> Reviewed-by: Fam Zheng <famz@redhat.com>
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-12-18 12:11 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-16 18:33 [Qemu-devel] [PATCH] block: use drained section around bdrv_snapshot_delete Paolo Bonzini
2015-12-17  1:55 ` Fam Zheng
2015-12-18 12:11   ` Kevin Wolf

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.