From: Peter Chubb <Peter.Chubb@data61.csiro.au>

Commit 901d2b9c introduced virCgroupGetMemoryStat and replaced
the LXC virLXCCgroupGetMemStat logic in commit e634c7cd0. However,
in doing so the replacement wasn't exact as the LXC logic used
getline() to process the cgroup controller data, while the new
virCgroupGetMemoryStat used "memory.stat" manual buffer read/
processing which neglected to forward through @line in order
to read each line in the output.

To fix that, we should be sure to carry forward the @line value
for each line read updating it beyond that current @newLine value
once we've calculated the values that we want.

Signed-off-by: John Ferlan <jferlan@redhat.com>
---
 src/util/vircgroupv1.c | 7 ++++++-
 src/util/vircgroupv2.c | 7 ++++++-
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c
index 28a74474ee..678ffda35c 100644
--- a/src/util/vircgroupv1.c
+++ b/src/util/vircgroupv1.c
@@ -1476,7 +1476,7 @@ virCgroupV1GetMemoryStat(virCgroupPtr group,
 
     line = stat;
 
-    while (line) {
+    while (line && *line) {
         char *newLine = strchr(line, '\n');
         char *valueStr = strchr(line, ' ');
         unsigned long long value;
@@ -1506,6 +1506,11 @@ virCgroupV1GetMemoryStat(virCgroupPtr group,
             inactiveFileVal = value >> 10;
         else if (STREQ(line, "unevictable"))
             unevictableVal = value >> 10;
+
+        if (newLine)
+            line = newLine + 1;
+        else
+            break;
     }
 
     *cache = cacheVal;
diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c
index 32adb06784..541e8e790e 100644
--- a/src/util/vircgroupv2.c
+++ b/src/util/vircgroupv2.c
@@ -1068,7 +1068,7 @@ virCgroupV2GetMemoryStat(virCgroupPtr group,
 
     line = stat;
 
-    while (line) {
+    while (line && *line) {
         char *newLine = strchr(line, '\n');
         char *valueStr = strchr(line, ' ');
         unsigned long long value;
@@ -1102,6 +1102,11 @@ virCgroupV2GetMemoryStat(virCgroupPtr group,
             inactiveFileVal = value >> 10;
         else if (STREQ(line, "unevictable"))
             unevictableVal = value >> 10;
+
+        if (newLine)
+            line = newLine + 1;
+        else
+            break;
     }
 
     *cache = cacheVal;
-- 
2.17.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

>>>>> "John" == John Ferlan <jferlan@redhat.com> writes:

John> Signed-off-by: John Ferlan <jferlan@redhat.com> ---

Signed-off-by: Peter Chubb <peter.chubb@data61.csiro.au>

--
Dr Peter Chubb         Tel: +61 2 9490 5852      http://ts.data61.csiro.au/
Trustworthy Systems Group                     Data61, CSIRO (formerly NICTA)

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Wed, Nov 07, 2018 at 06:57:39PM -0500, John Ferlan wrote:
> From: Peter Chubb <Peter.Chubb@data61.csiro.au>
> 
> Commit 901d2b9c introduced virCgroupGetMemoryStat and replaced
> the LXC virLXCCgroupGetMemStat logic in commit e634c7cd0. However,
> in doing so the replacement wasn't exact as the LXC logic used
> getline() to process the cgroup controller data, while the new
> virCgroupGetMemoryStat used "memory.stat" manual buffer read/
> processing which neglected to forward through @line in order
> to read each line in the output.
> 
> To fix that, we should be sure to carry forward the @line value
> for each line read updating it beyond that current @newLine value
> once we've calculated the values that we want.
> 
> Signed-off-by: John Ferlan <jferlan@redhat.com>
> ---
>  src/util/vircgroupv1.c | 7 ++++++-
>  src/util/vircgroupv2.c | 7 ++++++-
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c
> index 28a74474ee..678ffda35c 100644
> --- a/src/util/vircgroupv1.c
> +++ b/src/util/vircgroupv1.c
> @@ -1476,7 +1476,7 @@ virCgroupV1GetMemoryStat(virCgroupPtr group,
>  
>      line = stat;
>  
> -    while (line) {
> +    while (line && *line) {

There is no need to check line, it cannot be NULL.

>          char *newLine = strchr(line, '\n');
>          char *valueStr = strchr(line, ' ');
>          unsigned long long value;
> @@ -1506,6 +1506,11 @@ virCgroupV1GetMemoryStat(virCgroupPtr group,
>              inactiveFileVal = value >> 10;
>          else if (STREQ(line, "unevictable"))
>              unevictableVal = value >> 10;
> +
> +        if (newLine)
> +            line = newLine + 1;
> +        else
> +            break;
>      }
>  
>      *cache = cacheVal;
> diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c
> index 32adb06784..541e8e790e 100644
> --- a/src/util/vircgroupv2.c
> +++ b/src/util/vircgroupv2.c
> @@ -1068,7 +1068,7 @@ virCgroupV2GetMemoryStat(virCgroupPtr group,
>  
>      line = stat;
>  
> -    while (line) {
> +    while (line && *line) {

Same here.

>          char *newLine = strchr(line, '\n');
>          char *valueStr = strchr(line, ' ');
>          unsigned long long value;
> @@ -1102,6 +1102,11 @@ virCgroupV2GetMemoryStat(virCgroupPtr group,
>              inactiveFileVal = value >> 10;
>          else if (STREQ(line, "unevictable"))
>              unevictableVal = value >> 10;
> +
> +        if (newLine)
> +            line = newLine + 1;
> +        else
> +            break;
>      }
>  
>      *cache = cacheVal;
> -- 

Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list