[pve-devel] [PATCH qemu-server v6 3/3] use qmeventd to execute qm cleanup

[Dominik Csapak]

we reverse the direction of the event socket (this does not
prevent live migration) and point it to wher qmeventd listens

Signed-off-by: Dominik Csapak <***@proxmox.com>
---
changes from v5:
* fixed path to socket

PVE/QemuServer.pm | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 9f5d6f3..035d8b7 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -3434,8 +3434,7 @@ sub config_to_command {
push @$cmd, '-mon', "chardev=qmp,mode=control";

if (qemu_machine_feature_enabled($machine_type, $kvmver, 2, 12)) {
- my $eventsocket = qmp_socket($vmid, 0, 'event');
- push @$cmd, '-chardev', "socket,id=qmp-event,path=$eventsocket,server,nowait";
+ push @$cmd, '-chardev', "socket,id=qmp-event,path=/var/run/qmeventd.sock,reconnect=5";
push @$cmd, '-mon', "chardev=qmp-event,mode=control";
}

[Dominik Csapak]

this is intended to be used with qmeventd, to do
the necessary cleanups when qemu crashes or is being
shut down from within the guest

this can also be the point where we could introduce
shutdown/stop/reboot hooks

Signed-off-by: Dominik Csapak <***@proxmox.com>
---
PVE/CLI/qm.pm | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 62 insertions(+)

diff --git a/PVE/CLI/qm.pm b/PVE/CLI/qm.pm
index 46a7e2f..eceb9b3 100755
--- a/PVE/CLI/qm.pm
+++ b/PVE/CLI/qm.pm
@@ -18,6 +18,7 @@ use PVE::SafeSyslog;
use PVE::INotify;
use PVE::RPCEnvironment;
use PVE::Exception qw(raise_param_exc);
+use PVE::Network;
use PVE::QemuServer;
use PVE::QemuServer::ImportDisk;
use PVE::QemuServer::OVF;
@@ -722,6 +723,65 @@ __PACKAGE__->register_method({
return { result => $res };
}});

