Peter Xu <pet...@redhat.com> writes: > On Wed, Mar 23, 2016 at 10:33:09AM +0100, Markus Armbruster wrote: >> Peter Xu <pet...@redhat.com> writes: >> >> > On Tue, Mar 22, 2016 at 07:41:17PM +0100, Markus Armbruster wrote: >> >> Markus Armbruster <arm...@redhat.com> writes: >> >> >> +## >> >> >> +# @GICCapability: >> >> >> +# >> >> >> +# This struct describes capability for a specific GIC version. These >> >> >> +# bits are not only decided by QEMU/KVM software version, but also >> >> >> +# decided by the hardware that the program is running upon. >> >> >> +# >> >> >> +# @version: version of GIC to be described. >> >> >> +# >> >> >> +# @emulated: whether current QEMU/hardware supports emulated GIC >> >> >> +# device in user space. >> >> >> +# >> >> >> +# @kernel: whether current QEMU/hardware supports hardware >> >> >> +# accelerated GIC device in kernel. >> >> >> +# >> >> >> +# Since: 2.6 >> >> >> +## >> >> >> +{ 'struct': 'GICCapability', >> >> >> + 'data': { 'version': 'int', >> >> >> + 'emulated': 'bool', >> >> >> + 'kernel': 'bool' } } > > [*] Marking here... > >> So GICCapability essentially tells its users whether certain >> configurations have a chance to work. >> >> I think what's missing in your patch is the connection from >> GICCapability to the actual configuration options. As is, you just have >> to know what options the presence of each possible GICCapability value >> unlocks. It needs to be documented instead. > > What I understand is that, above [*] should have explained what does > each entry mean. E.g., as mentioned in the qapi-schema, there are > explainations about "version", "emulated" and "kernel" key words. If > we go deeper into e.g., "emulated" key word, we will got: > > "whether current QEMU/hardware supports emulated GIC device in user > space." > > So this boolean will tell just as it is explained. > > Maybe I failed to get the point of your review comment... :( Would > you please give an example on how should I better express this > relationship?
Can you tell me what a management application is supposed to do with the information returned by query-gic-capabilities? Not just in general terms, like "using this information, libvirt can warn the user during configuration of guests when specified GIC device type is not supported, but specifics. Something like "-frobnicate mutter=mumble won't work unless query-gic-capabilities reports emulated version 2 is supported" for every piece of configuration that should be vetted against query-gic-capabilities. > (btw, I have updated the commit message in v6 for current patch, to > tell more about why we need this, and why we decided to add this ad > hoc command. I'd be glad if we can continue the discussion based on > that one. Thanks!)