* [pve-devel] [PATCH qemu-server 1/7] qmeventd: add handling for -no-shutdown QEMU instances
2020-09-03 8:58 [pve-devel] [PATCH 0/7] Handle guest shutdown during backups Stefan Reiter
@ 2020-09-03 8:58 ` Stefan Reiter
2020-09-03 8:58 ` [pve-devel] [PATCH qemu-server 2/7] qmeventd: add last-ditch effort SIGKILL cleanup Stefan Reiter
` (7 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Stefan Reiter @ 2020-09-03 8:58 UTC (permalink / raw)
To: pve-devel
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>
---
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 393badc..35c77da 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: http://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..5616ae4 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->state = 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] 10+ messages in thread* [pve-devel] [PATCH qemu-server 2/7] qmeventd: add last-ditch effort SIGKILL cleanup
2020-09-03 8:58 [pve-devel] [PATCH 0/7] Handle guest shutdown during backups Stefan Reiter
2020-09-03 8:58 ` [pve-devel] [PATCH qemu-server 1/7] qmeventd: add handling for -no-shutdown QEMU instances Stefan Reiter
@ 2020-09-03 8:58 ` Stefan Reiter
2020-09-03 8:58 ` [pve-devel] [PATCH qemu-server 3/7] vzdump: connect to qmeventd for duration of backup Stefan Reiter
` (6 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Stefan Reiter @ 2020-09-03 8:58 UTC (permalink / raw)
To: pve-devel
'alarm' is used to schedule an additionaly cleanup round 5 seconds after
sending SIGTERM via terminate_client. This then calls SIGKILL, 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>
---
Unsure about this one, it's technically not necessary, but might help avoid
situations where a QEMU process is left over.
Also, is PID reuse an issue? Suppose QEMU quits correctly on SIGTERM, another
process starts up with the now free PID, and 5 seconds after we SIGKILL it. Not
sure how this could be solved though...
qmeventd/qmeventd.c | 54 ++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 53 insertions(+), 1 deletion(-)
diff --git a/qmeventd/qmeventd.c b/qmeventd/qmeventd.c
index 5616ae4..ed657b8 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
@@ -470,6 +472,12 @@ terminate_client(struct Client *client)
int err = kill(client->pid, SIGTERM);
log_neg(err, "kill");
+
+ // resets any other alarms, but will fire eventually and cleanup all
+ pid_t *pid_ptr = malloc(sizeof(pid_t));
+ *pid_ptr = client->pid;
+ forced_cleanups = g_slist_prepend(forced_cleanups, (void *)pid_ptr);
+ alarm(5);
}
void
@@ -545,6 +553,47 @@ 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 *pid_ptr, __attribute__((unused)) void *unused)
+{
+ pid_t pid = *((pid_t *)pid_ptr);
+ int err = kill(pid, SIGKILL);
+ if (err < 0) {
+ if (errno != ESRCH) {
+ fprintf(stderr, "SIGKILL cleanup of pid '%d' failed - %s\n",
+ pid, strerror(errno));
+ }
+ } else {
+ fprintf(stderr, "cleanup failed, terminating pid '%d' with SIGKILL\n", 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 +626,7 @@ main(int argc, char *argv[])
}
signal(SIGCHLD, SIG_IGN);
+ signal(SIGALRM, alarm_handler);
socket_path = argv[optind];
@@ -612,7 +662,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 +680,7 @@ main(int argc, char *argv[])
handle_client((struct Client *)events[n].data.ptr);
}
}
+
+ handle_forced_cleanup();
}
}
--
2.20.1
^ permalink raw reply [flat|nested] 10+ messages in thread* [pve-devel] [PATCH qemu-server 3/7] vzdump: connect to qmeventd for duration of backup
2020-09-03 8:58 [pve-devel] [PATCH 0/7] Handle guest shutdown during backups Stefan Reiter
2020-09-03 8:58 ` [pve-devel] [PATCH qemu-server 1/7] qmeventd: add handling for -no-shutdown QEMU instances Stefan Reiter
2020-09-03 8:58 ` [pve-devel] [PATCH qemu-server 2/7] qmeventd: add last-ditch effort SIGKILL cleanup Stefan Reiter
@ 2020-09-03 8:58 ` Stefan Reiter
2020-09-03 8:58 ` [pve-devel] [PATCH qemu-server 4/7] vzdump: use dirty bitmap for not running VMs too Stefan Reiter
` (5 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Stefan Reiter @ 2020-09-03 8:58 UTC (permalink / raw)
To: pve-devel
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 7297795..196739d 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;
@@ -507,6 +508,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 {
@@ -675,6 +677,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;
@@ -833,6 +836,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) = @_;
@@ -886,7 +917,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] 10+ messages in thread* [pve-devel] [PATCH qemu-server 4/7] vzdump: use dirty bitmap for not running VMs too
2020-09-03 8:58 [pve-devel] [PATCH 0/7] Handle guest shutdown during backups Stefan Reiter
` (2 preceding siblings ...)
2020-09-03 8:58 ` [pve-devel] [PATCH qemu-server 3/7] vzdump: connect to qmeventd for duration of backup Stefan Reiter
@ 2020-09-03 8:58 ` Stefan Reiter
2020-09-03 8:58 ` [pve-devel] [PATCH qemu-server 5/7] config_to_command: use -no-shutdown option Stefan Reiter
` (4 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Stefan Reiter @ 2020-09-03 8:58 UTC (permalink / raw)
To: pve-devel
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 196739d..074a5f4 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") {
@@ -546,7 +545,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] 10+ messages in thread* [pve-devel] [PATCH qemu-server 5/7] config_to_command: use -no-shutdown option
2020-09-03 8:58 [pve-devel] [PATCH 0/7] Handle guest shutdown during backups Stefan Reiter
` (3 preceding siblings ...)
2020-09-03 8:58 ` [pve-devel] [PATCH qemu-server 4/7] vzdump: use dirty bitmap for not running VMs too Stefan Reiter
@ 2020-09-03 8:58 ` Stefan Reiter
2020-09-03 8:58 ` [pve-devel] [PATCH qemu-server 6/7] fix vm_resume and allow vm_start with QMP status 'shutdown' Stefan Reiter
` (3 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Stefan Reiter @ 2020-09-03 8:58 UTC (permalink / raw)
To: pve-devel
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>
---
PVE/QemuServer.pm | 2 ++
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 +
19 files changed, 20 insertions(+)
diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 2747c66..a5ff16f 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -3027,6 +3027,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/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] 10+ messages in thread* [pve-devel] [PATCH qemu-server 6/7] fix vm_resume and allow vm_start with QMP status 'shutdown'
2020-09-03 8:58 [pve-devel] [PATCH 0/7] Handle guest shutdown during backups Stefan Reiter
` (4 preceding siblings ...)
2020-09-03 8:58 ` [pve-devel] [PATCH qemu-server 5/7] config_to_command: use -no-shutdown option Stefan Reiter
@ 2020-09-03 8:58 ` Stefan Reiter
2020-09-03 8:58 ` [pve-devel] [PATCH manager 7/7] ui: qemu: set correct disabled state for start button Stefan Reiter
` (2 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Stefan Reiter @ 2020-09-03 8:58 UTC (permalink / raw)
To: pve-devel
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 a5ff16f..982d0a4 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -4850,13 +4850,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};
@@ -5536,9 +5545,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) {
@@ -5549,6 +5561,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] 10+ messages in thread* [pve-devel] [PATCH manager 7/7] ui: qemu: set correct disabled state for start button
2020-09-03 8:58 [pve-devel] [PATCH 0/7] Handle guest shutdown during backups Stefan Reiter
` (5 preceding siblings ...)
2020-09-03 8:58 ` [pve-devel] [PATCH qemu-server 6/7] fix vm_resume and allow vm_start with QMP status 'shutdown' Stefan Reiter
@ 2020-09-03 8:58 ` Stefan Reiter
2020-09-07 9:36 ` [pve-devel] [PATCH 0/7] Handle guest shutdown during backups Thomas Lamprecht
2020-09-18 14:20 ` Dominik Csapak
8 siblings, 0 replies; 10+ messages in thread
From: Stefan Reiter @ 2020-09-03 8:58 UTC (permalink / raw)
To: pve-devel
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] 10+ messages in thread* Re: [pve-devel] [PATCH 0/7] Handle guest shutdown during backups
2020-09-03 8:58 [pve-devel] [PATCH 0/7] Handle guest shutdown during backups Stefan Reiter
` (6 preceding siblings ...)
2020-09-03 8:58 ` [pve-devel] [PATCH manager 7/7] ui: qemu: set correct disabled state for start button Stefan Reiter
@ 2020-09-07 9:36 ` Thomas Lamprecht
2020-09-18 14:20 ` Dominik Csapak
8 siblings, 0 replies; 10+ messages in thread
From: Thomas Lamprecht @ 2020-09-07 9:36 UTC (permalink / raw)
To: Proxmox VE development discussion, Dominik Csapak
On 03.09.20 10:58, 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.
>
>
@Dominik, I'd like a review from you on this series, no pressure though. :)
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [pve-devel] [PATCH 0/7] Handle guest shutdown during backups
2020-09-03 8:58 [pve-devel] [PATCH 0/7] Handle guest shutdown during backups Stefan Reiter
` (7 preceding siblings ...)
2020-09-07 9:36 ` [pve-devel] [PATCH 0/7] Handle guest shutdown during backups Thomas Lamprecht
@ 2020-09-18 14:20 ` Dominik Csapak
8 siblings, 0 replies; 10+ messages in thread
From: Dominik Csapak @ 2020-09-18 14:20 UTC (permalink / raw)
To: pve-devel
overall the series looks ok to me (and tested ok), though a few points:
* i'd really like for someone else to look over this too
maybe someone who is really good with c ( wolfgang, when you're back
from holidays? ;) )
* regarding the killing, i'd like better what we already discussed off-list:
maybe using either the 'quit' command via qmp, or using a pidfd
to avoid races
* at this point, i think a rewrite in rust could be good,
before we tack even more features onto this?
(we have all that we need to handle this, mio,serde_json, etc.)
On 9/3/20 10:58 AM, 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.
>
>
> 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'
>
> PVE/QemuServer.pm | 25 +-
> PVE/VZDump/QemuServer.pm | 40 ++-
> debian/control | 1 +
> qmeventd/Makefile | 4 +-
> qmeventd/qmeventd.c | 331 ++++++++++++++++--
> qmeventd/qmeventd.h | 41 ++-
> .../custom-cpu-model-defaults.conf.cmd | 1 +
> .../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 +
> .../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 +
> 24 files changed, 410 insertions(+), 50 deletions(-)
>
> manager: Stefan Reiter (1):
> ui: qemu: set correct disabled state for start button
>
> www/manager6/qemu/Config.js | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
^ permalink raw reply [flat|nested] 10+ messages in thread