On Thu, Oct 09, 2014 at 04:58:22PM +0800, Fam Zheng wrote: > On Wed, 10/08 11:05, Benoît Canet wrote: > > On Wed, Oct 08, 2014 at 02:53:38PM +0800, Fam Zheng wrote: > > > > > > Does this mean that after this series, all the throttle_states must be > > > contained inside its own throttle group? If so, we could embed > > > ThrottleGroup > > > fields in ThrottleState. > > > > > > It's weird when a function called throttle_group_compare takes a > > > parameter of > > > ThrottleState pointer, and cast it back to ThrottleGroup with > > > container_of. > > > > It's done like this to fullfill a design goal: the throttle should be > > reusable > > without the groups and any reference to block related stuff. > > So it's just a way to split the responsabilities. > > I see. > > Having both ThrottleGroup and ThrottleState interfaces is more complicated > than > just use ThrottleGroup, where a one-member group is exactly the same as > ThrottleState.
Here my interest conflict between a short term strategy (comply and getting these patches merged asap) and the technical advantage of keeping ThrottleState and ThrottleGroup separated. orthogonality ------------- An advantage to keep ThrottleState and ThrottleGroup separated is that the two roles of implementing the throttle itself and implementing the behavior of a group are keep separated. As a result each piece of code is simpler to write, review and test. genericity ---------- When writing the initial throttle code I carefully designed it to be independant of BlockDriverState. As a result the throttle code lives in util/ and is easilly reusable into whatever we want (a device model or network throttling). It is mean, designed and written to be as generic as possible. So yes merging ThrottleState and ThrottleGroup would result in the seemlingly nice thing that one structure is simpler than two structure. But it would also imply the fact that I really hate that this pesky BlockDriverState structure would become tied to ThrottleState. If we do this ThrottleState would move out of util/ to block/ and we would have lost the ability to reuse this infrastructure. It would be a short term gain and a long term loss. Another fact is that the rationale "1 structure is simpler than 2" is not alway relevant: look for BlockBackend that we are desesperately trying to move out of BlockDriverState. Best regards Benoît > > Fam
