From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55447) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VVvTO-0002Me-S1 for qemu-devel@nongnu.org; Mon, 14 Oct 2013 23:39:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VVvTF-0001nP-Cu for qemu-devel@nongnu.org; Mon, 14 Oct 2013 23:39:06 -0400 Received: from e23smtp01.au.ibm.com ([202.81.31.143]:60558) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VVvTE-0001mi-Hh for qemu-devel@nongnu.org; Mon, 14 Oct 2013 23:38:57 -0400 Received: from /spool/local by e23smtp01.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 15 Oct 2013 13:38:49 +1000 Received: from d23relay03.au.ibm.com (d23relay03.au.ibm.com [9.190.235.21]) by d23dlp03.au.ibm.com (Postfix) with ESMTP id 2D51F3578057 for ; Tue, 15 Oct 2013 14:38:46 +1100 (EST) Received: from d23av04.au.ibm.com (d23av04.au.ibm.com [9.190.235.139]) by d23relay03.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r9F3cSCS7799040 for ; Tue, 15 Oct 2013 14:38:34 +1100 Received: from d23av04.au.ibm.com (localhost [127.0.0.1]) by d23av04.au.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id r9F3cdSN008790 for ; Tue, 15 Oct 2013 14:38:39 +1100 Message-ID: <525CB8BD.1020903@linux.vnet.ibm.com> Date: Tue, 15 Oct 2013 11:38:37 +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> In-Reply-To: <87zjqbhqcd.fsf@blackfin.pond.sub.org> 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: Markus Armbruster Cc: kwolf@redhat.com, qemu-devel@nongnu.org, lcapitulino@redhat.com 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 - if (verbose) { - monitor_printf(mon, " images:\n"); ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - image_info = info->value->inserted->image; - while (1) { - bdrv_image_info_dump((fprintf_function)monitor_printf, - mon, image_info); - if (image_info->has_backing_image) { - image_info = image_info->backing_image; - } else { - break; - } + if (verbose) { + monitor_printf(mon, "\nImages:\n"); ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + image_info = info->value->inserted->image; + while (1) { + bdrv_image_info_dump((fprintf_function)monitor_printf, + mon, image_info); + if (image_info->has_backing_image) { + image_info = image_info->backing_image; + } else { + break; } } - } else { - monitor_printf(mon, " [not inserted]"); } It was changed to add this, so there maybe some reasons I think, Thanks Mike > 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"); > } > > Maybe more. The function probably needs a systematic newline review. > >