+__PACKAGE__->register_method({
+ name => 'cleanup',
+ path => 'cleanup',
+ method => 'POST',
+ protected => 1,
+ description => "Cleans up resources like tap devices, vgpus, etc. Called after a vm shuts down, crashes, etc.",
+ parameters => {
+ additionalProperties => 0,
+ properties => {
+ node => get_standard_option('pve-node'),
+ vmid => get_standard_option('pve-vmid', {
+ completion => \&PVE::QemuServer::complete_vmid_running }),
+ 'clean-shutdown' => {
+ type => 'boolean',
+ description => "Indicates if qemu shutdown cleanly.",
+ },
+ 'guest-requested' => {
+ type => 'boolean',
+ description => "Indicates if the shutdown was requested by the guest or via qmp.",
+ },
+ },
+ },
+ returns => { type => 'null', },
+ code => sub {
+ my ($param) = @_;
+
+ my $vmid = $param->{vmid};
+ my $clean = $param->{'clean-shutdown'};
+ my $guest = $param->{'guest-requested'};
+
+ my $storecfg = PVE::Storage::config();
+ warn "Starting cleanup for $vmid\n";
+
+ PVE::QemuConfig->lock_config($vmid, sub {
+ my $conf = PVE::QemuConfig->load_config ($vmid);
+ my $pid = PVE::QemuServer::check_running ($vmid);
+ die "vm still running\n" if $pid;
+
+ if (!$clean) {
+ # we have to cleanup the tap devices after a crash
+
+ foreach my $opt (keys %$conf) {
+ next if $opt !~ m/^net(\d)+$/;
+ my $interface = $1;
+ PVE::Network::tap_unplug("tap${vmid}i${interface}");
+ }
+ }
+
+ if (!$clean || $guest) {
+ # vm was shutdown from inside the guest or crashed, doing api cleanup
+ PVE::QemuServer::vm_stop_cleanup($storecfg, $vmid, $conf, 0, 0);
+ }
+ });
+
+ warn "Finished cleanup for $vmid\n";
+
+ return undef;
+ }});
+
my $print_agent_result = sub {
my ($data) = @_;

@@ -904,6 +964,8 @@ our $cmddef = {

importovf => [ __PACKAGE__, 'importovf', ['vmid', 'manifest', 'storage']],

+ cleanup => [ __PACKAGE__, 'cleanup', ['vmid', 'clean-shutdown', 'guest-requested'], { node => $nodename }],
+
};

1;

[Dominik Csapak]

This post might be inappropriate. Click to display it.

[Thomas Lamprecht]

Post by Dominik Csapak
this adds a program that can listen to qemu qmp events on a given socket
and if a shutdown event followed by a disconnected socket occurs,
executes qm cleanup with arguments that indicate if the
vm was closed gracefully and whether the guest initiated it
this is useful if we want to cleanup after the qemu process exited,
e.g. tap devices, vgpus, etc.
---
* fix typo
* fix function signature
* change exit(0) to exit(EXIT_SUCCESS)
Makefile | 19 ++-
debian/control | 2 +
debian/rules | 2 +-
qmeventd.c | 429 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
qmeventd.h | 55 +++++++
qmeventd.service | 10 ++
6 files changed, 513 insertions(+), 4 deletions(-)
create mode 100644 qmeventd.c
create mode 100644 qmeventd.h
create mode 100644 qmeventd.service
diff --git a/debian/control b/debian/control
index 912d7a9..15fb74a 100644
--- a/debian/control
+++ b/debian/control
@@ -3,7 +3,9 @@ Section: admin
Priority: optional
Build-Depends: debhelper (>= 7.0.50~),
+ docutils,

^^^^^^^^

I guess this is left over from earlier iterations where you had rst docs directly
here not in pve-docs? If so, please remove again from the dependency list...

Post by Dominik Csapak
libio-multiplex-perl,
+ libjson-c-dev,
libpve-common-perl,
libpve-guest-common-perl (>= 2.0-18),
libpve-storage-perl,

[Wolfgang Bumiller]

Post by Thomas Lamprecht

Post by Dominik Csapak
this adds a program that can listen to qemu qmp events on a given socket
and if a shutdown event followed by a disconnected socket occurs,
executes qm cleanup with arguments that indicate if the
vm was closed gracefully and whether the guest initiated it
this is useful if we want to cleanup after the qemu process exited,
e.g. tap devices, vgpus, etc.
---
* fix typo
* fix function signature
* change exit(0) to exit(EXIT_SUCCESS)
Makefile | 19 ++-
debian/control | 2 +
debian/rules | 2 +-
qmeventd.c | 429 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
qmeventd.h | 55 +++++++
qmeventd.service | 10 ++
6 files changed, 513 insertions(+), 4 deletions(-)
create mode 100644 qmeventd.c
create mode 100644 qmeventd.h
create mode 100644 qmeventd.service
diff --git a/debian/control b/debian/control
index 912d7a9..15fb74a 100644
--- a/debian/control
+++ b/debian/control
@@ -3,7 +3,9 @@ Section: admin
Priority: optional
Build-Depends: debhelper (>= 7.0.50~),
+ docutils,

^^^^^^^^
I guess this is left over from earlier iterations where you had rst docs directly
here not in pve-docs? If so, please remove again from the dependency list...

done

[Wolfgang Bumiller]

applied series

this series adds qmeventd, a binary which listens on a socket
and waits for qemu to connect to it, and thenfor the shutdown event of qemu
i use this to execute 'qm cleanup' when a vm exits, and can detect
if a qemu crashed/was stopped from within/etc.
like i discussed this with wolfgang off-list, i send it without the
docs patch
* fixed typo
* fixed function signature formatting
* changed exit(0) to exit(EXIT_SUCCESS)
* fixed the path to qmeventd socket in config_to_command
* incorporated feedback from wolfgang
- initialize with NULL
- use < len
- use stdbool
- better error handling for pid
- exit code 0 with -h
- dependecy
- whitespace
- put args on the stack
* sorted headers
* moved service socket to /var/run/qmeventd.sock and have
that directory as RequiresMountsFor=/var/run
(that fixed the issue that qmeventd could not start after a reboot because
/var/run/qemu-server was not created yet)
* added some logging to qm cleanup
* added missing build dependency
* fixed string handling in get_vmid_from_pid
* use strtoul instead of custom code
* changed multiple fatal errors to simply logging
* added SIGCLD handler
* changed manpage from rst to adoc and moved to pve-docs
* changed fds to have CLOEXEC
* changed from accept to accept4
* whitespace fixes
* changed error handling from macro to inline function
* put buf/vmid limits in struct and use sizeof instead of #define
* completely reversed the logic: instead of having one binary
per vm, we now have one proper daemon which listens and waits
for qemu to connect to it, this way the race is on the qemu side
(if it crashes/gets killed before connecting to the socket) instead
of with us (connecting to a non existing qemu process)
* dropped the --no-reboot patch/logic, as my patch upstream was not accepted,
but i will send another version of it there soon
* added a manpage, option parsing, etc.
* 1/5 is new and contains changes that we want for qemu 2.12
* incorporated feedback from w.bumiller
* fixed the -no-reboot check
add qmeventd
add 'qm cleanup'
use qmeventd to execute qm cleanup
Makefile | 19 ++-
PVE/CLI/qm.pm | 62 ++++++++
PVE/QemuServer.pm | 3 +-
debian/control | 2 +
debian/rules | 2 +-
qmeventd.c | 429 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
qmeventd.h | 55 +++++++
qmeventd.service | 10 ++
8 files changed, 576 insertions(+), 6 deletions(-)
create mode 100644 qmeventd.c
create mode 100644 qmeventd.h
create mode 100644 qmeventd.service
--
2.11.0