On 10/12/2015 09:45 PM, Stefan Hajnoczi wrote: > On Fri, Sep 25, 2015 at 02:17:30PM +0800, Wen Congyang wrote: >> Signed-off-by: Wen Congyang <we...@cn.fujitsu.com> >> Signed-off-by: zhanghailiang <zhang.zhanghaili...@huawei.com> >> Signed-off-by: Gonglei <arei.gong...@huawei.com> >> Reviewed-by: Jeff Cody <jc...@redhat.com> >> --- >> block/backup.c | 14 ++++++++++++++ >> blockjob.c | 11 +++++++++++ >> include/block/blockjob.h | 12 ++++++++++++ >> 3 files changed, 37 insertions(+) >> >> diff --git a/block/backup.c b/block/backup.c >> index c61e4c3..5e5995e 100644 >> --- a/block/backup.c >> +++ b/block/backup.c >> @@ -214,11 +214,25 @@ static void backup_iostatus_reset(BlockJob *job) >> } >> } >> >> +static void backup_do_checkpoint(BlockJob *job, Error **errp) >> +{ >> + BackupBlockJob *backup_job = container_of(job, BackupBlockJob, common); >> + >> + if (backup_job->sync_mode != MIRROR_SYNC_MODE_NONE) { >> + error_setg(errp, "The backup job only supports block checkpoint in" >> + " sync=none mode"); >> + return; >> + } >> + >> + hbitmap_reset_all(backup_job->bitmap); >> +} > > Is this a fast way to stop and then start a new backup blockjob without > emitting block job lifecycle events? > > Not sure the blockjob_do_checkpoint() interface is appropriate. Is > there any other block job type that will implement .do_checkpoint()?
Currently, the answer is no. > > COLO block replication could call a public backup_do_checkpoint() > function. That way the direct coupling between COLO and the backup > block job is obvious. I'm not convinced a generic interface like > blockjob_do_checkpoint() makes sense since it's really not a generic > operation that makes sense for other block job types. > > void backup_do_checkpoint(BlockJob *job, Error **errp) > { > BackupBlockJob *s; > > if (job->driver != backup_job_driver) { > error_setg(errp, "expected backup block job type for " > "checkpoint, got %d", job->driver->job_type); > return; > } > > s = container_of(job, BackupBlockJob, common); > ... > } In a older version, I implement it like this, but Paolo didn't like it. > > Please also make the function name and documentation more specific. > Instead of "do" maybe this should be "pre" or "post" to indicate whether > this happens before or after the checkpoint commit. What happens if OK > this function returns an error? We just return this error to COLO, and COLO will do failover. Thanks Wen Congyang > . >