public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH-SERIES qemu-server 0/6] VM-state property handling improvements
@ 2025-09-17 16:30 Fiona Ebner
  2025-09-17 16:30 ` [pve-devel] [PATCH qemu-server 1/6] partially fix #6805: api: clone: properly remove all snapshot-related info Fiona Ebner
                   ` (6 more replies)
  0 siblings, 7 replies; 8+ messages in thread
From: Fiona Ebner @ 2025-09-17 16:30 UTC (permalink / raw)
  To: pve-devel

Fix bug #6805, fix handling nets-host-mtu in more contexts like resume
from hibernation and printing the commandline of a snapshot. Disallow
setting that property by a user via API like is done for other
VM-state properties.

Sorry, I thought I had checked for all occurences of 'runningmachine'
when introducing 'running-nets-host-mtu', but apparently I missed some
in the QemuServer module :(

Also introduce proper privilege checking for VM-state properties.

Finally, detect and cleanup such properties upon VM start.

I haven't come around to testing these for stable-bookworm yet, but
will let you know if they apply/work cleanly there too.


Fiona Ebner (6):
  partially fix #6805: api: clone: properly remove all snapshot-related
    info
  partially fix #6805: api: modify vm config: privilege checks for
    VM-state-related properties
  api: create/update: disallow setting 'running-nets-host-mtu' via API
  resume from suspended: properly handle 'nets-host-mtu'
  vm commandline: handle 'nets-host-mtu' property in snapshot
  vm start: remove left-over VM-state-related properties

 src/PVE/API2/Qemu.pm  |  9 +++++++--
 src/PVE/QemuServer.pm | 24 ++++++++++++++++++++++--
 2 files changed, 29 insertions(+), 4 deletions(-)

-- 
2.47.2



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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [pve-devel] [PATCH qemu-server 1/6] partially fix #6805: api: clone: properly remove all snapshot-related info
  2025-09-17 16:30 [pve-devel] [PATCH-SERIES qemu-server 0/6] VM-state property handling improvements Fiona Ebner
@ 2025-09-17 16:30 ` Fiona Ebner
  2025-09-17 16:30 ` [pve-devel] [PATCH qemu-server 2/6] partially fix #6805: api: modify vm config: privilege checks for VM-state-related properties Fiona Ebner
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Fiona Ebner @ 2025-09-17 16:30 UTC (permalink / raw)
  To: pve-devel

When cloning from a snapshot, the current VM state is not copied. Not
all relevant properties were dropped, leading to some left-overs in
the configuration of the clone target.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/API2/Qemu.pm | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/PVE/API2/Qemu.pm b/src/PVE/API2/Qemu.pm
index 34f615d8..8097ea87 100644
--- a/src/PVE/API2/Qemu.pm
+++ b/src/PVE/API2/Qemu.pm
@@ -4356,7 +4356,10 @@ __PACKAGE__->register_method({
                     || $opt eq 'parent'
                     || $opt eq 'snaptime'
                     || $opt eq 'vmstate'
-                    || $opt eq 'snapstate';
+                    || $opt eq 'snapstate'
+                    || $opt eq 'runningcpu'
+                    || $opt eq 'runningmachine'
+                    || $opt eq 'running-nets-host-mtu';
 
                 # no need to copy unused images, because VMID(owner) changes anyways
                 next if $opt =~ m/^unused\d+$/;
-- 
2.47.2



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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [pve-devel] [PATCH qemu-server 2/6] partially fix #6805: api: modify vm config: privilege checks for VM-state-related properties
  2025-09-17 16:30 [pve-devel] [PATCH-SERIES qemu-server 0/6] VM-state property handling improvements Fiona Ebner
  2025-09-17 16:30 ` [pve-devel] [PATCH qemu-server 1/6] partially fix #6805: api: clone: properly remove all snapshot-related info Fiona Ebner
@ 2025-09-17 16:30 ` Fiona Ebner
  2025-09-17 16:30 ` [pve-devel] [PATCH qemu-server 3/6] api: create/update: disallow setting 'running-nets-host-mtu' via API Fiona Ebner
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Fiona Ebner @ 2025-09-17 16:30 UTC (permalink / raw)
  To: pve-devel

Currently, the VM-state-related properties 'runningcpu',
'runningmachine' and 'running-nets-host-mtu' are not supposed to end
up in the VM configuration of a remote-migratable VM, because a
suspended VM is not yet migratable. However, there was a bug and the
properties were not removed after cloning from a snapshot, see commit
"partially fix #6805: api: clone: properly remove all snapshot-related
info". Upon remote migration, the property would be encountered and
would be limited to root@pam only. Also, migrating suspended VMs might
be implemented in the future, i.e. BZ issue #2252.

To aid fixing bug #6805 and preparing for issue #2252 in the future,
do proper privilege checking for configuration properties related to
the running VM state.

Note that the 'vmstate' property is already checked for in the
check_vm_modify_config_perm() helper. Note that VM-state-related
properties cannot be set via API by a user.

Originally-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/API2/Qemu.pm | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/PVE/API2/Qemu.pm b/src/PVE/API2/Qemu.pm
index 8097ea87..594c5d48 100644
--- a/src/PVE/API2/Qemu.pm
+++ b/src/PVE/API2/Qemu.pm
@@ -732,6 +732,7 @@ my $check_cpu_model_access = sub {
 my $cpuoptions = {
     'cores' => 1,
     'cpu' => 1,
+    'runningcpu' => 1,
     'cpulimit' => 1,
     'cpuunits' => 1,
     'numa' => 1,
@@ -751,6 +752,7 @@ my $hwtypeoptions = {
     'hotplug' => 1,
     'kvm' => 1,
     'machine' => 1,
+    'runningmachine' => 1,
     'scsihw' => 1,
     'smbios1' => 1,
     'tablet' => 1,
@@ -957,7 +959,7 @@ my $check_vm_modify_config_perm = sub {
             $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.PowerMgmt']);
         } elsif ($diskoptions->{$opt}) {
             $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.Disk']);
-        } elsif ($opt =~ m/^net\d+$/) {
+        } elsif ($opt =~ m/^net\d+$/ || $opt eq 'running-nets-host-mtu') {
             $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.Network']);
         } elsif ($cloudinitoptions->{$opt} || $opt =~ m/^ipconfig\d+$/) {
             $rpcenv->check_vm_perm(
-- 
2.47.2



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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [pve-devel] [PATCH qemu-server 3/6] api: create/update: disallow setting 'running-nets-host-mtu' via API
  2025-09-17 16:30 [pve-devel] [PATCH-SERIES qemu-server 0/6] VM-state property handling improvements Fiona Ebner
  2025-09-17 16:30 ` [pve-devel] [PATCH qemu-server 1/6] partially fix #6805: api: clone: properly remove all snapshot-related info Fiona Ebner
  2025-09-17 16:30 ` [pve-devel] [PATCH qemu-server 2/6] partially fix #6805: api: modify vm config: privilege checks for VM-state-related properties Fiona Ebner
@ 2025-09-17 16:30 ` Fiona Ebner
  2025-09-17 16:30 ` [pve-devel] [PATCH qemu-server 4/6] resume from suspended: properly handle 'nets-host-mtu' Fiona Ebner
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Fiona Ebner @ 2025-09-17 16:30 UTC (permalink / raw)
  To: pve-devel

Like the other snapshot-related properties, it should not be possible
to set 'running-nets-host-mtu' via the API.

Fixes: 7ceb6b72 ("snapshot: introduce running-nets-host-mtu property")
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/QemuServer.pm | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index cbcad749..0ddddbf2 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -1856,6 +1856,7 @@ sub json_config_properties {
         parent => 1,
         snaptime => 1,
         vmstate => 1,
+        'running-nets-host-mtu' => 1,
         runningmachine => 1,
         runningcpu => 1,
         meta => 1,
-- 
2.47.2



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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [pve-devel] [PATCH qemu-server 4/6] resume from suspended: properly handle 'nets-host-mtu'
  2025-09-17 16:30 [pve-devel] [PATCH-SERIES qemu-server 0/6] VM-state property handling improvements Fiona Ebner
                   ` (2 preceding siblings ...)
  2025-09-17 16:30 ` [pve-devel] [PATCH qemu-server 3/6] api: create/update: disallow setting 'running-nets-host-mtu' via API Fiona Ebner
@ 2025-09-17 16:30 ` Fiona Ebner
  2025-09-17 16:30 ` [pve-devel] [PATCH qemu-server 5/6] vm commandline: handle 'nets-host-mtu' property in snapshot Fiona Ebner
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Fiona Ebner @ 2025-09-17 16:30 UTC (permalink / raw)
  To: pve-devel

Fixes: 7ceb6b72 ("snapshot: introduce running-nets-host-mtu property")
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/QemuServer.pm | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 0ddddbf2..e2d5358f 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -5681,10 +5681,12 @@ sub vm_start_nolock {
 
     my $forcemachine = $params->{forcemachine};
     my $forcecpu = $params->{forcecpu};
+    my $nets_host_mtu = $params->{'nets-host-mtu'};
     if ($resume) {
         # enforce machine and CPU type on suspended vm to ensure HW compatibility
         $forcemachine = $conf->{runningmachine};
         $forcecpu = $conf->{runningcpu};
+        $nets_host_mtu = $conf->{'running-nets-host-mtu'};
         print "Resuming suspended VM\n";
     }
 
@@ -5731,7 +5733,7 @@ sub vm_start_nolock {
                 'force-machine' => $forcemachine,
                 'force-cpu' => $forcecpu,
                 'live-restore-backing' => $params->{'live-restore-backing'},
-                'nets-host-mtu' => $params->{'nets-host-mtu'},
+                'nets-host-mtu' => $nets_host_mtu,
             },
         );
 
@@ -6035,7 +6037,7 @@ sub vm_start_nolock {
             PVE::Storage::deactivate_volumes($storecfg, [$vmstate]);
             PVE::Storage::vdisk_free($storecfg, $vmstate);
         }
-        delete $conf->@{qw(lock vmstate runningmachine runningcpu)};
+        delete $conf->@{qw(lock vmstate running-nets-host-mtu runningmachine runningcpu)};
         PVE::QemuConfig->write_config($vmid, $conf);
     }
 
-- 
2.47.2



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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [pve-devel] [PATCH qemu-server 5/6] vm commandline: handle 'nets-host-mtu' property in snapshot
  2025-09-17 16:30 [pve-devel] [PATCH-SERIES qemu-server 0/6] VM-state property handling improvements Fiona Ebner
                   ` (3 preceding siblings ...)
  2025-09-17 16:30 ` [pve-devel] [PATCH qemu-server 4/6] resume from suspended: properly handle 'nets-host-mtu' Fiona Ebner
@ 2025-09-17 16:30 ` Fiona Ebner
  2025-09-17 16:30 ` [pve-devel] [PATCH qemu-server 6/6] vm start: remove left-over VM-state-related properties Fiona Ebner
  2025-09-17 16:52 ` [pve-devel] applied: [PATCH-SERIES qemu-server 0/6] VM-state property handling improvements Thomas Lamprecht
  6 siblings, 0 replies; 8+ messages in thread
From: Fiona Ebner @ 2025-09-17 16:30 UTC (permalink / raw)
  To: pve-devel

Fixes: 7ceb6b72 ("snapshot: introduce running-nets-host-mtu property")
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/QemuServer.pm | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index e2d5358f..90a8be3e 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -6068,6 +6068,7 @@ sub vm_commandline {
         # check for machine or CPU overrides in snapshot
         $options->{'force-machine'} = $snapshot->{runningmachine};
         $options->{'force-cpu'} = $snapshot->{runningcpu};
+        $options->{'nets-host-mtu'} = $snapshot->{'running-nets-host-mtu'};
 
         $snapshot->{digest} = $conf->{digest}; # keep file digest for API
 
-- 
2.47.2



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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [pve-devel] [PATCH qemu-server 6/6] vm start: remove left-over VM-state-related properties
  2025-09-17 16:30 [pve-devel] [PATCH-SERIES qemu-server 0/6] VM-state property handling improvements Fiona Ebner
                   ` (4 preceding siblings ...)
  2025-09-17 16:30 ` [pve-devel] [PATCH qemu-server 5/6] vm commandline: handle 'nets-host-mtu' property in snapshot Fiona Ebner
@ 2025-09-17 16:30 ` Fiona Ebner
  2025-09-17 16:52 ` [pve-devel] applied: [PATCH-SERIES qemu-server 0/6] VM-state property handling improvements Thomas Lamprecht
  6 siblings, 0 replies; 8+ messages in thread
From: Fiona Ebner @ 2025-09-17 16:30 UTC (permalink / raw)
  To: pve-devel

When cloning from a snapshot, VM-state-related properties would be
accidentally copied, see commit "partially fix #6805: api: clone:
properly remove all snapshot-related info". Detect and remove such
left-over properties upon VM start.

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

diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 90a8be3e..f7f85436 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -5553,6 +5553,20 @@ sub vm_migrate_alloc_nbd_disks {
     return $nbd;
 }
 
+my sub remove_left_over_vmstate_opts {
+    my ($vmid, $conf) = @_;
+
+    my $found;
+    for my $opt (qw(running-nets-host-mtu runningmachine runningcpu)) {
+        if (defined($conf->{$opt})) {
+            print "No VM state set, removing left-over option '$opt'\n";
+            delete $conf->{$opt};
+            $found = 1;
+        }
+    }
+    PVE::QemuConfig->write_config($vmid, $conf) if $found;
+}
+
 # see vm_start_nolock for parameters, additionally:
 # migrate_opts:
 #   storagemap = parsed storage map for allocating NBD disks
@@ -6039,6 +6053,8 @@ sub vm_start_nolock {
         }
         delete $conf->@{qw(lock vmstate running-nets-host-mtu runningmachine runningcpu)};
         PVE::QemuConfig->write_config($vmid, $conf);
+    } elsif (!$conf->{vmstate}) {
+        remove_left_over_vmstate_opts($vmid, $conf);
     }
 
     PVE::GuestHelpers::exec_hookscript($conf, $vmid, 'post-start');
-- 
2.47.2



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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [pve-devel] applied: [PATCH-SERIES qemu-server 0/6] VM-state property handling improvements
  2025-09-17 16:30 [pve-devel] [PATCH-SERIES qemu-server 0/6] VM-state property handling improvements Fiona Ebner
                   ` (5 preceding siblings ...)
  2025-09-17 16:30 ` [pve-devel] [PATCH qemu-server 6/6] vm start: remove left-over VM-state-related properties Fiona Ebner
@ 2025-09-17 16:52 ` Thomas Lamprecht
  6 siblings, 0 replies; 8+ messages in thread
From: Thomas Lamprecht @ 2025-09-17 16:52 UTC (permalink / raw)
  To: pve-devel, Fiona Ebner

On Wed, 17 Sep 2025 18:30:24 +0200, Fiona Ebner wrote:
> Fix bug #6805, fix handling nets-host-mtu in more contexts like resume
> from hibernation and printing the commandline of a snapshot. Disallow
> setting that property by a user via API like is done for other
> VM-state properties.
> 
> Sorry, I thought I had checked for all occurences of 'runningmachine'
> when introducing 'running-nets-host-mtu', but apparently I missed some
> in the QemuServer module :(
> 
> [...]

Applied, thanks!

[1/6] partially fix #6805: api: clone: properly remove all snapshot-related info
      commit: 6572ff578488da700123281ac447d2ad6bd1771f
[2/6] partially fix #6805: api: modify vm config: privilege checks for VM-state-related properties
      commit: 7c05f0be9858543272c1254c83f4dfc266b97fe2
[3/6] api: create/update: disallow setting 'running-nets-host-mtu' via API
      commit: f1fe3b748967d204c85960703407e0e655bfbb39
[4/6] resume from suspended: properly handle 'nets-host-mtu'
      commit: 80236b100e8cf5660b3f5f459e78e0ba322e1f21
[5/6] vm commandline: handle 'nets-host-mtu' property in snapshot
      commit: bd1a858f433e01dff8510173515004b3a91638e5
[6/6] vm start: remove left-over VM-state-related properties
      commit: 0db14e60109ea91a558f0f08594583588ef9ec66


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


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2025-09-17 16:52 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-09-17 16:30 [pve-devel] [PATCH-SERIES qemu-server 0/6] VM-state property handling improvements Fiona Ebner
2025-09-17 16:30 ` [pve-devel] [PATCH qemu-server 1/6] partially fix #6805: api: clone: properly remove all snapshot-related info Fiona Ebner
2025-09-17 16:30 ` [pve-devel] [PATCH qemu-server 2/6] partially fix #6805: api: modify vm config: privilege checks for VM-state-related properties Fiona Ebner
2025-09-17 16:30 ` [pve-devel] [PATCH qemu-server 3/6] api: create/update: disallow setting 'running-nets-host-mtu' via API Fiona Ebner
2025-09-17 16:30 ` [pve-devel] [PATCH qemu-server 4/6] resume from suspended: properly handle 'nets-host-mtu' Fiona Ebner
2025-09-17 16:30 ` [pve-devel] [PATCH qemu-server 5/6] vm commandline: handle 'nets-host-mtu' property in snapshot Fiona Ebner
2025-09-17 16:30 ` [pve-devel] [PATCH qemu-server 6/6] vm start: remove left-over VM-state-related properties Fiona Ebner
2025-09-17 16:52 ` [pve-devel] applied: [PATCH-SERIES qemu-server 0/6] VM-state property handling improvements Thomas Lamprecht

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