From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43046) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z5cKi-0005Em-27 for qemu-devel@nongnu.org; Thu, 18 Jun 2015 12:06:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z5cKh-0007Es-5s for qemu-devel@nongnu.org; Thu, 18 Jun 2015 12:06:28 -0400 Date: Thu, 18 Jun 2015 17:06:18 +0100 From: Stefan Hajnoczi Message-ID: <20150618160618.GF822@stefanha-thinkpad.redhat.com> References: <1434617361-17778-1-git-send-email-wency@cn.fujitsu.com> <1434617361-17778-8-git-send-email-wency@cn.fujitsu.com> <20150618125533.GE25387@stefanha-thinkpad.redhat.com> <5582D777.8060202@gmail.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="KJY2Ze80yH5MUxol" Content-Disposition: inline In-Reply-To: <5582D777.8060202@gmail.com> Subject: Re: [Qemu-devel] [Qemu-block] [PATCH COLO-Block v6 07/16] Add new block driver interface to connect/disconnect the remote target List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Wen Congyang Cc: Kevin Wolf , Fam Zheng , Lai Jiangshan , qemu block , Stefan Hajnoczi , Jiang Yunhong , Dong Eddie , qemu devel , Max Reitz , Gonglei , Paolo Bonzini , Yang Hongyang , "Dr. David Alan Gilbert" , zhanghailiang --KJY2Ze80yH5MUxol Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Jun 18, 2015 at 10:36:39PM +0800, Wen Congyang wrote: > At 2015/6/18 20:55, Stefan Hajnoczi Wrote: > >On Thu, Jun 18, 2015 at 04:49:12PM +0800, Wen Congyang wrote: > >>+void bdrv_connect(BlockDriverState *bs, Error **errp) > >>+{ > >>+ BlockDriver *drv =3D bs->drv; > >>+ > >>+ if (drv && drv->bdrv_connect) { > >>+ drv->bdrv_connect(bs, errp); > >>+ } else if (bs->file) { > >>+ bdrv_connect(bs->file, errp); > >>+ } else { > >>+ error_setg(errp, "this feature or command is not currently sup= ported"); > >>+ } > >>+} > >>+ > >>+void bdrv_disconnect(BlockDriverState *bs) > >>+{ > >>+ BlockDriver *drv =3D bs->drv; > >>+ > >>+ if (drv && drv->bdrv_disconnect) { > >>+ drv->bdrv_disconnect(bs); > >>+ } else if (bs->file) { > >>+ bdrv_disconnect(bs->file); > >>+ } > >>+} > > > >Please add doc comments describing the semantics of these commands. >=20 > Where should it be documented? In the header file? block.h doesn't document prototypes in the header file, please document the function definition in block.c. (QEMU is not consistent here, some places do it the other way around.) > >Why are these operations needed when there is already a bs->drv =3D=3D N= ULL > >case which means the BDS is not ready for read/write? > > >=20 > The purpos is that: don't connect to nbd server when opening a nbd client. > connect/disconnect > to nbd server when we need to do it. >=20 > IIUC, if bs->drv is NULL, it means that the driver is ejected? Here, > connect/disconnect > means that connect/disconnect to remote target(The target may be in anoth= er > host). Connect/disconnect puts something on the QEMU command-line that isn't ready at startup time. How about using monitor commands to add objects when needed instead? That is cleaner because it doesn't introduce a new state (which is only implemented for nbd). --KJY2Ze80yH5MUxol Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJVgux6AAoJEJykq7OBq3PIw3QIAIKO3L49r8y6HXfxYp0f4tbb g9bez8XXrs/KIfK96yjrO7ejeU81jH4zWwt44UDH35bkr5hDHoHFGhGWpt1wM5qU e0F6pXgOpi1sEJ5RPhgDziVYhm/tWEAnfnkdgll08o1KE2kxF0ggBLUlmgSRh4Ci 3VXRxDIsBLYoH1FDakYcKxoq8UbUTJ4U9DocbhLcDV4ARVPaPU2Ci+Vxz5NaY+24 HLuLvl3wZexBvcSPnK7dSPqSvgBHe1QYgNlnAA6CtrndvYdiqv1p46diZM8hx3A+ AAGqeeqwd8quv7tTLQw+CcEOa0optqX+O/bIs+ib3vafc+k9h9g6G3rhFFaq4W8= =NOG6 -----END PGP SIGNATURE----- --KJY2Ze80yH5MUxol--