12:42 p.m.
We need to add a few bits to our /proc/meminfo virtualization to
make 'free' work correctly on Fedora 23.
https://bugzilla.redhat.com/show_bug.cgi?id=1300781
Cole Robinson (4):
lxc: fuse: Unindent meminfo logic
lxc: fuse: Fix /proc/meminfo size calculation
lxc: fuse: Fill in MemAvailable for /proc/meminfo
lxc: fuse: Stub out Slab bits in /proc/meminfo
src/lxc/lxc_fuse.c | 141 +++++++++++++++++++++++++++++------------------------
1 file changed, 76 insertions(+), 65 deletions(-)
--
2.5.0
12:42 p.m.
New subject: [libvirt] [PATCH 1/4] lxc: fuse: Unindent meminfo logic
Reverse the conditional at the start so we aren't stuffing all the logic
in an 'if' block
---
src/lxc/lxc_fuse.c | 122 ++++++++++++++++++++++++++---------------------------
1 file changed, 61 insertions(+), 61 deletions(-)
diff --git a/src/lxc/lxc_fuse.c b/src/lxc/lxc_fuse.c
index 862dca3..86c0d10 100644
--- a/src/lxc/lxc_fuse.c
+++ b/src/lxc/lxc_fuse.c
@@ -161,68 +161,68 @@ static int lxcProcReadMeminfo(char *hostpath, virDomainDefPtr def,
res = -1;
while (copied < size && getline(&line, &n, fd) > 0) {
char *ptr = strchr(line, ':');
- if (ptr) {
- *ptr = '\0';
-
- if (STREQ(line, "MemTotal") &&
- (virMemoryLimitIsSet(def->mem.hard_limit) ||
- virDomainDefGetMemoryActual(def))) {
- virBufferAsprintf(new_meminfo, "MemTotal: %8llu kB\n",
- meminfo.memtotal);
- } else if (STREQ(line, "MemFree") &&
- (virMemoryLimitIsSet(def->mem.hard_limit) ||
- virDomainDefGetMemoryActual(def))) {
- virBufferAsprintf(new_meminfo, "MemFree: %8llu kB\n",
- (meminfo.memtotal - meminfo.memusage));
- } else if (STREQ(line, "Buffers")) {
- virBufferAsprintf(new_meminfo, "Buffers: %8d kB\n", 0);
- } else if (STREQ(line, "Cached")) {
- virBufferAsprintf(new_meminfo, "Cached: %8llu kB\n",
- meminfo.cached);
- } else if (STREQ(line, "Active")) {
- virBufferAsprintf(new_meminfo, "Active: %8llu kB\n",
- (meminfo.active_anon + meminfo.active_file));
- } else if (STREQ(line, "Inactive")) {
- virBufferAsprintf(new_meminfo, "Inactive: %8llu kB\n",
- (meminfo.inactive_anon + meminfo.inactive_file));
- } else if (STREQ(line, "Active(anon)")) {
- virBufferAsprintf(new_meminfo, "Active(anon): %8llu kB\n",
- meminfo.active_anon);
- } else if (STREQ(line, "Inactive(anon)")) {
- virBufferAsprintf(new_meminfo, "Inactive(anon): %8llu kB\n",
- meminfo.inactive_anon);
- } else if (STREQ(line, "Active(file)")) {
- virBufferAsprintf(new_meminfo, "Active(file): %8llu kB\n",
- meminfo.active_file);
- } else if (STREQ(line, "Inactive(file)")) {
- virBufferAsprintf(new_meminfo, "Inactive(file): %8llu kB\n",
- meminfo.inactive_file);
- } else if (STREQ(line, "Unevictable")) {
- virBufferAsprintf(new_meminfo, "Unevictable: %8llu kB\n",
- meminfo.unevictable);
- } else if (STREQ(line, "SwapTotal") &&
- virMemoryLimitIsSet(def->mem.swap_hard_limit)) {
- virBufferAsprintf(new_meminfo, "SwapTotal: %8llu kB\n",
- (meminfo.swaptotal - meminfo.memtotal));
- } else if (STREQ(line, "SwapFree") &&
- virMemoryLimitIsSet(def->mem.swap_hard_limit)) {
- virBufferAsprintf(new_meminfo, "SwapFree: %8llu kB\n",
- (meminfo.swaptotal - meminfo.memtotal -
- meminfo.swapusage + meminfo.memusage));
- } else {
- *ptr = ':';
- virBufferAdd(new_meminfo, line, -1);
- }
-
- if (virBufferCheckError(new_meminfo) < 0) {
- res = -errno;
- goto cleanup;
- }
-
- copied += strlen(line);
- if (copied > size)
- copied = size;
+ if (!ptr)
+ continue;
+ *ptr = '\0';
+
+ if (STREQ(line, "MemTotal") &&
+ (virMemoryLimitIsSet(def->mem.hard_limit) ||
+ virDomainDefGetMemoryActual(def))) {
+ virBufferAsprintf(new_meminfo, "MemTotal: %8llu kB\n",
+ meminfo.memtotal);
+ } else if (STREQ(line, "MemFree") &&
+ (virMemoryLimitIsSet(def->mem.hard_limit) ||
+ virDomainDefGetMemoryActual(def))) {
+ virBufferAsprintf(new_meminfo, "MemFree: %8llu kB\n",
+ (meminfo.memtotal - meminfo.memusage));
+ } else if (STREQ(line, "Buffers")) {
+ virBufferAsprintf(new_meminfo, "Buffers: %8d kB\n", 0);
+ } else if (STREQ(line, "Cached")) {
+ virBufferAsprintf(new_meminfo, "Cached: %8llu kB\n",
+ meminfo.cached);
+ } else if (STREQ(line, "Active")) {
+ virBufferAsprintf(new_meminfo, "Active: %8llu kB\n",
+ (meminfo.active_anon + meminfo.active_file));
+ } else if (STREQ(line, "Inactive")) {
+ virBufferAsprintf(new_meminfo, "Inactive: %8llu kB\n",
+ (meminfo.inactive_anon + meminfo.inactive_file));
+ } else if (STREQ(line, "Active(anon)")) {
+ virBufferAsprintf(new_meminfo, "Active(anon): %8llu kB\n",
+ meminfo.active_anon);
+ } else if (STREQ(line, "Inactive(anon)")) {
+ virBufferAsprintf(new_meminfo, "Inactive(anon): %8llu kB\n",
+ meminfo.inactive_anon);
+ } else if (STREQ(line, "Active(file)")) {
+ virBufferAsprintf(new_meminfo, "Active(file): %8llu kB\n",
+ meminfo.active_file);
+ } else if (STREQ(line, "Inactive(file)")) {
+ virBufferAsprintf(new_meminfo, "Inactive(file): %8llu kB\n",
+ meminfo.inactive_file);
+ } else if (STREQ(line, "Unevictable")) {
+ virBufferAsprintf(new_meminfo, "Unevictable: %8llu kB\n",
+ meminfo.unevictable);
+ } else if (STREQ(line, "SwapTotal") &&
+ virMemoryLimitIsSet(def->mem.swap_hard_limit)) {
+ virBufferAsprintf(new_meminfo, "SwapTotal: %8llu kB\n",
+ (meminfo.swaptotal - meminfo.memtotal));
+ } else if (STREQ(line, "SwapFree") &&
+ virMemoryLimitIsSet(def->mem.swap_hard_limit)) {
+ virBufferAsprintf(new_meminfo, "SwapFree: %8llu kB\n",
+ (meminfo.swaptotal - meminfo.memtotal -
+ meminfo.swapusage + meminfo.memusage));
+ } else {
+ *ptr = ':';
+ virBufferAdd(new_meminfo, line, -1);
}
+
+ if (virBufferCheckError(new_meminfo) < 0) {
+ res = -errno;
+ goto cleanup;
+ }
+
+ copied += strlen(line);
+ if (copied > size)
+ copied = size;
}
res = copied;
memcpy(buf, virBufferCurrentContent(new_meminfo), copied);
--
2.5.0
12:42 p.m.
New subject: [libvirt] [PATCH 2/4] lxc: fuse: Fix /proc/meminfo size calculation
We virtualize bits of /proc/meminfo by replacing host values with
values specific to the container.
However for calculating the final size of the returned data, we are
using the size of the original file and not the altered copy, which
could give garbelled output.
---
src/lxc/lxc_fuse.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/src/lxc/lxc_fuse.c b/src/lxc/lxc_fuse.c
index 86c0d10..ffa6889 100644
--- a/src/lxc/lxc_fuse.c
+++ b/src/lxc/lxc_fuse.c
@@ -131,7 +131,6 @@ static int lxcProcHostRead(char *path, char *buf, size_t size, off_t
offset)
static int lxcProcReadMeminfo(char *hostpath, virDomainDefPtr def,
char *buf, size_t size, off_t offset)
{
- int copied = 0;
int res;
FILE *fd = NULL;
char *line = NULL;
@@ -159,7 +158,7 @@ static int lxcProcReadMeminfo(char *hostpath, virDomainDefPtr def,
}
res = -1;
- while (copied < size && getline(&line, &n, fd) > 0) {
+ while (getline(&line, &n, fd) > 0) {
char *ptr = strchr(line, ':');
if (!ptr)
continue;
@@ -219,13 +218,11 @@ static int lxcProcReadMeminfo(char *hostpath, virDomainDefPtr def,
res = -errno;
goto cleanup;
}
-
- copied += strlen(line);
- if (copied > size)
- copied = size;
}
- res = copied;
- memcpy(buf, virBufferCurrentContent(new_meminfo), copied);
+ res = strlen(virBufferCurrentContent(new_meminfo));
+ if (res > size)
+ res = size;
+ memcpy(buf, virBufferCurrentContent(new_meminfo), res);
cleanup:
VIR_FREE(line);
--
2.5.0
12:42 p.m.
New subject: [libvirt] [PATCH 3/4] lxc: fuse: Fill in MemAvailable for /proc/meminfo
'free' on Fedora 23 will use MemAvailable to calculate its 'available'
field, but we are passing through the host's value. Set it to match
MemFree, which is what 'free' will do for older linux that don't have
MemAvailable
https://bugzilla.redhat.com/show_bug.cgi?id=1300781
---
src/lxc/lxc_fuse.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/src/lxc/lxc_fuse.c b/src/lxc/lxc_fuse.c
index ffa6889..0a1c7dc 100644
--- a/src/lxc/lxc_fuse.c
+++ b/src/lxc/lxc_fuse.c
@@ -174,6 +174,14 @@ static int lxcProcReadMeminfo(char *hostpath, virDomainDefPtr def,
virDomainDefGetMemoryActual(def))) {
virBufferAsprintf(new_meminfo, "MemFree: %8llu kB\n",
(meminfo.memtotal - meminfo.memusage));
+ } else if (STREQ(line, "MemAvailable") &&
+ (virMemoryLimitIsSet(def->mem.hard_limit) ||
+ virDomainDefGetMemoryActual(def))) {
+ /* MemAvailable is actually MemFree + SRReclaimable +
+ some other bits, but MemFree is the closest approximation
+ we have */
+ virBufferAsprintf(new_meminfo, "MemAvailable: %8llu kB\n",
+ (meminfo.memtotal - meminfo.memusage));
} else if (STREQ(line, "Buffers")) {
virBufferAsprintf(new_meminfo, "Buffers: %8d kB\n", 0);
} else if (STREQ(line, "Cached")) {
--
2.5.0
12:43 p.m.
New subject: [libvirt] [PATCH 4/4] lxc: fuse: Stub out Slab bits in /proc/meminfo
'free' on fedora23 wants to use the Slab field for calculated used
memory. The equation is:
used = MemTotal - MemFree - (Cached + Slab) - Buffers
We already set Cached and Buffers to 0, do the same for Slab and its
related values
https://bugzilla.redhat.com/show_bug.cgi?id=1300781
---
src/lxc/lxc_fuse.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/lxc/lxc_fuse.c b/src/lxc/lxc_fuse.c
index 0a1c7dc..1988c19 100644
--- a/src/lxc/lxc_fuse.c
+++ b/src/lxc/lxc_fuse.c
@@ -217,6 +217,12 @@ static int lxcProcReadMeminfo(char *hostpath, virDomainDefPtr def,
virBufferAsprintf(new_meminfo, "SwapFree: %8llu kB\n",
(meminfo.swaptotal - meminfo.memtotal -
meminfo.swapusage + meminfo.memusage));
+ } else if (STREQ(line, "Slab")) {
+ virBufferAsprintf(new_meminfo, "Slab: %8d kB\n", 0);
+ } else if (STREQ(line, "SReclaimable")) {
+ virBufferAsprintf(new_meminfo, "SReclaimable: %8d kB\n", 0);
+ } else if (STREQ(line, "SUnreclaim")) {
+ virBufferAsprintf(new_meminfo, "SUnreclaim: %8d kB\n", 0);
} else {
*ptr = ':';
virBufferAdd(new_meminfo, line, -1);
--
2.5.0
4:02 a.m.
On Thu, Jan 21, 2016 at 01:42:56PM -0500, Cole Robinson wrote:
We need to add a few bits to our /proc/meminfo virtualization to
make 'free' work correctly on Fedora 23.
https://bugzilla.redhat.com/show_bug.cgi?id=1300781
ACK to all 4
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
7:37 a.m.
On 01/22/2016 05:02 AM, Daniel P. Berrange wrote:
On Thu, Jan 21, 2016 at 01:42:56PM -0500, Cole Robinson wrote:
> We need to add a few bits to our /proc/meminfo virtualization to
> make 'free' work correctly on Fedora 23.
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1300781
ACK to all 4
Regards,
Daniel
Thanks, pushed
- Cole
3257
days inactive
3258
days old
6 comments
2 participants
participants (2)
-
Cole Robinson -
Daniel P. Berrange