From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52328) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YQ0ba-0005FG-8L for qemu-devel@nongnu.org; Mon, 23 Feb 2015 16:32:00 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YQ0bT-0003DH-Po for qemu-devel@nongnu.org; Mon, 23 Feb 2015 16:31:54 -0500 Received: from mx1.redhat.com ([209.132.183.28]:45069) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YQ0bT-0003DC-Hk for qemu-devel@nongnu.org; Mon, 23 Feb 2015 16:31:47 -0500 Message-ID: <54EB9C36.4060606@redhat.com> Date: Mon, 23 Feb 2015 16:31:34 -0500 From: Max Reitz MIME-Version: 1.0 References: <1423710438-14377-1-git-send-email-wency@cn.fujitsu.com> <1423710438-14377-7-git-send-email-wency@cn.fujitsu.com> In-Reply-To: <1423710438-14377-7-git-send-email-wency@cn.fujitsu.com> Content-Type: text/plain; charset=iso-8859-15; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC PATCH 06/14] NBD client: connect to nbd server later List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Wen Congyang , qemu devel , Kevin Wolf , Stefan Hajnoczi , Paolo Bonzini Cc: Lai Jiangshan , Jiang Yunhong , Dong Eddie , "Dr. David Alan Gilbert" , Gonglei , Yang Hongyang , zhanghailiang 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 > Signed-off-by: zhanghailiang > Signed-off-by: Gonglei > --- > 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) > + > 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; > } > As I proposed before, can you make nbd_getlength() return -ENOTCONN or something unique in case s->connected is false? I think that'd be better than returning 0 (which is a valid value). Max > static int64_t nbd_getlength(BlockDriverState *bs) > @@ -335,6 +381,10 @@ static void nbd_detach_aio_context(BlockDriverState *bs) > { > BDRVNBDState *s = bs->opaque; > > + if (!s->connected) { > + return; > + } > + > nbd_client_session_detach_aio_context(&s->client); > } > > @@ -343,6 +393,10 @@ static void nbd_attach_aio_context(BlockDriverState *bs, > { > BDRVNBDState *s = bs->opaque; > > + if (!s->connected) { > + return; > + } > + > nbd_client_session_attach_aio_context(&s->client, new_context); > } > > @@ -445,11 +499,31 @@ static BlockDriver bdrv_nbd_unix = { > .bdrv_refresh_filename = nbd_refresh_filename, > }; > > +static BlockDriver bdrv_nbd_colo = { > + .format_name = "nbd+colo", > + .protocol_name = "nbd+colo", > + .instance_size = sizeof(BDRVNBDState), > + .bdrv_parse_filename = nbd_parse_filename, > + .bdrv_file_open = nbd_open_colo, > + .bdrv_co_readv = nbd_co_readv, > + .bdrv_co_writev = nbd_co_writev, > + .bdrv_close = nbd_close, > + .bdrv_co_flush_to_os = nbd_co_flush, > + .bdrv_co_discard = nbd_co_discard, > + .bdrv_getlength = nbd_getlength, > + .bdrv_detach_aio_context = nbd_detach_aio_context, > + .bdrv_attach_aio_context = nbd_attach_aio_context, > + .bdrv_refresh_filename = nbd_refresh_filename, > + > + .has_variable_length = true, > +}; > + > static void bdrv_nbd_init(void) > { > bdrv_register(&bdrv_nbd); > bdrv_register(&bdrv_nbd_tcp); > bdrv_register(&bdrv_nbd_unix); > + bdrv_register(&bdrv_nbd_colo); > } > > block_init(bdrv_nbd_init);