From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39995) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z7HtP-0003df-Ta for qemu-devel@nongnu.org; Tue, 23 Jun 2015 02:41:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z7HtL-0006Zi-2X for qemu-devel@nongnu.org; Tue, 23 Jun 2015 02:41:11 -0400 Message-ID: <55890069.6070902@cn.fujitsu.com> Date: Tue, 23 Jun 2015 14:44: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> In-Reply-To: <20150618160618.GF822@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 , 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 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? We have decieded use this way here: http://lists.gnu.org/archive/html/qemu-devel/2015-04/msg02975.html Thanks Wen Congyang > > That is cleaner because it doesn't introduce a new state (which is only > implemented for nbd). >