Bug 976831 - Inconsistent memory size input and output
Inconsistent memory size input and output
Status: CLOSED WONTFIX
Product: Virtualization Tools
Classification: Community
Component: libvirt (Show other bugs)
unspecified
Unspecified Unspecified
unspecified Severity unspecified
: ---
: ---
Assigned To: Libvirt Maintainers
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2013-06-21 11:09 EDT by Michal Novotny
Modified: 2014-02-02 17:39 EST (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2013-06-21 11:34:48 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Michal Novotny 2013-06-21 11:09:03 EDT
Description of problem:

Setting different unit (e.g. MiB) of memory or currentMemory in the domain XML will just calculate appropriate number of KiB and the unit remains KiB.

Version-Release number of selected component (if applicable):
all

How reproducible:
100%

Steps to Reproduce:
1. Create a domain XML file with e.g. "<currentMemory unit="MiB">512<currentMemory>"
2. Virsh dumpxml $DOMAIN
3. You can see: 

Actual results:

# virsh dumpxml srcd | grep currentMemory
  <currentMemory unit='KiB'>524288</currentMemory>
#

Expected results:

# virsh dumpxml srcd | grep currentMemory
  <currentMemory unit='MiB'>512</currentMemory>
#

Additional info:

This is minor issue however it may be confusing to some people. If you input memory of e.g. 16 GiB the result will be "<currentMemory unit='KiB'>16777216</currentMemory>" however I think the "<currentMemory unit='GiB'>16</currentMemory>" representation would be better (as it has better integrity with user input). It affects both <currentMemory> and <memory> tags.
Comment 1 Michal Privoznik 2013-06-21 11:18:53 EDT
I am not sure this is feasible. In the documentation we state that @unit attribute was introduced in the 0.9.11 release [1]. So if a mgmt application is using some ancient libvirt (e.g. without @unit attribute) and counting on KiB units, a libvirt update would break the mgmt application. Because instead of detecting  4194304, the application will see only 4. While I agree that your approach is more user friendly (is there really somebody who can tell just by taking a quick look that 4194304 KiB == 4 GiB?), I'm afraid we can't break applications here.

1: http://www.libvirt.org/formatdomain.html#elementsMemoryAllocation
Comment 2 Richard W.M. Jones 2013-06-21 11:21:25 EDT
It's a shame because I find it annoying too, but I agree with
Michal Privoznik's analysis in comment 1.

Can we add a comment to the XML output giving the size in more
familiar units (megabytes)?  AFAICT the qemu driver just prints
XML, so adding <!-- ... --> shouldn't cause any problems and
of course comments are ignored by consumers except human beings.
Comment 3 Michal Novotny 2013-06-21 11:25:27 EDT
I agree with this however there's API to determine libvirt version on each libvirt server so wouldn't it be worth considering to support this approach as mentioned in comment #0 for libvirt >= 0.9.11 with fallback to current approach for libvirt < 0.9.11 ?

Michal
Comment 4 Daniel Berrange 2013-06-21 11:34:48 EDT
The libvirt version number does not help. Regardless of what libvirt version is used in either the client or the server, the XML format must remain the same. 

Usability is non-goal of the XML format (or we wouldn't be using XML in the first place). The key requirement is to have a stable data format and that means no matter what units are input, we will always output in KiB.
Comment 5 Richard W.M. Jones 2013-06-21 11:35:22 EDT
(In reply to Michal Novotny from comment #3)
> I agree with this however there's API to determine libvirt version on each
> libvirt server so wouldn't it be worth considering to support this approach
> as mentioned in comment #0 for libvirt >= 0.9.11 with fallback to current
> approach for libvirt < 0.9.11 ?

I still don't think you can do it safely.  The problem is that
a client might have been written against libvirt < 0.9.11, so
it's not even expecting that there is a unit attribute; or
it might be programmed to ignore the unit attribute knowing that
in all released versions of libvirt it defaults to "KiB".
Comment 6 Michal Novotny 2013-06-21 11:37:40 EDT
(In reply to Richard W.M. Jones from comment #5)
> (In reply to Michal Novotny from comment #3)
> > I agree with this however there's API to determine libvirt version on each
> > libvirt server so wouldn't it be worth considering to support this approach
> > as mentioned in comment #0 for libvirt >= 0.9.11 with fallback to current
> > approach for libvirt < 0.9.11 ?
> 
> I still don't think you can do it safely.  The problem is that
> a client might have been written against libvirt < 0.9.11, so
> it's not even expecting that there is a unit attribute; or
> it might be programmed to ignore the unit attribute knowing that
> in all released versions of libvirt it defaults to "KiB".

Ok, I see. but why the @unit attribute has been added at the first place if we consider it might be useless except for "writing new memory size" but the "reading" have to be fixed in KiB always?

Michal
Comment 7 Daniel Berrange 2013-06-21 11:40:31 EDT
It was added to make it explicit what units a number was in, without the person having to read the documentation for the schema.
Comment 8 Michal Novotny 2013-06-21 11:43:11 EDT
(In reply to Daniel Berrange from comment #7)
> It was added to make it explicit what units a number was in, without the
> person having to read the documentation for the schema.

Ah, makes sense. However supporting other formats (like MiB, GiB) may confuse people if it's supported for changing the memory sizes but the output is always in KiB. I think many people would be confused saying "Why I input 4 GiB allocation but the result is not printed back in GiB if I entered it in GiB?" and similar so maybe just supporting "KiB" would be better to avoid the confusion.

Michal
Comment 9 Richard W.M. Jones 2013-06-26 06:55:42 EDT
(In reply to Michal Novotny from comment #8)
> (In reply to Daniel Berrange from comment #7)
> > It was added to make it explicit what units a number was in, without the
> > person having to read the documentation for the schema.
> 
> Ah, makes sense. However supporting other formats (like MiB, GiB) may
> confuse people if it's supported for changing the memory sizes but the
> output is always in KiB. I think many people would be confused saying "Why I
> input 4 GiB allocation but the result is not printed back in GiB if I
> entered it in GiB?" and similar

As Dan says, usability is a non-goal of the XML format.  If non-
programmers are editing the XML, we should consider that a failure.
Unfortunately there isn't a simple tool for editing the domain
configuration unless you go up to virt-manager, so that *is* a
failure (of us).

> so maybe just supporting "KiB" would be
> better to avoid the confusion.

We can't do that because clients may now be expecting to use
the unit attribute (when updating the XML).

Note You need to log in before you can comment on or make changes to this bug.