Even though the value in $conf->{$opt} contains a volume ID for unused mount points at the moment, this is not guaranteed to be true in the future. To ensure that a valid volume ID is used here, run call parse_volume() first.
No functional change is intended here as the values of $conf->{$opt} and $mp->{volume} are identical for unused mount points at the moment. Also add FIXME for the is_volume_in_use() check for the branch that handles mountpoint volumes that are in use. Currently, the check only works by accident. The value in $conf->{$opt} isn’t just a volume ID - it contains other things as well (such as mp=<path>). However, the check only considers 'rootfs' and entries like 'mp0', 'mp1', etc. as valid volume IDs. As a result, the volume is not detected to be in use. Using a valid volume id with the current implementation, the check would always report that the volume is in use and the mountpoint volume would not be deleted. The correct behavior should use a valid volume id to ensure that the volume can be safely removed. Signed-off-by: Michael Köppl <m.koe...@proxmox.com> --- src/PVE/LXC/Config.pm | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/PVE/LXC/Config.pm b/src/PVE/LXC/Config.pm index 0740e8c..052cc27 100644 --- a/src/PVE/LXC/Config.pm +++ b/src/PVE/LXC/Config.pm @@ -1473,8 +1473,9 @@ sub vmconfig_hotplug_pending { if ($LXC_FASTPLUG_OPTIONS->{$opt}) { # pass } elsif ($opt =~ m/^unused(\d+)$/) { - PVE::LXC::delete_mountpoint_volume($storecfg, $vmid, $conf->{$opt}) - if !$class->is_volume_in_use($conf, $conf->{$opt}, 1, 1); + my $volid = $class->parse_volume($opt, $conf->{$opt})->{volume}; + PVE::LXC::delete_mountpoint_volume($storecfg, $vmid, $volid) + if !$class->is_volume_in_use($conf, $volid, 1, 1); } elsif ($opt eq 'swap') { $hotplug_memory->(undef, 0); } elsif ($opt eq 'cpulimit') { @@ -1560,12 +1561,14 @@ sub vmconfig_apply_pending { if ($opt =~ m/^mp(\d+)$/) { my $mp = $class->parse_volume($opt, $conf->{$opt}); if ($mp->{type} eq 'volume') { + # FIXME: use $mp->{volume} for is_volume_in_use, fix conditions for check $class->add_unused_volume($conf, $mp->{volume}) if !$class->is_volume_in_use($conf, $conf->{$opt}, 1, 1); } } elsif ($opt =~ m/^unused(\d+)$/) { - PVE::LXC::delete_mountpoint_volume($storecfg, $vmid, $conf->{$opt}) - if !$class->is_volume_in_use($conf, $conf->{$opt}, 1, 1); + my $volid = $class->parse_volume($opt, $conf->{$opt})->{volume}; + PVE::LXC::delete_mountpoint_volume($storecfg, $vmid, $volid) + if !$class->is_volume_in_use($conf, $volid, 1, 1); } elsif ($opt =~ m/^net(\d+)$/) { if ($have_sdn) { my $net = $class->parse_lxc_network($conf->{$opt}); -- 2.39.5 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel