From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id 8EAFC1FF0AE for ; Tue, 13 Oct 2026 14:40:29 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 51528213B4; Tue, 13 Oct 2026 14:40:27 +0200 (CEST) Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Tue, 13 Oct 2026 14:40:21 +0200 Message-Id: From: "Daniel Kral" To: "Fiona Ebner" , Subject: Re: [PATCH-SERIES common/cluster/ha-manager/qemu-server/container v3 00/21] migration: strict config check for intra-cluster migration X-Mailer: aerc 0.22.0-10-g6373ac9d2179-dirty References: <20260918160841.128088-1-f.ebner@proxmox.com> In-Reply-To: <20260918160841.128088-1-f.ebner@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1791895222041 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.818 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: 3GY5ANN5GV4JPAMMHFHMCBFNZV5ZGIZG X-Message-ID-Hash: 3GY5ANN5GV4JPAMMHFHMCBFNZV5ZGIZG X-MailFrom: d.kral@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: I like the approach with the JSON commands over the storing and restoring of migration parameters in the migrate API handler itself, nice work and thanks for working on this! I tested both the passing the --with-conntrack-state and the new --skip-config-check options with cluster nodes being in outdated versions and all being on the correct version to use the new functionality. The JSON command and therefore the migration parameters were only passed if all of the cluster nodes had the correct version as expected. The same for whether the target node had at least the correct version to know the new config command to make a strict config check if --skip-config-check wasn't specified (tested it with the 'shiny-new' config property from v2). I tested the migrations with the {qm,pct} migrate as well as the ha-manager migrate API. On Fri Sep 18, 2026 at 6:08 PM CEST, Fiona Ebner wrote: > Changes in v3: > * Add patches to teach HA to pass along certain migration parameters > and fix #7053 by passing along 'with-conntrack-state'. > * Avoid re-using root-only 'force' flag, add a new 'skip-config-check' > flag that can also be used by regular users. > * Use pvecfg node version to detect if a target supports config > checking via mtunnel. > > Changes in v2: > * add patches for container > * log when skipping strict configuration check > > First part > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > Additional migration options can now be passed from the guest API > endpoints to the HA stack. From the HA resource migrate API endpoint, > the options are passed all the way to the LRM and resource plugins, > which will then use it again when invoking the resource's migrate API > endpoint itself. > > The migration API endpoints for containers and VMs use different > parameters. For example, the endpoint for containers does not have > the 'with-conntrack-state' option. To keep the schema for the more > abstract endpoint for HA resources clean, separate by type using a > oneOf schema and let the resource plugins declare additional > parameters via a migrate_json_properties() function. > > Known issue: because of cyclic dependencies, the PVE::API::{LXC,QEMU} > modules can't be included by the respective resource plugin, when the > build happens in a PVE_GENERATING_DOCS environment. Because of this, > the extra migration options for the HA endpoint won't show up in the > docs. > > To pass these along to CRM, support for JSON CRM commands is > introduced, with only the migration command being supported for now. > If not all nodes are recent enough to support the new-style JSON CRM > command, produce a warning and fall-back to the previous behavior > ignoring the additional migration options. > > Initially, this is used to fix #7053 by passing along the > 'with-conntrack-state' migration option. > > Second part > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > For remote migration, we already check that the config can be parsed > on the target. Do the same for intra-cluster migration, to avoid > issues like [0] for future new settings, with settings being > unexpectedly ignored on the target, and in the case of qemu-server > even relatively silently dropped (there are warnings in the target's > system logs). In case of containers, an example is migrating a > container with a mountpoint with 'keepattrs' to a node with a too old > pve-container version, resulting in the mountpoint not being mounted > on the target. > > Unfortunately, before patch "qm: mtunnel: reply when a command is > unknown", when a command is unknown, mtunnel did not reply at all. > Therefore, check the target node's version to see if it is new enough > to support the new mtunnel command. > > Still missing > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > UI integration for the new flag > HA manager tests with the new JSON commands > > Useful to pull in early > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > pvecfg node version helpers moved to pve-cluster rather than just > pve-guest-common, because David needs them in pve-cluster too [1]. > > [0]: https://bugzilla.proxmox.com/show_bug.cgi?id=3D7341 > [1]: https://lore.proxmox.com/pve-devel/20260724142528.109453-1-d.riley@p= roxmox.com/ > > Patch 15/21 qm: mtunnel: reply when a command is unknown would also > be nice to have. > > Dependencies > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > Dependency bump qemu-server,pve-container -> common,pve-cluster needed! > > qemu-server also build-depends on new common and pve-cluster. > > pve-manager needs to depend on new ha-manager so the node version > checking makes sense. > > Dependency bump pve-container -> guest-common needed!