[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] hmp: Add '\n' in monitor_printf()
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH] hmp: Add '\n' in monitor_printf() |
Date: |
Tue, 15 Oct 2013 11:31:45 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.2 (gnu/linux) |
Kevin Wolf <address@hidden> writes:
> Am 15.10.2013 um 05:38 hat mike geschrieben:
>> On 10/14/2013 10:36 PM, Markus Armbruster wrote:
>> >Mike Qiu <address@hidden> 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 <address@hidden>
>> >>---
>> >> 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?
This one made me think I do:
foo: tmp.img (raw)
Removable device: not locked, tray closed
[not inserted](qemu)
If the '[not inserted]' is wrong and needs to go, then I actually don't.
- [Qemu-devel] [PATCH] hmp: Add '\n' in monitor_printf(), Mike Qiu, 2013/10/14
- Re: [Qemu-devel] [PATCH] hmp: Add '\n' in monitor_printf(), Markus Armbruster, 2013/10/14
- Re: [Qemu-devel] [PATCH] hmp: Add '\n' in monitor_printf(), mike, 2013/10/14
- Re: [Qemu-devel] [PATCH] hmp: Add '\n' in monitor_printf(), mike, 2013/10/15
- Re: [Qemu-devel] [PATCH] hmp: Add '\n' in monitor_printf(), BenoƮt Canet, 2013/10/15
- Re: [Qemu-devel] [PATCH] hmp: Add '\n' in monitor_printf(), Wenchao Xia, 2013/10/16
- Re: [Qemu-devel] [PATCH] hmp: Add '\n' in monitor_printf(), Markus Armbruster, 2013/10/16
- Re: [Qemu-devel] [PATCH] hmp: Add '\n' in monitor_printf(), Wenchao Xia, 2013/10/16
- Re: [Qemu-devel] [PATCH] hmp: Add '\n' in monitor_printf(), mike, 2013/10/16