Bug 1145048

Summary: freepages argument has wrong unit and range
Product: Red Hat Enterprise Linux 7 Reporter: Jincheng Miao <jmiao>
Component: libvirtAssignee: Michal Privoznik <mprivozn>
Status: CLOSED ERRATA QA Contact: Virtualization Bugs <virt-bugs>
Severity: medium Docs Contact:
Priority: medium    
Version: 7.1CC: dyuan, honzhang, jiahu, mprivozn, mzhan, rbalakri
Target Milestone: rc   
Target Release: ---   
Hardware: x86_64   
OS: Linux   
Whiteboard:
Fixed In Version: libvirt-1.2.8-6.el7 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2015-03-05 07:45:00 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Jincheng Miao 2014-09-22 09:30:21 UTC
description of problem:
The unit of argument '--pagesize' for freepages is kibibytes, but it treats it as bytes
And '--pagesize' should only accept positive number.

version:
libvirt-1.2.8-2.el7.x86_64
qemu-kvm-rhev-2.1.0-3.el7.x86_64

How reproducible:
100%

step to reproduce:
1. check free 2M memory page
# virsh freepages --cellno -1 --pagesize 2048
error: Failed to open file '/sys/kernel/mm/hugepages/hugepages-2kB/free_hugepages': No such file or directory

# virsh help freepages
  NAME
    freepages - NUMA free pages

  SYNOPSIS
    freepages [--cellno <number>] [--pagesize <number>] [--all]

  DESCRIPTION
    display available free pages for the NUMA cell.

  OPTIONS
    --cellno <number>  NUMA cell number
    --pagesize <number>  page size (in kibibytes)
    --all            show free pages for all NUMA cells

2. pass negative number to '--pagesize'
# virsh freepages --cellno -1 --pagesize -2048
error: page size has to be a number
error: numerical overflow: value too large: 18446744073709549568

Expect result:
1. # virsh freepages --cellno -1 --pagesize 2048
2048KiB: 250

2. # virsh freepages --cellno -1 --pagesize -2048
error: page size must be positive.

Comment 2 Michal Privoznik 2014-09-23 08:29:43 UTC
I've just pushed patches upstream:

commit c46fa72e4090b7884b8c6edaf63f1638eb8e04f8
Author:     Jincheng Miao <jmiao>
AuthorDate: Mon Sep 22 18:14:28 2014 +0800
Commit:     Michal Privoznik <mprivozn>
CommitDate: Tue Sep 23 10:23:20 2014 +0200

    Fix typo of virNodeGetFreePages comment
    
    Signed-off-by: Jincheng Miao <jmiao>

commit 7db19366423dcab388907c12caf766c6e5870d00
Author:     Jincheng Miao <jmiao>
AuthorDate: Mon Sep 22 18:14:27 2014 +0800
Commit:     Michal Privoznik <mprivozn>
CommitDate: Tue Sep 23 10:23:20 2014 +0200

    nodeinfo: report error when given node is out of range
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1145050
    
    Signed-off-by: Jincheng Miao <jmiao>
    Signed-off-by: Michal Privoznik <mprivozn>

commit c3e2d5929c1bfaaeb2ef2df121dc90d767b228f5
Author:     Jincheng Miao <jmiao>
AuthorDate: Mon Sep 22 18:14:26 2014 +0800
Commit:     Michal Privoznik <mprivozn>
CommitDate: Tue Sep 23 10:23:20 2014 +0200

    virsh-host: fix pagesize unit of freepages
    
    The unit of '--pagesize' of freepages is kibibytes.
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1145048
    
    Signed-off-by: Jincheng Miao <jmiao>
    Signed-off-by: Michal Privoznik <mprivozn>

v1.2.8-230-gc46fa72

Comment 5 Hu Jianwei 2014-10-15 03:42:20 UTC
Maybe we need blow two upstream patches:

[hujianwei@localhost libvirt]$ git show 8baf0f02
commit 8baf0f025f975267e3f9a3af71f69137130907b7
Author: Jincheng Miao <jmiao>
Date:   Wed Sep 24 13:45:30 2014 +0800

    nodeinfo: fix nodeGetFreePages when max node is zero
    
    In nodeGetFreePages, if startCell is given by '0',
    and the max node number is '0' too. The for-loop
    wouldn't be executed.
    So convert it to while-loop.
    
    Before:
    > virsh freepages --cellno 0 --pagesize 4
    error: internal error: no suitable info found
    
    After:
    > virsh freepages --cellno 0 --pagesize 4
    4KiB: 472637
    
    Signed-off-by: Jincheng Miao <jmiao>
    Signed-off-by: Michal Privoznik <mprivozn>

diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 2459922..04ae382 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -2043,7 +2043,7 @@ nodeGetFreePages(unsigned int npages,
 
     lastCell = MIN(lastCell, startCell + cellCount);
 
-    for (cell = startCell; cell < lastCell; cell++) {
+    for (cell = startCell; cell <= lastCell; cell++) {
         for (i = 0; i < npages; i++) {
             unsigned int page_size = pages[i];
             unsigned int page_free;

[hujianwei@localhost libvirt]$ git show  4aa8a68faa8
commit 4aa8a68faa86d6e1c6ecaeabcec487ee30eff813
Author: Michal Privoznik <mprivozn>
Date:   Wed Sep 24 15:10:18 2014 +0200

    nodeGetFreePages: Push forgotten change
    
    In the previous patch I've changed the for loop bounds but forgot
    to 'git add' changes that adapt the rest of the code.
    
    Signed-off-by: Michal Privoznik <mprivozn>

diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 04ae382..3bc0c3c 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -2041,7 +2041,7 @@ nodeGetFreePages(unsigned int npages,
         goto cleanup;
     }
 
-    lastCell = MIN(lastCell, startCell + cellCount);
+    lastCell = MIN(lastCell, startCell + (int) cellCount - 1);
 
     for (cell = startCell; cell <= lastCell; cell++) {
         for (i = 0; i < npages; i++) {

Before the patches:
[root@ibm-x3850x5-05 ~]# rpm -q libvirt
libvirt-1.2.8-5.el7.x86_64

[root@ibm-x3850x5-05 ~]# numactl --hard
available: 2 nodes (0-1)
node 0 cpus: 0 1 2 3 4 5 6 7 16 17 18 19 20 21 22 23
node 0 size: 65514 MB
node 0 free: 62986 MB
node 1 cpus: 8 9 10 11 12 13 14 15 24 25 26 27 28 29 30 31
node 1 size: 65536 MB
node 1 free: 62785 MB
node distances:
node   0   1 
  0:  10  11 
  1:  11  10 
[root@ibm-x3850x5-05 ~]# virsh freepages --all
Node 0:
4KiB: 16124187
2048KiB: 0

error: internal error: no suitable info found

[root@ibm-x3850x5-05 ~]# virsh freepages --cellno 1 --pagesize 4
error: internal error: no suitable info found

After applying the patches:
[root@ibm-x3850x5-06 x86_64]# rpm -q libvirt
libvirt-1.2.8-5.el7_jiahu.x86_64
[root@ibm-x3850x5-06 x86_64]# virsh freepages --all
Node 0:
4KiB: 3626649
2048KiB: 0

Node 1:
4KiB: 3829784
2048KiB: 0


[root@ibm-x3850x5-06 x86_64]# virsh freepages --cellno 1 --pagesize 4
4KiB: 3829836

Comment 6 Michal Privoznik 2014-10-15 10:11:52 UTC
Yeah, so moving to POST again:

http://post-office.corp.redhat.com/archives/rhvirt-patches/2014-October/msg00372.html

Comment 8 Jincheng Miao 2014-11-06 05:51:16 UTC
The unit of pagesize is already fix in latest libvirt-1.2.8-6.el7:

# rpm -q libvirt
libvirt-1.2.8-6.el7.x86_64

# virsh freepages --cellno -1 --pagesize 4
4KiB: 1119867

# virsh freepages --cellno -1 --pagesize 4k
4KiB: 1119580

# virsh freepages --cellno -1 --pagesize 1G
1048576KiB: 2

# virsh freepages --cellno 0 --pagesize 1G
1048576KiB: 2

# virsh freepages --cellno 1 --pagesize 1G
error: internal error: start cell 1 out of range (0-0)


There is a little problem is that reporting numerical overflow for negative pagesize:
# virsh freepages --cellno -1 --pagesize -4
error: page size has to be a number
error: numerical overflow: value too large: 18446744073709551612

I had filed a bug https://bugzilla.redhat.com/show_bug.cgi?id=1159171 for it,
so change this status to VERIFIED.

Comment 10 errata-xmlrpc 2015-03-05 07:45:00 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://rhn.redhat.com/errata/RHSA-2015-0323.html