Re: [Qemu-devel] [PATCH 2/2] ramblock: add new hmp command "info ramblock"

From: Markus Armbruster <armbru@redhat.com>
To: Peter Xu <peterx@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	qemu-devel@nongnu.org,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 2/2] ramblock: add new hmp command "info ramblock"
Date: Thu, 27 Apr 2017 09:07:01 +0200	[thread overview]
Message-ID: <87h91ajpm2.fsf@dusky.pond.sub.org> (raw)
In-Reply-To: <20170427020931.GD26792@pxdev.xzpeter.org> (Peter Xu's message of "Thu, 27 Apr 2017 10:09:31 +0800")

Peter Xu <peterx@redhat.com> writes:

> On Wed, Apr 26, 2017 at 02:10:16PM +0200, Markus Armbruster wrote:
>> Peter Xu <peterx@redhat.com> writes:
>> 
>> > To dump information about ramblocks. It looks like:
>> >
>> > (qemu) info ramblock
>> >               Block Name    PSize              Offset               Used              Total
>> >             /objects/mem       2M  0x0000000000000000 0x0000000080000000 0x0000000080000000
>> >                 vga.vram       4K  0x0000000080060000 0x0000000001000000 0x0000000001000000
>> >     /rom@etc/acpi/tables       4K  0x00000000810b0000 0x0000000000020000 0x0000000000200000
>> >                  pc.bios       4K  0x0000000080000000 0x0000000000040000 0x0000000000040000
>> >   0000:00:03.0/e1000.rom       4K  0x0000000081070000 0x0000000000040000 0x0000000000040000
>> >                   pc.rom       4K  0x0000000080040000 0x0000000000020000 0x0000000000020000
>> >     0000:00:02.0/vga.rom       4K  0x0000000081060000 0x0000000000010000 0x0000000000010000
>> >    /rom@etc/table-loader       4K  0x00000000812b0000 0x0000000000001000 0x0000000000001000
>> >       /rom@etc/acpi/rsdp       4K  0x00000000812b1000 0x0000000000001000 0x0000000000001000
>> >
>> > Signed-off-by: Peter Xu <peterx@redhat.com>
>> 
>> You implemented this just for HMP.
>> 
>> In general, functionality available in HMP should also available in QMP.
>> Exceptions include functionality that makes no sense in QMP, or is of
>> use only for human users.  If you think your command is an exception,
>> please explain why in the commit message.
>> 
>> If it isn't, you need to implement it for QMP, then rewrite the HMP
>> version to reuse either the QMP command or a common core.
>
> I see. I think it should belongs to the first condition, which should
> only be used by human users, especially developers. Maybe I can repost
> with some more comment in the commit message to emphasize that.

Example for "makes no sense in QMP": setting the current CPU[1], because
a QMP monitor doesn't have a current CPU.

Examples for "is of use only for human users": HMP command "help", the
integrated pocket calculator[2].

> But, before that, I would also want to know whether anyone would think
> this an useful thing even to QMP... If so, I would be glad to provide
> QMP interface as well.

Can't say without understanding the use cases.  Perhaps you can start
with explaining the itch your HMP command scratches.

[1] For reasons lost in time, we added QMP command "cpu" anyway, it just
does nothing.

[2] I doubt that one is of much use to humans, either.