On Aug 16, 2017 11:41 PM, "Markus Armbruster" <arm...@redhat.com> wrote:
Eric Blake <ebl...@redhat.com> writes: > On 08/16/2017 11:13 AM, Markus Armbruster wrote: >> Markus Armbruster <arm...@redhat.com> writes: >> > >>> >>> Conclusion: no consensus, yet. >> >> All right, let's start over and try to resolve the impasse and/or >> misunderstanding. >> >> Type BlockIOThrottle lives in qapi/block-core.json, and is used by QMP >> command block_set_io_throttle. Since 1.1. >> >> Pradeep has a use case for throttling in fsdev. Instead of duplicating >> the relevant parts of BlockIOThrottle, qmp_block_set_io_throttle() and >> hmp_block_set_io_throttle(), he factors them out smartly, into >> >> * [PATCH 2] IOThrottle, base type of BlockIOThrottle >> >> * [PATCH 3] throttle_set_io_limits(), called by >> qmp_block_set_io_throttle() >> >> * [PATCH 4] hmp_initialize_io_throttle(), called by >> hmp_block_set_io_throttle() >> >> throttle_set_io_limits() goes into existing util/throttle.c, and >> hmp_initialize_io_throttle() goes into existing hmp.c. The question is >> where IOThrottle should go. > > Good summary. > >> >> Pradeep proposes to put it in new qapi/throttle.json. Certainly >> defensible, but I really don't like putting every little thing shared >> across subsystem boundaries into its own schema file. > > I agree with the dislike of creating new files, if an existing file is > adequate. > >> >> Let me step back and discuss why we split the QAPI schema into multiple >> files in the first place. For me, the one and only reason is >> MAINTAINERS. > > Indeed, that's a good description of why splits would be appropriate. > So the obvious next question is if this is a case that needs a new > maintainer. > >> >> If the block folks should continue to maintain IOThrottle, then it >> should stay put in block-core.json. > > I think Manos' work on making throttling a filter driver at the block > layer is proof enough that it it is still fine to keep throttling > maintained in block-core.json. > >> >> If somebody else should start maintaining it, it should move. We'd need >> a suitable entry in MAINTAINERS then. >> >> I don't see why maintenance should change, and therefore believe it >> should stay put. >> >> Eric? > > I think we're in violent agreement: don't create a new file, and having > the new factored type live in block-core.json is the best fit because we > haven't come up with any reasons why it needs to be split. Thanks, Eric. Pradeep, please put IOThrottle right before BlocIOThrottle in block-core.json. Use it in fsdev.json without including block-core.json. Sorry for the delay. Thanks for all the clarifications, sure I will move iothrottle code to block-core.json. Thanks, Pradeep