Markus Armbruster <arm...@redhat.com> writes: > Eric Blake <ebl...@redhat.com> writes: > >> On 08/10/2017 09:06 AM, Pradeep Jagadeesh wrote: >> >>>>> It's not "moving it back", it's keeping it where it is. But I see no big >>>>> problem with moving it to a common file either. >>>> >>>> I'd rather not put every struct shared across subsystem boundaries in >>>> its own file. >>>> >>>> We can keep it right where it is for now. Bonus: more readable diff. >>>> If we start sharing more throttle-related material than just a struct, >>>> we can reconsider. >>>> >>>> We could also move it to the existing file for common stuff: >>>> qapi/common.json. Not a great fit, though. >>> >>> So, the final conclusion is to move to common.json? >> >> No. >> >> If more than one .json file would benefit by including the definition, >> then put it in a separate file that both .json include from. > > This is the case. > > Your opinion is incompatible with mine, stated above. > >> But if only one .json file would be including a new file, then just >> inline the struct directly into that one original file (in this case, >> block-core.json) instead of creating a separate file (so no to needing >> iothrottle.json), or putting the code in yet a different file than the >> one that is using the struct (so no to putting it in common.json). > > This is no longer the case. > > 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. 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. 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. If the block folks should continue to maintain IOThrottle, then it should stay put 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?