From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53706) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VW0T2-0000Qx-7r for qemu-devel@nongnu.org; Tue, 15 Oct 2013 04:59:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VW0Sw-00079h-5L for qemu-devel@nongnu.org; Tue, 15 Oct 2013 04:59:04 -0400 Received: from mx1.redhat.com ([209.132.183.28]:53077) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VW0Sv-00079b-TN for qemu-devel@nongnu.org; Tue, 15 Oct 2013 04:58:58 -0400 Date: Tue, 15 Oct 2013 10:58:51 +0200 From: Kevin Wolf Message-ID: <20131015085851.GA3039@dhcp-200-207.str.redhat.com> References: <1381757489-3310-1-git-send-email-qiudayu@linux.vnet.ibm.com> <87zjqbhqcd.fsf@blackfin.pond.sub.org> <525CB8BD.1020903@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <525CB8BD.1020903@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [PATCH] hmp: Add '\n' in monitor_printf() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: mike Cc: lcapitulino@redhat.com, benoit@irqsave.net, Markus Armbruster , stefanha@redhat.com, qemu-devel@nongnu.org Am 15.10.2013 um 05:38 hat mike geschrieben: > On 10/14/2013 10:36 PM, Markus Armbruster wrote: > >Mike Qiu writes: > > > >>Without this, output of 'info block' > >> > >>scsi0-hd0: /images/f18-ppc64.qcow2 (qcow2) > >> [not inserted] > >>scsi0-cd2: [not inserted] > >> Removable device: not locked, tray closed > >> > >>floppy0: [not inserted] > >> Removable device: not locked, tray closed > >> > >>sd0: [not inserted] > >> Removable device: not locked, tray closed > >> > >>There will be no additional lines between scsi0-hd0 scsi0-cd2, > >>and break the info style. > >Just saw a similar one: > > > > (qemu) info block > > disk0: test.img (raw) > > [not inserted] > > cd: [not inserted] > > Removable device: not locked, tray closed > > > > foo: tmp.img (raw) > > Removable device: not locked, tray closed > > [not inserted](qemu) > > > >>This patch is to solve this. > >> > >>Signed-off-by: Mike Qiu > >>--- > >> hmp.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >>diff --git a/hmp.c b/hmp.c > >>index 5891507..2d2e5f8 100644 > >>--- a/hmp.c > >>+++ b/hmp.c > >>@@ -367,7 +367,7 @@ void hmp_info_block(Monitor *mon, const QDict *qdict) > >> info->value->inserted->iops_wr_max, > >> info->value->inserted->iops_size); > >> } else { > >>- monitor_printf(mon, " [not inserted]"); > >>+ monitor_printf(mon, " [not inserted]\n"); > >> } > >> if (verbose) { > > monitor_printf(mon, "\nImages:\n"); > > > >What about removing the newline before "Images"? > A good idea I think, it no need to add addition lines in one info. > > But see commit id: fbe2e26c15af35e4d157874dc80f6a19eebaa83b > [...] > It was changed to add this, so there maybe some reasons I think, Like everything else in that commit, I did that change because I found it more readable. The problem seems to be commit 3e9fab69 ('block: Add support for throttling burst max in QMP and the command line'), which added a bogus "[not inserted]" message. We simply need to drop it altogether instead of adding a newline. > > I think we should also drop this newline: > > > > if (info->value->removable) { > > monitor_printf(mon, " Removable device: %slocked, tray %s\n", > > info->value->locked ? "" : "not ", > > info->value->tray_open ? "open" : "closed"); > > } Why? Look: (qemu) info block scsi0-cd0: /tmp/cdrom.qcow2 (qcow2) Removable device: not locked, tray closed Backing file: /home/kwolf/images/iso/Fedora-18-x86_64-Live-Desktop.iso (chain depth: 1) I/O throttling: bps=1048576 bps_rd=0 bps_wr=0 bps_max=104857 bps_rd_max=0 bps_wr_max=0 iops=0 iops_rd=0 iops_wr=0 iops_max=0 iops_rd_max=0 iops_wr_max=0 iops_size=0 Do you really want to remove the newline? Kevin