5:04 p.m.
The virsh-command freecell hands out bytes but affixes those with kB.
The error ist found in virsh.c on line 1663 and following.
I have corrected this and altered the output to the method i chose for
virt-manager - in the days of 96 GB per socket (Nehalem-EP) no one cares
even about a fraction of a Megabyte.
--- a/src/virsh.c 2009-04-02 17:45:50.000000000 +0200
+++ b/src/virsh.c 2009-04-02 23:35:54.000000000 +0200
@@ -1661,10 +1661,23 @@
}
if (cell == -1)
- vshPrint(ctl, "%s: %llu kB\n", _("Total"), memory);
+ if (memory < 10*1024*1204*1024)
+ {
+ vshPrint(ctl, "%s: %.0f MB\n", _("Total"),
(float)memory/1024/1024);
+ }
+ else
+ {
+ vshPrint(ctl, "%s: %.2f GB\n", _("Total"),
(float)memory/1024/1024/1024);
+ }
else
- vshPrint(ctl, "%d: %llu kB\n", cell, memory);
-
+ if (memory < 10*1024*1204*1024)
+ {
+ vshPrint(ctl, "%d: %.0f MB\n", cell,
(float)memory/1024/1024);
+ }
+ else
+ {
+ vshPrint(ctl, "%d: %.2f GB\n", cell,
(float)memory/1024/1024/1024);
+ }
return TRUE;
}
eg:
rr016# virsh freecell; free -m
0: 24 MB
total used free shared buffers cached
Mem: 3884 3859 24 0 60 1306
-/+ buffers/cache: 2492 1392
Swap: 2047 248 1798
rr017# virsh freecell; free -m
0: 1490 MB
total used free shared buffers cached
Mem: 2991 1501 1489 0 100 1283
-/+ buffers/cache: 117 2873
Swap: 2047 0 2047
rr019# virsh freecell; free -m
0: 1616 MB
total used free shared buffers cached
Mem: 3952 2337 1615 0 5 1209
-/+ buffers/cache: 1121 2831
Swap: 2047 0 2047
Is this okay with you? Or just the fix for the output removing the
kB-suffix.
3:29 a.m.
Gerrit Slomma a écrit :
--- a/src/virsh.c 2009-04-02 17:45:50.000000000 +0200
+++ b/src/virsh.c 2009-04-02 23:35:54.000000000 +0200
@@ -1661,10 +1661,23 @@
}
if (cell == -1)
- vshPrint(ctl, "%s: %llu kB\n", _("Total"), memory);
+ if (memory < 10*1024*1204*1024)
If I may, it should probably read "1024", not "1204" here.
+ {
+ vshPrint(ctl, "%s: %.0f MB\n", _("Total"),
(float)memory/1024/1024);
+ }
+ else
+ {
+ vshPrint(ctl, "%s: %.2f GB\n", _("Total"),
(float)memory/1024/1024/1024);
+ }
else
- vshPrint(ctl, "%d: %llu kB\n", cell, memory);
-
+ if (memory < 10*1024*1204*1024)
and here.
+ {
+ vshPrint(ctl, "%d: %.0f MB\n", cell,
(float)memory/1024/1024);
+ }
+ else
+ {
+ vshPrint(ctl, "%d: %.2f GB\n", cell,
(float)memory/1024/1024/1024);
+ }
return TRUE;
}
Florian
11:43 a.m.
Florian Vichot schrieb:
Gerrit Slomma a écrit :
> if (cell == -1)
> - vshPrint(ctl, "%s: %llu kB\n", _("Total"), memory);
> + if (memory < 10*1024*1204*1024)
If I may, it should probably read "1024", not "1204" here.
and here.
> + {
> + vshPrint(ctl, "%d: %.0f MB\n", cell,
> (float)memory/1024/1024);
Florian
Yes you are right.
5:23 a.m.
On Fri, Apr 03, 2009 at 12:04:05AM +0200, Gerrit Slomma wrote:
The virsh-command freecell hands out bytes but affixes those with
kB.
The error ist found in virsh.c on line 1663 and following.
I have corrected this and altered the output to the method i chose for
virt-manager - in the days of 96 GB per socket (Nehalem-EP) no one cares
even about a fraction of a Megabyte.
For command line tools like virsh I prefer to have it consistently
report in the same units, so if someone wants to script it from
the shell they don't have to concern themselves with changing units.
All the other virsh commands report in KB, so the simple fix is to
just divide by 1024.
In checking this I discovered a whole bunch of other fun bugs in the
NUMA support :-) The QEMU impl was not returning the correct return
code - it used -1 instead of 0. The QEMU impl was also not setting an
error if the requested cell was out of range. The libvirtd remote
driver was not correctly seeing return value of -1 due to casting it
to an unsigned int. virsh was not checking return values correctly
either.
So I propose the following patch....
Index: qemud/remote.c
===================================================================
RCS file: /data/cvs/libvirt/qemud/remote.c,v
retrieving revision 1.65
diff -u -p -r1.65 remote.c
--- qemud/remote.c 16 Mar 2009 10:33:01 -0000 1.65
+++ qemud/remote.c 3 Apr 2009 10:18:38 -0000
@@ -662,6 +662,7 @@ remoteDispatchNodeGetCellsFreeMemory (st
remote_node_get_cells_free_memory_args *args,
remote_node_get_cells_free_memory_ret *ret)
{
+ int err;
if (args->maxCells > REMOTE_NODE_MAX_CELLS) {
remoteDispatchFormatError (rerr,
@@ -675,15 +676,16 @@ remoteDispatchNodeGetCellsFreeMemory (st
return -1;
}
- ret->freeMems.freeMems_len = virNodeGetCellsFreeMemory(conn,
- (unsigned long long
*)ret->freeMems.freeMems_val,
- args->startCell,
- args->maxCells);
- if (ret->freeMems.freeMems_len == 0) {
+ err = virNodeGetCellsFreeMemory(conn,
+ (unsigned long long *)ret->freeMems.freeMems_val,
+ args->startCell,
+ args->maxCells);
+ if (err <= 0) {
VIR_FREE(ret->freeMems.freeMems_val);
remoteDispatchConnError(rerr, conn);
return -1;
}
+ ret->freeMems.freeMems_len = err;
return 0;
}
Index: src/qemu_driver.c
===================================================================
RCS file: /data/cvs/libvirt/src/qemu_driver.c,v
retrieving revision 1.223
diff -u -p -r1.223 qemu_driver.c
--- src/qemu_driver.c 1 Apr 2009 09:54:20 -0000 1.223
+++ src/qemu_driver.c 3 Apr 2009 10:18:38 -0000
@@ -1878,15 +1878,23 @@ qemudNodeGetCellsFreeMemory(virConnectPt
{
int n, lastCell, numCells;
int ret = -1;
+ int maxCell;
if (numa_available() < 0) {
qemudReportError(conn, NULL, NULL, VIR_ERR_NO_SUPPORT,
"%s", _("NUMA not supported on this
host"));
goto cleanup;
}
+ maxCell = numa_max_node();
+ if (startCell > maxCell) {
+ qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _("start cell %d out of range (0-%d)"),
+ startCell, maxCell);
+ goto cleanup;
+ }
lastCell = startCell + maxCells - 1;
- if (lastCell > numa_max_node())
- lastCell = numa_max_node();
+ if (lastCell > maxCell)
+ lastCell = maxCell;
for (numCells = 0, n = startCell ; n <= lastCell ; n++) {
long long mem;
@@ -1906,7 +1914,7 @@ cleanup:
static unsigned long long
qemudNodeGetFreeMemory (virConnectPtr conn)
{
- unsigned long long freeMem = -1;
+ unsigned long long freeMem = 0;
int n;
if (numa_available() < 0) {
Index: src/virsh.c
===================================================================
RCS file: /data/cvs/libvirt/src/virsh.c,v
retrieving revision 1.198
diff -u -p -r1.198 virsh.c
--- src/virsh.c 1 Apr 2009 09:52:59 -0000 1.198
+++ src/virsh.c 3 Apr 2009 10:18:38 -0000
@@ -1654,6 +1654,8 @@ cmdFreecell(vshControl *ctl, const vshCm
cell = vshCommandOptInt(cmd, "cellno", &cell_given);
if (!cell_given) {
memory = virNodeGetFreeMemory(ctl->conn);
+ if (memory == 0)
+ return FALSE;
} else {
ret = virNodeGetCellsFreeMemory(ctl->conn, &memory, cell, 1);
if (ret != 1)
@@ -1661,9 +1663,9 @@ cmdFreecell(vshControl *ctl, const vshCm
}
if (cell == -1)
- vshPrint(ctl, "%s: %llu kB\n", _("Total"), memory);
+ vshPrint(ctl, "%s: %llu kB\n", _("Total"), (memory/1024));
else
- vshPrint(ctl, "%d: %llu kB\n", cell, memory);
+ vshPrint(ctl, "%d: %llu kB\n", cell, (memory/1024));
return TRUE;
}
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
6:16 a.m.
On Fri, Apr 03, 2009 at 11:23:59AM +0100, Daniel P. Berrange wrote:
On Fri, Apr 03, 2009 at 12:04:05AM +0200, Gerrit Slomma wrote:
> The virsh-command freecell hands out bytes but affixes those with kB.
> The error ist found in virsh.c on line 1663 and following.
> I have corrected this and altered the output to the method i chose for
> virt-manager - in the days of 96 GB per socket (Nehalem-EP) no one cares
> even about a fraction of a Megabyte.
For command line tools like virsh I prefer to have it consistently
report in the same units, so if someone wants to script it from
the shell they don't have to concern themselves with changing units.
All the other virsh commands report in KB, so the simple fix is to
just divide by 1024.
Agreed, let's not make it too hard for tools who would parse virsh
output, even though we should not encourage that practice :-)
In checking this I discovered a whole bunch of other fun bugs in the
NUMA support :-) The QEMU impl was not returning the correct return
code - it used -1 instead of 0. The QEMU impl was also not setting an
error if the requested cell was out of range. The libvirtd remote
driver was not correctly seeing return value of -1 due to casting it
to an unsigned int. virsh was not checking return values correctly
either.
Heh !
So I propose the following patch....
ACK, fine by me !
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
11:46 a.m.
Daniel P. Berrange schrieb:
For command line tools like virsh I prefer to have it consistently
report in the same units, so if someone wants to script it from
the shell they don't have to concern themselves with changing units.
All the other virsh commands report in KB, so the simple fix is to
just divide by 1024.
A-OK.
5715
days inactive
5716
days old
5 comments
4 participants
participants (4)
-
Daniel P. Berrange -
Daniel Veillard -
Florian Vichot -
Gerrit Slomma