Re: [libvirt] [PATCH 16/17] vircgroup: Introduce virCgroupGetMemoryStat

2018-08-10 Thread Pavel Hrdina 
On Fri, Aug 10, 2018 at 10:44:39AM +0200, Michal Privoznik wrote:
> On 08/09/2018 03:44 PM, Pavel Hrdina wrote:
> > Signed-off-by: Pavel Hrdina 
> > ---
> >  src/libvirt_private.syms |  1 +
> >  src/util/vircgroup.c | 88 
> >  src/util/vircgroup.h |  7 
> >  3 files changed, 96 insertions(+)
> > 
> > diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> > index 59d9bc380e..ee0dca6129 100644
> > --- a/src/libvirt_private.syms
> > +++ b/src/libvirt_private.syms
> > @@ -1532,6 +1532,7 @@ virCgroupGetDomainTotalCpuStats;
> >  virCgroupGetFreezerState;
> >  virCgroupGetMemoryHardLimit;
> >  virCgroupGetMemorySoftLimit;
> > +virCgroupGetMemoryStat;
> >  virCgroupGetMemoryUsage;
> >  virCgroupGetMemSwapHardLimit;
> >  virCgroupGetMemSwapUsage;
> > diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
> > index 37982a9607..b91acd13c7 100644
> > --- a/src/util/vircgroup.c
> > +++ b/src/util/vircgroup.c
> > @@ -2427,6 +2427,94 @@ virCgroupSetMemory(virCgroupPtr group, unsigned long 
> > long kb)
> >  }
> >  
> >  
> > +/**
> > + * virCgroupGetMemoryStat:
> > + *
> > + * @group: The cgroup to change memory for
> > + * @cache: page cache memory in KiB
> > + * @activeAnon: anonymous and swap cache memory in KiB
> > + * @inactiveAnon: anonymous and swap cache memory in KiB
> > + * @activeFile: file-backed memory in KiB
> > + * @inactiveFile: file-backed memory in KiB
> > + * @unevictable: memory that cannot be reclaimed KiB
> > + *
> > + * Returns: 0 on success, -1 on error
> > + */
> > +int
> > +virCgroupGetMemoryStat(virCgroupPtr group,
> > +   unsigned long long *cache,
> > +   unsigned long long *activeAnon,
> > +   unsigned long long *inactiveAnon,
> > +   unsigned long long *activeFile,
> > +   unsigned long long *inactiveFile,
> > +   unsigned long long *unevictable)
> > +{
> > +int ret = -1;
> > +char *stat = NULL;
> > +char *line = NULL;
> > +unsigned long long cacheVal = 0;
> > +unsigned long long activeAnonVal = 0;
> > +unsigned long long inactiveAnonVal = 0;
> > +unsigned long long activeFileVal = 0;
> > +unsigned long long inactiveFileVal = 0;
> > +unsigned long long unevictableVal = 0;
> > +
> > +if (virCgroupGetValueStr(group,
> > + VIR_CGROUP_CONTROLLER_MEMORY,
> > + "memory.stat",
> > + ) < 0) {
> > +return -1;
> > +}
> > +
> > +line = stat;
> > +
> > +while (line) {
> > +char *newLine = strchr(line, '\n');
> > +char *valueStr = strchr(line, ' ');
> > +unsigned long long value;
> > +
> > +if (newLine)
> > +*newLine = '\0';
> > +
> > +if (!valueStr) {
> > +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > +   _("Cannot parse 'memory.stat' cgroup file."));
> > +goto cleanup;
> > +}
> > +*valueStr = '\0';
> > +
> > +if (virStrToLong_ull(valueStr + 1, NULL, 10, ) < 0)
> > +goto cleanup;
> > +
> > +if (STREQ(line, "cache"))
> > +cacheVal = value >> 10;
> 
> 
> Can't we assign directly to *cache? Sure, you'd need to initialize it
> before, just like you're initializing cacheVal.

We can do that, some other functions in vircgroup.c are assigning values
directly into the passed parameters, I wanted to play it safe and update
the passed parameters only if everything succeeds.

Pavel


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 16/17] vircgroup: Introduce virCgroupGetMemoryStat

