[pve-devel] [PATCH-SERIES common/qemu-server 0/3] fix #7092: systemd: vmid.scope: add orderings to fix dbus-vmstate cleanup (regression)

[pve-devel] [PATCH-SERIES common/qemu-server 0/3] fix #7092: systemd: vmid.scope: add orderings to fix dbus-vmstate cleanup (regression)

[Fiona Ebner]

Fixes bug #7092 as well as a regression [0] in combination with HA
shutdown policy 'migrate' caused by commit 1f2c5146 ("migrate: remove
left-over dbus-vmstate instance when migrating without conntrack
state"). Currently, the HA migrations do not migrate the conntrack
state and before this patch, the dbus.service is stopped too early
during shutdown, before the LRM migrations. This led to failure when
trying to check for a left-over dbus-vmstate instance, because
the Net::DBus->system() call would fail.

It is not enough to record the After=dbus.service dependency, because
systemd would already stop the vmid.scope even while processes in it
are still running. Add a Before=pve-ha-lrm.service and for consistency
Before=pve-guests.service to record the ordering on shutdown. This
ensures that the scope units are still around until the LRM or guests
service had the chance to stop, and usually, handle the remaining
running guests itself.

Also guard more errors in qemu_del_dbus_vmstate().

Many thanks to Maximiliano and Fabian for discussing the issue and
solutions!

[0]: https://forum.proxmox.com/threads/169739/post-825256


Dependency bump qemu-server -> libpve-common-perl needed!


common:

Fiona Ebner (1):
  systemd: enter scope: allow specifying 'Before' and 'After' properties

 src/PVE/Systemd.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


qemu-server:

Fiona Ebner (2):
  systemd: vmid.scope: add orderings to fix dbus-vmstate cleanup
    (regression)
  dbus-vmstate: guard more errors in qemu_del_dbus_vmstate()

 src/PVE/QemuServer.pm             |  4 ++++
 src/PVE/QemuServer/DBusVMState.pm | 15 +++++++++++++--
 2 files changed, 17 insertions(+), 2 deletions(-)


Summary over all repositories:
  3 files changed, 18 insertions(+), 3 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 1/3] systemd: enter scope: allow specifying 'Before' and 'After' properties

[Fiona Ebner]

