[PATCHv2] LXC: Fix handling of RAM filesystem size units

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

 



Since 76b644c when the support for RAM filesystems was introduced,
libvirt accepted the following XML:
<source usage='1024' unit='KiB'/>

This was parsed correctly and internally stored in bytes, but it
was formatted as (with an extra 's'):
<source usage='1024' units='KiB'/>
When read again, this was treated as if the units were missing,
meaning libvirt was unable to parse its own XML correctly.

The usage attribute was documented as being in KiB, but it was not
scaled if the unit was missing. Transient domains still worked,
because this was balanced by an extra 'k' in the mount options.

This patch:
Outputs the units via the 'unit' attribute (fixing persistent domains),
but accepts both 'unit' and 'units' (for compatibility with older
libvirt and libvirt-sandbox).

Removes the extra 'k' from the tmpfs mount options, which is needed
because now we parse our own XML correctly.

Changes the default unit to KiB to match documentation, fixing:
https://bugzilla.redhat.com/show_bug.cgi?id=1015689
---
v1: https://www.redhat.com/archives/libvir-list/2013-October/msg00361.html
v2: keeps the output unit KiB
    changes the default input unit to KiB
    accepts 'units' as well as 'unit' in input XML

 docs/formatdomain.html.in                      |  6 ++--
 docs/schemas/domaincommon.rng                  |  5 ++++
 src/conf/domain_conf.c                         | 12 ++++++--
 src/lxc/lxc_container.c                        |  2 +-
 tests/domainschematest                         |  2 +-
 tests/lxcxml2xmldata/lxc-filesystem-ram.xml    | 41 ++++++++++++++++++++++++++
 tests/lxcxml2xmloutdata/lxc-filesystem-ram.xml | 41 ++++++++++++++++++++++++++
 tests/lxcxml2xmltest.c                         |  1 +
 8 files changed, 103 insertions(+), 7 deletions(-)
 create mode 100644 tests/lxcxml2xmldata/lxc-filesystem-ram.xml
 create mode 100644 tests/lxcxml2xmloutdata/lxc-filesystem-ram.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 3689399..dfd11bf 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2213,7 +2213,8 @@
         <dd>
           An in-memory filesystem, using memory from the host OS.
           The source element has a single attribute <code>usage</code>
-          which gives the memory usage limit in kibibytes. Only used
+          which gives the memory usage limit in KiB, unless units
+          are specified by the <code>unit</code> attribute. Only used
           by LXC driver.
           <span class="since"> (since 0.9.13)</span></dd>
         <dt><code>type='bind'</code></dt>
@@ -2279,7 +2280,8 @@
         <code>name</code> attribute must be used with
         <code>type='template'</code>, and the <code>dir</code> attribute must
         be used with <code>type='mount'</code>. The <code>usage</code> attribute
-        is used with <code>type='ram'</code> to set the memory limit in KB.
+        is used with <code>type='ram'</code> to set the memory limit KiB, unless
+        units are specified by the <code>unit</code> attribute.
       </dd>
 
       <dt><code>target</code></dt>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 5c5301d..582d4c3 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1736,6 +1736,11 @@
                   <ref name='unit'/>
                 </attribute>
               </optional>
