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

[Jan Kiszka <jan.kiszka@web.de>]

[-- Attachment #1: Type: text/plain, Size: 3839 bytes --]

Avi Kivity wrote:
> On 05/22/2010 11:18 AM, Jan Kiszka wrote:
>> From: Jan Kiszka<jan.kiszka@siemens.com>
>>
>> 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
>> unless
>> +a full dump is requested.
>> +
>> +Arguments:
>> +
>> +- "path": the device's qtree path or unique ID (json-string)
>>    
> 
> This may be ambiguous.

Can your elaborate what precisely is ambiguous?

> 
>> +- "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.

> 
>> +
>> +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
>> representation
>> +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. This would be
overkill for the intended purpose (though someone may find a different
use case one day).

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.

> 
>> +
>> +The element object array may be empty, otherwise it can contain
>> +
>> +- json-int objects
>> +- QMP buffer objects
>> +- field objects
>> +- arrays of json-ints, QMP buffers, or field objects
>> +
>> +Example:
>> +
>> +->  { "execute": "device_show", "arguments": { "path": "isa.0/i8042" } }
>> +<- { "return": { "device": "i8042", "id": "", "fields":
>> +                 [ { "name": "kbd", "size": 4, "elems":
>> +                     [ { "name": "write_cmd", "size": 1, "elems": [0] },
>> +                       { "name": "status", "size": 1, "elems": [25] },
>> +                       { "name": "mode", "size": 1, "elems": [3] },
>> +                       { "name": "pending", "size": 1, "elems": [1] }
>> +                     ] }
>> +                 ]
>> +               }
>> +   }
>> +
>> +EQMP
>>    
> 
> Looks good.  I am only worried about long term stability and documentation.
> 

Thanks,
Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]