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

[Filip Schauer <f.schauer@proxmox.com>]

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(