Re: [libvirt] [PATCH v3 14/16] Track symlinks for co-mounted cgroup controllers

2013-04-11 Thread Michal Privoznik
On 10.04.2013 12:08, Daniel P. Berrange wrote:
 From: Daniel P. Berrange berra...@redhat.com
 
 If a cgroup controller is co-mounted with another, eg
 
/sys/fs/cgroup/cpu,cpuacct
 
 Then it is a requirement that there exist symlinks at
 
/sys/fs/cgroup/cpu
/sys/fs/cgroup/cpuacct
 
 pointing to the real mount point. Add support to virCgroupPtr
 to detect and track these symlinks
 
 Signed-off-by: Daniel P. Berrange berra...@redhat.com
 ---
  src/util/vircgroup.c | 56 ++
  src/util/vircgrouppriv.h |  5 +
  tests/vircgroupmock.c| 58 
 
  tests/vircgrouptest.c| 36 +++---
  4 files changed, 143 insertions(+), 12 deletions(-)
 
 diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
 index 6202614..14af16e 100644
 --- a/src/util/vircgroup.c
 +++ b/src/util/vircgroup.c

 @@ -157,9 +166,46 @@ static int virCgroupDetectMounts(virCgroupPtr group)
   * first entry only
   */
  if (typelen == len  STREQLEN(typestr, tmp, len) 
 -!group-controllers[i].mountPoint 
 -!(group-controllers[i].mountPoint = 
 strdup(entry.mnt_dir)))
 -goto no_memory;
 +!group-controllers[i].mountPoint) {
 +char *linksrc;
 +struct stat sb;
 +char *tmp2;
 +
 +if (!(group-controllers[i].mountPoint = 
 strdup(entry.mnt_dir)))
 +goto no_memory;
 +
 +tmp2 = strrchr(entry.mnt_dir, '/');
 +if (!tmp2) {
 +errno = EINVAL;
 +goto error;
 +}
 +*tmp2 = '\0';
 +/* If it is a co-mount it has a filename like 
 cpu,cpuacct
 + * and we must identify the symlink path */
 +if (strchr(tmp2 + 1, ',')) {
 +if (virAsprintf(linksrc, %s/%s,
 +entry.mnt_dir, typestr)  0)
 +goto no_memory;
 +*tmp2 = '/';
 +
 +if (lstat(linksrc, sb)  0) {
 +if (errno == ENOENT) {
 +VIR_WARN(Controller %s co-mounted at %s is 
 missing symlink at %s,
 + typestr, entry.mnt_dir, linksrc);
 +VIR_FREE(linksrc);
 +} else {
 +goto error;
 +}
 +} else {
 +if (!S_ISLNK(sb.st_mode)) {
 +VIR_WARN(Expecting a symlink at %s for 
 controller %s,
 + linksrc, typestr);
 +} else {
 +group-controllers[i].linkPoint = linksrc;
 +}
 +}
 +}
 +}
  tmp = next;
  }
  }
 @@ -170,8 +216,10 @@ static int virCgroupDetectMounts(virCgroupPtr group)
  return 0;
  
  no_memory:
 +errno = ENOENT;

Any reason for not returning ENOMEM here? I don't see any.

 +error:
  VIR_FORCE_FCLOSE(mounts);
 -return -ENOMEM;
 +return -errno;
  }
  

ACK if errno fixed.

Michal

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


[libvirt] [PATCH v3 14/16] Track symlinks for co-mounted cgroup controllers

2013-04-10 Thread Daniel P. Berrange
From: Daniel P. Berrange berra...@redhat.com

If a cgroup controller is co-mounted with another, eg

   /sys/fs/cgroup/cpu,cpuacct

Then it is a requirement that there exist symlinks at

   /sys/fs/cgroup/cpu
   /sys/fs/cgroup/cpuacct

pointing to the real mount point. Add support to virCgroupPtr
to detect and track these symlinks

Signed-off-by: Daniel P. Berrange berra...@redhat.com
---
 src/util/vircgroup.c | 56 ++
 src/util/vircgrouppriv.h |  5 +
 tests/vircgroupmock.c| 58 
 tests/vircgrouptest.c| 36 +++---
 4 files changed, 143 insertions(+), 12 deletions(-)

diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 6202614..14af16e 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -75,6 +75,7 @@ void virCgroupFree(virCgroupPtr *group)
 
 for (i = 0 ; i  VIR_CGROUP_CONTROLLER_LAST ; i++) {
 VIR_FREE((*group)-controllers[i].mountPoint);
+VIR_FREE((*group)-controllers[i].linkPoint);
 VIR_FREE((*group)-controllers[i].placement);
 }
 
@@ -114,6 +115,14 @@ static int virCgroupCopyMounts(virCgroupPtr group,
 
 if (!group-controllers[i].mountPoint)
 return -ENOMEM;
+
+if (parent-controllers[i].linkPoint) {
+group-controllers[i].linkPoint =
+strdup(parent-controllers[i].linkPoint);
+
+if (!group-controllers[i].linkPoint)
+return -ENOMEM;
+}
 }
 return 0;
 }
@@ -157,9 +166,46 @@ static int virCgroupDetectMounts(virCgroupPtr group)
  * first entry only
  */
 if (typelen == len  STREQLEN(typestr, tmp, len) 
-!group-controllers[i].mountPoint 
-!(group-controllers[i].mountPoint = 
strdup(entry.mnt_dir)))
-goto no_memory;
+!group-controllers[i].mountPoint) {
+char *linksrc;
+struct stat sb;
+char *tmp2;
+
+if (!(group-controllers[i].mountPoint = 
strdup(entry.mnt_dir)))
+goto no_memory;
+
+tmp2 = strrchr(entry.mnt_dir, '/');
+if (!tmp2) {
+errno = EINVAL;
+goto error;
+}
+*tmp2 = '\0';
+/* If it is a co-mount it has a filename like cpu,cpuacct
+ * and we must identify the symlink path */
+if (strchr(tmp2 + 1, ',')) {
+if (virAsprintf(linksrc, %s/%s,
+entry.mnt_dir, typestr)  0)
+goto no_memory;
+*tmp2 = '/';
+
+if (lstat(linksrc, sb)  0) {
+if (errno == ENOENT) {
+VIR_WARN(Controller %s co-mounted at %s is 
missing symlink at %s,
+ typestr, entry.mnt_dir, linksrc);
+VIR_FREE(linksrc);
+} else {
+goto error;
+}
+} else {
+if (!S_ISLNK(sb.st_mode)) {
+VIR_WARN(Expecting a symlink at %s for 
controller %s,
+ linksrc, typestr);
+} else {
+group-controllers[i].linkPoint = linksrc;
+}
+}
+}
+}
 tmp = next;
 }
 }
@@ -170,8 +216,10 @@ static int virCgroupDetectMounts(virCgroupPtr group)
 return 0;
 
 no_memory:
+errno = ENOENT;
+error:
 VIR_FORCE_FCLOSE(mounts);
-return -ENOMEM;
+return -errno;
 }
 
 
diff --git a/src/util/vircgrouppriv.h b/src/util/vircgrouppriv.h
index cc8cc0b..582be79 100644
--- a/src/util/vircgrouppriv.h
+++ b/src/util/vircgrouppriv.h
@@ -34,6 +34,11 @@
 struct virCgroupController {
 int type;
 char *mountPoint;
+/* If mountPoint holds several controllers co-mounted,
+ * then linkPoint is path of the symlink to the mountPoint
+ * for just the one controller
+ */
+char *linkPoint;
 char *placement;
 };
 
diff --git a/tests/vircgroupmock.c b/tests/vircgroupmock.c
index e50f7e0..32f074b 100644
--- a/tests/vircgroupmock.c
+++ b/tests/vircgroupmock.c
@@ -32,6 +32,8 @@
 static int (*realopen)(const char *path, int flags, ...);
 static FILE *(*realfopen)(const char *path, const char *mode);
 static int (*realaccess)(const char *path, int mode);
+static int (*reallstat)(const char *path, struct stat *sb);
+static int (*real__lxstat)(int ver, const char *path, struct stat *sb);
 static int (*realmkdir)(const char