[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 2/2] qmp: Expose MachineClass::default_ram_id
From: |
Eduardo Habkost |
Subject: |
Re: [PATCH 2/2] qmp: Expose MachineClass::default_ram_id |
Date: |
Mon, 25 May 2020 14:06:03 -0400 |
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.
> # @default-cpu-type: default CPU model typename if none is requested via
> # the -cpu argument. (since 4.2)
> #
> @@ -361,7 +363,8 @@
> 'data': { 'name': 'str', '*alias': 'str',
> '*is-default': 'bool', 'cpu-max': 'int',
> 'hotpluggable-cpus': 'bool', 'numa-mem-supported': 'bool',
> - 'deprecated': 'bool', '*default-cpu-type': 'str' } }
> + 'deprecated': 'bool', 'default-ram-id': 'str',
> + '*default-cpu-type': 'str' } }
>
> ##
> # @query-machines:
> --
> 2.26.2
>
--
Eduardo