From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47001) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z710r-0003OZ-Qj for qemu-devel@nongnu.org; Mon, 22 Jun 2015 08:39:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z710q-0006pL-Hy for qemu-devel@nongnu.org; Mon, 22 Jun 2015 08:39:45 -0400 Date: Mon, 22 Jun 2015 13:39:35 +0100 From: Stefan Hajnoczi Message-ID: <20150622123935.GB783@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> <20150619104953.GB9807@stefanha-thinkpad.redhat.com> <5584DEA8.80005@gmail.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="tsOsTdHNUZQcU9Ye" Content-Disposition: inline In-Reply-To: <5584DEA8.80005@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 , qemu block , Jiang Yunhong , Dong Eddie , qemu devel , Max Reitz , zhanghailiang , Gonglei , Stefan Hajnoczi , Paolo Bonzini , Yang Hongyang , "Dr. David Alan Gilbert" , Lai Jiangshan --tsOsTdHNUZQcU9Ye Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sat, Jun 20, 2015 at 11:31:52AM +0800, Wen Congyang wrote: > At 2015/6/19 18:49, Stefan Hajnoczi Wrote: > >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? > >>> > >>>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 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 cl= ient. > >>>>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 a= nother > >>>>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). > >>> > >> > >>The problem is that, nbd client is one child of quorum, and quorum must= have 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). > > >=20 > For replication case, we always do checkpoint again and again after > migration. If the > disk is not synced before migration, we will use disk mirgation or mirror > job to sync > it. So we cannot start block replication when migration is running. We ne= ed > that nbd > client is not ready when migration is running, and it is ready between > migration ends > and checkpoint begins. Using a monotir command add the nbd client will ca= use > larger > downtime. So if the nbd client has been added(only not connect to the nbd > server), > we can connect to nbd server automatically. I'm not sure I understand you. Can you rephrase this? The NBD connect should not be performed during downtime, regardless of whether a monitor command is used or not. Stefan --tsOsTdHNUZQcU9Ye Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJViAIHAAoJEJykq7OBq3PI6RIIAMCQi9fEPXO2R5Xwj/AlyUQr ViUb/iw+EQ0ZtpCn7mBypfO5lze2uJurBlxKrLjq/vRO6hDLzOwrvjbp7SHU36Mx MWcKe/3jEX9hsEg4rWetvxUWVofB/g86iwfa7Oe139iBBIb8hEVW/9Umgarj1jwq E69eGoeOMUsdDBTreqzuXh3bYOfFh7BekkxIAHwvAYMfUfVLGjdXJh1zp626BPoS 4VtBW+pjHBcZM+wGUEfPCdiwj+f2tY6Cx+nObHSHzWmyt4+SprXF5SW+m4uhY0Z1 fjQ2rv/fSj+asU/UeuOHpd/U08tqjgZGuIMyaLQ1e+JvDfAYXM3w4zf5DD8uTP0= =R7YL -----END PGP SIGNATURE----- --tsOsTdHNUZQcU9Ye--