[libvirt] [PATCHv2] LXC: Fix handling of RAM filesystem size units
Daniel P. Berrange
berrange at redhat.com
Wed Oct 9 13:22:49 UTC 2013
On Wed, Oct 09, 2013 at 03:15:36PM +0200, Ján Tomko wrote:
> 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");
We should just be dropping the 'unit' attribute - it was undocumented
and broken.
> }
> } 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;
No, you should not be changing the attribute name here.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
More information about the libvir-list
mailing list