From 2109b3f2a7cfd9abf1ae17905508abdc1b492532 Mon Sep 17 00:00:00 2001

Message-Id: <2109b3f2a7cfd9abf1ae17905508abdc1b492532.1381871412.git.jdenemar@redhat.com>

From: =?UTF-8?q?J=C3=A1n=20Tomko?= <jtomko@redhat.com>

Date: Wed, 9 Oct 2013 17:49:36 +0200

Subject: [PATCH] LXC: Fix handling of RAM filesystem size units

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:

Changes the parser to use 'units' instead of 'unit', as the latter

was never documented (fixing persistent domains) and some programs

(libvirt-glib, libvirt-sandbox) already parse the 'units' attribute.

Removes the extra 'k' from the tmpfs mount options, which is needed

because now we parse our own XML correctly.

Changes the default input unit to KiB to match documentation, fixing:

https://bugzilla.redhat.com/show_bug.cgi?id=1015689

(cherry picked from commit 3f029fb5319b9dc9cc2fbf8d1ba4505ee9e4b1e3)

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>

---

 docs/formatdomain.html.in                      |  6 +++--

 docs/schemas/domaincommon.rng                  |  2 +-

 src/conf/domain_conf.c                         |  9 ++++---

 src/conf/domain_conf.h                         |  2 +-

 src/lxc/lxc_container.c                        |  2 +-

 tests/domainschematest                         |  2 +-

 tests/lxcxml2xmldata/lxc-filesystem-ram.xml    | 33 ++++++++++++++++++++++++++

 tests/lxcxml2xmloutdata/lxc-filesystem-ram.xml | 33 ++++++++++++++++++++++++++

 tests/lxcxml2xmltest.c                         |  1 +

 9 files changed, 79 insertions(+), 11 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 4308dbe..4fecdee 100644

--- a/docs/formatdomain.html.in

+++ b/docs/formatdomain.html.in

@@ -2169,7 +2169,8 @@

           An in-memory filesystem, using memory from the host OS.

           The source element has a single attribute usage

-          which gives the memory usage limit in kibibytes. Only used

+          which gives the memory usage limit in KiB, unless units

+          are specified by the units attribute. Only used

           by LXC driver.

            (since 0.9.13)

         
type='bind'

@@ -2235,7 +2236,8 @@

         name attribute must be used with

         type='template', and the dir attribute must

         be used with type='mount'. The usage attribute

-        is used with type='ram' to set the memory limit in KB.

+        is used with type='ram' to set the memory limit in KiB,

+        unless units are specified by the units attribute.

       
target

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng

index 838f657..5540745 100644

--- a/docs/schemas/domaincommon.rng

+++ b/docs/schemas/domaincommon.rng

@@ -1681,7 +1681,7 @@

                 <ref name="unsignedLong"/>

               </attribute>

               <optional>

-                <attribute name='unit'>

+                <attribute name='units'>

                   <ref name='unit'/>

                 </attribute>

               </optional>

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c

index bca2151..dea8130 100644

--- a/src/conf/domain_conf.c

+++ b/src/conf/domain_conf.c

@@ -5862,7 +5862,7 @@ virDomainFSDefParseXML(xmlNodePtr node,

     char *accessmode = NULL;

     char *wrpolicy = NULL;

     char *usage = NULL;

-    char *unit = NULL;

+    char *units = NULL;

     ctxt->node = node;

@@ -5918,7 +5918,7 @@ virDomainFSDefParseXML(xmlNodePtr node,

                     source = virXMLPropString(cur, "name");

                 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")) {

@@ -5988,8 +5988,7 @@ virDomainFSDefParseXML(xmlNodePtr node,

                            usage);

             goto error;

         }

-        if (unit &&

-            virScaleInteger(&def->usage, unit,

+        if (virScaleInteger(&def->usage, units,

                             1024, ULLONG_MAX) < 0)

             goto error;

     }

@@ -6011,7 +6010,7 @@ cleanup:

     VIR_FREE(accessmode);

     VIR_FREE(wrpolicy);

     VIR_FREE(usage);

-    VIR_FREE(unit);

+    VIR_FREE(units);

     VIR_FREE(format);

     return def;

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h

index 5d2cb83..5a21576 100644

--- a/src/conf/domain_conf.h

+++ b/src/conf/domain_conf.h

@@ -874,7 +874,7 @@ struct _virDomainFSDef {

     int accessmode; /* enum virDomainFSAccessMode */

     int wrpolicy; /* enum virDomainFSWrpolicy */

     int format; /* enum virStorageFileFormat */

-    unsigned long long usage;

+    unsigned long long usage; /* in bytes */

     char *src;

     char *dst;

     bool readonly;

diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c

index f851fcc..43178e2 100644

--- a/src/lxc/lxc_container.c

+++ b/src/lxc/lxc_container.c

@@ -1434,7 +1434,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..002fde6

--- /dev/null

+++ b/tests/lxcxml2xmldata/lxc-filesystem-ram.xml

@@ -0,0 +1,33 @@

+<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' 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..d2369a2

--- /dev/null

+++ b/tests/lxcxml2xmloutdata/lxc-filesystem-ram.xml

@@ -0,0 +1,33 @@

+<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' units='KiB'/>

+      <target dir='/mnt/mississippi'/>

+    </filesystem>

+    <filesystem type='ram' accessmode='passthrough'>

+      <source usage='1024' units='KiB'/>

+      <target dir='/mnt/antananarivo'/>

+    </filesystem>

+    <filesystem type='ram' accessmode='passthrough'>

+      <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/lxcxml2xmltest.c b/tests/lxcxml2xmltest.c

index ca05d29..f8b5959 100644

--- a/tests/lxcxml2xmltest.c

+++ b/tests/lxcxml2xmltest.c

@@ -131,6 +131,7 @@ mymain(void)

     DO_TEST("systemd");

     DO_TEST("hostdev");

     DO_TEST("disk-formats");

+    DO_TEST_DIFFERENT("filesystem-ram");

     virObjectUnref(caps);

     virObjectUnref(xmlopt);

-- 

1.8.3.2