Re: [PATCH 1/3] xml: output memory unit for clarity

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

 



On 02/23/2012 02:55 AM, Christophe Fergeau wrote:
> On Thu, Feb 23, 2012 at 08:56:50AM +0100, Matthias Bolte wrote:
>> 2012/2/23 Eric Blake <eblake@xxxxxxxxxx>:
>>> Make it obvious to 'dumpxml' readers what unit we are using,
>>> since our default of KiB (1024) differs from qemu's default of MiB.
>>
>>> diff --git a/tests/xml2vmxdata/xml2vmx-svga.xml b/tests/xml2vmxdata/xml2vmx-svga.xml
>>> index b7db4c8..9f93ef9 100644
>>> --- a/tests/xml2vmxdata/xml2vmx-svga.xml
>>> +++ b/tests/xml2vmxdata/xml2vmx-svga.xml
>>> @@ -1,7 +1,7 @@
>>>  <domain type='vmware'>
>>>   <name>minimal</name>
>>>   <uuid>564d9bef-acd9-b4e0-c8f0-aea8b9103515</uuid>
>>> -  <memory>4096</memory>
>>> +  <memory units='KiB'>4096</memory>
>>>   <os>
>>>     <type>hvm</type>
>>>   </os>
>>
>> Good idea, but why did you decide to name the attribute "units"
>> (plural) instead of "unit"? As a value can only have one unit the
>> attribute name "units" is misleading.
> 
> Agreed, especially since <volume><allocation> already uses "unit"

Eek.  conf/storage_conf.c is using 'k' as 1024, but documenting it as
kilobyte in formatstorage.html.in.

Yes, I can make the code for unit parsing shared between the two; for
consistency, the one letter suffix will be the power of 2, the two
letter with B suffix is power of 10, and the three letter with iB suffix
is power of 2 (this is how coreutils does it, by the way, so it's not
that unusual).

That means some refactoring - moving the unit parsing code into a shared
location that both storage_conf and domain_conf can use.

-- 
Eric Blake   eblake@xxxxxxxxxx    +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]