From 556cc662abe3f8303b01a0b0bf85dbb41db229c1 Mon Sep 17 00:00:00 2001 From: Fiona Ebner Date: Fri, 14 Jun 2024 11:21:34 +0200 Subject: [PATCH] qmeventd: go back to extracting vmid from commandline instead of cgroup file 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 Link: https://lore.proxmox.com/20240614092134.18729-1-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 cf2a1b27..000cddfd 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);