* [pve-devel] [PATCH v2 qemu-server 1/7] qmeventd: add handling for -no-shutdown QEMU instances
2020-10-19 12:18 [pve-devel] [PATCH v2 0/7] Handle guest shutdown during backups Stefan Reiter
@ 2020-10-19 12:18 ` Stefan Reiter
2020-10-19 12:18 ` [pve-devel] [PATCH v2 qemu-server 2/7] qmeventd: add last-ditch effort SIGKILL cleanup Stefan Reiter
` (6 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Stefan Reiter @ 2020-10-19 12:18 UTC (permalink / raw)
To: pve-devel; +Cc: d.csapak, w.bumiller
We take care of killing QEMU processes when a guest shuts down manually.
QEMU will not exit itself, if started with -no-shutdown, but it will
still emit a "SHUTDOWN" event, which we await and then send SIGTERM.
This additionally allows us to handle backups in such situations. A
vzdump instance will connect to our socket and identify itself as such
in the handshake, sending along a VMID which will be marked as backing
up until the file handle is closed.
When a SHUTDOWN event is received while the VM is backing up, we do not
kill the VM. And when the vzdump handle is closed, we check if the
guest has started up since, and only if it's determined to still be
turned off, we then finally kill QEMU.
We cannot wait for QEMU directly to finish the backup (i.e. with
query-backup), as that would kill the VM too fast for vzdump to send the
last 'query-backup' to mark the backup as successful and done.
For handling 'query-status' messages sent to QEMU, a state-machine-esque
protocol is implemented into the Client struct (ClientState). This is
necessary, since QMP works asynchronously, and results arrive on the
same channel as events and even the handshake.
For referencing QEMU Clients from vzdump messages, they are kept in a
hash table. This requires linking against glib.
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---
v2:
* fix init of client->type to CLIENT_NONE, previously client->state was wrongly
initialized twice
debian/control | 1 +
qmeventd/Makefile | 4 +-
qmeventd/qmeventd.c | 277 ++++++++++++++++++++++++++++++++++++++------
qmeventd/qmeventd.h | 41 ++++++-
4 files changed, 281 insertions(+), 42 deletions(-)
diff --git a/debian/control b/debian/control
index fad7c8f..c34d974 100644
--- a/debian/control
+++ b/debian/control
@@ -20,6 +20,7 @@ Build-Depends: debhelper (>= 11~),
pve-edk2-firmware,
pve-firewall,
pve-qemu-kvm,
+ libglib2.0-dev,
Standards-Version: 4.3.0
Homepage: https://www.proxmox.com
diff --git a/qmeventd/Makefile b/qmeventd/Makefile
index a68818d..c2fdc6d 100644
--- a/qmeventd/Makefile
+++ b/qmeventd/Makefile
@@ -9,8 +9,8 @@ include /usr/share/pve-doc-generator/pve-doc-generator.mk
CC ?= gcc
CFLAGS += -O2 -Werror -Wall -Wextra -Wpedantic -Wtype-limits -Wl,-z,relro -std=gnu11
-CFLAGS += $(shell pkg-config --cflags json-c)
-LDFLAGS += $(shell pkg-config --libs json-c)
+CFLAGS += $(shell pkg-config --cflags json-c glib-2.0)
+LDFLAGS += $(shell pkg-config --libs json-c glib-2.0)
qmeventd: qmeventd.c
$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $<
diff --git a/qmeventd/qmeventd.c b/qmeventd/qmeventd.c
index 77c6297..6b02a06 100644
--- a/qmeventd/qmeventd.c
+++ b/qmeventd/qmeventd.c
@@ -55,12 +55,15 @@
#include <sys/un.h>
#include <sys/wait.h>
#include <unistd.h>
+#include <gmodule.h>
#include "qmeventd.h"
static int verbose = 0;
static int epoll_fd = 0;
static const char *progname;
+GHashTable *vm_clients; // key=vmid (freed on remove), value=*Client (free manually)
+
/*
* Helper functions
*/
@@ -165,15 +168,39 @@ must_write(int fd, const char *buf, size_t len)
* qmp handling functions
*/
+static void
+send_qmp_cmd(struct Client *client, const char *buf, size_t len)
+{
+ if (!must_write(client->fd, buf, len - 1)) {
+ fprintf(stderr, "%s: cannot send QMP message\n", client->qemu.vmid);
+ cleanup_client(client);
+ }
+}
+
void
handle_qmp_handshake(struct Client *client)
{
- VERBOSE_PRINT("%s: got QMP handshake\n", client->vmid);
- static const char qmp_answer[] = "{\"execute\":\"qmp_capabilities\"}\n";
- if (!must_write(client->fd, qmp_answer, sizeof(qmp_answer) - 1)) {
- fprintf(stderr, "%s: cannot complete handshake\n", client->vmid);
+ VERBOSE_PRINT("pid%d: got QMP handshake, assuming QEMU client\n", client->pid);
+
+ // extract vmid from cmdline, now that we know it's a QEMU process
+ unsigned long vmid = get_vmid_from_pid(client->pid);
+ int res = snprintf(client->qemu.vmid, sizeof(client->qemu.vmid), "%lu", vmid);
+ if (vmid == 0 || res < 0 || res >= (int)sizeof(client->qemu.vmid)) {
+ fprintf(stderr, "could not get vmid from pid %d\n", client->pid);
cleanup_client(client);
+ return;
}
+
+ VERBOSE_PRINT("pid%d: assigned VMID: %s\n", client->pid, client->qemu.vmid);
+ client->type = CLIENT_QEMU;
+ if(!g_hash_table_insert(vm_clients, strdup(client->qemu.vmid), client)) {
+ // not fatal, just means backup handling won't work
+ fprintf(stderr, "%s: could not insert client into VMID->client table\n",
+ client->qemu.vmid);
+ }
+
+ static const char qmp_answer[] = "{\"execute\":\"qmp_capabilities\"}\n";
+ send_qmp_cmd(client, qmp_answer, sizeof(qmp_answer));
}
void
@@ -183,18 +210,156 @@ handle_qmp_event(struct Client *client, struct json_object *obj)
if (!json_object_object_get_ex(obj, "event", &event)) {
return;
}
- VERBOSE_PRINT("%s: got QMP event: %s\n", client->vmid,
+ VERBOSE_PRINT("%s: got QMP event: %s\n", client->qemu.vmid,
json_object_get_string(event));
+
+ if (client->state == STATE_TERMINATING) {
+ // QEMU sometimes sends a second SHUTDOWN after SIGTERM, ignore
+ VERBOSE_PRINT("%s: event was after termination, ignoring\n",
+ client->qemu.vmid);
+ return;
+ }
+
// event, check if shutdown and get guest parameter
if (!strcmp(json_object_get_string(event), "SHUTDOWN")) {
- client->graceful = 1;
+ client->qemu.graceful = 1;
struct json_object *data;
struct json_object *guest;
if (json_object_object_get_ex(obj, "data", &data) &&
json_object_object_get_ex(data, "guest", &guest))
{
- client->guest = (unsigned short)json_object_get_boolean(guest);
+ client->qemu.guest = (unsigned short)json_object_get_boolean(guest);
}
+
+ // check if a backup is running and kill QEMU process if not
+ terminate_check(client);
+ }
+}
+
+void
+terminate_check(struct Client *client)
+{
+ if (client->state != STATE_IDLE) {
+ // if we're already in a request, queue this one until after
+ VERBOSE_PRINT("%s: terminate_check queued\n", client->qemu.vmid);
+ client->qemu.term_check_queued = true;
+ return;
+ }
+
+ client->qemu.term_check_queued = false;
+
+ VERBOSE_PRINT("%s: query-status\n", client->qemu.vmid);
+ client->state = STATE_EXPECT_STATUS_RESP;
+ static const char qmp_req[] = "{\"execute\":\"query-status\"}\n";
+ send_qmp_cmd(client, qmp_req, sizeof(qmp_req));
+}
+
+void
+handle_qmp_return(struct Client *client, struct json_object *data, bool error)
+{
+ if (error) {
+ const char *msg = "n/a";
+ struct json_object *desc;
+ if (json_object_object_get_ex(data, "desc", &desc)) {
+ msg = json_object_get_string(desc);
+ }
+ fprintf(stderr, "%s: received error from QMP: %s\n",
+ client->qemu.vmid, msg);
+ client->state = STATE_IDLE;
+ goto out;
+ }
+
+ struct json_object *status;
+ json_bool has_status = data &&
+ json_object_object_get_ex(data, "status", &status);
+
+ bool active = false;
+ if (has_status) {
+ const char *status_str = json_object_get_string(status);
+ active = status_str &&
+ (!strcmp(status_str, "running") || !strcmp(status_str, "paused"));
+ }
+
+ switch (client->state) {
+ case STATE_EXPECT_STATUS_RESP:
+ client->state = STATE_IDLE;
+ if (active) {
+ VERBOSE_PRINT("%s: got status: VM is active\n", client->qemu.vmid);
+ } else if (!client->qemu.backup) {
+ terminate_client(client);
+ } else {
+ // if we're in a backup, don't do anything, vzdump will notify
+ // us when the backup finishes
+ VERBOSE_PRINT("%s: not active, but running backup - keep alive\n",
+ client->qemu.vmid);
+ }
+ break;
+
+ // this means we received the empty return from our handshake answer
+ case STATE_HANDSHAKE:
+ client->state = STATE_IDLE;
+ VERBOSE_PRINT("%s: QMP handshake complete\n", client->qemu.vmid);
+ break;
+
+ case STATE_IDLE:
+ case STATE_TERMINATING:
+ VERBOSE_PRINT("%s: spurious return value received\n",
+ client->qemu.vmid);
+ break;
+ }
+
+out:
+ if (client->qemu.term_check_queued) {
+ terminate_check(client);
+ }
+}
+
+/*
+ * VZDump specific client functions
+ */
+
+void
+handle_vzdump_handshake(struct Client *client, struct json_object *data)
+{
+ client->state = STATE_IDLE;
+
+ struct json_object *vmid_obj;
+ json_bool has_vmid = data && json_object_object_get_ex(data, "vmid", &vmid_obj);
+
+ if (!has_vmid) {
+ VERBOSE_PRINT("pid%d: invalid vzdump handshake: no vmid\n",
+ client->pid);
+ return;
+ }
+
+ const char *vmid_str = json_object_get_string(vmid_obj);
+
+ if (!vmid_str) {
+ VERBOSE_PRINT("pid%d: invalid vzdump handshake: vmid is not a string\n",
+ client->pid);
+ return;
+ }
+
+ int res = snprintf(client->vzdump.vmid, sizeof(client->vzdump.vmid), "%s", vmid_str);
+ if (res < 0 || res >= (int)sizeof(client->vzdump.vmid)) {
+ VERBOSE_PRINT("pid%d: invalid vzdump handshake: vmid too long or invalid\n",
+ client->pid);
+ return;
+ }
+
+ struct Client *vmc =
+ (struct Client*) g_hash_table_lookup(vm_clients, client->vzdump.vmid);
+ if (vmc) {
+ vmc->qemu.backup = true;
+
+ // only mark as VZDUMP once we have set everything up, otherwise 'cleanup'
+ // might try to access an invalid value
+ client->type = CLIENT_VZDUMP;
+ VERBOSE_PRINT("%s: vzdump backup started\n",
+ client->vzdump.vmid);
+ } else {
+ VERBOSE_PRINT("%s: vzdump requested backup start for unregistered VM\n",
+ client->vzdump.vmid);
}
}
@@ -206,30 +371,25 @@ void
add_new_client(int client_fd)
{
struct Client *client = calloc(sizeof(struct Client), 1);
+ client->state = STATE_HANDSHAKE;
+ client->type = CLIENT_NONE;
client->fd = client_fd;
client->pid = get_pid_from_fd(client_fd);
if (client->pid == 0) {
fprintf(stderr, "could not get pid from client\n");
goto err;
}
- unsigned long vmid = get_vmid_from_pid(client->pid);
- int res = snprintf(client->vmid, sizeof(client->vmid), "%lu", vmid);
- if (vmid == 0 || res < 0 || res >= (int)sizeof(client->vmid)) {
- fprintf(stderr, "could not get vmid from pid %d\n", client->pid);
- goto err;
- }
struct epoll_event ev;
ev.events = EPOLLIN;
ev.data.ptr = client;
- res = epoll_ctl(epoll_fd, EPOLL_CTL_ADD, client_fd, &ev);
+ int res = epoll_ctl(epoll_fd, EPOLL_CTL_ADD, client_fd, &ev);
if (res < 0) {
perror("epoll_ctl client add");
goto err;
}
- VERBOSE_PRINT("added new client, pid: %d, vmid: %s\n", client->pid,
- client->vmid);
+ VERBOSE_PRINT("added new client, pid: %d\n", client->pid);
return;
err:
@@ -237,20 +397,16 @@ err:
free(client);
}
-void
-cleanup_client(struct Client *client)
+static void
+cleanup_qemu_client(struct Client *client)
{
- VERBOSE_PRINT("%s: client exited, status: graceful: %d, guest: %d\n",
- client->vmid, client->graceful, client->guest);
- log_neg(epoll_ctl(epoll_fd, EPOLL_CTL_DEL, client->fd, NULL), "epoll del");
- (void)close(client->fd);
-
- unsigned short graceful = client->graceful;
- unsigned short guest = client->guest;
- char vmid[sizeof(client->vmid)];
- strncpy(vmid, client->vmid, sizeof(vmid));
- free(client);
- VERBOSE_PRINT("%s: executing cleanup\n", vmid);
+ unsigned short graceful = client->qemu.graceful;
+ unsigned short guest = client->qemu.guest;
+ char vmid[sizeof(client->qemu.vmid)];
+ strncpy(vmid, client->qemu.vmid, sizeof(vmid));
+ g_hash_table_remove(vm_clients, &vmid); // frees key, ignore errors
+ VERBOSE_PRINT("%s: executing cleanup (graceful: %d, guest: %d)\n",
+ vmid, graceful, guest);
int pid = fork();
if (pid < 0) {
@@ -275,10 +431,51 @@ cleanup_client(struct Client *client)
}
}
+void
+cleanup_client(struct Client *client)
+{
+ log_neg(epoll_ctl(epoll_fd, EPOLL_CTL_DEL, client->fd, NULL), "epoll del");
+ (void)close(client->fd);
+
+ struct Client *vmc;
+ switch (client->type) {
+ case CLIENT_QEMU:
+ cleanup_qemu_client(client);
+ break;
+
+ case CLIENT_VZDUMP:
+ vmc = (struct Client*) g_hash_table_lookup(vm_clients, client->vzdump.vmid);
+ if (vmc) {
+ VERBOSE_PRINT("%s: backup ended\n", client->vzdump.vmid);
+ vmc->qemu.backup = false;
+ terminate_check(vmc);
+ }
+ break;
+
+ case CLIENT_NONE:
+ // do nothing, only close socket
+ break;
+ }
+
+ free(client);
+}
+
+void
+terminate_client(struct Client *client)
+{
+ VERBOSE_PRINT("%s: terminating client (pid %d)\n",
+ client->qemu.vmid, client->pid);
+
+ client->state = STATE_TERMINATING;
+
+ int err = kill(client->pid, SIGTERM);
+ log_neg(err, "kill");
+}
+
void
handle_client(struct Client *client)
{
- VERBOSE_PRINT("%s: entering handle\n", client->vmid);
+ VERBOSE_PRINT("pid%d: entering handle\n", client->pid);
ssize_t len;
do {
len = read(client->fd, (client->buf+client->buflen),
@@ -292,12 +489,12 @@ handle_client(struct Client *client)
}
return;
} else if (len == 0) {
- VERBOSE_PRINT("%s: got EOF\n", client->vmid);
+ VERBOSE_PRINT("pid%d: got EOF\n", client->pid);
cleanup_client(client);
return;
}
- VERBOSE_PRINT("%s: read %ld bytes\n", client->vmid, len);
+ VERBOSE_PRINT("pid%d: read %ld bytes\n", client->pid, len);
client->buflen += len;
struct json_tokener *tok = json_tokener_new();
@@ -318,20 +515,26 @@ handle_client(struct Client *client)
handle_qmp_handshake(client);
} else if (json_object_object_get_ex(jobj, "event", &obj)) {
handle_qmp_event(client, jobj);
+ } else if (json_object_object_get_ex(jobj, "return", &obj)) {
+ handle_qmp_return(client, obj, false);
+ } else if (json_object_object_get_ex(jobj, "error", &obj)) {
+ handle_qmp_return(client, obj, true);
+ } else if (json_object_object_get_ex(jobj, "vzdump", &obj)) {
+ handle_vzdump_handshake(client, obj);
} // else ignore message
}
break;
case json_tokener_continue:
if (client->buflen >= sizeof(client->buf)) {
- VERBOSE_PRINT("%s, msg too large, discarding buffer\n",
- client->vmid);
+ VERBOSE_PRINT("pid%d: msg too large, discarding buffer\n",
+ client->pid);
memset(client->buf, 0, sizeof(client->buf));
client->buflen = 0;
} // else we have enough space try again after next read
break;
default:
- VERBOSE_PRINT("%s: parse error: %d, discarding buffer\n",
- client->vmid, jerr);
+ VERBOSE_PRINT("pid%d: parse error: %d, discarding buffer\n",
+ client->pid, jerr);
memset(client->buf, 0, client->buflen);
client->buflen = 0;
break;
@@ -402,6 +605,8 @@ main(int argc, char *argv[])
bail_neg(daemon(0, 1), "daemon");
}
+ vm_clients = g_hash_table_new_full(g_str_hash, g_str_equal, free, NULL);
+
int nevents;
for(;;) {
diff --git a/qmeventd/qmeventd.h b/qmeventd/qmeventd.h
index 0c2ffc1..30aea98 100644
--- a/qmeventd/qmeventd.h
+++ b/qmeventd/qmeventd.h
@@ -38,18 +38,51 @@ static inline void bail_neg(int errval, const char *msg)
}
}
+typedef enum {
+ CLIENT_NONE,
+ CLIENT_QEMU,
+ CLIENT_VZDUMP
+} ClientType;
+
+typedef enum {
+ STATE_HANDSHAKE,
+ STATE_IDLE,
+ STATE_EXPECT_STATUS_RESP,
+ STATE_TERMINATING
+} ClientState;
+
struct Client {
char buf[4096];
- char vmid[16];
+ unsigned int buflen;
+
int fd;
pid_t pid;
- unsigned int buflen;
- unsigned short graceful;
- unsigned short guest;
+
+ ClientType type;
+ ClientState state;
+
+ // only relevant for type=CLIENT_QEMU
+ struct {
+ char vmid[16];
+ unsigned short graceful;
+ unsigned short guest;
+ bool term_check_queued;
+ bool backup;
+ } qemu;
+
+ // only relevant for type=CLIENT_VZDUMP
+ struct {
+ // vmid of referenced backup
+ char vmid[16];
+ } vzdump;
};
void handle_qmp_handshake(struct Client *client);
void handle_qmp_event(struct Client *client, struct json_object *obj);
+void handle_qmp_return(struct Client *client, struct json_object *data, bool error);
+void handle_vzdump_handshake(struct Client *client, struct json_object *data);
void handle_client(struct Client *client);
void add_new_client(int client_fd);
void cleanup_client(struct Client *client);
+void terminate_client(struct Client *client);
+void terminate_check(struct Client *client);
--
2.20.1
^ permalink raw reply [flat|nested] 9+ messages in thread* [pve-devel] [PATCH v2 qemu-server 2/7] qmeventd: add last-ditch effort SIGKILL cleanup
2020-10-19 12:18 [pve-devel] [PATCH v2 0/7] Handle guest shutdown during backups Stefan Reiter
2020-10-19 12:18 ` [pve-devel] [PATCH v2 qemu-server 1/7] qmeventd: add handling for -no-shutdown QEMU instances Stefan Reiter
@ 2020-10-19 12:18 ` Stefan Reiter
2020-10-19 12:18 ` [pve-devel] [PATCH v2 qemu-server 3/7] vzdump: connect to qmeventd for duration of backup Stefan Reiter
` (5 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Stefan Reiter @ 2020-10-19 12:18 UTC (permalink / raw)
To: pve-devel; +Cc: d.csapak, w.bumiller
'alarm' is used to schedule an additionaly cleanup round 5 seconds after
sending SIGTERM via terminate_client. This then sends SIGKILL via a
pidfd (if supported by the kernel) or directly via kill, making sure
that the QEMU process is *really* dead and won't be left behind in an
undetermined state.
This shouldn't be an issue under normal circumstances, but can help
avoid dead processes lying around if QEMU hangs after SIGTERM.
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---
v2:
* use a pidfd to avoid pid-reuse races
qmeventd/qmeventd.c | 87 ++++++++++++++++++++++++++++++++++++++++++++-
qmeventd/qmeventd.h | 26 ++++++++++++++
2 files changed, 112 insertions(+), 1 deletion(-)
diff --git a/qmeventd/qmeventd.c b/qmeventd/qmeventd.c
index 6b02a06..57f1867 100644
--- a/qmeventd/qmeventd.c
+++ b/qmeventd/qmeventd.c
@@ -63,6 +63,8 @@ static int verbose = 0;
static int epoll_fd = 0;
static const char *progname;
GHashTable *vm_clients; // key=vmid (freed on remove), value=*Client (free manually)
+GSList *forced_cleanups;
+volatile sig_atomic_t alarm_triggered = 0;
/*
* Helper functions
@@ -468,8 +470,39 @@ terminate_client(struct Client *client)
client->state = STATE_TERMINATING;
+ // open a pidfd before kill for later cleanup
+ int pidfd = pidfd_open(client->pid, 0);
+ if (pidfd < 0) {
+ switch (errno) {
+ case ESRCH:
+ // process already dead for some reason, cleanup done
+ VERBOSE_PRINT("%s: failed to open pidfd, process already dead (pid %d)\n",
+ client->qemu.vmid, client->pid);
+ return;
+
+ // otherwise fall back to just using the PID directly, but don't
+ // print if we only failed because we're running on an older kernel
+ case ENOSYS:
+ break;
+ default:
+ perror("failed to open QEMU pidfd for cleanup");
+ break;
+ }
+ }
+
int err = kill(client->pid, SIGTERM);
log_neg(err, "kill");
+
+ struct CleanupData *data_ptr = malloc(sizeof(struct CleanupData));
+ struct CleanupData data = {
+ .pid = client->pid,
+ .pidfd = pidfd
+ };
+ *data_ptr = data;
+ forced_cleanups = g_slist_prepend(forced_cleanups, (void *)data_ptr);
+
+ // resets any other alarms, but will fire eventually and cleanup all
+ alarm(5);
}
void
@@ -545,6 +578,55 @@ handle_client(struct Client *client)
}
+/*
+ * SIGALRM and cleanup handling
+ *
+ * terminate_client will set an alarm for 5 seconds and add its client's PID to
+ * the forced_cleanups list - when the timer expires, we iterate the list and
+ * attempt to issue SIGKILL to all processes which haven't yet stopped.
+ */
+
+static void
+alarm_handler(__attribute__((unused)) int signum)
+{
+ alarm_triggered = 1;
+}
+
+static void
+sigkill(void *ptr, __attribute__((unused)) void *unused)
+{
+ struct CleanupData data = *((struct CleanupData *)ptr);
+ int err;
+
+ if (data.pidfd > 0) {
+ err = pidfd_send_signal(data.pidfd, SIGKILL, NULL, 0);
+ } else {
+ err = kill(data.pid, SIGKILL);
+ }
+
+ if (err < 0) {
+ if (errno != ESRCH) {
+ fprintf(stderr, "SIGKILL cleanup of pid '%d' failed - %s\n",
+ data.pid, strerror(errno));
+ }
+ } else {
+ fprintf(stderr, "cleanup failed, terminating pid '%d' with SIGKILL\n",
+ data.pid);
+ }
+}
+
+static void
+handle_forced_cleanup()
+{
+ if (alarm_triggered) {
+ alarm_triggered = 0;
+ g_slist_foreach(forced_cleanups, sigkill, NULL);
+ g_slist_free_full(forced_cleanups, free);
+ forced_cleanups = NULL;
+ }
+}
+
+
int
main(int argc, char *argv[])
{
@@ -577,6 +659,7 @@ main(int argc, char *argv[])
}
signal(SIGCHLD, SIG_IGN);
+ signal(SIGALRM, alarm_handler);
socket_path = argv[optind];
@@ -612,7 +695,7 @@ main(int argc, char *argv[])
for(;;) {
nevents = epoll_wait(epoll_fd, events, 1, -1);
if (nevents < 0 && errno == EINTR) {
- // signal happened, try again
+ handle_forced_cleanup();
continue;
}
bail_neg(nevents, "epoll_wait");
@@ -630,5 +713,7 @@ main(int argc, char *argv[])
handle_client((struct Client *)events[n].data.ptr);
}
}
+
+ handle_forced_cleanup();
}
}
diff --git a/qmeventd/qmeventd.h b/qmeventd/qmeventd.h
index 30aea98..1921ef3 100644
--- a/qmeventd/qmeventd.h
+++ b/qmeventd/qmeventd.h
@@ -21,6 +21,15 @@
Author: Dominik Csapak <d.csapak@proxmox.com>
*/
+#include <sys/syscall.h>
+
+#ifndef __NR_pidfd_open
+#define __NR_pidfd_open 434
+#endif
+#ifndef __NR_pidfd_send_signal
+#define __NR_pidfd_send_signal 424
+#endif
+
#define VERBOSE_PRINT(...) do { if (verbose) { printf(__VA_ARGS__); } } while (0)
static inline void log_neg(int errval, const char *msg)
@@ -38,6 +47,18 @@ static inline void bail_neg(int errval, const char *msg)
}
}
+static inline int
+pidfd_open(pid_t pid, unsigned int flags)
+{
+ return syscall(__NR_pidfd_open, pid, flags);
+}
+
+static inline int
+pidfd_send_signal(int pidfd, int sig, siginfo_t *info, unsigned int flags)
+{
+ return syscall(__NR_pidfd_send_signal, pidfd, sig, info, flags);
+}
+
typedef enum {
CLIENT_NONE,
CLIENT_QEMU,
@@ -77,6 +98,11 @@ struct Client {
} vzdump;
};
+struct CleanupData {
+ pid_t pid;
+ int pidfd;
+};
+
void handle_qmp_handshake(struct Client *client);
void handle_qmp_event(struct Client *client, struct json_object *obj);
void handle_qmp_return(struct Client *client, struct json_object *data, bool error);
--
2.20.1
^ permalink raw reply [flat|nested] 9+ messages in thread* [pve-devel] [PATCH v2 qemu-server 3/7] vzdump: connect to qmeventd for duration of backup
2020-10-19 12:18 [pve-devel] [PATCH v2 0/7] Handle guest shutdown during backups Stefan Reiter
2020-10-19 12:18 ` [pve-devel] [PATCH v2 qemu-server 1/7] qmeventd: add handling for -no-shutdown QEMU instances Stefan Reiter
2020-10-19 12:18 ` [pve-devel] [PATCH v2 qemu-server 2/7] qmeventd: add last-ditch effort SIGKILL cleanup Stefan Reiter
@ 2020-10-19 12:18 ` Stefan Reiter
2020-10-19 12:18 ` [pve-devel] [PATCH v2 qemu-server 4/7] vzdump: use dirty bitmap for not running VMs too Stefan Reiter
` (4 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Stefan Reiter @ 2020-10-19 12:18 UTC (permalink / raw)
To: pve-devel; +Cc: d.csapak, w.bumiller
Connect and send the vmid of the VM being backed up. This prevents
qmeventd from SIGTERMing the underlying QEMU instance, even if the guest
shuts itself down, until we close the socket connection (in cleanup,
which happens on success and abort, or if we crash the file handle will
be closed as well).
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---
PVE/VZDump/QemuServer.pm | 35 ++++++++++++++++++++++++++++++++++-
1 file changed, 34 insertions(+), 1 deletion(-)
diff --git a/PVE/VZDump/QemuServer.pm b/PVE/VZDump/QemuServer.pm
index c8094bd..ccfb214 100644
--- a/PVE/VZDump/QemuServer.pm
+++ b/PVE/VZDump/QemuServer.pm
@@ -8,6 +8,7 @@ use File::Path;
use IO::File;
use IPC::Open3;
use JSON;
+use POSIX qw(EINTR EAGAIN);
use PVE::Cluster qw(cfs_read_file);
use PVE::INotify;
@@ -515,6 +516,7 @@ sub archive_pbs {
my $devlist = _get_task_devlist($task);
$self->enforce_vm_running_for_backup($vmid);
+ $self->register_qmeventd_handle($vmid);
my $backup_job_uuid;
eval {
@@ -683,6 +685,7 @@ sub archive_vma {
my $devlist = _get_task_devlist($task);
$self->enforce_vm_running_for_backup($vmid);
+ $self->register_qmeventd_handle($vmid);
my $cpid;
my $backup_job_uuid;
@@ -841,6 +844,34 @@ sub enforce_vm_running_for_backup {
die $@ if $@;
}
+sub register_qmeventd_handle {
+ my ($self, $vmid) = @_;
+
+ my $fh;
+ my $peer = "/var/run/qmeventd.sock";
+ my $count = 0;
+
+ for (;;) {
+ $count++;
+ $fh = IO::Socket::UNIX->new(Peer => $peer, Blocking => 0, Timeout => 1);
+ last if $fh;
+ if ($! != EINTR && $! != EAGAIN) {
+ $self->log("warn", "unable to connect to qmeventd socket (vmid: $vmid) - $!\n");
+ return;
+ }
+ if ($count > 4) {
+ $self->log("warn", "unable to connect to qmeventd socket (vmid: $vmid)"
+ . " - timeout after $count retries\n");
+ return;
+ }
+ usleep(25000);
+ }
+
+ # send handshake to mark VM as backing up
+ print $fh to_json({vzdump => {vmid => "$vmid"}});
+ $self->{qmeventd_fh} = $fh;
+}
+
# resume VM againe once we got in a clear state (stop mode backup of running VM)
sub resume_vm_after_job_start {
my ($self, $task, $vmid) = @_;
@@ -894,7 +925,9 @@ sub snapshot {
sub cleanup {
my ($self, $task, $vmid) = @_;
- # nothing to do ?
+ if ($self->{qmeventd_fh}) {
+ close($self->{qmeventd_fh});
+ }
}
1;
--
2.20.1
^ permalink raw reply [flat|nested] 9+ messages in thread* [pve-devel] [PATCH v2 qemu-server 4/7] vzdump: use dirty bitmap for not running VMs too
2020-10-19 12:18 [pve-devel] [PATCH v2 0/7] Handle guest shutdown during backups Stefan Reiter
` (2 preceding siblings ...)
2020-10-19 12:18 ` [pve-devel] [PATCH v2 qemu-server 3/7] vzdump: connect to qmeventd for duration of backup Stefan Reiter
@ 2020-10-19 12:18 ` Stefan Reiter
2020-10-19 12:18 ` [pve-devel] [PATCH v2 qemu-server 5/7] config_to_command: use -no-shutdown option Stefan Reiter
` (3 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Stefan Reiter @ 2020-10-19 12:18 UTC (permalink / raw)
To: pve-devel; +Cc: d.csapak, w.bumiller
Now that VMs can be started during a backup, it makes sense to create a
dirty bitmap in these cases too, since the VM might be resumed and thus
continue running normally even after the backup is done.
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---
PVE/VZDump/QemuServer.pm | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/PVE/VZDump/QemuServer.pm b/PVE/VZDump/QemuServer.pm
index ccfb214..ee9896c 100644
--- a/PVE/VZDump/QemuServer.pm
+++ b/PVE/VZDump/QemuServer.pm
@@ -299,8 +299,7 @@ my $bitmap_action_to_human = sub {
my $action = $info->{action};
if ($action eq "not-used") {
- return "disabled (no support)" if $self->{vm_was_running};
- return "disabled (VM not running)";
+ return "disabled (no support)";
} elsif ($action eq "not-used-removed") {
return "disabled (old bitmap cleared)";
} elsif ($action eq "new") {
@@ -554,7 +553,7 @@ sub archive_pbs {
my $is_template = PVE::QemuConfig->is_template($self->{vmlist}->{$vmid});
$params->{'use-dirty-bitmap'} = JSON::true
- if $qemu_support->{'pbs-dirty-bitmap'} && $self->{vm_was_running} && !$is_template;
+ if $qemu_support->{'pbs-dirty-bitmap'} && !$is_template;
$params->{timeout} = 60; # give some time to connect to the backup server
--
2.20.1
^ permalink raw reply [flat|nested] 9+ messages in thread* [pve-devel] [PATCH v2 qemu-server 5/7] config_to_command: use -no-shutdown option
2020-10-19 12:18 [pve-devel] [PATCH v2 0/7] Handle guest shutdown during backups Stefan Reiter
` (3 preceding siblings ...)
2020-10-19 12:18 ` [pve-devel] [PATCH v2 qemu-server 4/7] vzdump: use dirty bitmap for not running VMs too Stefan Reiter
@ 2020-10-19 12:18 ` Stefan Reiter
2020-10-19 12:18 ` [pve-devel] [PATCH v2 qemu-server 6/7] fix vm_resume and allow vm_start with QMP status 'shutdown' Stefan Reiter
` (2 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Stefan Reiter @ 2020-10-19 12:18 UTC (permalink / raw)
To: pve-devel; +Cc: d.csapak, w.bumiller
Ignore shutdowns triggered from within the guest in favor of detecting
them via qmeventd and stopping the QEMU process that way.
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---
v2:
* as part of rebase: include newer tests (bootorder) in update
PVE/QemuServer.pm | 2 ++
test/cfg2cmd/bootorder-empty.conf.cmd | 1 +
test/cfg2cmd/bootorder-legacy.conf.cmd | 1 +
test/cfg2cmd/bootorder.conf.cmd | 1 +
test/cfg2cmd/custom-cpu-model-defaults.conf.cmd | 1 +
test/cfg2cmd/custom-cpu-model-host-phys-bits.conf.cmd | 1 +
test/cfg2cmd/custom-cpu-model.conf.cmd | 1 +
test/cfg2cmd/efi-raw-old.conf.cmd | 1 +
test/cfg2cmd/efi-raw.conf.cmd | 1 +
test/cfg2cmd/i440fx-win10-hostpci.conf.cmd | 1 +
test/cfg2cmd/minimal-defaults.conf.cmd | 1 +
test/cfg2cmd/netdev.conf.cmd | 1 +
test/cfg2cmd/pinned-version.conf.cmd | 1 +
test/cfg2cmd/q35-linux-hostpci-multifunction.conf.cmd | 1 +
test/cfg2cmd/q35-linux-hostpci.conf.cmd | 1 +
test/cfg2cmd/q35-win10-hostpci.conf.cmd | 1 +
test/cfg2cmd/simple-virtio-blk.conf.cmd | 1 +
test/cfg2cmd/simple1.conf.cmd | 1 +
test/cfg2cmd/spice-enhancments.conf.cmd | 1 +
test/cfg2cmd/spice-linux-4.1.conf.cmd | 1 +
test/cfg2cmd/spice-usb3.conf.cmd | 1 +
test/cfg2cmd/spice-win.conf.cmd | 1 +
22 files changed, 23 insertions(+)
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 3be7e24..c712671 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -3086,6 +3086,8 @@ sub config_to_command {
push @$cmd, '-name', $vmname;
+ push @$cmd, '-no-shutdown';
+
my $use_virtio = 0;
my $qmpsocket = PVE::QemuServer::Helpers::qmp_socket($vmid);
diff --git a/test/cfg2cmd/bootorder-empty.conf.cmd b/test/cfg2cmd/bootorder-empty.conf.cmd
index 1f2b2fb..066b0b7 100644
--- a/test/cfg2cmd/bootorder-empty.conf.cmd
+++ b/test/cfg2cmd/bootorder-empty.conf.cmd
@@ -1,6 +1,7 @@
/usr/bin/kvm \
-id 8006 \
-name simple \
+ -no-shutdown \
-chardev 'socket,id=qmp,path=/var/run/qemu-server/8006.qmp,server,nowait' \
-mon 'chardev=qmp,mode=control' \
-chardev 'socket,id=qmp-event,path=/var/run/qmeventd.sock,reconnect=5' \
diff --git a/test/cfg2cmd/bootorder-legacy.conf.cmd b/test/cfg2cmd/bootorder-legacy.conf.cmd
index f624ea2..fd49088 100644
--- a/test/cfg2cmd/bootorder-legacy.conf.cmd
+++ b/test/cfg2cmd/bootorder-legacy.conf.cmd
@@ -1,6 +1,7 @@
/usr/bin/kvm \
-id 8006 \
-name simple \
+ -no-shutdown \
-chardev 'socket,id=qmp,path=/var/run/qemu-server/8006.qmp,server,nowait' \
-mon 'chardev=qmp,mode=control' \
-chardev 'socket,id=qmp-event,path=/var/run/qmeventd.sock,reconnect=5' \
diff --git a/test/cfg2cmd/bootorder.conf.cmd b/test/cfg2cmd/bootorder.conf.cmd
index 86cae07..4cb81dc 100644
--- a/test/cfg2cmd/bootorder.conf.cmd
+++ b/test/cfg2cmd/bootorder.conf.cmd
@@ -1,6 +1,7 @@
/usr/bin/kvm \
-id 8006 \
-name simple \
+ -no-shutdown \
-chardev 'socket,id=qmp,path=/var/run/qemu-server/8006.qmp,server,nowait' \
-mon 'chardev=qmp,mode=control' \
-chardev 'socket,id=qmp-event,path=/var/run/qmeventd.sock,reconnect=5' \
diff --git a/test/cfg2cmd/custom-cpu-model-defaults.conf.cmd b/test/cfg2cmd/custom-cpu-model-defaults.conf.cmd
index ca8fcb0..084b397 100644
--- a/test/cfg2cmd/custom-cpu-model-defaults.conf.cmd
+++ b/test/cfg2cmd/custom-cpu-model-defaults.conf.cmd
@@ -1,6 +1,7 @@
/usr/bin/kvm \
-id 8006 \
-name customcpu-defaults \
+ -no-shutdown \
-chardev 'socket,id=qmp,path=/var/run/qemu-server/8006.qmp,server,nowait' \
-mon 'chardev=qmp,mode=control' \
-chardev 'socket,id=qmp-event,path=/var/run/qmeventd.sock,reconnect=5' \
diff --git a/test/cfg2cmd/custom-cpu-model-host-phys-bits.conf.cmd b/test/cfg2cmd/custom-cpu-model-host-phys-bits.conf.cmd
index fb6e8c8..ea5dfc8 100644
--- a/test/cfg2cmd/custom-cpu-model-host-phys-bits.conf.cmd
+++ b/test/cfg2cmd/custom-cpu-model-host-phys-bits.conf.cmd
@@ -1,6 +1,7 @@
/usr/bin/kvm \
-id 8006 \
-name customcpu \
+ -no-shutdown \
-chardev 'socket,id=qmp,path=/var/run/qemu-server/8006.qmp,server,nowait' \
-mon 'chardev=qmp,mode=control' \
-chardev 'socket,id=qmp-event,path=/var/run/qmeventd.sock,reconnect=5' \
diff --git a/test/cfg2cmd/custom-cpu-model.conf.cmd b/test/cfg2cmd/custom-cpu-model.conf.cmd
index b30163c..7ad2e9b 100644
--- a/test/cfg2cmd/custom-cpu-model.conf.cmd
+++ b/test/cfg2cmd/custom-cpu-model.conf.cmd
@@ -1,6 +1,7 @@
/usr/bin/kvm \
-id 8006 \
-name customcpu \
+ -no-shutdown \
-chardev 'socket,id=qmp,path=/var/run/qemu-server/8006.qmp,server,nowait' \
-mon 'chardev=qmp,mode=control' \
-chardev 'socket,id=qmp-event,path=/var/run/qmeventd.sock,reconnect=5' \
diff --git a/test/cfg2cmd/efi-raw-old.conf.cmd b/test/cfg2cmd/efi-raw-old.conf.cmd
index 666cdb1..f6c38d4 100644
--- a/test/cfg2cmd/efi-raw-old.conf.cmd
+++ b/test/cfg2cmd/efi-raw-old.conf.cmd
@@ -1,6 +1,7 @@
/usr/bin/kvm \
-id 8006 \
-name vm8006 \
+ -no-shutdown \
-chardev 'socket,id=qmp,path=/var/run/qemu-server/8006.qmp,server,nowait' \
-mon 'chardev=qmp,mode=control' \
-chardev 'socket,id=qmp-event,path=/var/run/qmeventd.sock,reconnect=5' \
diff --git a/test/cfg2cmd/efi-raw.conf.cmd b/test/cfg2cmd/efi-raw.conf.cmd
index cb0e984..edaf1be 100644
--- a/test/cfg2cmd/efi-raw.conf.cmd
+++ b/test/cfg2cmd/efi-raw.conf.cmd
@@ -1,6 +1,7 @@
/usr/bin/kvm \
-id 8006 \
-name vm8006 \
+ -no-shutdown \
-chardev 'socket,id=qmp,path=/var/run/qemu-server/8006.qmp,server,nowait' \
-mon 'chardev=qmp,mode=control' \
-chardev 'socket,id=qmp-event,path=/var/run/qmeventd.sock,reconnect=5' \
diff --git a/test/cfg2cmd/i440fx-win10-hostpci.conf.cmd b/test/cfg2cmd/i440fx-win10-hostpci.conf.cmd
index 2fe34a1..a9c2720 100644
--- a/test/cfg2cmd/i440fx-win10-hostpci.conf.cmd
+++ b/test/cfg2cmd/i440fx-win10-hostpci.conf.cmd
@@ -1,6 +1,7 @@
/usr/bin/kvm \
-id 8006 \
-name vm8006 \
+ -no-shutdown \
-chardev 'socket,id=qmp,path=/var/run/qemu-server/8006.qmp,server,nowait' \
-mon 'chardev=qmp,mode=control' \
-chardev 'socket,id=qmp-event,path=/var/run/qmeventd.sock,reconnect=5' \
diff --git a/test/cfg2cmd/minimal-defaults.conf.cmd b/test/cfg2cmd/minimal-defaults.conf.cmd
index 0735f43..304121f 100644
--- a/test/cfg2cmd/minimal-defaults.conf.cmd
+++ b/test/cfg2cmd/minimal-defaults.conf.cmd
@@ -1,6 +1,7 @@
/usr/bin/kvm \
-id 8006 \
-name vm8006 \
+ -no-shutdown \
-chardev 'socket,id=qmp,path=/var/run/qemu-server/8006.qmp,server,nowait' \
-mon 'chardev=qmp,mode=control' \
-chardev 'socket,id=qmp-event,path=/var/run/qmeventd.sock,reconnect=5' \
diff --git a/test/cfg2cmd/netdev.conf.cmd b/test/cfg2cmd/netdev.conf.cmd
index 4294fa0..91c6148 100644
--- a/test/cfg2cmd/netdev.conf.cmd
+++ b/test/cfg2cmd/netdev.conf.cmd
@@ -1,6 +1,7 @@
/usr/bin/kvm \
-id 8006 \
-name netdev \
+ -no-shutdown \
-chardev 'socket,id=qmp,path=/var/run/qemu-server/8006.qmp,server,nowait' \
-mon 'chardev=qmp,mode=control' \
-chardev 'socket,id=qmp-event,path=/var/run/qmeventd.sock,reconnect=5' \
diff --git a/test/cfg2cmd/pinned-version.conf.cmd b/test/cfg2cmd/pinned-version.conf.cmd
index a7d0ae2..cef2e8d 100644
--- a/test/cfg2cmd/pinned-version.conf.cmd
+++ b/test/cfg2cmd/pinned-version.conf.cmd
@@ -1,6 +1,7 @@
/usr/bin/kvm \
-id 8006 \
-name pinned \
+ -no-shutdown \
-chardev 'socket,id=qmp,path=/var/run/qemu-server/8006.qmp,server,nowait' \
-mon 'chardev=qmp,mode=control' \
-chardev 'socket,id=qmp-event,path=/var/run/qmeventd.sock,reconnect=5' \
diff --git a/test/cfg2cmd/q35-linux-hostpci-multifunction.conf.cmd b/test/cfg2cmd/q35-linux-hostpci-multifunction.conf.cmd
index a008939..cfef54d 100644
--- a/test/cfg2cmd/q35-linux-hostpci-multifunction.conf.cmd
+++ b/test/cfg2cmd/q35-linux-hostpci-multifunction.conf.cmd
@@ -1,6 +1,7 @@
/usr/bin/kvm \
-id 8006 \
-name vm8006 \
+ -no-shutdown \
-chardev 'socket,id=qmp,path=/var/run/qemu-server/8006.qmp,server,nowait' \
-mon 'chardev=qmp,mode=control' \
-chardev 'socket,id=qmp-event,path=/var/run/qmeventd.sock,reconnect=5' \
diff --git a/test/cfg2cmd/q35-linux-hostpci.conf.cmd b/test/cfg2cmd/q35-linux-hostpci.conf.cmd
index 7e829ae..14112ca 100644
--- a/test/cfg2cmd/q35-linux-hostpci.conf.cmd
+++ b/test/cfg2cmd/q35-linux-hostpci.conf.cmd
@@ -1,6 +1,7 @@
/usr/bin/kvm \
-id 8006 \
-name vm8006 \
+ -no-shutdown \
-chardev 'socket,id=qmp,path=/var/run/qemu-server/8006.qmp,server,nowait' \
-mon 'chardev=qmp,mode=control' \
-chardev 'socket,id=qmp-event,path=/var/run/qmeventd.sock,reconnect=5' \
diff --git a/test/cfg2cmd/q35-win10-hostpci.conf.cmd b/test/cfg2cmd/q35-win10-hostpci.conf.cmd
index 133c086..4572348 100644
--- a/test/cfg2cmd/q35-win10-hostpci.conf.cmd
+++ b/test/cfg2cmd/q35-win10-hostpci.conf.cmd
@@ -1,6 +1,7 @@
/usr/bin/kvm \
-id 8006 \
-name vm8006 \
+ -no-shutdown \
-chardev 'socket,id=qmp,path=/var/run/qemu-server/8006.qmp,server,nowait' \
-mon 'chardev=qmp,mode=control' \
-chardev 'socket,id=qmp-event,path=/var/run/qmeventd.sock,reconnect=5' \
diff --git a/test/cfg2cmd/simple-virtio-blk.conf.cmd b/test/cfg2cmd/simple-virtio-blk.conf.cmd
index 6933edf..4b94d65 100644
--- a/test/cfg2cmd/simple-virtio-blk.conf.cmd
+++ b/test/cfg2cmd/simple-virtio-blk.conf.cmd
@@ -1,6 +1,7 @@
/usr/bin/kvm \
-id 8006 \
-name simple \
+ -no-shutdown \
-chardev 'socket,id=qmp,path=/var/run/qemu-server/8006.qmp,server,nowait' \
-mon 'chardev=qmp,mode=control' \
-chardev 'socket,id=qmp-event,path=/var/run/qmeventd.sock,reconnect=5' \
diff --git a/test/cfg2cmd/simple1.conf.cmd b/test/cfg2cmd/simple1.conf.cmd
index 3485064..87a70cc 100644
--- a/test/cfg2cmd/simple1.conf.cmd
+++ b/test/cfg2cmd/simple1.conf.cmd
@@ -1,6 +1,7 @@
/usr/bin/kvm \
-id 8006 \
-name simple \
+ -no-shutdown \
-chardev 'socket,id=qmp,path=/var/run/qemu-server/8006.qmp,server,nowait' \
-mon 'chardev=qmp,mode=control' \
-chardev 'socket,id=qmp-event,path=/var/run/qmeventd.sock,reconnect=5' \
diff --git a/test/cfg2cmd/spice-enhancments.conf.cmd b/test/cfg2cmd/spice-enhancments.conf.cmd
index 3951c06..838a65a 100644
--- a/test/cfg2cmd/spice-enhancments.conf.cmd
+++ b/test/cfg2cmd/spice-enhancments.conf.cmd
@@ -1,6 +1,7 @@
/usr/bin/kvm \
-id 8006 \
-name vm8006 \
+ -no-shutdown \
-chardev 'socket,id=qmp,path=/var/run/qemu-server/8006.qmp,server,nowait' \
-mon 'chardev=qmp,mode=control' \
-chardev 'socket,id=qmp-event,path=/var/run/qmeventd.sock,reconnect=5' \
diff --git a/test/cfg2cmd/spice-linux-4.1.conf.cmd b/test/cfg2cmd/spice-linux-4.1.conf.cmd
index 2748cc9..1d308c0 100644
--- a/test/cfg2cmd/spice-linux-4.1.conf.cmd
+++ b/test/cfg2cmd/spice-linux-4.1.conf.cmd
@@ -1,6 +1,7 @@
/usr/bin/kvm \
-id 8006 \
-name spicelinux \
+ -no-shutdown \
-chardev 'socket,id=qmp,path=/var/run/qemu-server/8006.qmp,server,nowait' \
-mon 'chardev=qmp,mode=control' \
-chardev 'socket,id=qmp-event,path=/var/run/qmeventd.sock,reconnect=5' \
diff --git a/test/cfg2cmd/spice-usb3.conf.cmd b/test/cfg2cmd/spice-usb3.conf.cmd
index c515644..578e2e5 100644
--- a/test/cfg2cmd/spice-usb3.conf.cmd
+++ b/test/cfg2cmd/spice-usb3.conf.cmd
@@ -1,6 +1,7 @@
/usr/bin/kvm \
-id 8006 \
-name spiceusb3 \
+ -no-shutdown \
-chardev 'socket,id=qmp,path=/var/run/qemu-server/8006.qmp,server,nowait' \
-mon 'chardev=qmp,mode=control' \
-chardev 'socket,id=qmp-event,path=/var/run/qmeventd.sock,reconnect=5' \
diff --git a/test/cfg2cmd/spice-win.conf.cmd b/test/cfg2cmd/spice-win.conf.cmd
index 22dfa9d..78e3c5e 100644
--- a/test/cfg2cmd/spice-win.conf.cmd
+++ b/test/cfg2cmd/spice-win.conf.cmd
@@ -1,6 +1,7 @@
/usr/bin/kvm \
-id 8006 \
-name spiceusb3 \
+ -no-shutdown \
-chardev 'socket,id=qmp,path=/var/run/qemu-server/8006.qmp,server,nowait' \
-mon 'chardev=qmp,mode=control' \
-chardev 'socket,id=qmp-event,path=/var/run/qmeventd.sock,reconnect=5' \
--
2.20.1
^ permalink raw reply [flat|nested] 9+ messages in thread* [pve-devel] [PATCH v2 qemu-server 6/7] fix vm_resume and allow vm_start with QMP status 'shutdown'
2020-10-19 12:18 [pve-devel] [PATCH v2 0/7] Handle guest shutdown during backups Stefan Reiter
` (4 preceding siblings ...)
2020-10-19 12:18 ` [pve-devel] [PATCH v2 qemu-server 5/7] config_to_command: use -no-shutdown option Stefan Reiter
@ 2020-10-19 12:18 ` Stefan Reiter
2020-10-19 12:18 ` [pve-devel] [PATCH v2 manager 7/7] ui: qemu: set correct disabled state for start button Stefan Reiter
2020-11-05 12:35 ` [pve-devel] applied-series: [PATCH v2 0/7] Handle guest shutdown during backups Thomas Lamprecht
7 siblings, 0 replies; 9+ messages in thread
From: Stefan Reiter @ 2020-10-19 12:18 UTC (permalink / raw)
To: pve-devel; +Cc: d.csapak, w.bumiller
When the VM is in status 'shutdown', i.e. after the guest issues a
powerdown while a backup is running, QEMU requires a 'system_reset' to
be issued before 'cont' can boot the guest again.
Additionally, when the VM has been powered down during a backup, the
logically correct call would be a 'vm_start', so automatically vm_resume
from vm_start in case this situation occurs. This also means the GUI can
cope with this almost unchanged.
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---
PVE/QemuServer.pm | 23 ++++++++++++++++++++---
1 file changed, 20 insertions(+), 3 deletions(-)
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index c712671..22484ca 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -4860,13 +4860,22 @@ sub vm_start {
if !$params->{skiptemplate} && PVE::QemuConfig->is_template($conf);
my $has_suspended_lock = PVE::QemuConfig->has_lock($conf, 'suspended');
+ my $has_backup_lock = PVE::QemuConfig->has_lock($conf, 'backup');
+
+ my $running = check_running($vmid, undef, $migrate_opts->{migratedfrom});
+
+ if ($has_backup_lock && $running) {
+ # a backup is currently running, attempt to start the guest in the
+ # existing QEMU instance
+ return vm_resume($vmid);
+ }
PVE::QemuConfig->check_lock($conf)
if !($params->{skiplock} || $has_suspended_lock);
$params->{resume} = $has_suspended_lock || defined($conf->{vmstate});
- die "VM $vmid already running\n" if check_running($vmid, undef, $migrate_opts->{migratedfrom});
+ die "VM $vmid already running\n" if $running;
if (my $storagemap = $migrate_opts->{storagemap}) {
my $replicated = $migrate_opts->{replicated_volumes};
@@ -5546,9 +5555,12 @@ sub vm_resume {
PVE::QemuConfig->lock_config($vmid, sub {
my $res = mon_cmd($vmid, 'query-status');
my $resume_cmd = 'cont';
+ my $reset = 0;
- if ($res->{status} && $res->{status} eq 'suspended') {
- $resume_cmd = 'system_wakeup';
+ if ($res->{status}) {
+ return if $res->{status} eq 'running'; # job done, go home
+ $resume_cmd = 'system_wakeup' if $res->{status} eq 'suspended';
+ $reset = 1 if $res->{status} eq 'shutdown';
}
if (!$nocheck) {
@@ -5559,6 +5571,11 @@ sub vm_resume {
if !($skiplock || PVE::QemuConfig->has_lock($conf, 'backup'));
}
+ if ($reset) {
+ # required if a VM shuts down during a backup and we get a resume
+ # request before the backup finishes for example
+ mon_cmd($vmid, "system_reset");
+ }
mon_cmd($vmid, $resume_cmd);
});
}
--
2.20.1
^ permalink raw reply [flat|nested] 9+ messages in thread* [pve-devel] [PATCH v2 manager 7/7] ui: qemu: set correct disabled state for start button
2020-10-19 12:18 [pve-devel] [PATCH v2 0/7] Handle guest shutdown during backups Stefan Reiter
` (5 preceding siblings ...)
2020-10-19 12:18 ` [pve-devel] [PATCH v2 qemu-server 6/7] fix vm_resume and allow vm_start with QMP status 'shutdown' Stefan Reiter
@ 2020-10-19 12:18 ` Stefan Reiter
2020-11-05 12:35 ` [pve-devel] applied-series: [PATCH v2 0/7] Handle guest shutdown during backups Thomas Lamprecht
7 siblings, 0 replies; 9+ messages in thread
From: Stefan Reiter @ 2020-10-19 12:18 UTC (permalink / raw)
To: pve-devel; +Cc: d.csapak, w.bumiller
If a guest's QEMU process is 'running', but QMP says 'shutdown' or
'prelaunch', the VM is ready to be booted anew, so we can show the
button.
The 'shutdown' button is intentionally not touched, as we always want to
give the user the ability to 'stop' a VM (and thus kill any potentially
leftover processes).
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---
www/manager6/qemu/Config.js | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/www/manager6/qemu/Config.js b/www/manager6/qemu/Config.js
index a13bf0c5..346c71a7 100644
--- a/www/manager6/qemu/Config.js
+++ b/www/manager6/qemu/Config.js
@@ -411,7 +411,10 @@ Ext.define('PVE.qemu.Config', {
statusTxt.update({ lock: lock });
- startBtn.setDisabled(!caps.vms['VM.PowerMgmt'] || status === 'running' || template);
+ let guest_running = status === 'running' &&
+ !(qmpstatus === "shutdown" || qmpstatus === "prelaunch");
+ startBtn.setDisabled(!caps.vms['VM.PowerMgmt'] || template || guest_running);
+
shutdownBtn.setDisabled(!caps.vms['VM.PowerMgmt'] || status !== 'running');
me.down('#removeBtn').setDisabled(!caps.vms['VM.Allocate'] || status !== 'stopped');
consoleBtn.setDisabled(template);
--
2.20.1
^ permalink raw reply [flat|nested] 9+ messages in thread* [pve-devel] applied-series: [PATCH v2 0/7] Handle guest shutdown during backups
2020-10-19 12:18 [pve-devel] [PATCH v2 0/7] Handle guest shutdown during backups Stefan Reiter
` (6 preceding siblings ...)
2020-10-19 12:18 ` [pve-devel] [PATCH v2 manager 7/7] ui: qemu: set correct disabled state for start button Stefan Reiter
@ 2020-11-05 12:35 ` Thomas Lamprecht
7 siblings, 0 replies; 9+ messages in thread
From: Thomas Lamprecht @ 2020-11-05 12:35 UTC (permalink / raw)
To: Proxmox VE development discussion, Stefan Reiter; +Cc: w.bumiller
On 19.10.20 14:18, Stefan Reiter wrote:
> Use QEMU's -no-shutdown argument so the QEMU instance stays alive even if the
> guest shuts down. This allows running backups to continue.
>
> To handle cleanup of QEMU processes, this series extends the qmeventd to handle
> SHUTDOWN events not just for detecting guest triggered shutdowns, but also to
> clean the QEMU process via SIGTERM (which quits it even with -no-shutdown
> enabled).
>
> A VZDump instance can then signal qmeventd (via the /var/run/qmeventd.sock) to
> keep alive certain VM processes if they're backing up, and once the backup is
> done, they close their connection to the socket, and qmeventd knows that it can
> now safely kill the VM (as long as the guest hasn't booted again, which is
> possible with some changes to the vm_start code also done in this series).
>
> This series requires a lot of testing, since there can be quite a few edge cases
> lounging around. So far it's been doing well for me, aside from the VNC GUI
> looking a bit confused when you do the 'shutdown during backup' motion (i.e. the
> last image from the framebuffer stays in the VNC window, looks more like the
> guest has crashed than shut down) - but I haven't found a solution for that.
>
>
> v2:
> * use a pidfd (see `man pidfd_open`, though the manpage does not seem to be
> available on Debian atm - I suppose since they don't support kernel 5.3 yet?),
> fall back to regular racy kill() included, for people running older kernels
> * initialize client->type with CLIENT_NONE instead of client->state
> * rebase on latest master
>
>
> qemu-server: Stefan Reiter (6):
> qmeventd: add handling for -no-shutdown QEMU instances
> qmeventd: add last-ditch effort SIGKILL cleanup
> vzdump: connect to qmeventd for duration of backup
> vzdump: use dirty bitmap for not running VMs too
> config_to_command: use -no-shutdown option
> fix vm_resume and allow vm_start with QMP status 'shutdown'
applied series, thanks!
As talked off-list, may make sense to also disable the shutdown button for running backups,
as the user cannot use that to stop the backup, but rather should use the "stop" button
of the backup worker task, if they want to do that.
^ permalink raw reply [flat|nested] 9+ messages in thread