From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44315) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VW1Y9-00059V-Fo for qemu-devel@nongnu.org; Tue, 15 Oct 2013 06:09:22 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VW1XB-0004gq-II for qemu-devel@nongnu.org; Tue, 15 Oct 2013 06:08:25 -0400 Received: from e28smtp08.in.ibm.com ([122.248.162.8]:33503) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VW1XB-0004eV-0j for qemu-devel@nongnu.org; Tue, 15 Oct 2013 06:07:25 -0400 Received: from /spool/local by e28smtp08.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 15 Oct 2013 15:37:21 +0530 Received: from d28relay05.in.ibm.com (d28relay05.in.ibm.com [9.184.220.62]) by d28dlp01.in.ibm.com (Postfix) with ESMTP id 5ED61E0053 for ; Tue, 15 Oct 2013 15:38:43 +0530 (IST) Received: from d28av05.in.ibm.com (d28av05.in.ibm.com [9.184.220.67]) by d28relay05.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r9FA7H1c31391870 for ; Tue, 15 Oct 2013 15:37:17 +0530 Received: from d28av05.in.ibm.com (localhost [127.0.0.1]) by d28av05.in.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id r9FA7I95014114 for ; Tue, 15 Oct 2013 15:37:19 +0530 Message-ID: <525D13D4.3000704@linux.vnet.ibm.com> Date: Tue, 15 Oct 2013 18:07:16 +0800 From: mike MIME-Version: 1.0 References: <1381757489-3310-1-git-send-email-qiudayu@linux.vnet.ibm.com> <87zjqbhqcd.fsf@blackfin.pond.sub.org> <525CB8BD.1020903@linux.vnet.ibm.com> <20131015085851.GA3039@dhcp-200-207.str.redhat.com> In-Reply-To: <20131015085851.GA3039@dhcp-200-207.str.redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] hmp: Add '\n' in monitor_printf() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: qemu-devel@nongnu.org, Markus Armbruster , benoit@irqsave.net, stefanha@redhat.com, lcapitulino@redhat.com On 10/15/2013 04:58 PM, Kevin Wolf wrote: > 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. > Yes, I agree with you. but maybe need the author of the commit 3e9fab69 ('block: Add support for throttling burst max in QMP and the command line') to have some comments on this line, I think. >>> 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? I'm not, but Markus suggest to do so. Thanks Mike > Kevin > > >