Patchwork [v2,12/15] monitor: Add basic device state visualization

login
register
mail settings
Submitter Jan Kiszka
Date May 22, 2010, 8:18 a.m.
Message ID <1f557b9feb1965a61e64f7166bcf4918bed8d0ec.1274516288.git.jan.kiszka@web.de>
Download mbox | patch
Permalink /patch/53252/
State New
Headers show

Comments

Jan Kiszka - May 22, 2010, 8:18 a.m.
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.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/hw.h         |    2 +
 hw/qdev.c       |  244 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/qdev.h       |    2 +
 qemu-monitor.hx |   64 +++++++++++++++
 qerror.c        |    4 +
 qerror.h        |    3 +
 6 files changed, 319 insertions(+), 0 deletions(-)
Avi Kivity - May 22, 2010, 6:55 p.m.
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.

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

Is this needed for QMP?  The client can always truncate it to any length.

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

> +
> +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.
Jan Kiszka - May 23, 2010, 7:57 a.m.
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
Avi Kivity - May 23, 2010, 8:44 a.m.
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<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?
>    

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

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.
Jan Kiszka - May 23, 2010, 10:03 a.m.
Avi Kivity wrote:
> 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<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?
>>    
> 
> Can't the user choose the unique ID so that it aliases an unrelated
> qtree path?

True. I'll swap the search order and document this. Qtree paths should
always rule.

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

That would be unhandy.

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

I introduced .user_only as a monitor command tag and applied it on
device_show. But I also added the vmstate version to the device output,
maybe already helpful for users. All this will come with v3.

Jan
Avi Kivity - May 23, 2010, 10:42 a.m.
On 05/23/2010 01:03 PM, Jan Kiszka wrote:
>>>
>>> Can your elaborate what precisely is ambiguous?
>>>
>>>        
>> Can't the user choose the unique ID so that it aliases an unrelated
>> qtree path?
>>      
> True. I'll swap the search order and document this. Qtree paths should
> always rule.
>    

Well, I guess the user could avoid ambiguity by avoiding /es.

>    
>> I prefer having mutually exclusive 'path' and 'ref' arguments.
>>      
> That would be unhandy.
>    

Don't really see why.

>> 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.
>>      
> I introduced .user_only as a monitor command tag and applied it on
> device_show. But I also added the vmstate version to the device output,
> maybe already helpful for users. All this will come with v3.
>    

Thanks.
Luiz Capitulino - May 24, 2010, 12:51 p.m.
On Sun, 23 May 2010 09:57:43 +0200
Jan Kiszka <jan.kiszka@web.de> wrote:

> Avi Kivity wrote:

[...]

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

 We could have a form of optional key which is only present internally,
we have that async commands.
Anthony Liguori - May 24, 2010, 8:22 p.m.
On 05/24/2010 07:51 AM, Luiz Capitulino wrote:
> On Sun, 23 May 2010 09:57:43 +0200
> Jan Kiszka<jan.kiszka@web.de>  wrote:
>
>    
>> Avi Kivity wrote:
>>      
> [...]
>
>    
>>>        
>>>> +- "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.
>>      
>   We could have a form of optional key which is only present internally,
> we have that async commands.
>    

I'd rather see two separate commands with an internal mechanism to make 
direct QMP calls.  The human monitor can be implemented in terms of the 
QMP call but it should be in terms of the human monitor invoking the QMP 
command such that it can munge the output as it sees appropriate.

Regards,

Anthony Liguori
Anthony Liguori - May 24, 2010, 8:22 p.m.
On 05/22/2010 01:55 PM, 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.
>
>> +- "full": report full state (json-bool, optional)
>
> Is this needed for QMP?  The client can always truncate it to any length.
>
>> +
>> +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.

N.B. the association with qdev is only in identifying the device.  The 
contents of the device's state are not part of qdev but rather part of 
vmstate.  vmstate is something that we already guarantee to be stable 
since that's required for live migration compatibility.

I don't think that qdev device names and paths are something we have to 
worry much about changing over time since they reflect logical bus 
layout.  They should remain static provided the devices remain static.

The qdev properties are a different matter entirely.  A command like 
'info qdm' would be potentially difficult to support as part of QMP but 
the proposed command's output is actually already part of a backward 
compatible interface (vmstate).

Regards,

Anthony Liguori


>> +
>> +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.
>
Jan Kiszka - May 24, 2010, 8:35 p.m.
Anthony Liguori wrote:
> On 05/22/2010 01:55 PM, 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.
>>
>>> +- "full": report full state (json-bool, optional)
>>
>> Is this needed for QMP?  The client can always truncate it to any length.
>>
>>> +
>>> +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.
> 
> N.B. the association with qdev is only in identifying the device.  The
> contents of the device's state are not part of qdev but rather part of
> vmstate.  vmstate is something that we already guarantee to be stable
> since that's required for live migration compatibility.

