On 5/13/25 10:03, Michael Köppl wrote:
The value in $conf->{opt} is not necessarily a volume ID. To ensure that
a valid volume ID is used, it is retrieved by calling parse_volume().
The patch summary and/or description should contain more information
about the whereabouts, e.g. for here:
config: apply_pending: get unused volid through parse_volume()
nit: If there are no functional changes here, it's always nice to state
that in the commit message, so it's clear from the start ;).
Co-authored-by: Stefan Hrdlicka
Signed-off-by: Michael Köppl <m.koe...@proxmox.com>
---
src/PVE/LXC/Config.pm | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/PVE/LXC/Config.pm b/src/PVE/LXC/Config.pm
index 0740e8c..7f76b34 100644
--- a/src/PVE/LXC/Config.pm
+++ b/src/PVE/LXC/Config.pm
@@ -1557,15 +1557,16 @@ sub vmconfig_apply_pending {
foreach my $opt (sort keys %$pending_delete_hash) {
next if $selection && !$selection->{$opt};
eval {
+ my $mp = $class->parse_volume($opt, $conf->{$opt});
+
if ($opt =~ m/^mp(\d+)$/) {
- my $mp = $class->parse_volume($opt, $conf->{$opt});
if ($mp->{type} eq 'volume') {
$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);
+ if !$class->is_volume_in_use($conf, $mp->{volume}, 1, 1);
I was a little confused why $mp->{volume} needs to be passed here but at
other places (like is_volume_in_use for mp* above) $conf->{$opt} is fine.
AFAICS that is because valid_volume_keys(...) for the LXC config defines
rootfs and mp* as valid volume keys, but not unused*, which is then used
by foreach_volume(...) in the is_volume_in_use(...) helper, where it can
be found to be used.
"Coincidentally" $conf->{$opt} and $mp->{volume} have the same value
here, so this patch doesn't change the behavior here AFAICS, but a
$class->parse_volume(...) makes it more future proof if anything changes
here, because that helper is aware of unused and will correctly parse it
with $parse_ct_mountpoint_full->($class, $unused_desc, ...).
It would be great if this was better documented here, but that could be
a follow-up as it isn't really related to the series itself.
} elsif ($opt =~ m/^net(\d+)$/) {
if ($have_sdn) {
my $net = $class->parse_lxc_network($conf->{$opt});
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
