Re: [libvirt] [PATCH 1/3] util: add Intel x86 RDT/CMT support

2018-06-12 Thread Wang, Huaqiang
See my update inline. 

> -Original Message-
> From: Martin Kletzander [mailto:mklet...@redhat.com]
> Sent: Monday, June 11, 2018 4:40 PM
> To: Wang, Huaqiang 
> Cc: libvir-list@redhat.com; Feng, Shaohe ; Niu, Bing
> ; Ding, Jian-feng ; Zang, Rui
> 
> Subject: Re: [libvirt] [PATCH 1/3] util: add Intel x86 RDT/CMT support
> 
> On Fri, Jun 08, 2018 at 05:02:17PM +0800, Wang Huaqiang wrote:
> >Add RDT/CMT feature (Intel x86) by interacting with kernel resctrl file 
> >system.
> Integrate code into util/resctrl.
> >---
> > src/libvirt_private.syms |   9 ++
> > src/util/virresctrl.c| 316
> ++-
> > src/util/virresctrl.h|  44 +++
> > 3 files changed, 367 insertions(+), 2 deletions(-)
> >
> 
> This will not merge after some of the cleanups I made.  There is one more 
> patch
> that didn't get in and you clould look there for some inspiration as well, 
> but it's
> just about keeping the data in another part of the code.
> 
> Anyway the conflict is very easy to fix now.
> 
> Why isn't it just a matter of setting a boolean?
> 
> Aling the code and run the checks before posting to the list.  For more info 
> see
> contribution guidelines:
> 
>   https://libvirt.org/hacking.html

Yes, noticed your patch. I'd like to make changes accordingly.
Will follow the rules listed in "https://libvirt.org/hacking.html; and take 
care the coding style. Thanks very much. 

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


Re: [libvirt] [PATCH 1/3] util: add Intel x86 RDT/CMT support

2018-06-11 Thread Martin Kletzander

On Fri, Jun 08, 2018 at 05:02:17PM +0800, Wang Huaqiang wrote:

Add RDT/CMT feature (Intel x86) by interacting with kernel resctrl file system. 
Integrate code into util/resctrl.
---
src/libvirt_private.syms |   9 ++
src/util/virresctrl.c| 316 ++-
src/util/virresctrl.h|  44 +++
3 files changed, 367 insertions(+), 2 deletions(-)



This will not merge after some of the cleanups I made.  There is one more patch
that didn't get in and you clould look there for some inspiration as well, but
it's just about keeping the data in another part of the code.

Anyway the conflict is very easy to fix now.

Why isn't it just a matter of setting a boolean?

Aling the code and run the checks before posting to the list.  For more info see
contribution guidelines:

 https://libvirt.org/hacking.html

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

[libvirt] [PATCH 1/3] util: add Intel x86 RDT/CMT support

2018-06-08 Thread Wang Huaqiang
Add RDT/CMT feature (Intel x86) by interacting with kernel resctrl file system. 
Integrate code into util/resctrl.
---
 src/libvirt_private.syms |   9 ++
 src/util/virresctrl.c| 316 ++-
 src/util/virresctrl.h|  44 +++
 3 files changed, 367 insertions(+), 2 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index b4ab1f3..e16c3e0 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2634,6 +2634,15 @@ virResctrlAllocSetSize;
 virResctrlGetInfo;
 virResctrlInfoGetCache;
 virResctrlInfoNew;
+virResctrlMonNew;
+virResctrlMonSetID;
+virResctrlMonGetID;
+virResctrlMonDeterminePath;
+virResctrlMonAddPID;
+virResctrlMonCreate;
+virResctrlMonRemove;
+virResctrlMonIsRunning;
+virResctrlMonGetCacheOccupancy;
 
 
 # util/virrotatingfile.h
diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c
index fc11635..e5f5caf 100644
--- a/src/util/virresctrl.c
+++ b/src/util/virresctrl.c
@@ -224,6 +224,22 @@ struct _virResctrlAlloc {
 
 static virClassPtr virResctrlAllocClass;
 
+struct _virResctrlMon {
+virObject parent;
+
+/* pairedalloc: pointer to a resctrl allocaion it paried with.
+ * NULL for a resctrl monitoring group not associated with
+ * any allocation. */
+virResctrlAllocPtr pairedalloc;
+/* The identifier (any unique string for now) */
+char *id;
+/* libvirt-generated path, may be identical to alloction path
+ * may not if allocation is ready */
+char *path;
+};
+
+static virClassPtr virResctrlMonClass;
+
 static void
 virResctrlAllocDispose(void *obj)
 {
@@ -275,7 +291,28 @@ virResctrlAllocOnceInit(void)
 }
 
 
+static void
+virResctrlMonDispose(void *obj)
+{
+virResctrlMonPtr resctrlMon = obj;
+
+VIR_FREE(resctrlMon->id);
+VIR_FREE(resctrlMon->path);
+}
+
+
+static int
+virResctrlMonOnceInit(void)
+{
+if (!VIR_CLASS_NEW(virResctrlMon, virClassForObject()))
+return -1;
+
+return 0;
+}
+
+
 VIR_ONCE_GLOBAL_INIT(virResctrlAlloc)
+VIR_ONCE_GLOBAL_INIT(virResctrlMon)
 
 
 virResctrlAllocPtr
@@ -288,6 +325,16 @@ virResctrlAllocNew(void)
 }
 
 
+virResctrlMonPtr
+virResctrlMonNew(void)
+{
+if (virResctrlMonInitialize() < 0)
+return NULL;
+
+return virObjectNew(virResctrlMonClass);
+}
+
+
 /* Common functions */
 #ifdef __linux__
 static int
@@ -329,8 +376,6 @@ virResctrlLockWrite(void)
 #endif
 
 
-
-
 static int
 virResctrlUnlock(int fd)
 {
@@ -1646,3 +1691,270 @@ virResctrlAllocRemove(virResctrlAllocPtr alloc)
 
 return ret;
 }
+
+
+int
+virResctrlMonSetID(virResctrlMonPtr mon,
+ const char *id)
+{
+if (!id) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+_("Resctrl mon group 'id' cannot be NULL"));
+return -1;
+}
+
+return VIR_STRDUP(mon->id, id);
+}
+
+
+const char *
+virResctrlMonGetID(virResctrlMonPtr mon)
+{
+return mon->id;
+}
+
+
+int
+virResctrlMonDeterminePath(virResctrlMonPtr mon,
+ const char *machinename)
+{
+
+VIR_DEBUG("mon group,  mon->path=%s\n", mon->path);
+if (!mon->id) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+_("Resctrl mon group id must be set before creation"));
+return -1;
+}
+
+if (mon->path)
+return -1;
+
+if(mon->pairedalloc)
+{
+if (virAsprintf(>path, "%s/%s-%s",
+SYSFS_RESCTRL_PATH, machinename, mon->id) < 0)
+return -1;
+}
+else
+{
+if (virAsprintf(>path, "%s/mon_groups/%s-%s",
+SYSFS_RESCTRL_PATH, machinename, mon->id) < 0)
+return -1;
+}
+
+return 0;
+}
+
+
+int
+virResctrlMonAddPID(virResctrlMonPtr mon,
+  pid_t pid)
+{
+char *tasks = NULL;
+char *pidstr = NULL;
+int ret = 0;
+
+if (!mon->path) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+_("Cannot add pid to non-existing resctrl mon group"));
+return -1;
+}
+
+VIR_DEBUG("Add PID %d to domain %s\n",
+pid, mon->path);
+
+if (virAsprintf(, "%s/tasks", mon->path) < 0)
+return -1;
+
+if (virAsprintf(, "%lld", (long long int) pid) < 0)
+goto cleanup;
+
+if (virFileWriteStr(tasks, pidstr, 0) < 0) {
+virReportSystemError(errno,
+_("Cannot write pid in tasks file '%s'"),
+tasks);
+goto cleanup;
+}
+
+ret = 0;
+cleanup:
+VIR_FREE(tasks);
+VIR_FREE(pidstr);
+return ret;
+}
+
+
+int
+virResctrlMonCreate(virResctrlAllocPtr pairedalloc,
+  virResctrlMonPtr mon,
+  const char *machinename)
+{
+int ret = -1;
+int lockfd = -1;
+
+if (!mon)
+return 0;
+
+
+if (pairedalloc)
+{
+if (!virFileExists(pairedalloc->path)) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+_("For paired mon group, the resctrl