Re: [libvirt] Parsing of memory.stat in libvirt
>>>> "John" == John Ferlan
<jferlan(a)redhat.com> writes:
On 11/6/18 3:25 AM, Peter.Chubb(a)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(-)
>
John> Hmmmm... well I see quite true... nice catch!
John> An any case, the patch below is against an older version of
John> libvirt, we typically work from top of the git tree, but since
John> you're not a regular contributor I will put something together
John> and CC you using top of tree. Since we adhere to usage of
John> Signed-Off-By, I'd need to have you agree to have me add an
John> S-O-B with your name. For now I'll put my name with you as the
John> author.
Sure, I'll add it to the individual patches.
> --- libvirt.orig/src/util/vircgroup.c +++
> libvirt/src/util/vircgroup.c @@ -2477,7 +2477,7 @@
> virCgroupGetMemoryStat(virCgroupPtr grou
>
> line = stat;
>
> - while (line) { + while (*line) {
John> probably should be line && *line Since if line was for who knows
John> what reason NULL, then life wouldn't be happy.
line can't be NULL at this point. virCgroupGetValueStr() returns
non-zero if it hasn't set stat to something sensible.
An extra check doesn't hurt though.
Peter C
--
Dr Peter Chubb Tel: +61 2 9490 5852 http://ts.data61.csiro.au/
Trustworthy Systems Group Data61, CSIRO (formerly NICTA)