| Message ID | 1423710438-14377-5-git-send-email-wency@cn.fujitsu.com |
|---|---|
| State | New |
| Headers | show |
On 2015-02-11 at 22:07, Wen Congyang wrote: > Signed-off-by: Wen Congyang <wency@cn.fujitsu.com> > Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> > Signed-off-by: Gonglei <arei.gonglei@huawei.com> > --- > block.c | 36 ++++++++++++++++++++++++++++++++++++ > include/block/block.h | 10 ++++++++++ > include/block/block_int.h | 12 ++++++++++++ > 3 files changed, 58 insertions(+) > > diff --git a/block.c b/block.c > index 210fd5f..2335af1 100644 > --- a/block.c > +++ b/block.c > @@ -6156,3 +6156,39 @@ BlockAcctStats *bdrv_get_stats(BlockDriverState *bs) > { > return &bs->stats; > } > + > +int bdrv_start_replication(BlockDriverState *bs, int mode) > +{ > + BlockDriver *drv = bs->drv; > + if (drv && drv->bdrv_start_replication) { > + return drv->bdrv_start_replication(bs, mode); > + } else if (bs->file) { > + return bdrv_start_replication(bs->file, mode); > + } > + > + return -1; I'd prefer returning -errno here (like -ENOTSUP). Alternatively, you may want to use Error objects (which would probably actually be the preferable way). > +} > + > +int bdrv_do_checkpoint(BlockDriverState *bs) > +{ > + BlockDriver *drv = bs->drv; > + if (drv && drv->bdrv_do_checkpoint) { > + return drv->bdrv_do_checkpoint(bs); > + } else if (bs->file) { > + return bdrv_do_checkpoint(bs->file); > + } > + > + return -1; Same here. > +} > + > +int bdrv_stop_replication(BlockDriverState *bs) > +{ > + BlockDriver *drv = bs->drv; > + if (drv && drv->bdrv_stop_replication) { > + return drv->bdrv_stop_replication(bs); > + } else if (bs->file) { > + return bdrv_stop_replication(bs->file); > + } > + > + return -1; And here. > +} > diff --git a/include/block/block.h b/include/block/block.h > index 321295e..632b9fc 100644 > --- a/include/block/block.h > +++ b/include/block/block.h > @@ -557,4 +557,14 @@ void bdrv_flush_io_queue(BlockDriverState *bs); > > BlockAcctStats *bdrv_get_stats(BlockDriverState *bs); > > +/* Checkpoint control, called in migration/checkpoint thread */ > +enum { > + COLO_UNPROTECTED_MODE = 0, > + COLO_PRIMARY_MODE, > + COLO_SECONDARY_MODE, > +}; I have a feeling that you may want to define these values through QAPI... There's nothing wrong with this patch, but I don't yet really know what you want to do with these functions (the doc didn't really help me with them), so I'll have to look into the rest of the series before I can really say something useful about it. Max > +int bdrv_start_replication(BlockDriverState *bs, int mode); > +int bdrv_do_checkpoint(BlockDriverState *bs); > +int bdrv_stop_replication(BlockDriverState *bs); > + > #endif > diff --git a/include/block/block_int.h b/include/block/block_int.h > index 7ad1950..603f704 100644 > --- a/include/block/block_int.h > +++ b/include/block/block_int.h > @@ -273,6 +273,18 @@ struct BlockDriver { > void (*bdrv_io_unplug)(BlockDriverState *bs); > void (*bdrv_flush_io_queue)(BlockDriverState *bs); > > + > + /* Checkpoint control, called in migration/checkpoint thread */ > + int (*bdrv_start_replication)(BlockDriverState *bs, int mode); > + /* > + * Drop Disk buffer when doing checkpoint. > + */ > + int (*bdrv_do_checkpoint)(BlockDriverState *bs); > + /* After failover, we should flush Disk buffer into secondary disk > + * and stop block replication. > + */ > + int (*bdrv_stop_replication)(BlockDriverState *bs); > + > QLIST_ENTRY(BlockDriver) list; > }; >
On 02/23/2015 01:57 PM, Max Reitz wrote: > On 2015-02-11 at 22:07, Wen Congyang wrote: >> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com> >> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com> >> Signed-off-by: Gonglei <arei.gonglei@huawei.com> >> --- >> block.c | 36 ++++++++++++++++++++++++++++++++++++ >> include/block/block.h | 10 ++++++++++ >> include/block/block_int.h | 12 ++++++++++++ >> 3 files changed, 58 insertions(+) >> >> +++ b/include/block/block.h >> @@ -557,4 +557,14 @@ void bdrv_flush_io_queue(BlockDriverState *bs); >> BlockAcctStats *bdrv_get_stats(BlockDriverState *bs); >> +/* Checkpoint control, called in migration/checkpoint thread */ >> +enum { >> + COLO_UNPROTECTED_MODE = 0, >> + COLO_PRIMARY_MODE, >> + COLO_SECONDARY_MODE, >> +}; > > I have a feeling that you may want to define these values through QAPI... especially if you intend for a QMP command to output which mode a colo disk is in. >> +++ b/include/block/block_int.h >> @@ -273,6 +273,18 @@ struct BlockDriver { >> void (*bdrv_io_unplug)(BlockDriverState *bs); >> void (*bdrv_flush_io_queue)(BlockDriverState *bs); >> + >> + /* Checkpoint control, called in migration/checkpoint thread */ >> + int (*bdrv_start_replication)(BlockDriverState *bs, int mode); >> + /* >> + * Drop Disk buffer when doing checkpoint. >> + */ >> + int (*bdrv_do_checkpoint)(BlockDriverState *bs); Inconsistent comment style between one-line and multi-line.
diff --git a/block.c b/block.c index 210fd5f..2335af1 100644 --- a/block.c +++ b/block.c @@ -6156,3 +6156,39 @@ BlockAcctStats *bdrv_get_stats(BlockDriverState *bs) { return &bs->stats; } + +int bdrv_start_replication(BlockDriverState *bs, int mode) +{ + BlockDriver *drv = bs->drv; + if (drv && drv->bdrv_start_replication) { + return drv->bdrv_start_replication(bs, mode); + } else if (bs->file) { + return bdrv_start_replication(bs->file, mode); + } + + return -1; +} + +int bdrv_do_checkpoint(BlockDriverState *bs) +{ + BlockDriver *drv = bs->drv; + if (drv && drv->bdrv_do_checkpoint) { + return drv->bdrv_do_checkpoint(bs); + } else if (bs->file) { + return bdrv_do_checkpoint(bs->file); + } + + return -1; +} + +int bdrv_stop_replication(BlockDriverState *bs) +{ + BlockDriver *drv = bs->drv; + if (drv && drv->bdrv_stop_replication) { + return drv->bdrv_stop_replication(bs); + } else if (bs->file) { + return bdrv_stop_replication(bs->file); + } + + return -1; +} diff --git a/include/block/block.h b/include/block/block.h index 321295e..632b9fc 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -557,4 +557,14 @@ void bdrv_flush_io_queue(BlockDriverState *bs); BlockAcctStats *bdrv_get_stats(BlockDriverState *bs); +/* Checkpoint control, called in migration/checkpoint thread */ +enum { + COLO_UNPROTECTED_MODE = 0, + COLO_PRIMARY_MODE, + COLO_SECONDARY_MODE, +}; +int bdrv_start_replication(BlockDriverState *bs, int mode); +int bdrv_do_checkpoint(BlockDriverState *bs); +int bdrv_stop_replication(BlockDriverState *bs); + #endif diff --git a/include/block/block_int.h b/include/block/block_int.h index 7ad1950..603f704 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -273,6 +273,18 @@ struct BlockDriver { void (*bdrv_io_unplug)(BlockDriverState *bs); void (*bdrv_flush_io_queue)(BlockDriverState *bs); + + /* Checkpoint control, called in migration/checkpoint thread */ + int (*bdrv_start_replication)(BlockDriverState *bs, int mode); + /* + * Drop Disk buffer when doing checkpoint. + */ + int (*bdrv_do_checkpoint)(BlockDriverState *bs); + /* After failover, we should flush Disk buffer into secondary disk + * and stop block replication. + */ + int (*bdrv_stop_replication)(BlockDriverState *bs); + QLIST_ENTRY(BlockDriver) list; };