| Message ID | 1372744789-997-8-git-send-email-famz@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Jul 02, 2013 at 01:59:49PM +0800, Fam Zheng wrote: > Assign source image as the backing hd of target bs, so reading target bs > gets the point-in-time copy of data from source image. > > Signed-off-by: Fam Zheng <famz@redhat.com> > --- > block/backup.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/block/backup.c b/block/backup.c > index 4e9f927..2dd0540 100644 > --- a/block/backup.c > +++ b/block/backup.c > @@ -294,6 +294,11 @@ static void coroutine_fn backup_run(void *opaque) > hbitmap_free(job->bitmap); > > bdrv_iostatus_disable(target); > + > + bdrv_put_ref(target->backing_hd); > + target->backing_hd = NULL; > + target->backing_file[0] = '\0'; > + target->backing_format[0] = '\0'; > bdrv_put_ref(target); > > block_job_completed(&job->common, ret); > @@ -332,7 +337,15 @@ void backup_start(BlockDriverState *bs, BlockDriverState *target, > return; > } > > + target->backing_hd = bs; > + pstrcpy(target->backing_file, sizeof(target->backing_file), > + bs->filename); > + pstrcpy(target->backing_format, sizeof(target->backing_format), > + bs->drv->format_name); > bdrv_get_ref(target); > + /* Get another ref to source for backing_hd relationship */ > + bdrv_get_ref(bs); > + > job->on_source_error = on_source_error; > job->on_target_error = on_target_error; > job->target = target; This is a strange way of overriding the backing file. The target exists after drive-backup completes but now has a NULL backing_hd. Also, we set backing_hd even for a raw image. I thought this would be achieved as follows: 1. The management tool creates the qcow2 file. 2. drive-add if=none,id=target,file=backup.qcow2,backing_hd=drive0 3. drive-backup drive=drive0,target=target,mode=existing Something along these lines. The difference is that we do not force override the backing_hd. It is only done when the management tool/user decides explicitly to use backing files. Also, the target is left in a usable state after the blockjob completes. I know the drive-add approach has been shelved in favor of directly using drive-backup, but this patch seems a little too hacky IMO. Stefan
diff --git a/block/backup.c b/block/backup.c index 4e9f927..2dd0540 100644 --- a/block/backup.c +++ b/block/backup.c @@ -294,6 +294,11 @@ static void coroutine_fn backup_run(void *opaque) hbitmap_free(job->bitmap); bdrv_iostatus_disable(target); + + bdrv_put_ref(target->backing_hd); + target->backing_hd = NULL; + target->backing_file[0] = '\0'; + target->backing_format[0] = '\0'; bdrv_put_ref(target); block_job_completed(&job->common, ret); @@ -332,7 +337,15 @@ void backup_start(BlockDriverState *bs, BlockDriverState *target, return; } + target->backing_hd = bs; + pstrcpy(target->backing_file, sizeof(target->backing_file), + bs->filename); + pstrcpy(target->backing_format, sizeof(target->backing_format), + bs->drv->format_name); bdrv_get_ref(target); + /* Get another ref to source for backing_hd relationship */ + bdrv_get_ref(bs); + job->on_source_error = on_source_error; job->on_target_error = on_target_error; job->target = target;
Assign source image as the backing hd of target bs, so reading target bs gets the point-in-time copy of data from source image. Signed-off-by: Fam Zheng <famz@redhat.com> --- block/backup.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)