[pve-devel] [PATCH v2 qemu-server 1/4] qmeventd: improve getting VMID from PID in presence of legacy cgroup entries

[pve-devel] [PATCH v2 qemu-server 1/4] qmeventd: improve getting VMID from PID in presence of legacy cgroup entries

[Fiona Ebner]

On a hybrid cgroup system, the /proc/<PID>/cgroup file looks like

> 13:pids:/qemu.slice/110.scope
> 12:perf_event:/
> 11:devices:/qemu.slice
> 10:misc:/
> 9:hugetlb:/
> 8:freezer:/
> 7:cpu,cpuacct:/qemu.slice/110.scope
> 6:memory:/qemu.slice/110.scope
> 5:rdma:/
> 4:cpuset:/
> 3:blkio:/qemu.slice
> 2:net_cls,net_prio:/
> 1:name=systemd:/qemu.slice/110.scope
> 0::/qemu.slice/110.scope

but the order doesn't seem to be deterministic, so it can happen that
an entry like '11:devices:/qemu.slice' is the first to match the
'/qemu.slice' part, which previously made the code expect to find the
VMID.

To improve detection, as a first step, match the trailing slash too.

Reported in the community forum:
https://forum.proxmox.com/threads/129320/post-571654

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

Changes in v2:
    * Improve commit message, seems like the order is not the same on
      every boot.

 qmeventd/qmeventd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/qmeventd/qmeventd.c b/qmeventd/qmeventd.c
index 921c0dca..503bce69 100644
--- a/qmeventd/qmeventd.c
+++ b/qmeventd/qmeventd.c
@@ -109,7 +109,7 @@ get_vmid_from_pid(pid_t pid)
 	}
 	cgroup_path++;
 
-	if (strncmp(cgroup_path, "/qemu.slice", 11)) {
+	if (strncmp(cgroup_path, "/qemu.slice/", 12)) {
 	    continue;
 	}
 
@@ -142,7 +142,7 @@ get_vmid_from_pid(pid_t pid)
     if (errno) {
 	fprintf(stderr, "error parsing vmid for %d: %s\n", pid, strerror(errno));
     } else {
-	fprintf(stderr, "error parsing vmid for %d: no qemu.slice cgroup entry\n", pid);
+	fprintf(stderr, "error parsing vmid for %d: no matching qemu.slice cgroup entry\n", pid);
     }
 
 ret:
-- 
2.39.2

[pve-devel] [PATCH v2 qemu-server 2/4] qmeventd: further improve getting VMID from PID

[Fiona Ebner]

by also expecting the ".scope" part and trying the next entry if it is
not present instead of immediately failing.

It's still unexpected to encounter such entries, so keep the log line.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

Changes in v2:
    * Keep setting vmid to zero in the error case!
    * Still print error to indicate it's unexpected. Can potentially
      be helpful for debugging.

 qmeventd/qmeventd.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/qmeventd/qmeventd.c b/qmeventd/qmeventd.c
index 503bce69..0130103d 100644
--- a/qmeventd/qmeventd.c
+++ b/qmeventd/qmeventd.c
@@ -128,12 +128,14 @@ get_vmid_from_pid(pid_t pid)
 	errno = 0;
 	char *endptr = NULL;
 	vmid = strtoul(vmid_start, &endptr, 10);
+	if (!endptr || strncmp(endptr, ".scope", 6)) {
+	    fprintf(stderr, "unexpected cgroup entry %s\n", buf);
+	    vmid = 0;
+	    continue;
+	}
 	if (errno != 0) {
 	    fprintf(stderr, "error parsing vmid for %d: %s\n", pid, strerror(errno));
 	    vmid = 0;
-	} else if (*endptr != '.') {
-	    fprintf(stderr, "unexpected cgroup entry %s\n", buf);
-	    vmid = 0;
 	}
 
 	goto ret;
-- 
2.39.2

[pve-devel] [PATCH v2 qemu-server 3/4] qmeventd: VMID from PID: don't fail immediately when encountering unexpected entry

[Fiona Ebner]

While such entries shuoldn't exist in practice without manually
messing around, it's not hard to imagine, and there's still a good
chance the actual VMID can still be found.

It's still unexpected to encounter such entries so keep the log line.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

New in v2.

 qmeventd/qmeventd.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/qmeventd/qmeventd.c b/qmeventd/qmeventd.c
index 0130103d..76a894cb 100644
--- a/qmeventd/qmeventd.c
+++ b/qmeventd/qmeventd.c
@@ -105,7 +105,7 @@ get_vmid_from_pid(pid_t pid)
 	char *cgroup_path = strrchr(buf, ':');
 	if (!cgroup_path) {
 	    fprintf(stderr, "unexpected cgroup entry %s\n", buf);
-	    goto ret;
+	    continue;
 	}
 	cgroup_path++;
 
@@ -116,13 +116,13 @@ get_vmid_from_pid(pid_t pid)
 	char *vmid_start = strrchr(buf, '/');
 	if (!vmid_start) {
 	    fprintf(stderr, "unexpected cgroup entry %s\n", buf);
-	    goto ret;
+	    continue;
 	}
 	vmid_start++;
 
 	if (vmid_start[0] == '-' || vmid_start[0] == '\0') {
 	    fprintf(stderr, "invalid vmid in cgroup entry %s\n", buf);
-	    goto ret;
+	    continue;
 	}
 
 	errno = 0;
-- 
2.39.2

[pve-devel] [PATCH v2 qemu-server 4/4] qmeventd: VMID from PID: avoid goto

[Fiona Ebner]

If strtol() returns an errno, make sure not to print the error message
twice after the refactoring.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

New in v2.

 qmeventd/qmeventd.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/qmeventd/qmeventd.c b/qmeventd/qmeventd.c
index 76a894cb..002b2ac4 100644
--- a/qmeventd/qmeventd.c
+++ b/qmeventd/qmeventd.c
@@ -134,20 +134,18 @@ get_vmid_from_pid(pid_t pid)
 	    continue;
 	}
 	if (errno != 0) {
-	    fprintf(stderr, "error parsing vmid for %d: %s\n", pid, strerror(errno));
 	    vmid = 0;
 	}
 
-	goto ret;
+	break;
     }
 
     if (errno) {
 	fprintf(stderr, "error parsing vmid for %d: %s\n", pid, strerror(errno));
-    } else {
+    } else if (!vmid) {
 	fprintf(stderr, "error parsing vmid for %d: no matching qemu.slice cgroup entry\n", pid);
     }
 
-ret:
     free(buf);
     fclose(fp);
     return vmid;
-- 
2.39.2

[pve-devel] applied-series: [PATCH v2 qemu-server 1/4] qmeventd: improve getting VMID from PID in presence of legacy cgroup entries

[Wolfgang Bumiller]

applied series, thanks