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

[libvirt] [PATCHv2 10/15] xml: allow scaled memory on input



Output is still in kibibytes, but input can now be in different
scales for ease of typing.

* src/conf/domain_conf.c (virDomainParseMemory): New helper.
(virDomainDefParseXML): Use it when parsing.
* docs/schemas/domaincommon.rng: Expand XML; rename memoryKBElement
to memoryElement and update callers.
* docs/formatdomain.html.in (elementsMemoryAllocation): Document
scaling.
* tests/qemuxml2argvdata/qemuxml2argv-memtune.xml: Adjust test.
* tests/qemuxml2xmltest.c: Likewise.
* tests/qemuxml2xmloutdata/qemuxml2xmlout-memtune.xml: New file.
---

v2: reuse code introduced earlier in series, add tests

 docs/formatdomain.html.in                          |   42 +++++++--
 docs/schemas/domaincommon.rng                      |   23 ++----
 src/conf/domain_conf.c                             |   93 +++++++++++++++----
 tests/qemuxml2argvdata/qemuxml2argv-memtune.xml    |    6 +-
 .../qemuxml2xmloutdata/qemuxml2xmlout-memtune.xml  |   31 +++++++
 tests/qemuxml2xmltest.c                            |    2 +-
 6 files changed, 147 insertions(+), 50 deletions(-)
 create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-memtune.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 0855c7f..1e4a990 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -415,8 +415,8 @@
 <pre>
 &lt;domain&gt;
   ...
-  &lt;memory&gt;524288&lt;/memory&gt;
-  &lt;currentMemory&gt;524288&lt;/currentMemory&gt;
+  &lt;memory unit='KiB'&gt;524288&lt;/memory&gt;
+  &lt;currentMemory unit='KiB'&gt;524288&lt;/currentMemory&gt;
   ...
 &lt;/domain&gt;
 </pre>
@@ -424,12 +424,30 @@
     <dl>
       <dt><code>memory</code></dt>
       <dd>The maximum allocation of memory for the guest at boot time.
-        The units for this value are kibibytes (i.e. blocks of 1024 bytes)</dd>
+        The units for this value are determined by the optional
+        atttribute <code>unit</code>, which defaults to "KiB"
+        (kibibytes, 2<sup>10</sup> or blocks of 1024 bytes).  Valid
+        units are "b" or "bytes" for bytes, "KB" for kilobytes
+        (10<sup>3</sup> or 1,000), "k" or "KiB" for kibibytes (1024),
+        "MB" for megabytes (10<sup>6</sup> or 1,000,000), "M" or "MiB"
+        for mebibytes (2<sup>20</sup> or 1,048,576), "GB" for
+        gigabytes (10<sup>9</sup> or 1,000,000,000), "G" or "GiB" for
+        gibibytes (2<sup>30</sup> or 1,073,741,824), "TB" for
+        terabytes (10<sup>12</sup> or 1,000,000,000,000), or "T" or
+        "TiB" for tebibytes (2<sup>40</sup> or 1,099,511,627,776).
+        However, the value will be rounded up to the nearest kibibyte
+        by libvirt, and may be further rounded to the granularity
+        supported by the hypervisor.  Some hypervisors also enforce a
+        minimum, such as
+        4000KiB. <span class='since'><code>unit</code> since
+        0.9.11</span></dd>
       <dt><code>currentMemory</code></dt>
       <dd>The actual allocation of memory for the guest. This value can
         be less than the maximum allocation, to allow for ballooning
         up the guests memory on the fly. If this is omitted, it defaults
-        to the same value as the <code>memory</code> element</dd>
+        to the same value as the <code>memory</code> element.
+        The <code>unit</code> attribute behaves the same as
+        for <code>memory</code>.</dd>
     </dl>


@@ -460,10 +478,10 @@
 &lt;domain&gt;
   ...
   &lt;memtune&gt;
-    &lt;hard_limit&gt;1048576&lt;/hard_limit&gt;
-    &lt;soft_limit&gt;131072&lt;/soft_limit&gt;
-    &lt;swap_hard_limit&gt;2097152&lt;/swap_hard_limit&gt;
-    &lt;min_guarantee&gt;65536&lt;/min_guarantee&gt;
+    &lt;hard_limit unit='G'&gt;1&lt;/hard_limit&gt;
+    &lt;soft_limit unit='M'&gt;128&lt;/soft_limit&gt;
+    &lt;swap_hard_limit unit='G'&gt;2&lt;/swap_hard_limit&gt;
+    &lt;min_guarantee unit='bytes'&gt;67108864&lt;/min_guarantee&gt;
   &lt;/memtune&gt;
   ...
 &lt;/domain&gt;
@@ -477,7 +495,13 @@
         parameters are applied to the QEMU process as a whole. Thus, when
         counting them, one needs to add up guest RAM, guest video RAM, and
         some memory overhead of QEMU itself. The last piece is hard to
