On Wed, 2019-08-21 at 13:31 +0200, Markus Armbruster wrote: > Maxim Levitsky <mlevi...@redhat.com> writes: > > > On Thu, 2019-08-15 at 10:00 -0500, Eric Blake wrote: > > > On 8/15/19 9:44 AM, Maxim Levitsky wrote: > > > > > > > > > > Does the idea of a union type with a default value for the > > > > > > > discriminator > > > > > > > help? Maybe we have a discriminator which defaults to 'auto', > > > > > > > and add a > > > > > > > union branch 'auto':'any'. During creation, if the > > > > > > > "driver":"auto" > > > > > > > branch is selected (usually implicitly by omitting "driver", but > > > > > > > also > > > > > > > possible explicitly), the creation attempt is rejected as invalid > > > > > > > regardless of the contents of the remaining 'any'. But during > > > > > > > amend > > > > > > > usage, if the 'auto' branch is selected, we then add in the proper > > > > > > > "driver":"xyz" and reparse the QAPI object to determine if the > > > > > > > remaining > > > > > > > fields in 'any' still meet the specification for the required > > > > > > > driver branch. > > > > > > > > > > > > > > This idea may still require some tweaks to the QAPI generator, > > > > > > > but it's > > > > > > > the best I can come up with for a way to parse an arbitrary JSON > > > > > > > object > > > > > > > with unknown validation, then reparse it again after adding more > > > > > > > information that would constrain the parse differently. > > > > > > > > > > > > Feels like this would be a lot of code just to allow the client to > > > > > > omit > > > > > > passing a value that it knows anyway. If this were a human > > > > > > interface, I > > > > > > could understand the desire to make commands less verbose, but for > > > > > > QMP I > > > > > > honestly don't see the point when it's not trivial. > > > > > > > > > > Seconded. > > > > > > > > > > > > But what about my suggestion of adding something like: > > > > > > > > { 'union': 'BlockdevAmendOptions', > > > > > > > > 'base': { > > > > 'node-name': 'str' }, > > > > > > > > 'discriminator': { 'get_block_driver(node-name)' } , > > > > > > Not worth it. It makes the QAPI generator more complex (to invoke > > > arbitrary code instead of a fixed name) just to avoid a little bit of > > > complexity in the caller (which is assumed to be a computer, and thus > > > shouldn't have a hard time providing a sane 'driver' unconditionally). > > > An HMP wrapper around the QMP command can do whatever magic it needs to > > > omit driver, but making driver mandatory for QMP is just fine. > > > > All right! I kind of not agree with that, since I think even though QMP is > > a machine language, > > it still should be consistent since humans still use it, even if this is > > humans that code some > > tool that use it. > > > > I won't argue with you though, let it be like that. > > Software's fundamental limit is complexity. We need to pick what we use > it for. Sometimes, that means saying no to things that would be nice to > have.
I fully agree with that and that is usually the exact reason I argue about such things: Sometimes avoiding complexity in one place, and/or breaking consistency can introduce complexity in other place (like libvirt). In this particular case, I won't argue about this, but still it kind of feels like it is a precedent of requiring the user to supply redundant data. Of all issues all of you pointed out (thanks!!) this is probably the least important one that I should be arguing about, so let it be like you say. Best regards, Maxim Levitsky