In contrast to save/loadvm, device_show does not provide a
backward-compatible output. Not only field names can change (as a result
of internal refactoring), fields may even disappear, new ones may pop
up. vmstate makes this transparent for reading of old states, but not
for visualization.

That said, I see no use case yet that would justify the effort for
making state dumps as stable as vmstates.

Jan
Anthony Liguori - May 24, 2010, 9:49 p.m.
On 05/24/2010 03:35 PM, Jan Kiszka wrote:
> In contrast to save/loadvm, device_show does not provide a
> backward-compatible output. Not only field names can change (as a result
> of internal refactoring),

Field names could change, but that seems unlikely and unnecessary.

>   fields may even disappear,

That would break live migration.  The output of device_show when 
specifying -M pc-0.13 should always be the same.  If it's not, I believe 
that's a bug.

The output of device_show across multiple qemu versions without 
specifying a -M could certainly be different but that's to be expected 
since you have a different hardware model.

>   new ones may pop
> up. vmstate makes this transparent for reading of old states, but not
> for visualization.
>
> That said, I see no use case yet that would justify the effort for
> making state dumps as stable as vmstates.
>    

They already are.  It's just a matter of whether we declare it as such 
AFAICT.

Regards,

Anthony Liguori

> Jan
>
Jan Kiszka - May 24, 2010, 10:12 p.m.
Anthony Liguori wrote:
> On 05/24/2010 03:35 PM, Jan Kiszka wrote:
>> In contrast to save/loadvm, device_show does not provide a
>> backward-compatible output. Not only field names can change (as a result
>> of internal refactoring),
> 
> Field names could change, but that seems unlikely and unnecessary.

Actually, this may happen to improve the output format for device_show,
either via renaming the fields or annotating it (once we have the latter
mechanism).

> 
>>   fields may even disappear,
> 
> That would break live migration. 

Nope. We can (and I think we did before) perfectly read some field up to
version X and convert it into the new format but skip that field for X+1
upward.

> The output of device_show when
> specifying -M pc-0.13 should always be the same.  If it's not, I believe
> that's a bug.
> 
> The output of device_show across multiple qemu versions without
> specifying a -M could certainly be different but that's to be expected
> since you have a different hardware model.

It will be different as soon as the version_id of a dumped vmstate
changes, even independent of -M.

Jan
Anthony Liguori - May 24, 2010, 10:27 p.m.
On 05/24/2010 05:12 PM, Jan Kiszka wrote:
> Anthony Liguori wrote:
>    
>> On 05/24/2010 03:35 PM, Jan Kiszka wrote:
>>      
>>> In contrast to save/loadvm, device_show does not provide a
>>> backward-compatible output. Not only field names can change (as a result
>>> of internal refactoring),
>>>        
>> Field names could change, but that seems unlikely and unnecessary.
>>      
> Actually, this may happen to improve the output format for device_show,
> either via renaming the fields or annotating it (once we have the latter
> mechanism).

I don't think it's a huge win vs. stability.

>> The output of device_show when
>> specifying -M pc-0.13 should always be the same.  If it's not, I believe
>> that's a bug.
>>
>> The output of device_show across multiple qemu versions without
>> specifying a -M could certainly be different but that's to be expected
>> since you have a different hardware model.
>>      
> It will be different as soon as the version_id of a dumped vmstate
> changes, even independent of -M.
>    

But QMP stability is only guaranteed for releases and -M pc-0.12 should 
(in a perfect world) always dump out the same versions regardless of 
whether it's qemu-0.12, qemu-0.13, etc.

Regards,

Anthony Liguori

> Jan
>
>
Avi Kivity - May 25, 2010, 7:23 a.m.
On 05/24/2010 11:22 PM, Anthony Liguori wrote:
>> 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.
>
>
> N.B. the association with qdev is only in identifying the device.  The 
> contents of the device's state are not part of qdev but rather part of 
> vmstate.  vmstate is something that we already guarantee to be stable 
> since that's required for live migration compatibility.

That removes out ability to deprecate older vmstate as time passes.  Not 
a blocker but something to consider.

> I don't think that qdev device names and paths are something we have 
> to worry much about changing over time since they reflect logical bus 
> layout.  They should remain static provided the devices remain static.

Modulo mistakes.  We already saw one (lack of pci domains).  To reduce 
the possibility of mistakes, we need reviewable documentation.

Note sysfs had similar assumptions and problems.

> The qdev properties are a different matter entirely.  A command like 
> 'info qdm' would be potentially difficult to support as part of QMP 
> but the proposed command's output is actually already part of a 
> backward compatible interface (vmstate).

