[libvirt] Parsing of memory.stat in libvirt

[John Ferlan]

On 11/6/18 3:25 AM, Peter.Chubb at data61.csiro.au 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;
> 
>