Bug 976831 - Inconsistent memory size input and output
Summary: Inconsistent memory size input and output
Keywords:
Status: CLOSED WONTFIX
Alias: None
Product: Virtualization Tools
Classification: Community
Component: libvirt
Version: unspecified
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Libvirt Maintainers
QA Contact:
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2013-06-21 15:09 UTC by Michal Novotny
Modified: 2014-02-02 22:39 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2013-06-21 15:34:48 UTC
Embargoed:


Attachments (Terms of Use)

Description Michal Novotny 2013-06-21 15:09:03 UTC
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 15:18:53 UTC
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 15:21:25 UTC
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 15:25:27 UTC
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 Berrangé 2013-06-21 15:34:48 UTC
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 15:35:22 UTC
(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 15:37:40 UTC
(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 Berrangé 2013-06-21 15:40:31 UTC
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 15:43:11 UTC
(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 10:55:42 UTC
(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.