On 02/24/2015 05:31 AM, Max Reitz wrote: > On 2015-02-11 at 22:07, Wen Congyang wrote: >> The secondary qemu starts later than the primary qemu, so we >> cannot connect to nbd server in bdrv_open(). >> >> 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> >> --- >> block/nbd.c | 100 >> ++++++++++++++++++++++++++++++++++++++++++++++++++++-------- >> 1 file changed, 87 insertions(+), 13 deletions(-) >> >> diff --git a/block/nbd.c b/block/nbd.c >> index b05d1d0..19b9200 100644 >> --- a/block/nbd.c >> +++ b/block/nbd.c >> @@ -44,6 +44,8 @@ >> typedef struct BDRVNBDState { >> NbdClientSession client; >> QemuOpts *socket_opts; >> + char *export; >> + bool connected; >> } BDRVNBDState; >> static int nbd_parse_uri(const char *filename, QDict *options) >> @@ -247,20 +249,10 @@ static int nbd_establish_connection(BlockDriverState >> *bs, Error **errp) >> return sock; >> } >> -static int nbd_open(BlockDriverState *bs, QDict *options, int flags, >> - Error **errp) >> +static int nbd_connect_server(BlockDriverState *bs, Error **errp) >> { >> BDRVNBDState *s = bs->opaque; >> - char *export = NULL; >> int result, sock; >> - Error *local_err = NULL; >> - >> - /* Pop the config into our state object. Exit if invalid. */ >> - nbd_config(s, options, &export, &local_err); >> - if (local_err) { >> - error_propagate(errp, local_err); >> - return -EINVAL; >> - } >> /* establish TCP connection, return error if it fails >> * TODO: Configurable retry-until-timeout behaviour. >> @@ -271,16 +263,57 @@ static int nbd_open(BlockDriverState *bs, QDict >> *options, int flags, >> } >> /* NBD handshake */ >> - result = nbd_client_session_init(&s->client, bs, sock, export, errp); >> - g_free(export); >> + result = nbd_client_session_init(&s->client, bs, sock, s->export, errp); >> + g_free(s->export); >> + s->export = NULL; >> + if (!result) { >> + s->connected = true; >> + } >> + >> return result; >> } >> +static int nbd_open(BlockDriverState *bs, QDict *options, int flags, >> + Error **errp) >> +{ >> + BDRVNBDState *s = bs->opaque; >> + Error *local_err = NULL; >> + >> + /* Pop the config into our state object. Exit if invalid. */ >> + nbd_config(s, options, &s->export, &local_err); >> + if (local_err) { >> + error_propagate(errp, local_err); >> + return -EINVAL; >> + } >> + >> + return nbd_connect_server(bs, errp); >> +} >> + >> +static int nbd_open_colo(BlockDriverState *bs, QDict *options, int flags, >> + Error **errp) >> +{ >> + BDRVNBDState *s = bs->opaque; >> + Error *local_err = NULL; >> + >> + /* Pop the config into our state object. Exit if invalid. */ >> + nbd_config(s, options, &s->export, &local_err); >> + if (local_err) { >> + error_propagate(errp, local_err); >> + return -EINVAL; >> + } >> + >> + return 0; >> +} >> + >> static int nbd_co_readv(BlockDriverState *bs, int64_t sector_num, >> int nb_sectors, QEMUIOVector *qiov) >> { >> BDRVNBDState *s = bs->opaque; >> + if (!s->connected) { >> + return -EIO; >> + } >> + >> return nbd_client_session_co_readv(&s->client, sector_num, >> nb_sectors, qiov); >> } >> @@ -290,6 +323,10 @@ static int nbd_co_writev(BlockDriverState *bs, int64_t >> sector_num, >> { >> BDRVNBDState *s = bs->opaque; >> + if (!s->connected) { >> + return 0; >> + } > > Would it break anything to return -EIO here as well? (And in all the > following functions)
1. nbd_co_writev() If one child returns error, quorum will report it. There may be many write requests before we connect to nbd server, so there are too many qapi events... 2. nbd_co_flush() If quorum only have two children, and nbd client is the last one, quorum_co_flush() will return -EIO. 3. nbd_co_discard() quorum doens't call bdrv_co_discard(), so it is OK to return -EIO here. So only nbd_co_discard() can return -EIO. Thanks Wen Congyang > >> + >> return nbd_client_session_co_writev(&s->client, sector_num, >> nb_sectors, qiov); >> } >> @@ -298,6 +335,10 @@ static int nbd_co_flush(BlockDriverState *bs) >> { >> BDRVNBDState *s = bs->opaque; >> + if (!s->connected) { >> + return 0; >> + } >> + >> return nbd_client_session_co_flush(&s->client); >> } >> @@ -312,6 +353,10 @@ static int nbd_co_discard(BlockDriverState *bs, >> int64_t sector_num, >> { >> BDRVNBDState *s = bs->opaque; >> + if (!s->connected) { >> + return 0; >> + } >> + >> return nbd_client_session_co_discard(&s->client, sector_num, >> nb_sectors); >> } >> @@ -322,6 +367,7 @@ static void nbd_close(BlockDriverState *bs) >> qemu_opts_del(s->socket_opts); >> nbd_client_session_close(&s->client); >> + s->connected = false; >> } >> >