Re: [Qemu-devel] [Patch v12 resend 00/10] Block replication for continuous checkpoints

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Wen Congyang (wency@cn.fujitsu.com) wrote:
> On 12/23/2015 06:04 PM, Stefan Hajnoczi wrote:
> > On Thu, Dec 17, 2015 at 02:22:14PM +0800, Wen Congyang wrote:
> >> Stefan:Ping...
> >>
> >> What about this feature? I have worked for it about 1 year, but it is still in the
> >> way...
> > 
> > The code still has TODOs.  What is the plan for supporting replication
> > after failover?  This feature seems critical because anyone who wants FT
> > won't be able to use this code unless it supports FT after the first
> > failure.
> 
> We have implemented it based on an old version qemu. To keep the logical
> simple, we don't post them now. We will post them after this feature is merged
> into qemu.

It's probably best to post them, or at least say how you intend to do it,
so people can get an understanding of which way you're going.

However, why is anything new needed to continue replication after failover?
Shouldn't it be possible to build the secondary's disk structure in a way
that it can (by device/disk add/remove) into a structure that looks the same
as a primary, and then we just start a new migration to the new secondary?

(There's a separate problem of getting that to work with the rest of COLO as
well)

Dave

> 
> > 
> > ---
> > 
> > Adding new block layer APIs that are replication-specific is not clean.
> > Only the replication block driver cares about the start/stop/checkpoint
> > interface.
> > 
> > It is cleaner to have a separate API and data structure for block
> > replication.
> > 
> > The replication code should define its own BlockReplicationOps struct
> > and allow objects to register themselves.  Then it's no longer necessary
> > to modify the core block layer to forward start/stop/checkpoint calls.
> > 
> > Something like:
> > 
> > typedef struct BlockReplicationOps BlockReplicationOps;
> > typedef struct BlockReplicationState {
> >     const BlockReplicationOps *ops;
> >     QLIST_ENTRY(BlockReplicationState) list;
> > } BlockReplicationState;
> > 
> > typedef struct {
> >     void start(BlockReplicationState *brs, Error **errp);
> >     void stop(BlockReplicationState *brs, Error **errp);
> >     void checkpoint(BlockReplicationState *brs, Error **errp);
> > } BlockReplicationOps;
> > 
> > static QLIST_HEAD(BlockReplicationState) block_replication_states;
> > 
> > void block_replication_add(BlockReplicationState *brs);
> > void block_replication_remove(BlockReplicationState *brs);
> > 
> > The replication block driver would add/remove itself.  The quorum block
> > driver probably doesn't need to be modified (I think in your current
> > patches you modify it just to forward the start/stop/checkpoint calls to
> > a particular quorum child).
> 
> Yes, it is the major purpose. We also do some check in the quorum driver: 
> we don't allow more than one child support block replication.
> 
> Thanks
> Wen Congyang
> 
> > 
> > Stefan
> > 
> 
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK