On 11/6/18 3:25 AM, Peter.Chubb@xxxxxxxxxxxxxxx wrote: > > Hi Folks, > libvirt currently spams the logs with > > error : virCgroupGetMemoryStat:2490 : internal error: Cannot parse 'memory.stat' cgroup file > > whenever someone does ps in a container. > > This is because the parser for memory/stat is incorrect: the `line' > variable is never updated, so each time through the loop, the same > start-of-line is compared for the token; and as all spaces are > eventually replaced with NUL the error exit is taken instead of > ending the loop properly. > > Here is a strawman patch to fix the problem. Please note, I'm not > subscribed to this list; I reported the bug as > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=913023 and was asked > to send the patch upstream. > > src/util/vircgroup.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > Hmmmm... well I see quite true... nice catch! Seems commit id 901d2b9c87 added the code and commit e634c7cd0 used it for LXC's virLXCCgroupGetMemStat (libvirt 4.7.0, so rather recent fortunately). However that conversion seemed to missed the nuance that getline() is a bit different than the algorithm generated parsing through the read file. Too bad an existing test wasn't utilized for it, because that would have failed miserably. An any case, the patch below is against an older version of libvirt, we typically work from top of the git tree, but since you're not a regular contributor I will put something together and CC you using top of tree. Since we adhere to usage of Signed-Off-By, I'd need to have you agree to have me add an S-O-B with your name. For now I'll put my name with you as the author. > --- libvirt.orig/src/util/vircgroup.c > +++ libvirt/src/util/vircgroup.c > @@ -2477,7 +2477,7 @@ virCgroupGetMemoryStat(virCgroupPtr grou > > line = stat; > > - while (line) { > + while (*line) { probably should be line && *line Since if line was for who knows what reason NULL, then life wouldn't be happy. Tks, John updated patch will be sent shortly (with any luck). > char *newLine = strchr(line, '\n'); > char *valueStr = strchr(line, ' '); > unsigned long long value; > @@ -2507,6 +2507,11 @@ virCgroupGetMemoryStat(virCgroupPtr grou > inactiveFileVal = value >> 10; > else if (STREQ(line, "unevictable")) > unevictableVal = value >> 10; > + > + if (newLine) > + line = newLine + 1; > + else > + break; > } > > *cache = cacheVal; > > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
