[pve-devel] [PATCH container v7 3/4] config: ensure valid volid through parse_volume

[Michael Köppl]

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.koeppl at 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