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

[Wenchao Xia]

于 2013/10/16 15:47, Markus Armbruster 写道:
> Wenchao Xia<address@hidden>  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<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.
> > > 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.