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

[Michal Privoznik]

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 <address@hidden>
>
> 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