Re: [Qemu-devel] [PATCH v8 13/13] dump: add 'query-dump-guest-memory-capability' command

[Laszlo Ersek]

On 02/11/14 00:09, Paolo Bonzini wrote:
> Il 10/02/2014 23:02, Laszlo Ersek ha scritto:
>> On 02/10/14 20:10, Luiz Capitulino wrote:
>>> On Tue, 28 Jan 2014 14:22:06 +0800
>>> qiaonuohan <address@hidden> wrote:
>>>
>>>> 'query-dump-guest-memory-capability' is used to query whether option
>>>> 'format'
>>>> is available for 'dump-guest-memory' and the available format. The
>>>> output
>>>> of the command will be like:
>>>>
>>>> -> { "execute": "query-dump-guest-memory-capability" }
>>>> <- { "return": {
>>>>         "format-option": "optional",
>>>>         "capabilities": [
>>>>             {"available": true, "format": "elf"},
>>>>             {"available": true, "format": "kdump-zlib"},
>>>>             {"available": true, "format": "kdump-lzo"},
>>>>             {"available": true, "format": "kdump-snappy"}
>>>>         ]
>>>>     }
>>>
>>> I don't want to hold this series anymore, this series is long and I
>>> know it
>>> took you and Laszlo's a long time to get it right. On the other hand
>>> we can't
>>> allow every single command to have its own introspection protocol.
>>>
>>> I think I'm fine accepting this one now, as long as it's fine for
>>> libvirt
>>> too. Eric, can you confirm this please?
>>
>> We discussed this before, and Eric participated. In fact the custom
>> introspection was one of his recommendations.
>>
>> http://thread.gmane.org/gmane.comp.emulators.qemu/221270/focus=246650
>>
>> (Which I agreed with because it would give us the most independence.)
>>
>> Of course I'm not trying to imply that this one specific interface will
>> doubtlessly serve all of libvirt's needs wrt. the kdump feature. We
>> certainly need Eric to sign off on that.
> 
> I think the justification here is that even if you defined an Enum for
> ['elf','kdump-zlib', 'kdump-lzo', 'kdump-snappy'], it would not be
> enough to describe which values were compiled in (as opposed to
> supported by the particular QEMU version).
> 
> However, I don't see the point in having the "format-option" field. What
> about:
> 
> -> { "execute": "query-dump-guest-memory-capabilities" }
> <- { "return": { "formats":
>           ["elf", "kdump-zlib", "kdump-lzo", "kdump-snappy"] }

Technically you might be right. However, this partial introspection
feature is entirely a wart whose existence is exclusively justified by
non-technical reasons, such as deadlines, and not wanting to be blocked
indefinitely by architecture astronautics around full introspection. I
don't see the point of polishing it beyond bare usability, at least not
after I've reviewed three versions of the patchset.

That's of course just my opinion... :)

Laszlo