[pve-devel] [PATCH qemu-server] qmeventd: go back to extracting vmid from commandline instead of cgroup file

* [pve-devel] [PATCH qemu-server] qmeventd: go back to extracting vmid from commandline instead of cgroup file
@ 2024-06-14  9:21 Fiona Ebner
  2025-04-07  8:32 ` Fiona Ebner
  2025-04-07 22:53 ` [pve-devel] applied: " Thomas Lamprecht
  0 siblings, 2 replies; 3+ messages in thread
From: Fiona Ebner @ 2024-06-14  9:21 UTC (permalink / raw)
  To: pve-devel

In spirit, this is a revert of 502870a0 ("qmeventd: extract vmid from
cgroup file instead of cmdline"), but instead of relying on the custom
'id' commandline option that's added by a Proxmox VE patch to QEMU,
rely on the standard 'pidfile' option to extract the VM ID.

As reported in the community forum [0], at least during stop mode
backup, it seems to be possible to end up with the VM process having
> 0::/system.slice/pvescheduler.service
as its single cgroup entry. It's not clear what exactly happens and
there was no success to reproduce the issue. Might be a rare bug in
systemd or in pve-common's enter_systemd_scope() code.

This was not the first time relying on the cgroup entry caused issues,
see d0b58753 ("qmeventd: improve getting VMID from PID in presence of
legacy cgroup entries").

To avoid such edge cases and issues in the future, go back to
extracting the VM ID from the process's commandline.

It's enough to care about the first occurrence of the 'pidfile'
option, because that's the one added by Proxmox VE, so the 'continue's
in the loop turn into 'break's. Even though a later option would
override the first for QEMU itself to use, that's not supported
anyways and the important part is the VM ID which is present in the
first.

[0]: https://forum.proxmox.com/threads/147409/

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 qmeventd/qmeventd.c | 29 +++++++++++++----------------
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/qmeventd/qmeventd.c b/qmeventd/qmeventd.c
index d8f3ee72..164bffce 100644
--- a/qmeventd/qmeventd.c
+++ b/qmeventd/qmeventd.c
@@ -81,7 +81,7 @@ static unsigned long
 get_vmid_from_pid(pid_t pid)
 {
     char filename[32] = { 0 };
-    int len = snprintf(filename, sizeof(filename), "/proc/%d/cgroup", pid);
+    int len = snprintf(filename, sizeof(filename), "/proc/%d/cmdline", pid);
     if (len < 0) {
 	fprintf(stderr, "error during snprintf for %d: %s\n", pid,
 		strerror(errno));
@@ -101,37 +101,34 @@ get_vmid_from_pid(pid_t pid)
     char *buf = NULL;
     size_t buflen = 0;
 
-    while (getline(&buf, &buflen, fp) >= 0) {
-	char *cgroup_path = strrchr(buf, ':');
-	if (!cgroup_path) {
-	    fprintf(stderr, "unexpected cgroup entry %s\n", buf);
+    while (getdelim(&buf, &buflen, '\0', fp) >= 0) {
+	if (strcmp(buf, "-pidfile")) {
 	    continue;
 	}
-	cgroup_path++;
 
-	if (strncmp(cgroup_path, "/qemu.slice/", 12)) {
-	    continue;
+	if (getdelim(&buf, &buflen, '\0', fp) < 0) {
+	    break;
 	}
 
 	char *vmid_start = strrchr(buf, '/');
 	if (!vmid_start) {
-	    fprintf(stderr, "unexpected cgroup entry %s\n", buf);
-	    continue;
+	    fprintf(stderr, "unexpected pidfile option %s\n", buf);
+	    break;
 	}
 	vmid_start++;
 
 	if (vmid_start[0] == '-' || vmid_start[0] == '\0') {
-	    fprintf(stderr, "invalid vmid in cgroup entry %s\n", buf);
-	    continue;
+	    fprintf(stderr, "invalid vmid in pidfile option %s\n", buf);
+	    break;
 	}
 
 	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);
+	if (!endptr || strcmp(endptr, ".pid")) {
+	    fprintf(stderr, "unexpected pidfile option %s\n", buf);
 	    vmid = 0;
-	    continue;
+	    break;
 	}
 	if (errno != 0) {
 	    vmid = 0;
@@ -143,7 +140,7 @@ get_vmid_from_pid(pid_t pid)
     if (errno) {
 	fprintf(stderr, "error parsing vmid for %d: %s\n", pid, strerror(errno));
     } else if (!vmid) {
-	fprintf(stderr, "error parsing vmid for %d: no matching qemu.slice cgroup entry\n", pid);
+	fprintf(stderr, "error parsing vmid for %d: no matching pidfile option\n", pid);
     }
 
     free(buf);
-- 
2.39.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 3+ messages in thread