From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NImZS-0007m3-Gz for qemu-devel@nongnu.org; Thu, 10 Dec 2009 12:12:54 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1NImZN-0007f4-JO for qemu-devel@nongnu.org; Thu, 10 Dec 2009 12:12:54 -0500 Received: from [199.232.76.173] (port=42557 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NImZN-0007ep-CO for qemu-devel@nongnu.org; Thu, 10 Dec 2009 12:12:49 -0500 Received: from mx1.redhat.com ([209.132.183.28]:28865) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NImZN-0005FD-PY for qemu-devel@nongnu.org; Thu, 10 Dec 2009 12:12:50 -0500 Date: Thu, 10 Dec 2009 15:12:30 -0200 From: Luiz Capitulino Subject: Re: [Qemu-devel] [PATCH 10/19] monitor: Convert do_info_name() to QObject Message-ID: <20091210151230.056fb9d9@doriath> In-Reply-To: <4B2129AD.3080701@redhat.com> References: <1260376078-8694-1-git-send-email-lcapitulino@redhat.com> <1260376078-8694-11-git-send-email-lcapitulino@redhat.com> <20091210095237.38cafe5c@doriath> <4B20F003.6070409@linux.vnet.ibm.com> <4B2119F9.6010003@redhat.com> <4B211BE2.60907@linux.vnet.ibm.com> <4B211D6F.7020906@redhat.com> <4B211FDF.4070509@linux.vnet.ibm.com> <4B2120C6.1020004@redhat.com> <20091210145457.25ba1f09@doriath> <4B2129AD.3080701@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Avi Kivity Cc: Anthony Liguori , Markus Armbruster , qemu-devel@nongnu.org On Thu, 10 Dec 2009 19:02:37 +0200 Avi Kivity wrote: > On 12/10/2009 06:54 PM, Luiz Capitulino wrote: > > On Thu, 10 Dec 2009 18:24:38 +0200 > > Avi Kivity wrote: > > > > > >>> Let me put it another way, I don't think adding null to the json > >>> parser and incorporating it into this command is a good idea at this > >>> stage in the release so if we want to do something like this, we need > >>> to defer it to 0.13. > >>> > >>> I agree there are some instances where null could be useful. I think > >>> we can get away without it here though. > >>> > >> For 'name', definitely, since it's known to exist. It would be nice to > >> have consistency in how features are presented, though. > >> > > Following what you propose, if it's known to exist then we should > > never return an empty dict. > > > > Right, but if we can't support null (why?) then we can make an exception > for 'name'. I'm ok with the exception too and the problem with adding null support now is that it requires some changes to the parser which seem a bit late to be done. > > There are other commands that might require adjustments, for example > > 'info kvm' has a 'present' key. If QEMU is built w/o KVM support, then > > this key will be 'false'. Should we return an empty dict then? > > > > No. > > > HPET is another example, currently it's only compiled in if the > > target is i386. Otherwise the command won't even be available, and > > we have more commands with conditional features/compilation. > > > > > > That's fine, as long as command availability is discoverable. Yes, it's. > > So, what I arguably did wrong here was starting the conversion > > work before defining all these rules. > > > > On the other hand, we can often only find out something by implementing > it incorrectly first. Sure and even being inconsistent a bit I'm quite sure that it's a lot better than parsing the old user monitor. :) > > An option we have is: libvirt actually uses four or five of those > > info commands. So, we could drop all the rest and guarantee that > > only those libvirt ones are 100% correct. > > > > My worry is with the commands that parse or emit comma-separated option > strings. I tried to fix all emissions, but we accept it like in the uri argument of migrate. I don't think it's a big deal though, as it's easy to add a new key for that (if needed) w/o breaking compatibility.