On 03/26/2015 03:12 PM, Fam Zheng wrote:
> On Wed, 03/25 17:36, 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>
>> Cc: Luiz Capitulino <lcapitul...@redhat.com>
>> Cc: Michael Roth <mdr...@linux.vnet.ibm.com>
>> ---
>> block.c | 39 +++++++++++++++++++++++++++++++++++++++
>> include/block/block.h | 4 ++++
>> include/block/block_int.h | 11 +++++++++++
>> qapi/block.json | 16 ++++++++++++++++
>> 4 files changed, 70 insertions(+)
>>
>> diff --git a/block.c b/block.c
>> index 0fe97de..0ff5cf8 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -6196,3 +6196,42 @@ BlockAcctStats *bdrv_get_stats(BlockDriverState *bs)
>> {
>> return &bs->stats;
>> }
>> +
>> +void bdrv_start_replication(BlockDriverState *bs, COLOMode mode, Error
>> **errp)
>> +{
>> + BlockDriver *drv = bs->drv;
>> +
>> + if (drv && drv->bdrv_start_replication) {
>> + drv->bdrv_start_replication(bs, mode, errp);
>> + } else if (bs->file) {
>> + bdrv_start_replication(bs->file, mode, errp);
>> + } else {
>> + error_set(errp, QERR_UNSUPPORTED);
>
> I think we should use error_setg in new code? (The same to following ones)
Hmm, do you mean that don't use QERR_UNSUPPORTED here?
>
>> + }
>> +}
>> +
>> +void bdrv_do_checkpoint(BlockDriverState *bs, Error **errp)
>> +{
>> + BlockDriver *drv = bs->drv;
>> +
>> + if (drv && drv->bdrv_do_checkpoint) {
>> + drv->bdrv_do_checkpoint(bs, errp);
>> + } else if (bs->file) {
>> + bdrv_do_checkpoint(bs->file, errp);
>> + } else {
>> + error_set(errp, QERR_UNSUPPORTED);
>> + }
>> +}
>> +
>> +void bdrv_stop_replication(BlockDriverState *bs, Error **errp)
>> +{
>> + BlockDriver *drv = bs->drv;
>> +
>> + if (drv && drv->bdrv_stop_replication) {
>> + drv->bdrv_stop_replication(bs, errp);
>> + } else if (bs->file) {
>> + bdrv_stop_replication(bs->file, errp);
>> + } else {
>> + error_set(errp, QERR_UNSUPPORTED);
>> + }
>> +}
>> diff --git a/include/block/block.h b/include/block/block.h
>> index 4c57d63..68f3b1a 100644
>> --- a/include/block/block.h
>> +++ b/include/block/block.h
>> @@ -569,4 +569,8 @@ void bdrv_flush_io_queue(BlockDriverState *bs);
>>
>> BlockAcctStats *bdrv_get_stats(BlockDriverState *bs);
>>
>> +void bdrv_start_replication(BlockDriverState *bs, COLOMode mode, Error
>> **errp);
>> +void bdrv_do_checkpoint(BlockDriverState *bs, Error **errp);
>> +void bdrv_stop_replication(BlockDriverState *bs, Error **errp);
>> +
>> #endif
>> diff --git a/include/block/block_int.h b/include/block/block_int.h
>> index dccb092..08dd8ba 100644
>> --- a/include/block/block_int.h
>> +++ b/include/block/block_int.h
>> @@ -290,6 +290,17 @@ struct BlockDriver {
>> */
>> int (*bdrv_probe_geometry)(BlockDriverState *bs, HDGeometry *geo);
>>
>> +
>> + void (*bdrv_start_replication)(BlockDriverState *bs, COLOMode mode,
>> + Error **errp);
>
> Need some documentation, but I have a generic question:
>
> Why is a single interface with modes better than different functions for each
> mode (bdrv_start_replication_{primary,secondary}? Asking because the behavior
> is very different between them, and I don't see much sharing -- you implement
> primary operation in quorum, and secondary in qcow2+colo.
No special reason.
>
>> + /* Drop Disk buffer when doing checkpoint. */
>> + void (*bdrv_do_checkpoint)(BlockDriverState *bs, Error **errp);
>> + /*
>> + * After failover, we should flush Disk buffer into secondary disk
>> + * and stop block replication.
>> + */
>> + void (*bdrv_stop_replication)(BlockDriverState *bs, Error **errp);
>> +
>> QLIST_ENTRY(BlockDriver) list;
>> };
>>
>> diff --git a/qapi/block.json b/qapi/block.json
>> index e313465..e640566 100644
>> --- a/qapi/block.json
>> +++ b/qapi/block.json
>> @@ -40,6 +40,22 @@
>> 'data': ['auto', 'none', 'lba', 'large', 'rechs']}
>>
>> ##
>> +# @COLOMode
>> +#
>> +# An enumeration of COLO mode.
>> +#
>> +# @unprotected: COLO is not started or after failover
>> +#
>> +# @primary: Primary mode, the vm's state will be sent to secondary QEMU.
>> +#
>> +# @secondary: Secondary mode, receive the vm's state from primary QEMU.
>> +#
>> +# Since: 2.4
>> +##
>> +{ 'enum' : 'COLOMode',
>> + 'data' : ['unprotected', 'primary', 'secondary']}
>
> If split bdrv_start_replication, do we still need an enum? I can't find the
> usage in QMP interface, is it in some other series?
I will check it.
Thanks
Wen Congyang
>
> Fam
>
>> +
>> +##
>> # @BlockdevSnapshotInternal
>> #
>> # @device: the name of the device to generate the snapshot from
>> --
>> 2.1.0
>>
> .
>