That's all good.  But documentation is critical for this.  Not only to 
improve quality, but also so that tool authors would have something to 
code against instead of trial and error (which invariably misses some 
corner cases).
Anthony Liguori - May 25, 2010, 1:03 p.m.
On 05/25/2010 02:23 AM, Avi Kivity wrote:
> On 05/24/2010 11:22 PM, Anthony Liguori wrote:
>>> 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.
>>
>>
>> N.B. the association with qdev is only in identifying the device.  
>> The contents of the device's state are not part of qdev but rather 
>> part of vmstate.  vmstate is something that we already guarantee to 
>> be stable since that's required for live migration compatibility.
>
> That removes out ability to deprecate older vmstate as time passes.  
> Not a blocker but something to consider.
>
>> I don't think that qdev device names and paths are something we have 
>> to worry much about changing over time since they reflect logical bus 
>> layout.  They should remain static provided the devices remain static.
>
> Modulo mistakes.  We already saw one (lack of pci domains).  To reduce 
> the possibility of mistakes, we need reviewable documentation.

pci domains was only a mistake as a nice-to-have.  We can add pci 
domains in a backwards compatible way.

The arguments you're making about the importance of backwards 
compatibility and what's needed to strongly guarantee it are equally 
applicable to the live migration protocol.  We really do need to 
formally document the live migration protocol in such a way that it's 
reviewable if we hope to truly make it compatible across versions.

Regards,

Anthony Liguori

> Note sysfs had similar assumptions and problems.
>
>> The qdev properties are a different matter entirely.  A command like 
>> 'info qdm' would be potentially difficult to support as part of QMP 
>> but the proposed command's output is actually already part of a 
>> backward compatible interface (vmstate).
>
> That's all good.  But documentation is critical for this.  Not only to 
> improve quality, but also so that tool authors would have something to 
> code against instead of trial and error (which invariably misses some 
> corner cases).
>
Avi Kivity - May 25, 2010, 1:19 p.m.
On 05/25/2010 04:03 PM, Anthony Liguori wrote:
>>
>>> I don't think that qdev device names and paths are something we have 
>>> to worry much about changing over time since they reflect logical 
>>> bus layout.  They should remain static provided the devices remain 
>>> static.
>>
>> Modulo mistakes.  We already saw one (lack of pci domains).  To 
>> reduce the possibility of mistakes, we need reviewable documentation.
>
>
> pci domains was only a mistake as a nice-to-have.  We can add pci 
> domains in a backwards compatible way.

It adds a new level to the qdev tree.  Of course we can hide the new 
level for older clients, and newer clients can drop the level for older 
qemus, but it will be oh-so-painful.

>
> The arguments you're making about the importance of backwards 
> compatibility and what's needed to strongly guarantee it are equally 
> applicable to the live migration protocol.  We really do need to 
> formally document the live migration protocol in such a way that it's 
> reviewable if we hope to truly make it compatible across versions.

Mostly agreed.  I think live migration has a faster/easier deprecation 
schedule (easier not to support migration from 0.n-k to 0.n than to 
remove qmp support for a feature introduced in 0.n-k when releasing 
0.n).  But that's a minor concern, improving our externally visible 
interface documentation is a good thing and badly needed.
Anthony Liguori - May 25, 2010, 1:31 p.m.
On 05/25/2010 08:19 AM, Avi Kivity wrote:
> On 05/25/2010 04:03 PM, Anthony Liguori wrote:
>>>
>>>> I don't think that qdev device names and paths are something we 
>>>> have to worry much about changing over time since they reflect 
>>>> logical bus layout.  They should remain static provided the devices 
>>>> remain static.
>>>
>>> Modulo mistakes.  We already saw one (lack of pci domains).  To 
>>> reduce the possibility of mistakes, we need reviewable documentation.
>>
>>
>> pci domains was only a mistake as a nice-to-have.  We can add pci 
>> domains in a backwards compatible way.
>
> It adds a new level to the qdev tree.

