From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47041) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WPXxy-0003Dr-J4 for qemu-devel@nongnu.org; Mon, 17 Mar 2014 09:52:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WPXxr-0003A4-Mk for qemu-devel@nongnu.org; Mon, 17 Mar 2014 09:52:34 -0400 Received: from lputeaux-656-01-25-125.w80-12.abo.wanadoo.fr ([80.12.84.125]:56253 helo=paradis.irqsave.net) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WPXxr-00039s-DX for qemu-devel@nongnu.org; Mon, 17 Mar 2014 09:52:27 -0400 Date: Mon, 17 Mar 2014 14:52:25 +0100 From: =?iso-8859-1?Q?Beno=EEt?= Canet Message-ID: <20140317135225.GA3047@irqsave.net> References: <20140314155756.GC3324@irqsave.net> <20140317031231.GA28582@T430.nay.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <20140317031231.GA28582@T430.nay.redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] n ways block filters List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng Cc: =?iso-8859-1?Q?Beno=EEt?= Canet , kwolf@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com, mreitz@redhat.com The Monday 17 Mar 2014 =E0 11:12:31 (+0800), Fam Zheng wrote : > On Fri, 03/14 16:57, Beno=EEt Canet wrote: > >=20 > > Hello list, > >=20 > > I plan to convert throttling to a block filter and write n way thrott= ling > > support. > >=20 > > I discussed a bit with Stefan on the list and we came to the conclusi= on that the > > block filter API need group support. > >=20 > > filter group: > > ------------- > >=20 > > My current plan to implement this is to add the following fields to t= he BlockDriver > > structure. > >=20 > > int bdrv_add_filter_group(const char *name, QDict options); > > int bdrv_reconfigure_filter_group(const char *name, QDict options); > > int bdrv_destroy_filter_group(const char *name); > >=20 > > These three extra method would allow to create, reconfigure or destro= y a block > > filter group. A block filter group contain the shared or non shared s= tate of the > > blockfilter. For throttling it would contains the ThrottleState struc= ture. > >=20 > > Each block filter driver would contains a linked list of linked list = where the > > BDS are registered grouped by filter groups state. >=20 > Sorry I don't fully understand this. Does a filter group contain multip= le block > filters, and every block filter has effect on multiple BDSes? Could you= give an > example? The driver module says block/throttle.c would contains a linked list of T= hrottleGroup in a static variable. Each ThrottleGroup would have a name, a ThrottleState (as in utils/thrott= le.c) and a linked list of members BDS. The s variable of the bdrv_* method of throttle.c would have a pointer to= the ThrottleGroup this BDS is a member of hence the driver can access the ThrottleState So each BDS filter would have only one BDS parent and one BDS ->file chil= d but would also be member of a ThrottleGroup. This would allow to do throttling on a group of devices. Throttling only one device would create a ThrottleGroup of name bds->devi= ce_name containing only the throttled BDS. >=20 > >=20 > > The regular bdrv_open callback would be used to instantiate a block f= ilter and > > add it to a filter group. This method would also take a new-node-name= for the new > > filter. This node-name would become the name of the new filter. > > bdrv_close would cleanup and deregister from a filter group. > >=20 > > An extra filter-group field in the option dict would allow the bdrv_o= pen method > > to register the newly opened block filter in it's filter group. > > The BDS structure would have a direct pointer to it's filter group st= ate. > >=20 > > Utility methods to do the bdrv_add_filter_group bdrv_open then bdrv_s= wap to > > install a new filter can be provided by block.c. The same can be done= for filter > > close and desinstallation. >=20 > So you are defining block filter as a new kind of block driver. Is a fi= lter > always on top above everything else by definition? Yes, A filter is instanciated on top of another BDS. Filters can be stacked on top of another to form a chain. =20 > But I am afraid BlockDriverState is already taking too many responsibil= ities > here (backend, protocol driver, format driver, filter...). I was wonder= ing if > it is clearer to rather introduce bs->filter_list to point to a list of > BlockFilter (a new sturcture, tailored for a block filter), and don't b= other > with bdrv_swap. Hmm blkverify and quorum are already block filters reusing the BlockDrive= r interface. The good this with this is that it would allow to move some co= de from block.c to block/filter_name*.c: block throttling for example. Crypto would be another use case. Best regards Beno=EEt >=20 > Thanks, > Fam >=20 > >=20 > > Legacy throttling QMP API > > ------------------------- > >=20 > > The legacy throttling API would create throttling filters groups cont= aining only > > one BDS. > >=20 > > By default for every 1 way block filter block.c would create a filter= group > > using the BDS id or node-name as group name. This allow for easy file= r removal > > with the bds reference. > >=20 > > Group throttling API > > -------------------- > >=20 > > Commands would be added to create throttling filter groups reconfigur= e and remove > > them. > >=20 > > Two additional commands would be added to create and insert a block f= ilter in a > > given group or close and remove it. > >=20 > > Before I start implementing something what are your thougths on this = ? > >=20 > > Best regards > >=20 > > Beno=EEt > >=20 > >=20 > >=20 >=20