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

[Eric Blake]

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 621 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20131009/8020ce59/attachment-0001.sig>