Re: [Qemu-devel] n ways block filters

[Fam Zheng <famz@redhat.com>]

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
> 
> 
>