2018-08-10 Thread Michal Privoznik 
On 08/09/2018 03:44 PM, Pavel Hrdina wrote:
> Signed-off-by: Pavel Hrdina 
> ---
>  src/libvirt_private.syms |  1 +
>  src/util/vircgroup.c | 88 
>  src/util/vircgroup.h |  7 
>  3 files changed, 96 insertions(+)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 59d9bc380e..ee0dca6129 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1532,6 +1532,7 @@ virCgroupGetDomainTotalCpuStats;
>  virCgroupGetFreezerState;
>  virCgroupGetMemoryHardLimit;
>  virCgroupGetMemorySoftLimit;
> +virCgroupGetMemoryStat;
>  virCgroupGetMemoryUsage;
>  virCgroupGetMemSwapHardLimit;
>  virCgroupGetMemSwapUsage;
> diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
> index 37982a9607..b91acd13c7 100644
> --- a/src/util/vircgroup.c
> +++ b/src/util/vircgroup.c
> @@ -2427,6 +2427,94 @@ virCgroupSetMemory(virCgroupPtr group, unsigned long 
> long kb)
>  }
>  
>  
> +/**
> + * virCgroupGetMemoryStat:
> + *
> + * @group: The cgroup to change memory for
> + * @cache: page cache memory in KiB
> + * @activeAnon: anonymous and swap cache memory in KiB
> + * @inactiveAnon: anonymous and swap cache memory in KiB
> + * @activeFile: file-backed memory in KiB
> + * @inactiveFile: file-backed memory in KiB
> + * @unevictable: memory that cannot be reclaimed KiB
> + *
> + * Returns: 0 on success, -1 on error
> + */
> +int
> +virCgroupGetMemoryStat(virCgroupPtr group,
> +   unsigned long long *cache,
> +   unsigned long long *activeAnon,
> +   unsigned long long *inactiveAnon,
> +   unsigned long long *activeFile,
> +   unsigned long long *inactiveFile,
> +   unsigned long long *unevictable)
> +{
> +int ret = -1;
> +char *stat = NULL;
> +char *line = NULL;
> +unsigned long long cacheVal = 0;
> +unsigned long long activeAnonVal = 0;
> +unsigned long long inactiveAnonVal = 0;
> +unsigned long long activeFileVal = 0;
> +unsigned long long inactiveFileVal = 0;
> +unsigned long long unevictableVal = 0;
> +
> +if (virCgroupGetValueStr(group,
> + VIR_CGROUP_CONTROLLER_MEMORY,
> + "memory.stat",
> + ) < 0) {
> +return -1;
> +}
> +
> +line = stat;
> +
> +while (line) {
> +char *newLine = strchr(line, '\n');
> +char *valueStr = strchr(line, ' ');
> +unsigned long long value;
> +
> +if (newLine)
> +*newLine = '\0';
> +
> +if (!valueStr) {
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +   _("Cannot parse 'memory.stat' cgroup file."));
> +goto cleanup;
> +}
> +*valueStr = '\0';
> +
> +if (virStrToLong_ull(valueStr + 1, NULL, 10, ) < 0)
> +goto cleanup;
> +
> +if (STREQ(line, "cache"))
> +cacheVal = value >> 10;


Can't we assign directly to *cache? Sure, you'd need to initialize it
before, just like you're initializing cacheVal.

Michal

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

[libvirt] [PATCH 16/17] vircgroup: Introduce virCgroupGetMemoryStat

2018-08-09 Thread Pavel Hrdina 
Signed-off-by: Pavel Hrdina 
---
 src/libvirt_private.syms |  1 +
 src/util/vircgroup.c | 88 
 src/util/vircgroup.h |  7 
 3 files changed, 96 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 59d9bc380e..ee0dca6129 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1532,6 +1532,7 @@ virCgroupGetDomainTotalCpuStats;
 virCgroupGetFreezerState;
 virCgroupGetMemoryHardLimit;
 virCgroupGetMemorySoftLimit;
+virCgroupGetMemoryStat;
 virCgroupGetMemoryUsage;
 virCgroupGetMemSwapHardLimit;
 virCgroupGetMemSwapUsage;
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 37982a9607..b91acd13c7 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -2427,6 +2427,94 @@ virCgroupSetMemory(virCgroupPtr group, unsigned long 
long kb)
 }
 
 
