From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57926) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VWN1H-00032J-1T for qemu-devel@nongnu.org; Wed, 16 Oct 2013 05:04:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VWN18-0000My-3l for qemu-devel@nongnu.org; Wed, 16 Oct 2013 05:03:54 -0400 Received: from e23smtp09.au.ibm.com ([202.81.31.142]:41878) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VWN17-0000IH-Fh for qemu-devel@nongnu.org; Wed, 16 Oct 2013 05:03:46 -0400 Received: from /spool/local by e23smtp09.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 16 Oct 2013 19:03:40 +1000 Received: from d23relay03.au.ibm.com (d23relay03.au.ibm.com [9.190.235.21]) by d23dlp03.au.ibm.com (Postfix) with ESMTP id 63CE83578065 for ; Wed, 16 Oct 2013 20:03:37 +1100 (EST) Received: from d23av02.au.ibm.com (d23av02.au.ibm.com [9.190.235.138]) by d23relay03.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r9G93P9i4260124 for ; Wed, 16 Oct 2013 20:03:25 +1100 Received: from d23av02.au.ibm.com (localhost [127.0.0.1]) by d23av02.au.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id r9G93YbW008026 for ; Wed, 16 Oct 2013 20:03:35 +1100 Message-ID: <525E5664.10100@linux.vnet.ibm.com> Date: Wed, 16 Oct 2013 17:03:32 +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> <525D13D4.3000704@linux.vnet.ibm.com> <525E4045.8060500@linux.vnet.ibm.com> <8738o1odxm.fsf@blackfin.pond.sub.org> <525E46B0.5030600@linux.vnet.ibm.com> In-Reply-To: <525E46B0.5030600@linux.vnet.ibm.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH] hmp: Add '\n' in monitor_printf() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Wenchao Xia Cc: Kevin Wolf , benoit@irqsave.net, Markus Armbruster , qemu-devel@nongnu.org, lcapitulino@redhat.com, stefanha@redhat.com On 10/16/2013 03:56 PM, Wenchao Xia wrote: > 于 2013/10/16 15:47, Markus Armbruster 写道: >> Wenchao Xia writes: >> >>> 于 2013/10/15 18:07, mike 写道: >>>> 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. >>>> >>> Why the new line matters? I think there is a QMP interface available, >>> so the hmp output format >>> would not bother much, just need to tip clear the info. >> I want this fixed: >> >> $ qemu -nodefaults -nographic -monitor stdio -S -drive >> if=none,file=tmp.img >> QEMU 1.6.50 monitor - type 'help' for more information >> (qemu) info block >> none0: tmp.img (raw) >> Removable device: not locked, tray closed >> [not inserted](qemu) >> >> Output doesn't end with newline, messing up the prompt. >> > I see, no end with newline is bad, should fix. > > OK, I will make a patch to drop this ' [not inserted]' line instead of add a new line. Thanks Mike >> Elswhere in the thread, we concluded that the offending '[not inserted]' >> is bogus, and should be dropped. >> > > > >