The value is an array reference with a list of unit names.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/Systemd.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/PVE/Systemd.pm b/src/PVE/Systemd.pm
index 8fc06a5..6ff0dc8 100644
--- a/src/PVE/Systemd.pm
+++ b/src/PVE/Systemd.pm
@@ -113,7 +113,7 @@ sub enter_systemd_scope {
     my $properties = [[PIDs => [dbus_uint32($$)]]];
 
     foreach my $key (keys %extra) {
-        if ($key eq 'Slice' || $key eq 'KillMode') {
+        if ($key eq 'Slice' || $key eq 'KillMode' || $key eq 'After' || $key eq 'Before') {
             push @{$properties}, [$key, $extra{$key}];
         } elsif ($key eq 'SendSIGKILL') {
             push @{$properties}, [$key, dbus_boolean($extra{$key})];
-- 
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 2/3] systemd: vmid.scope: add orderings to fix dbus-vmstate cleanup (regression)

[Fiona Ebner]

Fixes bug #7092 as well as a regression [0] in combination with HA
shutdown policy 'migrate' caused by commit 1f2c5146 ("migrate: remove
left-over dbus-vmstate instance when migrating without conntrack
state"). Currently, the HA migrations do not migrate the conntrack
state and before this patch, the dbus.service is stopped too early
during shutdown, before the LRM migrations. This led to failure when
trying to check for a left-over dbus-vmstate instance, because
the Net::DBus->system() call would fail.

It is not enough to record the After=dbus.service dependency, because
systemd would already stop the vmid.scope even while processes in it
are still running. Add a Before=pve-ha-lrm.service and for consistency
Before=pve-guests.service to record the ordering on shutdown. This
ensures that the scope units are still around until the LRM or guests
service had the chance to stop, and usually, handle the remaining
running guests itself.

[0]: https://forum.proxmox.com/threads/169739/post-825256

Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=7092
Co-developed-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
Co-developed-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/QemuServer.pm | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index d634251b..69991843 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -5718,6 +5718,10 @@ sub vm_start_nolock {
         KillMode => 'process',
         SendSIGKILL => 0,
         TimeoutStopUSec => ULONG_MAX, # infinity
+        After => ['dbus.service'],
+        # The point is ordering the VMID.scope after these during stop.
+        # During start-up, the slice/scopes are not enabled.
+        Before => ['pve-ha-lrm.service', 'pve-guests.service'],
     );
 
     if (PVE::CGroup::cgroup_mode() == 2) {
-- 
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 3/3] dbus-vmstate: guard more errors in qemu_del_dbus_vmstate()

[Fiona Ebner]

Before commit "systemd: vmid.scope: add orderings to fix dbus-vmstate
cleanup (regression)", an issue with hard errors during cleanup of the
dbus-vmstate surfaced if the dbus.service was already not running. In
particular, the Net::DBus->system() call would fail when the
dbus.service is not running. Guard errors there and for the
get_bus_object() call, like other errors in qemu_del_dbus_vmstate()
already are guarded. Do not honor the 'quiet' option for these and
log failures in any case, because not having a working system DBus
is a more fundamental failure than the other failure scenarios in the
function and should still be recorded.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/QemuServer/DBusVMState.pm | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/src/PVE/QemuServer/DBusVMState.pm b/src/PVE/QemuServer/DBusVMState.pm
index 480d9f70..3b5fe301 100644
--- a/src/PVE/QemuServer/DBusVMState.pm
+++ b/src/PVE/QemuServer/DBusVMState.pm
@@ -82,8 +82,19 @@ sub qemu_del_dbus_vmstate {
     my ($vmid, %params) = @_;
 
     my $num_entries = undef;
-    my $dbus = Net::DBus->system();
-    my $dbus_obj = $dbus->get_bus_object();
+    my $dbus = eval { Net::DBus->system(); };
+    if (my $err = $@) {
+        # log fundamental error even if $params{quiet} is set
+        syslog('warn', "failed to connect to DBus system bus: $err");
+        return undef;
+    }
+
+    my $dbus_obj = eval { $dbus->get_bus_object(); };
+    if (my $err = $@) {
+        # log fundamental error even if $params{quiet} is set
+        syslog('warn', "failed to get DBus bus object: $err");
+        return undef;
+    }
 
     my $owners = eval { $dbus_obj->ListQueuedOwners('org.qemu.VMState1') };
     if (my $err = $@) {
-- 
2.47.3



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

Re: [pve-devel] [PATCH-SERIES common/qemu-server 0/3] fix #7092: systemd: vmid.scope: add orderings to fix dbus-vmstate cleanup (regression)

[Maximiliano Sandoval]

Fiona Ebner <f.ebner@proxmox.com> writes:

> Fixes bug #7092 as well as a regression [0] in combination with HA
> shutdown policy 'migrate' caused by commit 1f2c5146 ("migrate: remove
> left-over dbus-vmstate instance when migrating without conntrack
> state"). Currently, the HA migrations do not migrate the conntrack
> state and before this patch, the dbus.service is stopped too early
> during shutdown, before the LRM migrations. This led to failure when
> trying to check for a left-over dbus-vmstate instance, because
> the Net::DBus->system() call would fail.
>
> It is not enough to record the After=dbus.service dependency, because
> systemd would already stop the vmid.scope even while processes in it
> are still running. Add a Before=pve-ha-lrm.service and for consistency
> Before=pve-guests.service to record the ordering on shutdown. This
> ensures that the scope units are still around until the LRM or guests
> service had the chance to stop, and usually, handle the remaining
> running guests itself.
>
> Also guard more errors in qemu_del_dbus_vmstate().
>
> Many thanks to Maximiliano and Fabian for discussing the issue and
> solutions!
>
> [0]: https://forum.proxmox.com/threads/169739/post-825256
>
>
> Dependency bump qemu-server -> libpve-common-perl needed!

Thanks for working on this!

This series fixes the issue described at 7092 and the forum thread.

My testing setup is:
 - Create a 2 node cluster
 - Set shutdown policy to migrate
 - Create VM on node 1
 - Start the VM
 - Add the VM to the HA resources
 - Run `$ shutdown now` from node 1

and with this patch the migration succeeds without warnings and the node
is able to properly shut down.

Tested-by: Maximiliano Sandoval <m.sandoval@proxmox.com>

-- 
Maximiliano


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

[pve-devel] applied-series: [PATCH-SERIES common/qemu-server 0/3] fix #7092: systemd: vmid.scope: add orderings to fix dbus-vmstate cleanup (regression)

[Fabian Grünbichler]

On Wed, 17 Dec 2025 14:17:09 +0100, Fiona Ebner wrote:
> Fixes bug #7092 as well as a regression [0] in combination with HA
> shutdown policy 'migrate' caused by commit 1f2c5146 ("migrate: remove
> left-over dbus-vmstate instance when migrating without conntrack
> state"). Currently, the HA migrations do not migrate the conntrack
> state and before this patch, the dbus.service is stopped too early
> during shutdown, before the LRM migrations. This led to failure when
> trying to check for a left-over dbus-vmstate instance, because
> the Net::DBus->system() call would fail.
> 
> [...]

Applied, thanks!


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