From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39054) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VW2aO-0003uX-KF for qemu-devel@nongnu.org; Tue, 15 Oct 2013 07:14:54 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VW2aI-0003Q8-JG for qemu-devel@nongnu.org; Tue, 15 Oct 2013 07:14:48 -0400 Received: from nodalink.pck.nerim.net ([62.212.105.220]:48218 helo=paradis.irqsave.net) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VW2aH-0003PV-LJ for qemu-devel@nongnu.org; Tue, 15 Oct 2013 07:14:42 -0400 Date: Tue, 15 Oct 2013 13:14:33 +0200 From: =?iso-8859-1?Q?Beno=EEt?= Canet Message-ID: <20131015111432.GA2878@irqsave.net> 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> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <525D13D4.3000704@linux.vnet.ibm.com> Content-Transfer-Encoding: quoted-printable 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: Kevin Wolf , lcapitulino@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com, Markus Armbruster Hello, >Le Tuesday 15 Oct 2013 =E0 18:07:16 (+0800), mike a =E9crit : > 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 bogu= s > >"[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 li= ne') > to have some comments on this line, I think. Hello, I do not remember even thinking about adding this monitor_printf in 3e9fa= b69. It must be the result of a bad conflict resolve or something like that. Sorry for adding this. Do you want me to send a two liner to remove this ?=20 Best regards Beno=EEt > >>>I think we should also drop this newline: > >>> > >>> if (info->value->removable) { > >>> monitor_printf(mon, " Removable device: %slocked, t= ray %s\n", > >>> info->value->locked ? "" : "not ", > >>> info->value->tray_open ? "open" : "clos= ed"); > >>> } > >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-De= sktop.iso (chain depth: 1) > > I/O throttling: bps=3D1048576 bps_rd=3D0 bps_wr=3D0 bps_max=3D1= 04857 bps_rd_max=3D0 bps_wr_max=3D0 iops=3D0 iops_rd=3D0 iops_wr=3D0 iops= _max=3D0 iops_rd_max=3D0 iops_wr_max=3D0 iops_size=3D0 > > > >Do you really want to remove the newline? > I'm not, but Markus suggest to do so. >=20 > Thanks > Mike > >Kevin > > > > > > >=20 >=20