From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:50446) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Suhrx-0002TK-GF for qemu-devel@nongnu.org; Fri, 27 Jul 2012 06:34:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Suhrr-0007VF-E8 for qemu-devel@nongnu.org; Fri, 27 Jul 2012 06:34:05 -0400 Received: from mx1.redhat.com ([209.132.183.28]:64851) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Suhrr-0007Uf-4g for qemu-devel@nongnu.org; Fri, 27 Jul 2012 06:33:59 -0400 Date: Fri, 27 Jul 2012 11:33:52 +0100 From: "Daniel P. Berrange" Message-ID: <20120727103352.GR2225@redhat.com> References: <1343384448-21828-1-git-send-email-xiawenc@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <1343384448-21828-1-git-send-email-xiawenc@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [PATCH] let qemu-img info genereate json output Reply-To: "Daniel P. Berrange" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Wenchao Xia Cc: pbonzini@redhat.com, aliguori@us.ibm.com, qemu-devel@nongnu.org, stefanha@linux.vnet.ibm.com On Fri, Jul 27, 2012 at 06:20:48PM +0800, Wenchao Xia wrote: > This patch would add option -j in qemu-img info command, which > would generate json output in stdout. I like this idea in general, because currently apps (oVirt, OpenStack, etc) rely on parsing the human format, which is just as evil as libvirt relying on -help format. It would be helpful if you actually included the JSON output in your commit message. For the benefit of other reviews, it generates the following: #qemu-img info -j /var/lib/libvirt/images/bar.qcow2 { "information": { "actual_size": "139264", "fmt": "qcow2", "virtual_size": "10485760", "filename": "/var/lib/libvirt/images/bar.qcow2", "cluster_size": 65536, "encrypted": 0, "snapshot_list": [ ], "dirty_flag": 0, "backing_filename": "/dev/sda1" }, "return": 0 } IIUC,the 'return' element here is just duplicating the qemu-img exit status. I think this is rather dubious, and would rather just see the stuff in the 'information' sub-block be output directly. It also seems to forget to mention the backing file format. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|