[RFC pve-container 1/1] mountpoint backup: allow changing backup flag at runtime

[RFC pve-container 1/1] mountpoint backup: allow changing backup flag at runtime

[Thomas Ellmenreich]

Previously, setting the backup flag of a running container would first
place it into a pending state, before applying it at a later time. However,
since the flag does not affect the actual mount point, it is now applied
directly.

In situations where multiple settings are changed at once and backup
happens to be one of them, the flag is not applied directly, but instead
marked as pending alongside the other changes.

Suggested-by: Filip Schauer <f.schauer@proxmox.com>
Signed-off-by: Thomas Ellmenreich <t.ellmenreich@proxmox.com>
---
Ticking the backup flag on a running mountpoint can be directly applied,
even if the container is running, as it has no effect on the mountpoint
itself.

This functionality is already present for VMs, but not for containers.

As also mentioned in the commit message, a change to the backup flag is
only directly applied if it is the only change. In cases where other
configuration changes are also made, all of them are set to pending as
a single unit.

Open Questions
--------------

- As far as I understand, qemu-server currently decides whether to change a
  configuration at runtime based on a set of 'fast_plug_option's as well as a
  set of 'non-hotpluggable options'. For the sake of simplicity, I have
  decided to stick with a list of 'fast plug options'. Is this the right
  approach? I considered creating an inverse list, but that would have
  involved more work. Having 'pending' as the default is the correct approach,
  in my opinion.

- Are there any other properties that could mabye be made fast-pluggable?

- Instead of moving the skip and apply control flow into the
  `apply_pending_mountpoint` subroutine, I considered leaving it where
  it was and taking the config parsing to the caller. However, I decided
  against this option, as delegating the config parsing to the caller
  seemed like the wrong decision.

Testing
-------

- Only changing the `backup` setting does precisely that, skipping the
  pending state.

- When combining `backup` with changing a different setting, they are both
  marked as pending.

 src/PVE/LXC/Config.pm | 29 +++++++++++++++++++++++++----
 1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/src/PVE/LXC/Config.pm b/src/PVE/LXC/Config.pm
index 7c81834..c28fb04 100644
--- a/src/PVE/LXC/Config.pm
+++ b/src/PVE/LXC/Config.pm
@@ -3,6 +3,7 @@ package PVE::LXC::Config;
 use strict;
 use warnings;
 
+use List::Util qw(uniq first);
 use Fcntl qw(O_RDONLY);
 
 use PVE::AbstractConfig;
