From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33516) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z7ZAl-0000Q2-9s for qemu-devel@nongnu.org; Tue, 23 Jun 2015 21:08:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z7ZAe-0006s9-T2 for qemu-devel@nongnu.org; Tue, 23 Jun 2015 21:08:15 -0400 Message-ID: <558A03DD.9050407@cn.fujitsu.com> Date: Wed, 24 Jun 2015 09:11:57 +0800 From: Wen Congyang MIME-Version: 1.0 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> In-Reply-To: <20150619104953.GB9807@stefanha-thinkpad.redhat.com> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit 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: Stefan Hajnoczi Cc: Kevin Wolf , Fam Zheng , Wen Congyang , qemu block , Stefan Hajnoczi , Jiang Yunhong , Dong Eddie , qemu devel , Max Reitz , Alberto Garcia , zhanghailiang , Gonglei , Paolo Bonzini , Yang Hongyang , "Dr. David Alan Gilbert" , Lai Jiangshan On 06/19/2015 06:49 PM, 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 = 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 = 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 == 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 client. >>>> 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 another >>>> 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). > CCing Alberto Garcia for the quorum block driver.