Re: [Qemu-devel] [PATCH] hmp: Add '\n' in monitor_printf()

[Wenchao Xia <xiawenc@linux.vnet.ibm.com>]

于 2013/10/16 15:47, Markus Armbruster 写道:
> Wenchao Xia<xiawenc@linux.vnet.ibm.com>  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<qiudayu@linux.vnet.ibm.com>  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<qiudayu@linux.vnet.ibm.com>
>>>>>>> ---
>>>>>>> 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.


> Elswhere in the thread, we concluded that the offending '[not inserted]'
> is bogus, and should be dropped.
>