| Message ID | 1423710438-14377-8-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/nbd.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 55 insertions(+) So by now to me it looks like you're using bdrv_start_replication() on the primary VM to start transferring data through the NBD connection to the secondary VM. I guess this is what you were discussing with Fam and John, whether there'd be a better way to do it by using functionality that is already (or is about to become) part of qemu, right? > diff --git a/block/nbd.c b/block/nbd.c > index 19b9200..1ff6ecf 100644 > --- a/block/nbd.c > +++ b/block/nbd.c > @@ -445,6 +445,58 @@ static void nbd_refresh_filename(BlockDriverState *bs) > bs->full_open_options = opts; > } > > +static int nbd_start_replication(BlockDriverState *bs, int mode) > +{ > + BDRVNBDState *s = bs->opaque; > + Error *local_err = NULL; > + int ret; > + > + /* > + * TODO: support COLO_SECONDARY_MODE if we allow secondary > + * QEMU becoming primary QEMU. > + */ > + if (mode != COLO_PRIMARY_MODE) { > + return -1; Once again, I'd like -ENOTSUP more (or -EINVAL or whatever you prefer). > + } > + > + if (s->connected) { > + return -1; > + } > + > + /* TODO: NBD client should be one child of quorum, how to verify it? */ Again, why would you care about that? Other than "It's how it's intended to be used". > + ret = nbd_connect_server(bs, &local_err); > + if (local_err) { > + error_free(local_err); > + } If you'd use the Error API you'd be able to propagate the error. Max > + > + return ret; > +} > + > +static int nbd_do_checkpoint(BlockDriverState *bs) > +{ > + BDRVNBDState *s = bs->opaque; > + > + if (!s->connected) { > + return -1; > + } > + > + return 0; > +} > + > +static int nbd_stop_replication(BlockDriverState *bs) > +{ > + BDRVNBDState *s = bs->opaque; > + > + if (!s->connected) { > + return -1; > + } > + > + nbd_client_session_close(&s->client); > + s->connected = false; > + > + return 0; > +} > + > static BlockDriver bdrv_nbd = { > .format_name = "nbd", > .protocol_name = "nbd", > @@ -514,6 +566,9 @@ static BlockDriver bdrv_nbd_colo = { > .bdrv_detach_aio_context = nbd_detach_aio_context, > .bdrv_attach_aio_context = nbd_attach_aio_context, > .bdrv_refresh_filename = nbd_refresh_filename, > + .bdrv_start_replication = nbd_start_replication, > + .bdrv_do_checkpoint = nbd_do_checkpoint, > + .bdrv_stop_replication = nbd_stop_replication, > > .has_variable_length = true, > };
On 23/02/2015 22:41, Max Reitz wrote: >> >> } >> +static int nbd_start_replication(BlockDriverState *bs, int mode) >> +{ >> + BDRVNBDState *s = bs->opaque; >> + Error *local_err = NULL; >> + int ret; >> + >> + /* >> + * TODO: support COLO_SECONDARY_MODE if we allow secondary >> + * QEMU becoming primary QEMU. >> + */ >> + if (mode != COLO_PRIMARY_MODE) { >> + return -1; > > Once again, I'd like -ENOTSUP more (or -EINVAL or whatever you prefer). Using the Error API is the right thing to do here. Paolo
diff --git a/block/nbd.c b/block/nbd.c index 19b9200..1ff6ecf 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -445,6 +445,58 @@ static void nbd_refresh_filename(BlockDriverState *bs) bs->full_open_options = opts; } +static int nbd_start_replication(BlockDriverState *bs, int mode) +{ + BDRVNBDState *s = bs->opaque; + Error *local_err = NULL; + int ret; + + /* + * TODO: support COLO_SECONDARY_MODE if we allow secondary + * QEMU becoming primary QEMU. + */ + if (mode != COLO_PRIMARY_MODE) { + return -1; + } + + if (s->connected) { + return -1; + } + + /* TODO: NBD client should be one child of quorum, how to verify it? */ + ret = nbd_connect_server(bs, &local_err); + if (local_err) { + error_free(local_err); + } + + return ret; +} + +static int nbd_do_checkpoint(BlockDriverState *bs) +{ + BDRVNBDState *s = bs->opaque; + + if (!s->connected) { + return -1; + } + + return 0; +} + +static int nbd_stop_replication(BlockDriverState *bs) +{ + BDRVNBDState *s = bs->opaque; + + if (!s->connected) { + return -1; + } + + nbd_client_session_close(&s->client); + s->connected = false; + + return 0; +} + static BlockDriver bdrv_nbd = { .format_name = "nbd", .protocol_name = "nbd", @@ -514,6 +566,9 @@ static BlockDriver bdrv_nbd_colo = { .bdrv_detach_aio_context = nbd_detach_aio_context, .bdrv_attach_aio_context = nbd_attach_aio_context, .bdrv_refresh_filename = nbd_refresh_filename, + .bdrv_start_replication = nbd_start_replication, + .bdrv_do_checkpoint = nbd_do_checkpoint, + .bdrv_stop_replication = nbd_stop_replication, .has_variable_length = true, };