Hi David,
Thank you for the input. Please find my comments below.
> Is the idea to have something like hmp_info_numa() ("info NUMA"), just
> for qmp? And so it also works without NUMA?
..
> We also have query-memory-devices for that already.
The initial idea was to have a command for both HMP and QMP which would
report
a short summary regarding to memory which was assigned to a VM taking into
account
all the relevant "kinds" of memory (base/static, hot-plugged etc). Instead
of calling
"info memdev" + "info numa" + "info balloon" (if relevant) -- "info memory"
/ "query-memory"
can provide short summary within single monitor call. What do you think
about this in general?
> I don't think ballooning belongs into this at all.
..
> Can't you simply change query-balloon to show 0 in case no balloon is
> around? Or why can't your caller simply deal with the fact that querying
> the balloon might fail? Making a qmp interface directly copy the data
> from another qmp interface looks strange.
This can be removed from patch if it's confusing. The reason why it
appeared here is
the short discussion in https://lists.gnu.org/archive/
html/qemu-devel/2012-07/msg01472.html
where it was proposed.
> This seems to be x86 specific.
> Some machines (e.g. s390x) will not be properly accounted here.
> (e.g. they round ram_size up/down) or don't use DIMMs. I think we should
> query the machines instead.
In patch v3 (
http://lists.nongnu.org/archive/html/qemu-devel/2017-06/msg03475.html)
the command reports zero hot-plugged memory on machines which have
CONFIG_MEM_HOTPLUG disabled. No error emitted for such machines. Patch was
submitted before I received input from you and of course v4 will consider
all the points.
> So what you could do:
>
> - total_base memory
> - total hotplugged memory
> - base_memory per NUMA node
> - hotplugged memory per NUMA node
I will provide an extra patch which will extend "info numa" output to
additionally provide
information about hot-plugged memory per NUMA node and current patch will
utilize
this functionality. Actual "info memory" command (as well as
"query-memory") will provide
information as you suggested.
Thank you,
Vadim
On Mon, Jun 19, 2017 at 9:47 AM, David Hildenbrand <da...@redhat.com> wrote:
> On 13.06.2017 14:55, Vadim Galitsyn wrote:
> > Commands above provide the following memory information in bytes:
>
> Is the idea to have something like hmp_info_numa() ("info NUMA"), just
> for qmp? And so it also works without NUMA?
>
> I think, for this command to be helpful, you should include a per-NUMA
> node information.
>
> So what you could do:
>
> - total_base memory
> - total hotplugged memory
> - base_memory per NUMA node
> - hotplugged memory per NUMA node
>
> >
> > * hot-plug-memory - amount of memory that was hot-plugged.
> >
>
> We also have query-memory-devices for that already.
>
> > * ballooned-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, zero value is reported.
>
> I don't think ballooning belongs into this at all.
>
> >
> > NOTE:
> >
> > Parameter @ballooned-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.
>
> Can't you simply change query-balloon to show 0 in case no balloon is
> around? Or why can't your caller simply deal with the fact that querying
> the balloon might fail? Making a qmp interface directly copy the data
> from another qmp interface looks strange.
>
> >
> > Signed-off-by: Vasilis Liaskovitis <vasilis.liaskovi...@profitbricks.com
> >
> > Signed-off-by: Mohammed Gamal <mohammed.ga...@profitbricks.com>
> > Signed-off-by: Eduardo Otubo <eduardo.ot...@profitbricks.com>
> > Signed-off-by: Vadim Galitsyn <vadim.galit...@profitbricks.com>
> > Reviewed-by: Eugene Crosser <evgenii.cherkas...@profitbricks.com>
> > Cc: Dr. David Alan Gilbert <dgilb...@redhat.com>
> > Cc: Markus Armbruster <arm...@redhat.com>
> > Cc: qemu-devel@nongnu.org
> > ---
> [...]
> > +
> > +MemoryInfo *qmp_query_memory(Error **errp)
> > +{
> > + MemoryInfo *mem_info = g_malloc0(sizeof(MemoryInfo));
> > + BalloonInfo *balloon_info;
> > + Error *local_err = NULL;
> > +
> > + mem_info->base_memory = ram_size;
> > + mem_info->hot_plug_memory = pc_existing_dimms_capacity(&local_err);
>
> This seems to be x86 specific.
> Some machines (e.g. s390x) will not be properly accounted here.
> (e.g. they round ram_size up/down) or don't use DIMMs. I think we should
> query the machines instead.
>
> > + if (local_err) {
> > + error_setg(errp, "could not get hot-plug memory info: %s",
> > + error_get_pretty(local_err));
> > + g_free(mem_info);
> > + return NULL;
> > + }
> > +
> > + /* In case if it is not possible to get balloon info, just ignore
> it. */
> > + balloon_info = qmp_query_balloon(&local_err);
> > + if (local_err) {
> > + mem_info->ballooned_actual_memory = 0;
> > + error_free(local_err);
> > + } else {
> > + mem_info->ballooned_actual_memory = balloon_info->actual;
> > + }
>
> --
>
> Thanks,
>
> David
>
--
Vadim Galitsin
Software Engineer
ProfitBricks GmbH
Greifswalder Str. 207
D - 10405 Berlin
Fax: +49 30 577 008 299
Email: vadim.galit...@profitbricks.com
URL: https://www.profitbricks.de
Sitz der Gesellschaft: Berlin
Registergericht: Amtsgericht Charlottenburg, HRB 125506 B
Geschäftsführer: Achim Weiss
