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

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

 



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

Attachment: signature.asc
Description: OpenPGP digital signature

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