Re: [PATCH 2/2] qmp: Expose MachineClass::default_ram_id

[Michal Privoznik <mprivozn@redhat.com>]

On 5/25/20 8:06 PM, Eduardo Habkost wrote:
> On Mon, May 25, 2020 at 07:03:28PM +0200, Michal Privoznik wrote:
>> If a management application (like Libvirt) want's to preserve
>> migration ability and switch to '-machine memory-backend' it
>> needs to set exactly the same RAM id as QEMU would. Since the id
>> is machine type dependant, expose it under 'query-machines'
>> result.
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> 
> The code looks good, but documentation was a bit confusing:
> 
>> ---
> [...]
>> +# @default-ram-id: the default name of initial RAM memory region (since 5.1)
>> +#
> 
> Everywhere else in the commit message you call it "id", but here
> you say "name".  Also, I don't think we have any references to a
> "memory region" abstraction in the docs for the QAPI schema,
> -machine options, or memory backend objects.
> 
> I had to look it up in the code, to finally understand you were
> talking about the memory backend object ID.
> 
> To make it consistent with terminology used for -machine and
> QAPI, I suggest:
> 
>    @default-ram-id: the default ID of initial RAM memory backend (since 5.1)
> 
> I can change it before committing, if you agree.

Thanks for the offer, but I will post a v2, because as I was developing 
patches for libvirt to consume this I found out that some machine types 
don't have the attribute set (riscv is one of them). Therefore I will 
have to make this optional.

Michal