Re: [RFC PATCH 02/12] conf: Add support for parsing and formatting max memory and slot count

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 01/30/2015 06:20 AM, Peter Krempa wrote:
> Add a XML element that will allow to specify maximum supportable memory

s/a XML/an XML/

> and the count of memory slots to use with memory hotplug.

Might be nice to demonstrate that XML here in the commit message, not
just in formatdomain.html.

> 
> To avoid possible confusion and misuse of the new element this patch
> also explicitly forbids the use of the maxMemory setting in individual
> drivers's post parse callbacks. This limitation will be lifted when the

s/drivers's/drivers'/

> support will be implemented.
> ---
>  docs/formatdomain.html.in            | 19 +++++++++++
>  docs/schemas/domaincommon.rng        |  8 +++++
>  src/bhyve/bhyve_domain.c             |  4 +++
>  src/conf/domain_conf.c               | 64 ++++++++++++++++++++++++++++++++++++
>  src/conf/domain_conf.h               |  7 ++++
>  src/libvirt_private.syms             |  1 +
>  src/libxl/libxl_domain.c             |  5 +++
>  src/lxc/lxc_domain.c                 |  4 +++
>  src/openvz/openvz_driver.c           | 11 +++++--
>  src/parallels/parallels_driver.c     |  6 +++-
>  src/phyp/phyp_driver.c               |  6 +++-
>  src/qemu/qemu_domain.c               |  4 +++
>  src/uml/uml_driver.c                 |  6 +++-
>  src/vbox/vbox_common.c               |  6 +++-
>  src/vmware/vmware_driver.c           |  6 +++-
>  src/vmx/vmx.c                        |  6 +++-
>  src/xen/xen_driver.c                 |  4 +++
>  src/xenapi/xenapi_driver.c           |  6 +++-
>  tests/domainschemadata/maxMemory.xml | 19 +++++++++++
>  19 files changed, 183 insertions(+), 9 deletions(-)
>  create mode 100644 tests/domainschemadata/maxMemory.xml
> 
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index f8d5f89..12f7ede 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -664,6 +664,7 @@
>  <pre>
>  &lt;domain&gt;
>    ...
> +  &lt;maxMemory slots='123' unit='KiB'&gt;1524288&lt;/maxMemory&gt;

123 is unusual; the example would look more realistic with a power of 2.

>    &lt;memory unit='KiB'&gt;524288&lt;/memory&gt;
>    &lt;currentMemory unit='KiB'&gt;524288&lt;/currentMemory&gt;

Hmm.  Historically, <memory> was the maximum memory, then ballooning was
introduced and <currentMemory> was added to show the live difference
between the ballooned current value and the boot-up maximum.

But with the idea of hot-plug, I see where you are coming from - the
balloon can only inflate or deflate up to the amount of memory currently
plugged in, so <maxMemory> is the startup maximum, <memory> becomes the
amount plugged in, and <currentMemory> reflects the balloon value (that
is, current <= memory <= max).  So I guess this makes sense; it may be
more interesting figuring out how to expose it all through virsh.

>    ...
> @@ -697,6 +698,24 @@
>          <span class='since'><code>unit</code> since 0.9.11</span>,
>          <span class='since'><code>dumpCore</code> since 0.10.2
>          (QEMU only)</span></dd>
> +      <dt><code>maxMemory</code></dt>
> +      <dd>The run time maximum memory allocation of the guest. The initial
> +        memory specified by <code>&lt;memory&gt;</code> can be increased by
> +        hot-plugging of memory to the limit specified by this element.
> +
> +        The <code>unit</code> attribute behaves the same as for
> +        <code>memory</code>.
> +
> +        The <code>slots</code> attribute specifies the number of slots
> +        available for adding memory to the guest. The bounds are hypervisor
> +        specific.
> +
> +        Note that due to alignment of the memory chunks added via memory
> +        hotplug the full size allocation specified by this element may be
> +        impossible to achieve.

Is a hypervisor free to reject requests that aren't aligned properly?
With <memory>, we had the odd situation that we allowed the hypervisor
to round requests up, so that live numbers were different than the
original startup numbers; it might be easier to not repeat that.


> +      <optional>
> +        <element name="maxMemory">
> +          <ref name="scaledInteger"/>
> +          <attribute name="slots">
> +            <ref name="unsignedInt"/>
> +          </attribute>

This says the slots attribute mandatory; is there ever a reason to allow
it to be optional (defaulting to one slot, an all-or-none hotplug)?


> +/**
> + * virDomainDefCheckUnsupportedMemoryHotplug:
> + * @def: domain definition
> + *
> + * Returns -1 if the domain definition would enable memory hotplug via the
> + * <maxMemory> tunable and reports an error. Otherwise returns 0.
> + */
> +int
> +virDomainDefCheckUnsupportedMemoryHotplug(virDomainDefPtr def)
> +{
> +    /* memory hotplug tunables are not supported by this driver */
> +    if (def->mem.max_memory > 0 || def->mem.memory_slots > 0) {

Based on the XML, mem.memory_slots cannot be specified unless max_memory
is also present (at least, assuming that you enforce that <maxMemory> be
>= <memory>).  But I guess it doesn't hurt to check both values.


> @@ -12735,6 +12756,16 @@ virDomainDefParseXML(xmlDocPtr xml,
>                               &def->mem.cur_balloon, false, true) < 0)
>          goto error;
> 
> +    if (virDomainParseMemory("./maxMemory[1]", NULL, ctxt,
> +                             &def->mem.max_memory, false, false) < 0)
> +        goto error;
> +
> +    if (virXPathUInt("string(./maxMemory[1]/@slots)", ctxt, &def->mem.memory_slots) == -2) {
> +        virReportError(VIR_ERR_XML_ERROR, "%s",
> +                       _("Failed to parse memory slot count"));

At least this agrees with the RelaxNG that slots is mandatory, for now.

> 
> +    if ((def->mem.max_memory || def->mem.memory_slots) &&
> +        !(def->mem.max_memory && def->mem.memory_slots)) {

Shorter, but not necessarily easier to understand, as:

if (!def->mem.max_memory != !def->mem.memory_slots) {

> +        virReportError(VIR_ERR_XML_ERROR, "%s",
> +                       _("both maximum memory size and "
> +                         "memory slot count must be sepcified"));

s/sepcified/specified/

Overall, looks reasonable for now, although I want to get through the
series first to see if anything else might be needed.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]