On Fri, Feb 20, 2015 at 10:15:00 -0500, John Ferlan wrote:
>
>
> On 02/18/2015 09:16 AM, Peter Krempa wrote:
> > As there are two possible approaches to define a domain's memory size -
> > one used with legacy, non-NUMA VMs configured in the <memory> element
> > and per-node based approach on NUMA machines - the user needs to make
> > sure that both are specified correctly in the NUMA case.
> >
> > To avoid this burden on the user I'd like to replace the NUMA case with
> > automatic totaling of the memory size. To achieve this I need to replace
> > direct access to the virDomainMemtune's 'max_balloon' field with
> > two separate getters depending on the desired size.
> >
> > The two sizes are needed as:
> > 1) Startup memory size doesn't include memory modules in some
> > hypervisors.
> > 2) After startup these count as the usable memory size.
> >
> > Note that the comments for the functions are future aware and document
> > state that will be present after a few later patches.
> > ---
> > src/conf/domain_conf.c | 66 ++++++++++++++++++++++++++++++++++------
> > src/conf/domain_conf.h | 4 +++
> > src/hyperv/hyperv_driver.c | 2 +-
> > src/libvirt_private.syms | 3 ++
> > src/libxl/libxl_conf.c | 2 +-
> > src/libxl/libxl_driver.c | 8 ++---
> > src/lxc/lxc_cgroup.c | 2 +-
> > src/lxc/lxc_driver.c | 10 +++---
> > src/lxc/lxc_fuse.c | 4 +--
> > src/lxc/lxc_native.c | 4 +--
> > src/openvz/openvz_driver.c | 2 +-
> > src/parallels/parallels_driver.c | 2 +-
> > src/parallels/parallels_sdk.c | 12 ++++----
> > src/phyp/phyp_driver.c | 11 ++++---
> > src/qemu/qemu_command.c | 18 +++++++----
> > src/qemu/qemu_driver.c | 19 ++++++------
> > src/qemu/qemu_hotplug.c | 8 +++--
> > src/qemu/qemu_process.c | 2 +-
> > src/test/test_driver.c | 8 ++---
> > src/uml/uml_driver.c | 8 ++---
> > src/vbox/vbox_common.c | 4 +--
> > src/vmware/vmware_driver.c | 2 +-
> > src/vmx/vmx.c | 12 ++++----
> > src/xen/xm_internal.c | 14 ++++-----
> > src/xenapi/xenapi_driver.c | 2 +-
> > src/xenapi/xenapi_utils.c | 4 +--
> > src/xenconfig/xen_common.c | 8 +++--
> > src/xenconfig/xen_sxpr.c | 9 +++---
> > 28 files changed, 160 insertions(+), 90 deletions(-)
> >
> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > index 6ef499d..e95851a 100644
> > --- a/src/conf/domain_conf.c
> > +++ b/src/conf/domain_conf.c
> > @@ -3172,24 +3172,26 @@ virDomainDefPostParseInternal(virDomainDefPtr def,
> > return -1;
> > }
> >
> > - if (def->mem.cur_balloon > def->mem.max_balloon) {
> > + if (def->mem.cur_balloon > virDomainDefGetMemoryCurrent(def)) {
> > /* Older libvirt could get into this situation due to
> > * rounding; if the discrepancy is less than 4MiB, we silently
> > * round down, otherwise we flag the issue. */
> > if (VIR_DIV_UP(def->mem.cur_balloon, 4096) >
> > - VIR_DIV_UP(def->mem.max_balloon, 4096)) {
> > + VIR_DIV_UP(virDomainDefGetMemoryCurrent(def), 4096)) {
> > virReportError(VIR_ERR_XML_ERROR,
> > _("current memory '%lluk' exceeds "
> > "maximum '%lluk'"),
> > - def->mem.cur_balloon, def->mem.max_balloon);
> > + def->mem.cur_balloon,
> > + virDomainDefGetMemoryCurrent(def));
> > return -1;
> > } else {
> > VIR_DEBUG("Truncating current %lluk to maximum %lluk",
> > - def->mem.cur_balloon, def->mem.max_balloon);
> > - def->mem.cur_balloon = def->mem.max_balloon;
> > + def->mem.cur_balloon,
> > + virDomainDefGetMemoryCurrent(def));
> > + def->mem.cur_balloon = virDomainDefGetMemoryCurrent(def);
> > }
> > } else if (def->mem.cur_balloon == 0) {
> > - def->mem.cur_balloon = def->mem.max_balloon;
> > + def->mem.cur_balloon = virDomainDefGetMemoryCurrent(def);
> > }
> >
> > /*
> > @@ -6882,6 +6884,51 @@ virDomainParseMemory(const char *xpath,
> > }
> >
> >
> > +/**
> > + * virDomainDefGetMemoryInitial:
> > + * @def: domain definition
> > + *
> > + * Returns the size of the initial amount of guest memory. The initial amount
> > + * is the memory size is either the configured amount in the <memory> element
> > + * or the sum of memory sizes of NUMA nodes in case NUMA is enabled in @def.
> > + */
> > +unsigned long long
> > +virDomainDefGetMemoryInitial(virDomainDefPtr def)
> > +{
> > + return def->mem.max_balloon;
> > +}
> > +
> > +
> > +/**
> > + * virDomainDefSetMemoryInitial:
> > + * @def: domain definition
> > + * @size: size to set
> > + *
> > + * Sets the initial memory size in @def.
> > + */
> > +void
> > +virDomainDefSetMemoryInitial(virDomainDefPtr def,
> > + unsigned long long size)
> > +{
> > + def->mem.max_balloon = size;
> > +}
>
> Odd concept - changing Initial memory, but I don't have a better name in
> mind...
>
> > +
> > +
> > +/**
> > + * virDomainDefGetMemoryCurrent:
> > + * @def: domain definition
> > + *
> > + * Returns the current maximum memory size usable by the domain described by
> > + * @def. This size is a sum of size returned by virDomainDefGetMemoryInitial
> > + * and possible additional memory devices.
> > + */
> > +unsigned long long
> > +virDomainDefGetMemoryCurrent(virDomainDefPtr def)
> > +{
> > + return virDomainDefGetMemoryInitial(def);
> > +}
>
> Ok - so part of me expect this one to return mem.cur_balloon - perhaps a
> name change to GetMemoryUsable would help?
I'll try using GetMemoryAvailable for the next version.
>
> and of course begs the question why not bite the bullet and return
> mem.cur_balloon in a GetMemoryCurrent accessor...
I didn't need it currently. I'll look into it separately if I find it
necessary.
>
> > +
> > +
> > static int
> > virDomainControllerModelTypeFromString(const virDomainControllerDef *def,
> > const char *model)
> > @@ -15956,10 +16003,11 @@ virDomainDefCheckABIStability(virDomainDefPtr src,
> > goto error;
> > }
> >
> > - if (src->mem.max_balloon != dst->mem.max_balloon) {
> > + if (virDomainDefGetMemoryInitial(src) != virDomainDefGetMemoryInitial(dst)) {
> > virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> > _("Target domain max memory %lld does not match source %lld"),
> > - dst->mem.max_balloon, src->mem.max_balloon);
> > + virDomainDefGetMemoryInitial(dst),
> > + virDomainDefGetMemoryInitial(src));
> > goto error;
> > }
>
> Since the "future" (I assume) is that MemoryCurrent will include the hot
> (un)plug memory - does it make sense to also compare src/dst
> MemoryCurrent values even though for now it's essentially the same
> value? Although I believe that gets done in your later series and it's
> hard to keep them in my memory.
The memory modules will be compared individually so it doesn't matter
much.
>
> Of the remaining uses of GetInitial and {Get|Set}Current the ones I was
> wondering about are:
>
> virLXCCgroupSetupMemTune
> libxlMakeDomBuildInfo
> phypBuildLpar
>
> Wouldn't they be the corollary to qemuBuildCommandLine w/r/t using
> Initial instead of Current in order to define the memory for the guest
> as it starts up?
They are. I'll change them to use the Current/Available accessor
although it won't make a difference for now as memory hotplug is
explicitly forbidden for drivers other than qemu.
>
> John
Peter
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list