-        determine so one needs guess and try.</dd>
+        determine so one needs guess and try.  For each tunable, it
+        is possible to designate which unit the number is in on
+        input, using the same values as
+        for <code>&lt;memory&gt;</code>.  For backwards
+        compatibility, output is always in
+        KiB.  <span class='since'><code>unit</code>
+        since 0.9.11</span></dd>
       <dt><code>hard_limit</code></dt>
       <dd> The optional <code>hard_limit</code> element is the maximum memory
         the guest can use. The units for this value are kibibytes (i.e. blocks
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 99b3861..dfa1d1e 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -412,11 +412,11 @@
   <define name="resources">
     <interleave>
       <element name="memory">
-        <ref name='memoryKBElement'/>
+        <ref name='scaledInteger'/>
       </element>
       <optional>
         <element name="currentMemory">
-          <ref name='memoryKBElement'/>
+          <ref name='scaledInteger'/>
         </element>
       </optional>
       <optional>
@@ -461,25 +461,25 @@
           <!-- Maximum memory the VM can use -->
           <optional>
             <element name="hard_limit">
-              <ref name='memoryKBElement'/>
+              <ref name='scaledInteger'/>
             </element>
           </optional>
           <!-- Minimum memory ascertained for the VM during contention -->
           <optional>
             <element name="soft_limit">
-              <ref name='memoryKBElement'/>
+              <ref name='scaledInteger'/>
             </element>
           </optional>
           <!-- Minimum amount of memory required to start the VM -->
           <optional>
             <element name="min_guarantee">
-              <ref name='memoryKBElement'/>
+              <ref name='scaledInteger'/>
             </element>
           </optional>
           <!-- Maximum swap area the VM can use -->
           <optional>
             <element name="swap_hard_limit">
-              <ref name='memoryKBElement'/>
+              <ref name='scaledInteger'/>
             </element>
           </optional>
         </element>
@@ -3082,17 +3082,6 @@
       <param name="pattern">[0-9]+</param>
     </data>
   </define>
-  <!-- Memory as an element, with optional unit attribute -->
-  <define name='memoryKBElement'>
-    <optional>
-      <attribute name='unit'>
-        <value>KiB</value>
-      </attribute>
-    </optional>
-    <data type='unsignedInt'>
-      <param name='pattern'>[0-9]+</param>
-    </data>
-  </define>
   <define name="domainName">
     <data type="string">
       <!-- Use literal newline instead of \n for bug in libxml2 2.7.6 -->
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index fa44d3e..3bc2fae 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7169,6 +7169,60 @@ static int virDomainDefMaybeAddController(virDomainDefPtr def,
     return 0;
 }

+
+/* Parse a memory element located at XPATH within CTXT, and store the
+ * result into MEM.  If REQUIRED, then the value must exist;
+ * otherwise, the value is optional.  The value is in blocks of 1024.
+ * Return 0 on success, -1 on failure after issuing error.  */
+static int
+virDomainParseMemory(const char *xpath, xmlXPathContextPtr ctxt,
+                     unsigned long long *mem, bool required)
+{
+    char *xpath_full = NULL;
+    char *unit = NULL;
+    int ret = -1;
+    unsigned long long bytes;
+    unsigned long long max;
+
+    *mem = 0;
+    if (virAsprintf(&xpath_full, "string(%s)", xpath) < 0) {
+        virReportOOMError();
+        goto cleanup;
+    }
+    if (virXPathULongLong(xpath_full, ctxt, &bytes) < 0) {
+        if (required)
+            virDomainReportError(VIR_ERR_XML_ERROR,
+                                 "%s", _("missing memory element"));
+        else
+            ret = 0;
+        goto cleanup;
+    }
+    VIR_FREE(xpath_full);
+
+    if (virAsprintf(&xpath_full, "string(%s/@unit)", xpath) < 0) {
+        virReportOOMError();
+        goto cleanup;
+    }
+    unit = virXPathString(xpath_full, ctxt);
+    /* On 32-bit machines, our bound is 0xffffffff * KiB. On 64-bit
+     * machines, our bound is off_t (2^63).  */
+    if (sizeof(unsigned long) < sizeof(long long))
+        max = 1024ull * ULONG_MAX;
+    else
+        max = LLONG_MAX;
+    if (virScaleInteger(&bytes, unit, 1024, max) < 0)
+        goto cleanup;
+
+    /* Yes, we really do use kibibytes for our internal sizing.  */
+    *mem = VIR_DIV_UP(bytes, 1024);
+    ret = 0;
+cleanup:
+    VIR_FREE(xpath_full);
+    VIR_FREE(unit);
+    return ret;
+}
+
+
 static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
                                             xmlDocPtr xml,
                                             xmlNodePtr root,
@@ -7292,22 +7346,21 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
         goto error;

     /* Extract domain memory */
-    if (virXPathULongLong("string(./memory[1])", ctxt,
-                          &def->mem.max_balloon) < 0) {
-        virDomainReportError(VIR_ERR_INTERNAL_ERROR,
-                             "%s", _("missing memory element"));
+    if (virDomainParseMemory("./memory[1]", ctxt,
+                             &def->mem.max_balloon, true) < 0)
         goto error;
-    }

-    if (virXPathULongLong("string(./currentMemory[1])", ctxt,
-                          &def->mem.cur_balloon) < 0)
-        def->mem.cur_balloon = def->mem.max_balloon;
+    if (virDomainParseMemory("./currentMemory[1]", ctxt,
+                             &def->mem.cur_balloon, false) < 0)
+        goto error;

     if (def->mem.cur_balloon > def->mem.max_balloon) {
         virDomainReportError(VIR_ERR_INTERNAL_ERROR,
                              _("current memory '%lluk' exceeds maximum '%lluk'"),
                              def->mem.cur_balloon, def->mem.max_balloon);
         goto error;
+    } else if (def->mem.cur_balloon == 0) {
+        def->mem.cur_balloon = def->mem.max_balloon;
     }

     node = virXPathNode("./memoryBacking/hugepages", ctxt);
@@ -7346,21 +7399,21 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
     VIR_FREE(nodes);

     /* Extract other memory tunables */
-    if (virXPathULongLong("string(./memtune/hard_limit)", ctxt,
-                          &def->mem.hard_limit) < 0)
-        def->mem.hard_limit = 0;
+    if (virDomainParseMemory("./memtune/hard_limit[1]", ctxt,
+                             &def->mem.hard_limit, false) < 0)
+        goto error;

-    if (virXPathULongLong("string(./memtune/soft_limit[1])", ctxt,
-                          &def->mem.soft_limit) < 0)
-        def->mem.soft_limit = 0;
+    if (virDomainParseMemory("./memtune/soft_limit[1]", ctxt,
+                             &def->mem.soft_limit, false) < 0)
+        goto error;

-    if (virXPathULongLong("string(./memtune/min_guarantee[1])", ctxt,
-                          &def->mem.min_guarantee) < 0)
-        def->mem.min_guarantee = 0;
+    if (virDomainParseMemory("./memtune/min_guarantee[1]", ctxt,
+                             &def->mem.min_guarantee, false) < 0)
+        goto error;

-    if (virXPathULongLong("string(./memtune/swap_hard_limit[1])", ctxt,
-                          &def->mem.swap_hard_limit) < 0)
-        def->mem.swap_hard_limit = 0;
+    if (virDomainParseMemory("./memtune/swap_hard_limit[1]", ctxt,
+                             &def->mem.swap_hard_limit, false) < 0)
+        goto error;

     n = virXPathULong("string(./vcpu[1])", ctxt, &count);
     if (n == -2) {
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-memtune.xml b/tests/qemuxml2argvdata/qemuxml2argv-memtune.xml
index b6c8c95..ee08753 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-memtune.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-memtune.xml
@@ -1,12 +1,12 @@
 <domain type='qemu'>
   <name>QEMUGuest1</name>
   <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
-  <memory unit='KiB'>219136</memory>
+  <memory unit='MiB'>214</memory>
   <currentMemory unit='KiB'>219136</currentMemory>
   <memtune>
     <hard_limit unit='KiB'>512000</hard_limit>
-    <soft_limit unit='KiB'>128000</soft_limit>
-    <swap_hard_limit unit='KiB'>1024000</swap_hard_limit>
+    <soft_limit unit='bytes'>131071999</soft_limit>
+    <swap_hard_limit unit='KB'>1048576</swap_hard_limit>
   </memtune>
   <vcpu>1</vcpu>
   <os>
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-memtune.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-memtune.xml
new file mode 100644
index 0000000..b6c8c95
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-memtune.xml
@@ -0,0 +1,31 @@
+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219136</memory>
+  <currentMemory unit='KiB'>219136</currentMemory>
+  <memtune>
+    <hard_limit unit='KiB'>512000</hard_limit>
+    <soft_limit unit='KiB'>128000</soft_limit>
+    <swap_hard_limit unit='KiB'>1024000</swap_hard_limit>
+  </memtune>
+  <vcpu>1</vcpu>
+  <os>
+    <type arch='i686' machine='pc'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu</emulator>
+    <disk type='block' device='disk'>
+      <source dev='/dev/HostVG/QEMUGuest1'/>
+      <target dev='hda' bus='ide'/>
+      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+    </disk>
+    <controller type='usb' index='0'/>
+    <controller type='ide' index='0'/>
+    <memballoon model='virtio'/>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 4a2e016..629737f 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -194,7 +194,7 @@ mymain(void)
     DO_TEST("pci-rom");

     DO_TEST("encrypted-disk");
-    DO_TEST("memtune");
+    DO_TEST_DIFFERENT("memtune");
     DO_TEST("blkiotune");
     DO_TEST("blkiotune-device");
     DO_TEST("cputune");
-- 
1.7.7.6


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