From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47007) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aOcDH-0006eB-9U for qemu-devel@nongnu.org; Wed, 27 Jan 2016 21:21:36 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aOcDE-0005tM-46 for qemu-devel@nongnu.org; Wed, 27 Jan 2016 21:21:35 -0500 Received: from [59.151.112.132] (port=63223 helo=heian.cn.fujitsu.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aOcDD-0005kK-Nu for qemu-devel@nongnu.org; Wed, 27 Jan 2016 21:21:32 -0500 Message-ID: <56A97B4B.8040606@cn.fujitsu.com> Date: Thu, 28 Jan 2016 10:22:03 +0800 From: Changlong Xie MIME-Version: 1.0 References: <1452676712-24239-1-git-send-email-xiecl.fnst@cn.fujitsu.com> <1452676712-24239-4-git-send-email-xiecl.fnst@cn.fujitsu.com> <20160127160541.GA29236@stefanha-x1.localdomain> In-Reply-To: <20160127160541.GA29236@stefanha-x1.localdomain> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v14 3/8] Backup: clear all bitmap when doing block checkpoint List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Kevin Wolf , Fam Zheng , zhanghailiang , fnstml-hwcolo@cn.fujitsu.com, qemu devel , Max Reitz , Gonglei , Stefan Hajnoczi , Paolo Bonzini On 01/28/2016 12:05 AM, Stefan Hajnoczi wrote: > On Wed, Jan 13, 2016 at 05:18:27PM +0800, Changlong Xie wrote: >> diff --git a/blockjob.c b/blockjob.c >> index 80adb9d..0c8edfe 100644 >> --- a/blockjob.c >> +++ b/blockjob.c >> @@ -533,3 +533,14 @@ void block_job_txn_add_job(BlockJobTxn *txn, BlockJob *job) >> QLIST_INSERT_HEAD(&txn->jobs, job, txn_list); >> block_job_txn_ref(txn); >> } >> + >> +void block_job_do_checkpoint(BlockJob *job, Error **errp) >> +{ >> + if (!job->driver->do_checkpoint) { >> + error_setg(errp, "The job %s doesn't support block checkpoint", >> + BlockJobType_lookup[job->driver->job_type]); >> + return; >> + } >> + >> + job->driver->do_checkpoint(job, errp); >> +} >> diff --git a/include/block/blockjob.h b/include/block/blockjob.h >> index d84ccd8..abdba7c 100644 >> --- a/include/block/blockjob.h >> +++ b/include/block/blockjob.h >> @@ -70,6 +70,9 @@ typedef struct BlockJobDriver { >> * never both. >> */ >> void (*abort)(BlockJob *job); >> + >> + /** Optional callback for job types that support checkpoint. */ >> + void (*do_checkpoint)(BlockJob *job, Error **errp); > > The COLO/replication-specific callbacks have been moved out of > BlockDriver into their own replication struct. Similar reasoning > applies to BlockJobDriver: > > The do_checkpoint() callback is only implemented by one type of job and > its purpose is related to COLO rather than jobs. This is a strong > indication that this shouldn't be part of the generic BlockJobDriver > struct. > > Please drop changes to the generic blockjob interface. Instead, make > backup_do_checkpoint() public and add assert(job->driver->type == > BLOCK_JOB_TYPE_BACKUP) into the function. > > Then the replication filter can call backup_do_checkpoint() directly. > Will fix it in next version. Thanks -Xie > Stefan >