[pve-devel] [PATCH container] Remove an unused volume from the config if it is pending to be re-added

[Oguz Bektas]

hi,

found an issue here:
1 - create a new mp on running container
2 - mp gets added as pending, but disk is not created yet
3 - revert the pending mp
4 - this adds an unused disk, which doesn't exist and can't be removed via GUI

when i try to revert a new mp, 'guests:1'

this is what $mp is:

$VAR1 = {
'type' => 'volume',
'mp' => 'abc',
'volume' => 'guests:1'
};

so it passes your checks since it's defined and type is 'volume'

you could add another check to see if the disk is new, but then it might break the hotplug mount

On Tue, Nov 26, 2019 at 12:51:38PM +0100, Fabian Ebner wrote:
> This makes the behavior more similar to what we do for VM configs.
> If we have a pending change re-adding an unused volume as a mount point, then
> the unused volume will not show up anymore. And if we revert such a change, the
> volume will re-appear as an unused volume.
> Like this a user with a running container won't be able to re-add an unused
> volume multiple times via the web GUI.
> 
> Signed-off-by: Fabian Ebner <f.ebner at proxmox.com>
> ---
>  src/PVE/LXC/Config.pm | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/src/PVE/LXC/Config.pm b/src/PVE/LXC/Config.pm
> index ffc5911..c2ae166 100644
> --- a/src/PVE/LXC/Config.pm
> +++ b/src/PVE/LXC/Config.pm
> @@ -839,12 +839,14 @@ sub write_pct_config {
>      delete $conf->{snapstate}; # just to be sure
>  
>      my $volidlist = PVE::LXC::Config->get_vm_volumes($conf);
> +    push @$volidlist, @{PVE::LXC::Config->get_vm_volumes($conf->{pending})};
>      my $used_volids = {};
>      foreach my $vid (@$volidlist) {
>          $used_volids->{$vid} = 1;
>      }
>  
>      # remove 'unusedX' settings if the volume is still used
> +    # or if it's pending to be re-added
>      foreach my $key (keys %$conf) {
>          my $value = $conf->{$key};
>          if ($key =~ m/^unused/ && $used_volids->{$value}) {
> @@ -904,6 +906,15 @@ sub update_pct_config {
>      my $storage_cfg = PVE::Storage::config();
>  
>      foreach my $opt (@$revert) {
> +	# unused volume that was pending to become a mount point should show up again
> +	if ($opt =~ m/^mp(\d+)$/) {
> +	    my $mp = eval { $class->parse_ct_mountpoint($conf->{pending}->{$opt}); };
> +	    if (defined($mp) && $mp->{type} eq 'volume') {
> +		$class->add_unused_volume($conf, $mp->{volume})
> +		    if !$class->is_volume_in_use($conf, $conf->{pending}->{$opt}, 1, 0);
> +	    }
> +	}
> +
>  	delete $conf->{pending}->{$opt};
>  	$class->remove_from_pending_delete($conf, $opt); # also remove from deletion queue
>      }
> -- 
> 2.20.1
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>