Re: [Qemu-devel] [PATCH v4] hmp, qmp: introduce memory-size-summary commands

[Vadim Galitsyn]

Hi Guys,

Thank you for the feedback! Unfortunately, I am almost off for vacation and
will not be able to provide next patch in the following couple of weeks.
Nevertheless, will do so as soon as I return back.

I still have a question whether we need to provide NUMA information here?
>From my point of view it is a little bit out of scope of these commands
since it gives a bit more detailed information while the commands aimed to
give a short summary. I have a small patch which extends "info numa" with
hotplugged memory information per NUMA node. Maybe it is better to go this
way? I can include it into this patch series.

In the next patch (2 weeks later unfortunately) I will take into account
all the remaining points from this thread:

1. Turn into patch series by splitting HMP/QMP into separate patches;
2. Drop ballooning information;
3. Include "info numa" patch which extents output with hotplugged memory
info per NUMA node.

Thank you for the feedback once again,
Vadim


On Fri, Jul 7, 2017 at 10:58 AM, Markus Armbruster <address@hidden>
wrote:

> "Dr. David Alan Gilbert" <address@hidden> writes:
>
> > * Markus Armbruster (address@hidden) wrote:
> >> Sorry for the late review, got a bit overwhelmed...
> >>
> >> Vadim Galitsyn <address@hidden> writes:
> >>
> >> > Commands above provide the following memory information in bytes:
> >> >
> >> >   * base-memory - amount of unremovable memory specified
> >> >     with '-m' option at the start of the QEMU process.
> >> >
> >> >   * hotpluggable-memory - amount of memory that was hot-plugged.
> >> >     If target does not have CONFIG_MEM_HOTPLUG enabled, no
> >> >     value is reported.
> >> >
> >> >   * balloon-actual-memory - size of the memory that remains
> >> >     available to the guest after ballooning, as reported by the
> >> >     guest. If the guest has not reported its memory, this value
> >> >     equals to @base-memory + @hot-plug-memory. If ballooning
> >> >     is not enabled, no value is reported.
> >> >
> >> > NOTE:
> >> >
> >> >     Parameter @balloon-actual-memory reports the same as
> >> >     "info balloon" command when ballooning is enabled. The idea
> >> >     to have it in scope of this command(s) comes from
> >> >     https://lists.gnu.org/archive/html/qemu-devel/2012-07/
> msg01472.html.
> [...]
> >> >  hmp-commands-info.hx                               | 17 ++++++++++++
> >> >  hmp.c                                              | 23
> ++++++++++++++++
> >> >  hmp.h                                              |  1 +
> >> >  hw/mem/pc-dimm.c                                   |  6 +++++
> >> >  include/hw/mem/pc-dimm.h                           |  1 +
> >> >  qapi-schema.json                                   | 28
> +++++++++++++++++++
> >> >  qmp.c                                              | 31
> ++++++++++++++++++++++
> >> >  stubs/Makefile.objs                                |  2 +-
> >> >  stubs/{qmp_pc_dimm_device_list.c => qmp_pc_dimm.c} |  5 ++++
> >> >  9 files changed, 113 insertions(+), 1 deletion(-)
> >> >  rename stubs/{qmp_pc_dimm_device_list.c => qmp_pc_dimm.c} (60%)
> >>
> >> No test coverage?
> >>
> >> I prefer to add pairs of QMP / HMP commands in separate patches, QMP
> >> first, for easier review.  This patch seems small enough to tolerate
> >> adding them in a single patch.  But do consider splitting if you have to
> >> respin.
> >
> > The HMP tester scans all 'info' commands so it's not needed for the HMP
> > side.
>
> Correct.  I want a similar test for QMP, but I'm not asking Vadim to
> provide it :)
>