On Fri, 03/14 16:57, Benoît Canet wrote: > > Hello list, > > I plan to convert throttling to a block filter and write n way throttling > support. > > I discussed a bit with Stefan on the list and we came to the conclusion that > the > block filter API need group support. > > filter group: > ------------- > > My current plan to implement this is to add the following fields to the > BlockDriver > structure. > > 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); > > These three extra method would allow to create, reconfigure or destroy a block > filter group. A block filter group contain the shared or non shared state of > the > blockfilter. For throttling it would contains the ThrottleState structure. > > Each block filter driver would contains a linked list of linked list where the > BDS are registered grouped by filter groups state.
Sorry I don't fully understand this. Does a filter group contain multiple block filters, and every block filter has effect on multiple BDSes? Could you give an example? > > The regular bdrv_open callback would be used to instantiate a block filter 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. > > An extra filter-group field in the option dict would allow the bdrv_open > 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 state. > > Utility methods to do the bdrv_add_filter_group bdrv_open then bdrv_swap to > install a new filter can be provided by block.c. The same can be done for > filter > close and desinstallation. So you are defining block filter as a new kind of block driver. Is a filter always on top above everything else by definition? But I am afraid BlockDriverState is already taking too many responsibilities here (backend, protocol driver, format driver, filter...). I was wondering 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 bother with bdrv_swap. Thanks, Fam > > Legacy throttling QMP API > ------------------------- > > The legacy throttling API would create throttling filters groups containing > only > one BDS. > > 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 filer removal > with the bds reference. > > Group throttling API > -------------------- > > Commands would be added to create throttling filter groups reconfigure and > remove > them. > > Two additional commands would be added to create and insert a block filter in > a > given group or close and remove it. > > Before I start implementing something what are your thougths on this ? > > Best regards > > Benoît > > >
