Re: [Qemu-devel] [PATCH v14 3/8] Backup: clear all bitmap when doing block checkpoint

[Stefan Hajnoczi]

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.

Stefan

signature.asc
Description: PGP signature