[v2,07/11] block: hold hard reference for backup/mirror target
diff mbox

Message ID 1374054136-28741-8-git-send-email-famz@redhat.com
State New
Headers show

Commit Message

Fam Zheng July 17, 2013, 9:42 a.m. UTC
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/backup.c | 3 ++-
 block/mirror.c | 4 ++--
 2 files changed, 4 insertions(+), 3 deletions(-)

Comments

Stefan Hajnoczi July 23, 2013, 9:52 a.m. UTC | #1
On Wed, Jul 17, 2013 at 05:42:12PM +0800, Fam Zheng wrote:
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  block/backup.c | 3 ++-
>  block/mirror.c | 4 ++--
>  2 files changed, 4 insertions(+), 3 deletions(-)

Should we update the blockjob.c in_use code instead of adding
refcounting to specific block jobs?  This ought to be handled
generically for all block jobs.
Fam Zheng July 25, 2013, 6:08 a.m. UTC | #2
On Tue, 07/23 11:52, Stefan Hajnoczi wrote:
> On Wed, Jul 17, 2013 at 05:42:12PM +0800, Fam Zheng wrote:
> > Signed-off-by: Fam Zheng <famz@redhat.com>
> > ---
> >  block/backup.c | 3 ++-
> >  block/mirror.c | 4 ++--
> >  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> Should we update the blockjob.c in_use code instead of adding
> refcounting to specific block jobs?  This ought to be handled
> generically for all block jobs.

Target is not common in block jobs (e.g. doesn't apply to block-commit),
so it seems only specific block job knows about this.
Stefan Hajnoczi July 25, 2013, 7:59 a.m. UTC | #3
On Thu, Jul 25, 2013 at 02:08:42PM +0800, Fam Zheng wrote:
> On Tue, 07/23 11:52, Stefan Hajnoczi wrote:
> > On Wed, Jul 17, 2013 at 05:42:12PM +0800, Fam Zheng wrote:
> > > Signed-off-by: Fam Zheng <famz@redhat.com>
> > > ---
> > >  block/backup.c | 3 ++-
> > >  block/mirror.c | 4 ++--
> > >  2 files changed, 4 insertions(+), 3 deletions(-)
> > 
> > Should we update the blockjob.c in_use code instead of adding
> > refcounting to specific block jobs?  This ought to be handled
> > generically for all block jobs.
> 
> Target is not common in block jobs (e.g. doesn't apply to block-commit),
> so it seems only specific block job knows about this.

Of course you are right.

Stefan

Patch
diff mbox

diff --git a/block/backup.c b/block/backup.c
index 16105d4..b82f601 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -294,7 +294,7 @@  static void coroutine_fn backup_run(void *opaque)
     hbitmap_free(job->bitmap);
 
     bdrv_iostatus_disable(target);
-    bdrv_delete(target);
+    bdrv_unref(target, true);
 
     block_job_completed(&job->common, ret);
 }
@@ -332,6 +332,7 @@  void backup_start(BlockDriverState *bs, BlockDriverState *target,
         return;
     }
 
+    bdrv_ref(target, true);
     job->on_source_error = on_source_error;
     job->on_target_error = on_target_error;
     job->target = target;
diff --git a/block/mirror.c b/block/mirror.c
index bed4a7e..decdedb 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -479,8 +479,7 @@  immediate_exit:
         }
         bdrv_swap(s->target, s->common.bs);
     }
-    bdrv_close(s->target);
-    bdrv_delete(s->target);
+    bdrv_unref(s->target, true);
     block_job_completed(&s->common, ret);
 }
 
@@ -574,6 +573,7 @@  void mirror_start(BlockDriverState *bs, BlockDriverState *target,
     s->granularity = granularity;
     s->buf_size = MAX(buf_size, granularity);
 
+    bdrv_ref(target, true);
     bdrv_set_dirty_tracking(bs, granularity);
     bdrv_set_enable_write_cache(s->target, true);
     bdrv_set_on_error(s->target, on_target_error, on_target_error);