From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37513) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z5ts2-0003hW-RE for qemu-devel@nongnu.org; Fri, 19 Jun 2015 06:50:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z5ts1-0002IH-T0 for qemu-devel@nongnu.org; Fri, 19 Jun 2015 06:50:02 -0400 Date: Fri, 19 Jun 2015 11:49:53 +0100 From: Stefan Hajnoczi Message-ID: <20150619104953.GB9807@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> <20150618160618.GF822@stefanha-thinkpad.redhat.com> <55836860.2010401@cn.fujitsu.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="GRPZ8SYKNexpdSJ7" Content-Disposition: inline In-Reply-To: <55836860.2010401@cn.fujitsu.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 , Wen Congyang , qemu block , Stefan Hajnoczi , Jiang Yunhong , Dong Eddie , qemu devel , Max Reitz , zhanghailiang , Gonglei , Paolo Bonzini , Yang Hongyang , "Dr. David Alan Gilbert" , Lai Jiangshan --GRPZ8SYKNexpdSJ7 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Jun 19, 2015 at 08:54:56AM +0800, Wen Congyang wrote: > On 06/19/2015 12:06 AM, Stefan Hajnoczi wrote: > > 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 = supported"); > >>>> + } > >>>> +} > >>>> + > >>>> +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. > >> > >> Where should it be documented? In the header file? > >=20 > > 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.) > >=20 > >>> Why are these operations needed when there is already a bs->drv =3D= =3D NULL > >>> case which means the BDS is not ready for read/write? > >>> > >> > >> The purpos is that: don't connect to nbd server when opening a nbd cli= ent. > >> connect/disconnect > >> to nbd server when we need to do it. > >> > >> 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 an= other > >> host). > >=20 > > Connect/disconnect puts something on the QEMU command-line that isn't > > ready at startup time. > >=20 > > How about using monitor commands to add objects when needed instead? > >=20 > > That is cleaner because it doesn't introduce a new state (which is only > > implemented for nbd). > >=20 >=20 > The problem is that, nbd client is one child of quorum, and quorum must h= ave more > than one child. The nbd server is not ready until colo is running. A monitor command to hot add/remove quorum children solves this problem and could also be used in other scenarios (e.g. user decides to take a quorum child offline). --GRPZ8SYKNexpdSJ7 Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJVg/PRAAoJEJykq7OBq3PI30AIAKKteffUrRuazAVpmhRI3W/m VJfdE4JngKd+ugf6TfXSRiU8XJUmDuhXtMB7xQqqIlc9l/qoanMtEqjyl2Stzmij Qed5QrlnJJ6GkUlA3b7bH4hAclLQ/nMEbWjJD3AsJAG/QBvqslcCAR5oYtb/jz74 jkeJSVCncptYrybviQQXvY/6kk5FNr2UYOhhRsGdgux1MJ+K2V91UkiiS4GmY9Nq xAeClkZ4pB2Peszy1u7ZoyVGpvjkfI7WzkpKnEnLwhio+JHK2ggTsX/sc5WyXtQ9 kBA0wAOurQcF2wFupmOXl/iS0j61/FHrm79hJpw/GKbBkMbPEuHDyf0EwgArm4g= =cQAq -----END PGP SIGNATURE----- --GRPZ8SYKNexpdSJ7--