Re: [libvirt] [PATCHv2] LXC: Fix handling of RAM filesystem size units
On 10/09/2013 07:22 AM, Daniel P. Berrange wrote:
> 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.
Why are you dropping 'in'? I agree that 'KiB' is better than
'KB', but
still think you need 'in'.
> +++ b/docs/schemas/domaincommon.rng
> @@ -1736,6 +1736,11 @@
> <ref name='unit'/>
> </attribute>
> </optional>
> + <optional>
> + <attribute name='units'>
> + <ref name='unit'/>
> + </attribute>
> + </optional>
Only accept one or the other; don't allow validation if both are present:
<optional>
<choice>
<attribute name='unit'>...
<attribute name='units'>...
</choice>
</optional>
> @@ -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.
I'm not sure I agree - for consistency, all the rest of our XML uses 'unit'.
<memory unit='KiB'>1048576</memory>
<currentMemory unit='KiB'>1048576</currentMemory>
> @@ -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.
Consistent output, as long as we accept exactly one of the two names on
input, seems reasonable to me. Then again, if we released documentation
of one particular spelling, it's better to preserve that spelling for
back-compat, even if inconsistent. As we didn't document ANY name
(whether 'unit' or 'units'), I'd much rather output a consistent
name.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 621 bytes)