[pve-devel] [PATCH-SERIES common/qemu-server v3 0/2] migration: conntrack: fix race adding dbus-vmstate object to QEMU

[pve-devel] [PATCH-SERIES common/qemu-server v3 0/2] migration: conntrack: fix race adding dbus-vmstate object to QEMU

[Fiona Ebner]

Changes in v3 (thanks to Thomas!):
* Expand commit message for sd_notify() helper.
* Use $socket->{send,shutdown) methods.
* Print $IO::Socket::errstr in case of error.
* Unset NOTIFY_SOCKET environment variable only after sending the
  message.

Changes in v2:
* Dropped already applied patches.
* Introduce sd_notify() helper.
* Different approach, make the service type=notify instead of waiting
  in a sleep+check-loop until the object shows up via QMP 'qom-list'.

As reported in the community forum [0], it might happen that the
dbus-vmstate object is not added (quickly enough) to the target QEMU
instance, before the migration state is loaded. This would result in
a crash of the target instance.

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


Dependency bump qemu-server -> pve-common needed.


pve-common:

Fiona Ebner (1):
  systemd: add sd_notify() helper

 src/PVE/Systemd.pm | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)


qemu-server:

Fiona Ebner (1):
  migration: conntrack: avoid crash when dbus-vmstate object cannot be
    added (quickly enough)

 src/usr/dbus-vmstate              | 3 +++
 src/usr/pve-dbus-vmstate@.service | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)


Summary over all repositories:
  3 files changed, 35 insertions(+), 1 deletions(-)

-- 
Generated by git-murpp 0.5.0


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

[pve-devel] [PATCH common v3 1/2] systemd: add sd_notify() helper

[Fiona Ebner]

Implement a pure Perl reimplementation of systemd's sd_notify() as
defined in systemd/sd-daemon.h, see also 'man 3 sd_notify'.

The initial user of this helper is intended to be the pve-dbus-vmstate
service, so it can notify startup completion only once the
dbus-vmstate QEMU object is ready to be used.

EAGAIN is not checked for, because it does not occur for blocking
Unix domain sockets, see 'man 2 send'.

Co-developed-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

Changes in v3:
* Expand commit message.
* Use $socket->{send,shutdown) methods.
* Print $IO::Socket::errstr in case of error.
* Unset NOTIFY_SOCKET environment variable only after sending the
  message.

 src/PVE/Systemd.pm | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/src/PVE/Systemd.pm b/src/PVE/Systemd.pm
index e6d6f88..d0a291d 100644
--- a/src/PVE/Systemd.pm
+++ b/src/PVE/Systemd.pm
@@ -3,9 +3,12 @@ package PVE::Systemd;
 use strict;
 use warnings;
 
+use IO::Socket::UNIX;
 use Net::DBus qw(dbus_uint32 dbus_uint64 dbus_boolean);
 use Net::DBus::Callback;
 use Net::DBus::Reactor;
+use POSIX qw(EINTR);
+use Socket qw(SOCK_DGRAM);
 
 use PVE::Tools qw(file_set_contents file_get_contents trim);
 
@@ -282,4 +285,32 @@ sub write_ini {
     file_set_contents($filename, $content);
 }
 
+# This is a pure Perl reimplementation of systemd's sd_notify() as defined in systemd/sd-daemon.h
+sub sd_notify {
+    my ($unset_environment, $state) = @_;
+
+    my $socket_path = $ENV{NOTIFY_SOCKET};
+
+    my $socket = IO::Socket::UNIX->new(
+        Type => SOCK_DGRAM(),
+        Peer => $socket_path,
+    ) or die "unable to connect to socket $socket_path to notify systemd - $IO::Socket::errstr\n";
+
+    # we won't be reading from the socket
+    $socket->shutdown(SHUT_RD);
+
+    my $sent = 0;
+    my $total = length($state);
+    while ($sent < $total) {
+        my $res = $socket->send($state);
+        die "sending to $socket_path failed - $!" if !$res && $! != EINTR;
+        $sent += $res if $res;
+    }
+    $socket->flush();
+
+    close($socket);
+
+    delete($ENV{NOTIFY_SOCKET}) if $unset_environment;
+}
+
 1;
-- 
2.47.3



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

[pve-devel] [PATCH qemu-server v3 2/2] migration: conntrack: avoid crash when dbus-vmstate object cannot be added (quickly enough)

[Fiona Ebner]

As reported in the community forum [0], it might happen that the
dbus-vmstate object is not added (quickly enough) to the target QEMU
instance, before the migration state is loaded. This would result in
a crash of the target instance:

> kvm: Unknown savevm section or instance 'dbus-vmstate/dbus-vmstate'
> 0. Make sure that your current VM setup matches your saved VM setup,
> including any hotplugged devices
> kvm: load of migration failed: Invalid argument

This is after the configuration is already moved and thus there also
is no source instance running anymore.

Change the type of the 'pve-dbus-vmstate@' service to 'notify', so
that starting the service returns success only after the
'dbus-vmstate' object has been added to the QEMU instance.

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

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

Dependency bump qemu-server -> pve-common needed.

No changes in v3.

 src/usr/dbus-vmstate              | 3 +++
 src/usr/pve-dbus-vmstate@.service | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/usr/dbus-vmstate b/src/usr/dbus-vmstate
index ac6f8cfb..b2baa840 100755
--- a/src/usr/dbus-vmstate
+++ b/src/usr/dbus-vmstate
@@ -15,6 +15,7 @@ use Net::DBus::Reactor;
 use PVE::QemuServer::Helpers;
 use PVE::QemuServer::QMPHelpers qw(qemu_objectadd qemu_objectdel);
 use PVE::SafeSyslog;
+use PVE::Systemd;
 use PVE::Tools;
 
 use base qw(Net::DBus::Object);
@@ -165,4 +166,6 @@ qemu_objectadd($vmid, 'pve-vmstate', 'dbus-vmstate',
     'id-list' => "pve-vmstate-$vmid",
 );
 
+PVE::Systemd::sd_notify(0, "READY=1\n");
+
 Net::DBus::Reactor->main()->run();
diff --git a/src/usr/pve-dbus-vmstate@.service b/src/usr/pve-dbus-vmstate@.service
index 56b4e285..616f6979 100644
--- a/src/usr/pve-dbus-vmstate@.service
+++ b/src/usr/pve-dbus-vmstate@.service
@@ -6,5 +6,5 @@ PartOf=%i.scope
 
 [Service]
 Slice=qemu.slice
-Type=simple
+Type=notify
 ExecStart=/usr/libexec/qemu-server/dbus-vmstate %i
-- 
2.47.3



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