@@ -1741,10 +1742,6 @@ sub vmconfig_hotplug_pending {
                     $hotplug_memory->($conf->{pending}->{memory}, $conf->{pending}->{swap});
                 }
             } elsif ($opt =~ m/^mp(\d+)$/) {
-                if (exists($conf->{$opt})) {
-                    die "skip\n"; # don't try to hotplug over existing mp
-                }
-
                 $class->apply_pending_mountpoint($vmid, $conf, $opt, $storecfg, 1);
                 # apply_pending_mountpoint modifies the value if it creates a new disk
                 $value = $conf->{pending}->{$opt};
@@ -1888,11 +1885,35 @@ my $rescan_volume = sub {
     warn "Could not rescan volume size - $@\n" if $@;
 };
 
+# Checks whether the changed properties can be applied at runtime. It only
+# returns true if all changes can be applied in this way. If even one
+# property cannot be applied, returns false to ensure changes are applied
+# as a unit.
+my $mountpoint_config_fast_plug_safe = sub {
+    my ($new, $old) = @_;
+    my @fast_plug_safe_keys = ("backup");
+
+    for my $key (uniq(keys %$new, keys %$old)) {
+        next if first { $_ eq $key } @fast_plug_safe_keys;
+        return 0 if ($new->{$key} // '') ne ($old->{$key} // '');
+    }
+
+    1;
+};
+
 sub apply_pending_mountpoint {
     my ($class, $vmid, $conf, $opt, $storecfg, $running) = @_;
 
     my $mp = $class->parse_volume($opt, $conf->{pending}->{$opt});
     my $old = $conf->{$opt};
+
+    if (exists($conf->{$opt}) && $running) {
+        if ($mountpoint_config_fast_plug_safe->($mp, $class->parse_volume($opt, $old))) {
+            return;
+        }
+        die "skip\n"; # don't try to hotplug over existing mp
+    }
+
     if ($mp->{type} eq 'volume' && $mp->{volume} =~ $PVE::LXC::NEW_DISK_RE) {
         my $original_value = $conf->{pending}->{$opt};
         my $vollist = PVE::LXC::create_disks(
-- 
2.47.3

Re: [RFC pve-container 1/1] mountpoint backup: allow changing backup flag at runtime

[Filip Schauer]

On 27/08/2026 14:27, Thomas Ellmenreich wrote:
> Previously, setting the backup flag of a running container would first
> place it into a pending state, before applying it at a later time. However,
> since the flag does not affect the actual mount point, it is now applied
> directly.
> 
> In situations where multiple settings are changed at once and backup
> happens to be one of them, the flag is not applied directly, but instead
> marked as pending alongside the other changes.
> 
> Suggested-by: Filip Schauer <f.schauer@proxmox.com>
> Signed-off-by: Thomas Ellmenreich <t.ellmenreich@proxmox.com>
> ---
> Ticking the backup flag on a running mountpoint can be directly applied,
> even if the container is running, as it has no effect on the mountpoint
> itself.
> 
> This functionality is already present for VMs, but not for containers.
> 
> As also mentioned in the commit message, a change to the backup flag is
> only directly applied if it is the only change. In cases where other
> configuration changes are also made, all of them are set to pending as
> a single unit.
> 
> Open Questions
> --------------
> 
> - As far as I understand, qemu-server currently decides whether to change a
>    configuration at runtime based on a set of 'fast_plug_option's as well as a
>    set of 'non-hotpluggable options'. For the sake of simplicity, I have
>    decided to stick with a list of 'fast plug options'. Is this the right
>    approach? I considered creating an inverse list, but that would have
>    involved more work. Having 'pending' as the default is the correct approach,
>    in my opinion.

Sounds reasonable to me.

> 
> - Are there any other properties that could mabye be made fast-pluggable?

The `replicate` ("Skip Replication") property comes to mind.

> 
> - Instead of moving the skip and apply control flow into the
>    `apply_pending_mountpoint` subroutine, I considered leaving it where
>    it was and taking the config parsing to the caller. However, I decided
>    against this option, as delegating the config parsing to the caller
>    seemed like the wrong decision.
> 
> Testing
> -------
> 
> - Only changing the `backup` setting does precisely that, skipping the
>    pending state.
> 
> - When combining `backup` with changing a different setting, they are both
>    marked as pending.
> 
>   src/PVE/LXC/Config.pm | 29 +++++++++++++++++++++++++----
>   1 file changed, 25 insertions(+), 4 deletions(-)
> 
> diff --git a/src/PVE/LXC/Config.pm b/src/PVE/LXC/Config.pm
> index 7c81834..c28fb04 100644
> --- a/src/PVE/LXC/Config.pm
> +++ b/src/PVE/LXC/Config.pm
> @@ -3,6 +3,7 @@ package PVE::LXC::Config;
>   use strict;
>   use warnings;
>   
> +use List::Util qw(uniq first);
>   use Fcntl qw(O_RDONLY);
>   
>   use PVE::AbstractConfig;
> @@ -1741,10 +1742,6 @@ sub vmconfig_hotplug_pending {
>                       $hotplug_memory->($conf->{pending}->{memory}, $conf->{pending}->{swap});
>                   }
>               } elsif ($opt =~ m/^mp(\d+)$/) {
> -                if (exists($conf->{$opt})) {
> -                    die "skip\n"; # don't try to hotplug over existing mp
> -                }
> -
>                   $class->apply_pending_mountpoint($vmid, $conf, $opt, $storecfg, 1);
>                   # apply_pending_mountpoint modifies the value if it creates a new disk
>                   $value = $conf->{pending}->{$opt};
> @@ -1888,11 +1885,35 @@ my $rescan_volume = sub {
>       warn "Could not rescan volume size - $@\n" if $@;
>   };
>   
> +# Checks whether the changed properties can be applied at runtime. It only
> +# returns true if all changes can be applied in this way. If even one
> +# property cannot be applied, returns false to ensure changes are applied
> +# as a unit.
> +my $mountpoint_config_fast_plug_safe = sub {
> +    my ($new, $old) = @_;
> +    my @fast_plug_safe_keys = ("backup");
> +
> +    for my $key (uniq(keys %$new, keys %$old)) {
> +        next if first { $_ eq $key } @fast_plug_safe_keys;
> +        return 0 if ($new->{$key} // '') ne ($old->{$key} // '');

 From testing I noticed that when toggling `backup` on a mount point
specifying a custom `idmap`, the `backup` property is still marked as
pending.
The reason seems to be that this is comparing array references. Even if
the arrays both hold the same data, the references still differ.

> +    }
> +
> +    1;
> +};
> +
>   sub apply_pending_mountpoint {
>       my ($class, $vmid, $conf, $opt, $storecfg, $running) = @_;
>   
>       my $mp = $class->parse_volume($opt, $conf->{pending}->{$opt});
>       my $old = $conf->{$opt};
> +
> +    if (exists($conf->{$opt}) && $running) {
> +        if ($mountpoint_config_fast_plug_safe->($mp, $class->parse_volume($opt, $old))) {
> +            return;
> +        }
> +        die "skip\n"; # don't try to hotplug over existing mp

Doesn't this make the later `die "skip\n" if $running && defined($old);`
unreachable?

> +    }
> +
>       if ($mp->{type} eq 'volume' && $mp->{volume} =~ $PVE::LXC::NEW_DISK_RE) {
>           my $original_value = $conf->{pending}->{$opt};
>           my $vollist = PVE::LXC::create_disks(