From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50548) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aihnW-0002Jh-3m for qemu-devel@nongnu.org; Wed, 23 Mar 2016 08:22:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aihnV-0000Ot-6N for qemu-devel@nongnu.org; Wed, 23 Mar 2016 08:22:02 -0400 From: Markus Armbruster References: <1458271654-23706-1-git-send-email-peterx@redhat.com> <1458271654-23706-2-git-send-email-peterx@redhat.com> <87zitqjrhj.fsf@blackfin.pond.sub.org> <87vb4eicdu.fsf@blackfin.pond.sub.org> <20160323025819.GD28183@pxdev.xzpeter.org> <87shzhbktm.fsf@blackfin.pond.sub.org> <20160323114830.GR28183@pxdev.xzpeter.org> Date: Wed, 23 Mar 2016 13:21:52 +0100 In-Reply-To: <20160323114830.GR28183@pxdev.xzpeter.org> (Peter Xu's message of "Wed, 23 Mar 2016 19:48:30 +0800") Message-ID: <8737rh8jvj.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH v5 1/5] arm: qmp: add GICCapability struct List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Xu Cc: wei@redhat.com, peter.maydell@linaro.org, drjones@redhat.com, mdroth@linux.vnet.ibm.com, qemu-devel@nongnu.org, abologna@redhat.com, qemu-arm@nongnu.org Peter Xu writes: > On Wed, Mar 23, 2016 at 10:33:09AM +0100, Markus Armbruster wrote: >> Peter Xu writes: >> >> > On Tue, Mar 22, 2016 at 07:41:17PM +0100, Markus Armbruster wrote: >> >> Markus Armbruster 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!)