+              <optional>
+                <attribute name='units'>
+                  <ref name='unit'/>
+                </attribute>
+              </optional>
               <empty/>
             </element>
           </interleave>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0d63845..0870047 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5917,6 +5917,7 @@ virDomainFSDefParseXML(xmlNodePtr node,
     char *wrpolicy = NULL;
     char *usage = NULL;
     char *unit = NULL;
+    char *units = NULL;
 
     ctxt->node = node;
 
@@ -5973,6 +5974,7 @@ virDomainFSDefParseXML(xmlNodePtr node,
                 else if (def->type == VIR_DOMAIN_FS_TYPE_RAM) {
                     usage = virXMLPropString(cur, "usage");
                     unit = virXMLPropString(cur, "unit");
+                    units = virXMLPropString(cur, "units");
                 }
             } else if (!target &&
                        xmlStrEqual(cur->name, BAD_CAST "target")) {
@@ -6042,8 +6044,11 @@ virDomainFSDefParseXML(xmlNodePtr node,
                            usage);
             goto error;
         }
-        if (unit &&
-            virScaleInteger(&def->usage, unit,
+        /* Older libvirt only parsed 'unit' here, despite
+         * printing it in the 'units' attribute.
+         *
+         * The default was bytes when 'unit' wasn't present */
+        if (virScaleInteger(&def->usage, units ? units : unit,
                             1024, ULLONG_MAX) < 0)
             goto error;
     }
@@ -6066,6 +6071,7 @@ cleanup:
     VIR_FREE(wrpolicy);
     VIR_FREE(usage);
     VIR_FREE(unit);
+    VIR_FREE(units);
     VIR_FREE(format);
 
     return def;
@@ -14764,7 +14770,7 @@ virDomainFSDefFormat(virBufferPtr buf,
         break;
 
     case VIR_DOMAIN_FS_TYPE_RAM:
-        virBufferAsprintf(buf, "      <source usage='%lld' units='KiB'/>\n",
+        virBufferAsprintf(buf, "      <source usage='%lld' unit='KiB'/>\n",
                           def->usage / 1024);
         break;
     }
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index b1f429c..7c722cc 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -1428,7 +1428,7 @@ static int lxcContainerMountFSTmpfs(virDomainFSDefPtr fs,
     VIR_DEBUG("usage=%lld sec=%s", fs->usage, sec_mount_options);
 
     if (virAsprintf(&data,
-                    "size=%lldk%s", fs->usage, sec_mount_options) < 0)
+                    "size=%lld%s", fs->usage, sec_mount_options) < 0)
         goto cleanup;
 
     if (virFileMakePath(fs->dst) < 0) {
diff --git a/tests/domainschematest b/tests/domainschematest
index 0e360ca..9ebf0b9 100755
--- a/tests/domainschematest
+++ b/tests/domainschematest
@@ -7,7 +7,7 @@
 DIRS=""
 DIRS="$DIRS domainschemadata qemuxml2argvdata sexpr2xmldata"
 DIRS="$DIRS xmconfigdata xml2sexprdata qemuxml2xmloutdata "
-DIRS="$DIRS lxcxml2xmldata"
+DIRS="$DIRS lxcxml2xmldata lxcxml2xmloutdata"
 SCHEMA="domain.rng"
 
 check_schema "$DIRS" "$SCHEMA"
diff --git a/tests/lxcxml2xmldata/lxc-filesystem-ram.xml b/tests/lxcxml2xmldata/lxc-filesystem-ram.xml
new file mode 100644
index 0000000..4b2abb5
--- /dev/null
+++ b/tests/lxcxml2xmldata/lxc-filesystem-ram.xml
@@ -0,0 +1,41 @@
+<domain type='lxc'>
+  <name>demo</name>
+  <uuid>8369f1ac-7e46-e869-4ca5-759d51478066</uuid>
+  <memory unit='KiB'>500000</memory>
+  <currentMemory unit='KiB'>500000</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='x86_64'>exe</type>
+    <init>/bin/sh</init>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/libexec/libvirt_lxc</emulator>
+    <filesystem type='ram'>
+        <source usage='1048576'/>
+        <target dir='/mnt/mississippi'/>
+    </filesystem>
+    <filesystem type='ram'>
+        <source usage='1048576' unit='bytes'/>
+        <target dir='/mnt/titicaca'/>
+    </filesystem>
+    <filesystem type='ram'>
+        <source usage='1024' unit='KiB'/>
+        <target dir='/mnt/orinoco'/>
+    </filesystem>
+    <filesystem type='ram'>
+        <source usage='1048576' units='bytes'/>
+        <target dir='/mnt/antananarivo'/>
+    </filesystem>
+    <filesystem type='ram'>
+        <source usage='1024' units='KiB'/>
+        <target dir='/mnt/ouagadougou'/>
+    </filesystem>
+    <console type='pty'>
+      <target type='lxc' port='0'/>
+    </console>
+  </devices>
+</domain>
diff --git a/tests/lxcxml2xmloutdata/lxc-filesystem-ram.xml b/tests/lxcxml2xmloutdata/lxc-filesystem-ram.xml
new file mode 100644
index 0000000..65d07dd
--- /dev/null
+++ b/tests/lxcxml2xmloutdata/lxc-filesystem-ram.xml
@@ -0,0 +1,41 @@
+<domain type='lxc'>
+  <name>demo</name>
+  <uuid>8369f1ac-7e46-e869-4ca5-759d51478066</uuid>
+  <memory unit='KiB'>500000</memory>
+  <currentMemory unit='KiB'>500000</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='x86_64'>exe</type>
+    <init>/bin/sh</init>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/libexec/libvirt_lxc</emulator>
+    <filesystem type='ram' accessmode='passthrough'>
+      <source usage='1048576' unit='KiB'/>
+      <target dir='/mnt/mississippi'/>
+    </filesystem>
+    <filesystem type='ram' accessmode='passthrough'>
+      <source usage='1024' unit='KiB'/>
+      <target dir='/mnt/titicaca'/>
+    </filesystem>
+    <filesystem type='ram' accessmode='passthrough'>
+      <source usage='1024' unit='KiB'/>
+      <target dir='/mnt/orinoco'/>
+    </filesystem>
+    <filesystem type='ram' accessmode='passthrough'>
+      <source usage='1024' unit='KiB'/>
+      <target dir='/mnt/antananarivo'/>
+    </filesystem>
+    <filesystem type='ram' accessmode='passthrough'>
+      <source usage='1024' unit='KiB'/>
+      <target dir='/mnt/ouagadougou'/>
+    </filesystem>
+    <console type='pty'>
+      <target type='lxc' port='0'/>
+    </console>
+  </devices>
+</domain>
diff --git a/tests/lxcxml2xmltest.c b/tests/lxcxml2xmltest.c
index 5846ab0..1692e4b 100644
--- a/tests/lxcxml2xmltest.c
+++ b/tests/lxcxml2xmltest.c
@@ -136,6 +136,7 @@ mymain(void)
     DO_TEST("systemd");
     DO_TEST("hostdev");
     DO_TEST("disk-formats");
+    DO_TEST_DIFFERENT("filesystem-ram");
 
     virObjectUnref(caps);
     virObjectUnref(xmlopt);
-- 
1.8.1.5

--
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]