+/**
+ * virCgroupGetMemoryStat:
+ *
+ * @group: The cgroup to change memory for
+ * @cache: page cache memory in KiB
+ * @activeAnon: anonymous and swap cache memory in KiB
+ * @inactiveAnon: anonymous and swap cache memory in KiB
+ * @activeFile: file-backed memory in KiB
+ * @inactiveFile: file-backed memory in KiB
+ * @unevictable: memory that cannot be reclaimed KiB
+ *
+ * Returns: 0 on success, -1 on error
+ */
+int
+virCgroupGetMemoryStat(virCgroupPtr group,
+   unsigned long long *cache,
+   unsigned long long *activeAnon,
+   unsigned long long *inactiveAnon,
+   unsigned long long *activeFile,
+   unsigned long long *inactiveFile,
+   unsigned long long *unevictable)
+{
+int ret = -1;
+char *stat = NULL;
+char *line = NULL;
+unsigned long long cacheVal = 0;
+unsigned long long activeAnonVal = 0;
+unsigned long long inactiveAnonVal = 0;
+unsigned long long activeFileVal = 0;
+unsigned long long inactiveFileVal = 0;
+unsigned long long unevictableVal = 0;
+
+if (virCgroupGetValueStr(group,
+ VIR_CGROUP_CONTROLLER_MEMORY,
+ "memory.stat",
+ ) < 0) {
+return -1;
+}
+
+line = stat;
+
+while (line) {
+char *newLine = strchr(line, '\n');
+char *valueStr = strchr(line, ' ');
+unsigned long long value;
+
+if (newLine)
+*newLine = '\0';
+
+if (!valueStr) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("Cannot parse 'memory.stat' cgroup file."));
+goto cleanup;
+}
+*valueStr = '\0';
+
+if (virStrToLong_ull(valueStr + 1, NULL, 10, ) < 0)
+goto cleanup;
+
+if (STREQ(line, "cache"))
+cacheVal = value >> 10;
+else if (STREQ(line, "active_anon"))
+activeAnonVal = value >> 10;
+else if (STREQ(line, "inactive_anon"))
+inactiveAnonVal = value >> 10;
+else if (STREQ(line, "active_file"))
+activeFileVal = value >> 10;
+else if (STREQ(line, "inactive_file"))
+inactiveFileVal = value >> 10;
+else if (STREQ(line, "unevictable"))
+unevictableVal = value >> 10;
+}
+
+*cache = cacheVal;
+*activeAnon = activeAnonVal;
+*inactiveAnon = inactiveAnonVal;
+*activeFile = activeFileVal;
+*inactiveFile = inactiveFileVal;
+*unevictable = unevictableVal;
+
+ret = 0;
+
+ cleanup:
+VIR_FREE(stat);
+return ret;
+}
+
+
 /**
  * virCgroupGetMemoryUsage:
  *
diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h
index 48be077aba..c7fdaaede4 100644
--- a/src/util/vircgroup.h
+++ b/src/util/vircgroup.h
@@ -177,6 +177,13 @@ int virCgroupGetBlkioDeviceWriteBps(virCgroupPtr group,
 unsigned long long *wbps);
 
 int virCgroupSetMemory(virCgroupPtr group, unsigned long long kb);
+int virCgroupGetMemoryStat(virCgroupPtr group,
+   unsigned long long *cache,
+   unsigned long long *activeAnon,
+   unsigned long long *inactiveAnon,
+   unsigned long long *activeFile,
+   unsigned long long *inactiveFile,
+   unsigned long long *unevictable);
 int virCgroupGetMemoryUsage(virCgroupPtr group, unsigned long *kb);
 
 int virCgroupSetMemoryHardLimit(virCgroupPtr group, unsigned long long kb);
-- 
2.17.1

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