The tree is not organized like that today.  IOW, the PCI hierarchy is 
not reflected in the qdev hierarchy.  All PCI devices (regardless of 
whether they're a function or a full slot) simply sit below the PCI bus.

>>
>> The arguments you're making about the importance of backwards 
>> compatibility and what's needed to strongly guarantee it are equally 
>> applicable to the live migration protocol.  We really do need to 
>> formally document the live migration protocol in such a way that it's 
>> reviewable if we hope to truly make it compatible across versions.
>
> Mostly agreed.  I think live migration has a faster/easier deprecation 
> schedule (easier not to support migration from 0.n-k to 0.n than to 
> remove qmp support for a feature introduced in 0.n-k when releasing 
> 0.n).  But that's a minor concern, improving our externally visible 
> interface documentation is a good thing and badly needed.
>

Regards,

Anthony Liguori
Avi Kivity - May 25, 2010, 1:41 p.m.
On 05/25/2010 04:31 PM, Anthony Liguori wrote:
> On 05/25/2010 08:19 AM, Avi Kivity wrote:
>> On 05/25/2010 04:03 PM, Anthony Liguori wrote:
>>>>
>>>>> I don't think that qdev device names and paths are something we 
>>>>> have to worry much about changing over time since they reflect 
>>>>> logical bus layout.  They should remain static provided the 
>>>>> devices remain static.
>>>>
>>>> Modulo mistakes.  We already saw one (lack of pci domains).  To 
>>>> reduce the possibility of mistakes, we need reviewable documentation.
>>>
>>>
>>> pci domains was only a mistake as a nice-to-have.  We can add pci 
>>> domains in a backwards compatible way.
>>
>> It adds a new level to the qdev tree.
>
> The tree is not organized like that today.  IOW, the PCI hierarchy is 
> not reflected in the qdev hierarchy.  All PCI devices (regardless of 
> whether they're a function or a full slot) simply sit below the PCI bus.

That's a bug IMO, but regardless, s/qdev tree/pci device component of 
the qdev path/.
Markus Armbruster - May 29, 2010, 8 a.m.
Avi Kivity <avi@redhat.com> writes:

> 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<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?
>>    
>
> 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.

Don't think that's necessary.  If the string starts with '/', it's an
absolute path.  Else, it's a relative path rooted at the node with the
ID equal to the first component.

Currently breaks down when IDs contain '/', but permitting that is a
bug.  There may be more problems; the path lookup code is way too
clever.
Jan Kiszka - May 29, 2010, 8:14 a.m.
Markus Armbruster wrote:
> Avi Kivity <avi@redhat.com> writes:
> 
>> 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<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?
>>>    
>> 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.
> 
> Don't think that's necessary.  If the string starts with '/', it's an
> absolute path.  Else, it's a relative path rooted at the node with the
> ID equal to the first component.

'pci.0' can be both a (badly chosen) ID as well as an abbreviated qtree
path.

> 
> Currently breaks down when IDs contain '/', but permitting that is a
> bug.  There may be more problems; the path lookup code is way too
> clever.

Indeed. Less can sometimes be more. My impression is that some of the
cleverness was motivated by lacking qtree completion for the HMP.

Jan
Avi Kivity - May 30, 2010, 8:26 a.m.
On 05/29/2010 11:14 AM, Jan Kiszka wrote:
>> Currently breaks down when IDs contain '/', but permitting that is a
>> bug.  There may be more problems; the path lookup code is way too
>> clever.
>>      
> Indeed. Less can sometimes be more. My impression is that some of the
> cleverness was motivated by lacking qtree completion for the HMP.
>    

Can we disable abbreviations for QMP and only allow them for HMP?

We can support that by adding a hidden argument to commands specifying 
whether the input comes from a human or machine.  Abbreviations are 
dangerous if they become ambiguous; a human can recover while a machine 
can't.
Jan Kiszka - May 30, 2010, 12:36 p.m.
Avi Kivity wrote:
> On 05/29/2010 11:14 AM, Jan Kiszka wrote:
>>> Currently breaks down when IDs contain '/', but permitting that is a
>>> bug.  There may be more problems; the path lookup code is way too
>>> clever.
>>>      
>> Indeed. Less can sometimes be more. My impression is that some of the
>> cleverness was motivated by lacking qtree completion for the HMP.
>>    
> 
> Can we disable abbreviations for QMP and only allow them for HMP?
> 
> We can support that by adding a hidden argument to commands specifying
> whether the input comes from a human or machine.  Abbreviations are
> dangerous if they become ambiguous; a human can recover while a machine
> can't.
> 

Both QMP _and_ HMP suffer from ambitious and inconsistent addressing
schemes. Therefore, I'm more and more in favor of [1]. We just need to
add command line completion for option values, something that would be
beneficial for 'drive_add file=...' as well.

Jan

[1] http://article.gmane.org/gmane.comp.emulators.qemu/72152
Markus Armbruster - May 31, 2010, 8:46 a.m.
Jan Kiszka <jan.kiszka@web.de> writes:

> Avi Kivity wrote:
>> On 05/29/2010 11:14 AM, Jan Kiszka wrote:
>>>> Currently breaks down when IDs contain '/', but permitting that is a
>>>> bug.  There may be more problems; the path lookup code is way too
>>>> clever.
>>>>      
>>> Indeed. Less can sometimes be more. My impression is that some of the
>>> cleverness was motivated by lacking qtree completion for the HMP.
>>>    
>> 
>> Can we disable abbreviations for QMP and only allow them for HMP?
>> 
>> We can support that by adding a hidden argument to commands specifying
>> whether the input comes from a human or machine.  Abbreviations are
>> dangerous if they become ambiguous; a human can recover while a machine
>> can't.
>> 
>
> Both QMP _and_ HMP suffer from ambitious and inconsistent addressing
> schemes. Therefore, I'm more and more in favor of [1]. We just need to
> add command line completion for option values, something that would be
> beneficial for 'drive_add file=...' as well.
>
> Jan
>
> [1] http://article.gmane.org/gmane.comp.emulators.qemu/72152

[1] = abolish the clever abbreviations in path lookup.  I agree we
should do that.
Jan Kiszka - May 31, 2010, 8:58 a.m.
Markus Armbruster wrote:
> Jan Kiszka <jan.kiszka@web.de> writes:
> 
>> Avi Kivity wrote:
>>> On 05/29/2010 11:14 AM, Jan Kiszka wrote:
>>>>> Currently breaks down when IDs contain '/', but permitting that is a
>>>>> bug.  There may be more problems; the path lookup code is way too
>>>>> clever.
>>>>>      
>>>> Indeed. Less can sometimes be more. My impression is that some of the
>>>> cleverness was motivated by lacking qtree completion for the HMP.
>>>>    
>>> Can we disable abbreviations for QMP and only allow them for HMP?
>>>
>>> We can support that by adding a hidden argument to commands specifying
>>> whether the input comes from a human or machine.  Abbreviations are
>>> dangerous if they become ambiguous; a human can recover while a machine
>>> can't.
>>>
>> Both QMP _and_ HMP suffer from ambitious and inconsistent addressing
>> schemes. Therefore, I'm more and more in favor of [1]. We just need to
>> add command line completion for option values, something that would be
>> beneficial for 'drive_add file=...' as well.
>>
>> Jan
>>
>> [1] http://article.gmane.org/gmane.comp.emulators.qemu/72152
> 
> [1] = abolish the clever abbreviations in path lookup.  I agree we
> should do that.

Fine. No concerns regarding overlaying IDs and path specifications as well?

Jan
Markus Armbruster - May 31, 2010, 11:07 a.m.
Jan Kiszka <jan.kiszka@siemens.com> writes:

> Markus Armbruster wrote:
>> Jan Kiszka <jan.kiszka@web.de> writes:
>> 
>>> Avi Kivity wrote:
>>>> On 05/29/2010 11:14 AM, Jan Kiszka wrote:
>>>>>> Currently breaks down when IDs contain '/', but permitting that is a
>>>>>> bug.  There may be more problems; the path lookup code is way too
>>>>>> clever.
>>>>>>      
>>>>> Indeed. Less can sometimes be more. My impression is that some of the
>>>>> cleverness was motivated by lacking qtree completion for the HMP.
>>>>>    
>>>> Can we disable abbreviations for QMP and only allow them for HMP?
>>>>
>>>> We can support that by adding a hidden argument to commands specifying
>>>> whether the input comes from a human or machine.  Abbreviations are
>>>> dangerous if they become ambiguous; a human can recover while a machine
>>>> can't.
>>>>
>>> Both QMP _and_ HMP suffer from ambitious and inconsistent addressing
>>> schemes. Therefore, I'm more and more in favor of [1]. We just need to
>>> add command line completion for option values, something that would be
>>> beneficial for 'drive_add file=...' as well.
>>>
>>> Jan
>>>
>>> [1] http://article.gmane.org/gmane.comp.emulators.qemu/72152
>> 
>> [1] = abolish the clever abbreviations in path lookup.  I agree we
>> should do that.
>
> Fine. No concerns regarding overlaying IDs and path specifications as well?

I'm fine with that.

Calling the overlayed argument "id" is not so nice.  We got a bunch of
other not-so-nice names in QMP, maybe we'll have a flag day to clean
them all up.
Jan Kiszka - May 31, 2010, 11:11 a.m.
Markus Armbruster wrote:
> Jan Kiszka <jan.kiszka@siemens.com> writes:
> 
>> Markus Armbruster wrote:
>>> Jan Kiszka <jan.kiszka@web.de> writes:
>>>
>>>> Avi Kivity wrote:
>>>>> On 05/29/2010 11:14 AM, Jan Kiszka wrote:
>>>>>>> Currently breaks down when IDs contain '/', but permitting that is a
>>>>>>> bug.  There may be more problems; the path lookup code is way too
>>>>>>> clever.
>>>>>>>      
>>>>>> Indeed. Less can sometimes be more. My impression is that some of the
>>>>>> cleverness was motivated by lacking qtree completion for the HMP.
>>>>>>    
>>>>> Can we disable abbreviations for QMP and only allow them for HMP?
>>>>>
>>>>> We can support that by adding a hidden argument to commands specifying
>>>>> whether the input comes from a human or machine.  Abbreviations are
>>>>> dangerous if they become ambiguous; a human can recover while a machine
>>>>> can't.
>>>>>
>>>> Both QMP _and_ HMP suffer from ambitious and inconsistent addressing
>>>> schemes. Therefore, I'm more and more in favor of [1]. We just need to
>>>> add command line completion for option values, something that would be
>>>> beneficial for 'drive_add file=...' as well.
>>>>
>>>> Jan
>>>>
>>>> [1] http://article.gmane.org/gmane.comp.emulators.qemu/72152
>>> [1] = abolish the clever abbreviations in path lookup.  I agree we
>>> should do that.
>> Fine. No concerns regarding overlaying IDs and path specifications as well?
> 
> I'm fine with that.
> 
> Calling the overlayed argument "id" is not so nice.  We got a bunch of
> other not-so-nice names in QMP, maybe we'll have a flag day to clean
> them all up.

For this case (device_del and device_show), I think we should simply
call it 'device'.

Jan

Patch

diff --git a/hw/hw.h b/hw/hw.h
index fc2d184..cc4bd5f 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -299,6 +299,7 @@  enum VMStateFlags {
 
 typedef struct {
     const char *name;
+    const char *start_index;
     size_t offset;
     size_t size;
     size_t start;
@@ -413,6 +414,7 @@  extern const VMStateInfo vmstate_info_unused_buffer;
     .size       = sizeof(_type),                                     \
     .flags      = VMS_ARRAY,                                         \
     .offset     = vmstate_offset_sub_array(_state, _field, _type, _start), \
+    .start_index = (stringify(_start)),                              \
 }
 
 #define VMSTATE_VARRAY_INT32(_field, _state, _field_num, _version, _info, _type) {\
diff --git a/hw/qdev.c b/hw/qdev.c
index 7db839f..a30ac56 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -29,6 +29,9 @@ 
 #include "qdev.h"
 #include "sysemu.h"
 #include "monitor.h"
+#include "qjson.h"
+#include "qint.h"
+#include "qbuffer.h"
 
 static int qdev_hotplug = 0;
 
@@ -890,3 +893,244 @@  int do_device_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
     }
     return qdev_unplug(dev);
 }
+
+#define NAME_COLUMN_WIDTH 23
+
+static void print_field(Monitor *mon, const QDict *qfield, int indent);
+
+static void print_elem(Monitor *mon, const QObject *qelem, size_t size,
+                       int column_pos, int indent)
+{
+    int64_t data_size;
+    const void *data;
+    int n;
+
+    if (qobject_type(qelem) == QTYPE_QDICT) {
+        if (column_pos >= 0) {
+            monitor_printf(mon, ".\n");
+        }
+    } else {
+        monitor_printf(mon, ":");
+        column_pos++;
+        if (column_pos < NAME_COLUMN_WIDTH) {
+            monitor_printf(mon, "%*c", NAME_COLUMN_WIDTH - column_pos, ' ');
+        }
+    }
+
+    switch (qobject_type(qelem)) {
+    case QTYPE_QDICT:
+        print_field(mon, qobject_to_qdict(qelem), indent + 2);
+        break;
+    case QTYPE_QBUFFER:
+        data = qbuffer_get_data(qobject_to_qbuffer(qelem));
+        data_size = qbuffer_get_size(qobject_to_qbuffer(qelem));
+        for (n = 0; n < data_size; ) {
+            monitor_printf(mon, " %02x", *((uint8_t *)data+n));
+            if (++n < size) {
+                if (n % 16 == 0) {
+                    monitor_printf(mon, "\n%*c", NAME_COLUMN_WIDTH, ' ');
+                } else if (n % 8 == 0) {
+                    monitor_printf(mon, " -");
+                }
+            }
+        }
+        if (data_size < size) {
+            monitor_printf(mon, " ...");
+        }
+        monitor_printf(mon, "\n");
+        break;
+    case QTYPE_QINT:
+        monitor_printf(mon, " %0*" PRIx64 "\n", (int)size * 2,
+                       qint_get_int(qobject_to_qint(qelem)));
+        break;
+    default:
+        assert(0);
+    }
+}
+
+static void print_field(Monitor *mon, const QDict *qfield, int indent)
+{
+    const char *name = qdict_get_str(qfield, "name");
+    const char *start = qdict_get_try_str(qfield, "start");
+    int64_t size = qdict_get_int(qfield, "size");
+    QList *qlist = qdict_get_qlist(qfield, "elems");
+    QListEntry *entry, *sub_entry;
+    QList *sub_list;
+    int elem_no = 0;
+
+    QLIST_FOREACH_ENTRY(qlist, entry) {
+        QObject *qelem = qlist_entry_obj(entry);
+        int pos = indent + strlen(name);
+
+        if (qobject_type(qelem) == QTYPE_QLIST) {
+            monitor_printf(mon, "%*c%s", indent, ' ', name);
+            if (start) {
+                pos += monitor_printf(mon, "[%s+%02x]", start, elem_no);
+            } else {
+                pos += monitor_printf(mon, "[%02x]", elem_no);
+            }
+            sub_list = qobject_to_qlist(qelem);
+            QLIST_FOREACH_ENTRY(sub_list, sub_entry) {
+                print_elem(mon, qlist_entry_obj(sub_entry), size, pos,
+                           indent + 2);
+                pos = -1;
+            }
+        } else {
+            if (elem_no == 0) {
+                monitor_printf(mon, "%*c%s", indent, ' ', name);
+            } else {
+                pos = -1;
+            }
+            print_elem(mon, qelem, size, pos, indent);
+        }
+        elem_no++;
+    }
+}
+
+void device_user_print(Monitor *mon, const QObject *data)
+{
+    QDict *qdict = qobject_to_qdict(data);
+    QList *qlist = qdict_get_qlist(qdict, "fields");
+    QListEntry *entry;
+
+    monitor_printf(mon, "dev: %s, id \"%s\"\n",
+                   qdict_get_str(qdict, "device"),
+                   qdict_get_str(qdict, "id"));
+
+    QLIST_FOREACH_ENTRY(qlist, entry) {
+        print_field(mon, qobject_to_qdict(qlist_entry_obj(entry)), 2);
+    }
+}
+
+static size_t parse_vmstate(const VMStateDescription *vmsd, void *opaque,
+                            QList *qlist, int full_buffers)
+{
+    VMStateField *field = vmsd->fields;
+    size_t overall_size = 0;
+
+    if (vmsd->pre_save) {
+        vmsd->pre_save(opaque);
+    }
+    while(field->name) {
+        if (!field->field_exists ||
+            field->field_exists(opaque, vmsd->version_id)) {
+            void *base_addr = opaque + field->offset;
+            int i, n_elems = 1;
+            int is_array = 1;
+            size_t size = field->size;
+            size_t real_size = 0;
+            size_t dump_size;
+            QDict *qfield = qdict_new();
+            QList *qelems = qlist_new();
+
+            qlist_append_obj(qlist, QOBJECT(qfield));
+
+            qdict_put_obj(qfield, "name",
+                          QOBJECT(qstring_from_str(field->name)));
+            qdict_put_obj(qfield, "elems", QOBJECT(qelems));
+
+            if (field->flags & VMS_VBUFFER) {
+                size = *(int32_t *)(opaque + field->size_offset);
+                if (field->flags & VMS_MULTIPLY) {
+                    size *= field->size;
+                }
+            }
+            if (field->start_index) {
+                qdict_put_obj(qfield, "start",
+                              QOBJECT(qstring_from_str(field->start_index)));
+            }
+
+            if (field->flags & VMS_ARRAY) {
+                n_elems = field->num;
+            } else if (field->flags & VMS_VARRAY_INT32) {
+                n_elems = *(int32_t *)(opaque + field->num_offset);
+            } else if (field->flags & VMS_VARRAY_UINT16) {
+                n_elems = *(uint16_t *)(opaque + field->num_offset);
+            } else {
+                is_array = 0;
+            }
+            if (field->flags & VMS_POINTER) {
+                base_addr = *(void **)base_addr + field->start;
+            }
+            for (i = 0; i < n_elems; i++) {
+                void *addr = base_addr + size * i;
+                QList *sub_elems = qelems;
+                int val;
+
+                if (is_array) {
+                    sub_elems = qlist_new();
+                    qlist_append_obj(qelems, QOBJECT(sub_elems));
+                }
+                if (field->flags & VMS_ARRAY_OF_POINTER) {
+                    addr = *(void **)addr;
+                }
+                if (field->flags & VMS_STRUCT) {
+                    real_size = parse_vmstate(field->vmsd, addr,
+                                              sub_elems, full_buffers);
+                } else {
+                    real_size = size;
+                    if (field->flags & (VMS_BUFFER | VMS_VBUFFER)) {
+                        dump_size = (full_buffers || size <= 16) ? size : 16;
+                        qlist_append_obj(sub_elems,
+                                QOBJECT(qbuffer_from_data(addr, dump_size)));
+                    } else {
+                        switch (size) {
+                        case 1:
+                            val = *(uint8_t *)addr;
+                            break;
+                        case 2:
+                            val = *(uint16_t *)addr;
+                            break;
+                        case 4:
+                            val = *(uint32_t *)addr;
+                            break;
+                        case 8:
+                            val = *(uint64_t *)addr;
+                            break;
+                        default:
+                            assert(0);
+                        }
+                        qlist_append_obj(sub_elems,
+                                         QOBJECT(qint_from_int(val)));
+                    }
+                }
+                overall_size += real_size;
+            }
+            qdict_put_obj(qfield, "size", QOBJECT(qint_from_int(real_size)));
+        }
+        field++;
+    }
+    return overall_size;
+}
+
+int do_device_show(Monitor *mon, const QDict *qdict, QObject **ret_data)
+{
+    const char *path = qdict_get_str(qdict, "path");
+    const VMStateDescription *vmsd;
+    DeviceState *dev;
+    QList *qlist;
+
+    dev = qdev_find_recursive(main_system_bus, path);
+    if (!dev) {
+        dev = qdev_find_internal(path, true);
+        if (!dev) {
+            return -1;
+        }
+    }
+
+    vmsd = dev->info->vmsd;
+    if (!vmsd) {
+        qerror_report(QERR_DEVICE_NO_STATE, dev->info->name);
+        error_printf_unless_qmp("Note: device may simply lack complete qdev "
+                                "conversion\n");
+        return -1;
+    }
+
+    *ret_data = qobject_from_jsonf("{ 'device': %s, 'id': %s }",
+                                   dev->info->name, dev->id ? : "");
+    qlist = qlist_new();
+    parse_vmstate(vmsd, dev, qlist, qdict_get_int(qdict, "full"));
+    qdict_put_obj(qobject_to_qdict(*ret_data), "fields", QOBJECT(qlist));
+
+    return 0;
+}
diff --git a/hw/qdev.h b/hw/qdev.h
index b27d33b..447d13c 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -183,6 +183,8 @@  void do_info_qtree(Monitor *mon);
 void do_info_qdm(Monitor *mon);
 int do_device_add(Monitor *mon, const QDict *qdict, QObject **ret_data);
 int do_device_del(Monitor *mon, const QDict *qdict, QObject **ret_data);
+void device_user_print(Monitor *mon, const QObject *data);
+int do_device_show(Monitor *mon, const QDict *qdict, QObject **ret_data);
 
 /*** qdev-properties.c ***/
 
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index eb257a6..cff16d7 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -734,6 +734,70 @@  Example:
 EQMP
 
     {
+        .name       = "device_show",
+        .args_type  = "path:Q,full:-f",
+        .params     = "device [-f]",
+        .help       = "show device state (specify -f for full buffer dumping)",
+        .user_print = device_user_print,
+        .mhandler.cmd_new = do_device_show,
+    },
+
+STEXI
+@item device_show @var{path} [@code{-f}]
+
+Show state of device @var{path} in a human-readable form. @var{path} can be
+an unique id specified during device creation or a full path in the device
+tree (see @code{info qtree}). Buffers are cut after 16 bytes unless a full
+dump is requested via @code{-f}.
+ETEXI
+SQMP
+device_show
+-----------
+
+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)
+- "full": report full state (json-bool, optional)
+
+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.
+
+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
+
+    {
         .name       = "cpu",
         .args_type  = "index:i",
         .params     = "index",
diff --git a/qerror.c b/qerror.c
index 034c7de..c50ff91 100644
--- a/qerror.c
+++ b/qerror.c
@@ -101,6 +101,10 @@  static const QErrorStringTable qerror_table[] = {
         .desc      = "Device '%(device)' has no child bus",
     },
     {
+        .error_fmt = QERR_DEVICE_NO_STATE,
+        .desc      = "No state available for device '%(device)'",
+    },
+    {
         .error_fmt = QERR_DUPLICATE_ID,
         .desc      = "Duplicate ID '%(id)' for %(object)",
     },
diff --git a/qerror.h b/qerror.h
index c98c61a..e5de6dd 100644
--- a/qerror.h
+++ b/qerror.h
@@ -91,6 +91,9 @@  QError *qobject_to_qerror(const QObject *obj);
 #define QERR_DEVICE_NO_BUS \
     "{ 'class': 'DeviceNoBus', 'data': { 'device': %s } }"
 
+#define QERR_DEVICE_NO_STATE \
+    "{ 'class': 'DeviceNoState', 'data': { 'device': %s } }"
+
 #define QERR_DUPLICATE_ID \
     "{ 'class': 'DuplicateId', 'data': { 'id': %s, 'object': %s } }"