public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Stefan Hanreich <s.hanreich@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
	Christoph Heiss <c.heiss@proxmox.com>
Subject: Re: [pve-devel] [PATCH ve-rs/firewall/qemu-server/manager v2 00/13] fix #5180: migrate conntrack state on live migration
Date: Tue, 3 Jun 2025 11:35:53 +0200	[thread overview]
Message-ID: <902b7cc1-8962-4bf6-be1e-a996f24798e3@proxmox.com> (raw)
In-Reply-To: <20250424111941.730528-1-c.heiss@proxmox.com>

Gave this a spin on my test cluster, and it worked as advertised. As
already mentioned and talked off-list we should imo add documentation
for this setting and mention the current restrictions (no migration of
NAT entries). We might also want to mention that only CT entries created
*after* the firewall is enabled for a VM get migrated, although that is
probably rather uncommon.

Other than that consider this:

Tested-by: Stefan Hanreich <s.hanreich@proxmox.com>

I've also mainly looked at the firewall + pve-manager code, maybe
someone with more knowledge about QEMU can chime in on the DBus server
parts. Consider the firewall and ui patches:

Reviewed-by: Stefan Hanreich <s.hanreich@proxmox.com>


On 4/24/25 13:19, Christoph Heiss wrote:
> Fixes #5180 [0].
> 
> This implements migration of per-VM conntrack state on live-migration.
> 
> The core of the implementation are in patch #7 & #8. See there for more
> details.
> 
> Patch #1 - #3 implement CONNMARK'ing any VM traffic with their unique
> VMID. This is needed later on to filter conntrack entries for the
> migration. These three patches can be applied independently,
> CONNMARK'ing traffic does not have any visible impact.
> 
> Currently, remote/inter-cluster migration is not supported and indicated
> to the user with a warning. See also patch #8 for a bit more in-depth
> explanation.
> 
> Needed dependency bumps between packages are indicated in the notes
> appropriately.
> 
> [0] https://bugzilla.proxmox.com/show_bug.cgi?id=5180
> 
> Testing
> =======
> 
> I've primarily tested intra-cluster live-migrations, with both the
> iptables-based and nftables-based firewall), using the reproducer as
> described in #5180. I further verified that the D-Bus servers get
> started as expected and are _always_ stopped, even in the case of some
> migration error.
> 
> Finally, I also checked using `conntrack -L -m <vmid>` tool that the
> conntrack entries are 
> a) added/updated on the target node and 
> b) removed from the source node afterwards
> 
> Also tested was the migration from/to an "old" (unpatched) node, which
> results in the issue as per #5180 & appropriate warnings in the UI.
> 
> For remote migrations, only tested that the warning is logged as
> expected.
> 
> History
> =======
> 
> v1: https://lore.proxmox.com/pve-devel/20250317141152.1247324-1-c.heiss@proxmox.com/
> 
> Changes v1 -> v2:
>   * rebased as necessary
>   * "un-rfc'd" firewall conntrack flushing patches
>   * use an instanced systemd service instead of fork+exec for the
>     pve-dbus-vmstate helper
> 
> Diffstat
> ========
> 
> pve-firewall:
> 
> Christoph Heiss (2):
>   firewall: add connmark rule with VMID to all guest chains
>   firewall: helpers: add sub for flushing conntrack entries by mark
> 
>  debian/control              |  3 ++-
>  src/PVE/Firewall.pm         |  7 +++++--
>  src/PVE/Firewall/Helpers.pm | 11 +++++++++++
>  3 files changed, 18 insertions(+), 3 deletions(-)
> 
> proxmox-firewall:
> 
> Christoph Heiss (1):
>   firewall: add connmark rule with VMID to all guest chains
> 
>  proxmox-firewall/src/firewall.rs              | 14 +++-
>  .../integration_tests__firewall.snap          | 84 +++++++++++++++++++
>  proxmox-nftables/src/expression.rs            |  9 ++
>  proxmox-nftables/src/statement.rs             | 10 ++-
>  4 files changed, 114 insertions(+), 3 deletions(-)
> 
> proxmox-ve-rs:
> 
> Christoph Heiss (1):
>   config: guest: allow access to raw Vmid value
> 
>  proxmox-ve-config/src/guest/types.rs | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> qemu-server:
> 
> Christoph Heiss (5):
>   qmp helpers: allow passing structured args via qemu_objectadd()
>   api2: qemu: add module exposing node migration capabilities
>   fix #5180: libexec: add QEMU dbus-vmstate daemon for migrating
>     conntrack
>   fix #5180: migrate: integrate helper for live-migrating conntrack info
>   migrate: flush old VM conntrack entries after successful migration
> 
>  Makefile                               |   7 +-
>  PVE/API2/Qemu.pm                       |  72 +++++++++++
>  PVE/API2/Qemu/Makefile                 |   2 +-
>  PVE/API2/Qemu/Migration.pm             |  46 +++++++
>  PVE/CLI/qm.pm                          |   5 +
>  PVE/QemuMigrate.pm                     |  69 ++++++++++
>  PVE/QemuServer.pm                      |   6 +
>  PVE/QemuServer/DBusVMState.pm          | 120 ++++++++++++++++++
>  PVE/QemuServer/Makefile                |   1 +
>  PVE/QemuServer/QMPHelpers.pm           |   4 +-
>  dbus-vmstate/Makefile                  |   7 ++
>  dbus-vmstate/dbus-vmstate              | 168 +++++++++++++++++++++++++
>  dbus-vmstate/org.qemu.VMState1.conf    |  11 ++
>  dbus-vmstate/pve-dbus-vmstate@.service |  10 ++
>  debian/control                         |   7 +-
>  15 files changed, 530 insertions(+), 5 deletions(-)
>  create mode 100644 PVE/API2/Qemu/Migration.pm
>  create mode 100644 PVE/QemuServer/DBusVMState.pm
>  create mode 100644 dbus-vmstate/Makefile
>  create mode 100755 dbus-vmstate/dbus-vmstate
>  create mode 100644 dbus-vmstate/org.qemu.VMState1.conf
>  create mode 100644 dbus-vmstate/pve-dbus-vmstate@.service
> 
> pve-manager:
> 
> Christoph Heiss (4):
>   api2: capabilities: explicitly import CPU capabilities module
>   api2: capabilities: proxy index endpoints to respective nodes
>   api2: capabilities: expose new qemu/migration endpoint
>   ui: window: Migrate: add checkbox for migrating VM conntrack state
> 
>  PVE/API2/Capabilities.pm       |  9 +++++
>  www/manager6/window/Migrate.js | 73 ++++++++++++++++++++++++++++++++--
>  2 files changed, 78 insertions(+), 4 deletions(-)
> 



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


  parent reply	other threads:[~2025-06-03  9:35 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-24 11:19 Christoph Heiss
2025-04-24 11:19 ` [pve-devel] [PATCH proxmox-ve-rs v2 1/13] config: guest: allow access to raw Vmid value Christoph Heiss
2025-04-24 11:19 ` [pve-devel] [PATCH proxmox-firewall v2 2/13] firewall: add connmark rule with VMID to all guest chains Christoph Heiss
2025-04-24 11:19 ` [pve-devel] [PATCH pve-firewall v2 3/13] " Christoph Heiss
2025-04-24 11:19 ` [pve-devel] [PATCH pve-firewall v2 4/13] firewall: helpers: add sub for flushing conntrack entries by mark Christoph Heiss
2025-04-24 11:19 ` [pve-devel] [PATCH qemu-server v2 5/13] qmp helpers: allow passing structured args via qemu_objectadd() Christoph Heiss
2025-04-24 11:19 ` [pve-devel] [PATCH qemu-server v2 6/13] api2: qemu: add module exposing node migration capabilities Christoph Heiss
2025-04-24 12:02   ` Fiona Ebner
2025-04-25  8:40     ` Christoph Heiss
2025-04-24 11:19 ` [pve-devel] [PATCH qemu-server v2 7/13] fix #5180: libexec: add QEMU dbus-vmstate daemon for migrating conntrack Christoph Heiss
2025-06-03  9:34   ` Stefan Hanreich
2025-06-11 10:44     ` Christoph Heiss
2025-04-24 11:19 ` [pve-devel] [PATCH qemu-server v2 8/13] fix #5180: migrate: integrate helper for live-migrating conntrack info Christoph Heiss
2025-04-24 11:19 ` [pve-devel] [PATCH qemu-server v2 9/13] migrate: flush old VM conntrack entries after successful migration Christoph Heiss
2025-04-24 11:19 ` [pve-devel] [PATCH manager v2 10/13] api2: capabilities: explicitly import CPU capabilities module Christoph Heiss
2025-04-24 11:19 ` [pve-devel] [PATCH manager v2 11/13] api2: capabilities: proxy index endpoints to respective nodes Christoph Heiss
2025-04-24 11:19 ` [pve-devel] [PATCH manager v2 12/13] api2: capabilities: expose new qemu/migration endpoint Christoph Heiss
2025-04-24 11:19 ` [pve-devel] [PATCH manager v2 13/13] ui: window: Migrate: add checkbox for migrating VM conntrack state Christoph Heiss
2025-06-03  9:32   ` Stefan Hanreich
2025-05-30 11:34 ` [pve-devel] [PATCH ve-rs/firewall/qemu-server/manager v2 00/13] fix #5180: migrate conntrack state on live migration Stefan Hanreich
2025-06-03  9:35 ` Stefan Hanreich [this message]
2025-06-04  9:00   ` Christoph Heiss

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=902b7cc1-8962-4bf6-be1e-a996f24798e3@proxmox.com \
    --to=s.hanreich@proxmox.com \
    --cc=c.heiss@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal