[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-devel] Re: [PATCH v2 12/15] monitor: Add basic device state visual

From: Avi Kivity
Subject: [Qemu-devel] Re: [PATCH v2 12/15] monitor: Add basic device state visualization
Date: Sun, 23 May 2010 11:44:43 +0300
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv: Gecko/20100330 Fedora/3.0.4-1.fc12 Thunderbird/3.0.4

On 05/23/2010 10:57 AM, Jan Kiszka wrote:
Avi Kivity wrote:
On 05/22/2010 11:18 AM, Jan Kiszka wrote:
From: Jan Kiszka<address@hidden>

This introduces device_show, a monitor command that saves the vmstate of
a qdev device and visualizes it. QMP is also supported. Buffers are cut
after 16 byte by default, but the full content can be requested via
'-f'. To pretty-print sub-arrays, vmstate is extended to store the start
index name. A new qerror is introduced to signal a missing vmstate. And
it comes with documentation.

+Dump a snapshot of the device state. Buffers are cut after 16 bytes
+a full dump is requested.
+- "path": the device's qtree path or unique ID (json-string)

This may be ambiguous.
Can your elaborate what precisely is ambiguous?

Can't the user choose the unique ID so that it aliases an unrelated qtree path?

I prefer having mutually exclusive 'path' and 'ref' arguments.

+- "full": report full state (json-bool, optional)

Is this needed for QMP?  The client can always truncate it to any length.
The effect may not be needed for QMP, but I do need this channel from
the command line to the monitor pretty-printer. I could just stick
"full": json-bool back into the return dict, but that would look somehow
strange IMO.

So we could disallow it as a QMP input, but allow it as an HMP input.

+Schema of returned object:
+{ "device": json-string, "id": json-string, "fields" : [
field-objects ] }
+The field object array may be empty, otherwise it consists of
+{ "name": json-string, "size": json-int, "elems": [ element-objects ] }
+"size" describes the real number of bytes required for a binary
+of a single field element in the array. The actually transfered
amount may be
+smaller unless a full dump was requested.

This converts the entire qdev tree into an undocumented stable protocol
(the qdev paths were already in this state I believe).  This really
worries me.
Being primarily a debugging tool, device_show exports the entire
(qdev'ified) vmstates via QMP. Unlike the migration protocol, it does
not provide something like backward compatibility.

Should be explicitly documented. All QMP commands should be backwards and forwards compatible unless noted.

This would be
overkill for the intended purpose (though someone may find a different
use case one day).

Even for simply showing things, a GUI may depend on the presence of certain fields. If we document that the fields may change, a correctly written GUI can fall back to a simpler display.

I think we have the following options:
  - disable device_show via QMP, limit it to the monitor console
  - declare its output inherently unstable, maybe at least adding the
    vmstate version to each device so that potential QMP consumers notice
    that they may have to update their tools or switch to a different
    processing function

Given that vmstate annotations will most probably require some work on
the output structure (and I don't have a QMP use case ATM anyway), I
would be fine with the first option for now. Still, I don't think we
will ever get beyond the second option because this service is tight to
some internals of QEMU we don't want to freeze.

I agree. This feature is very useful as a debugging aid, and as I don't think we'll have debugging GUIs any time soon, it's better to defer the problem until we really need to solve it.

error compiling committee.c: too many arguments to function

reply via email to

[Prev in Thread] Current Thread [Next in Thread]