[PATCH storage v2 6/7] fix #7143: ensure qcow2 volume (de)activation is limited to target volume chain

[Fiona Ebner <f.ebner@proxmox.com>]

When using tag-based (de)activation for qcow2 volumes, volumes with
the same name (or snapshot volumes based on a volume with the same
name) on other storages are affected too. Including the storage ID in
the tag would be a possibility, but would mean updating the tags after
moving the volume and would break moving volumes or renaming outside
the Proxmox VE stack and would be rather involved to do for already
existing images.

Use a regex with '--select' instead of using tags to select the
volumes in the qcow2 volume chain.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

New in v2.

 src/PVE/Storage/LVMPlugin.pm | 66 ++++++++++++++++++++++++++++--------
 1 file changed, 51 insertions(+), 15 deletions(-)

diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm
index 79a52c5..b4cb0c5 100644
--- a/src/PVE/Storage/LVMPlugin.pm
+++ b/src/PVE/Storage/LVMPlugin.pm
@@ -713,7 +713,12 @@ my sub alloc_lvm_image {
     die "not enough free space ($free < $size)\n" if $free < $size;
 
     my $tags = ["pve-vm-$vmid"];
-    #tags all snapshots volumes with the main volume tag for easier activation of the whole group
+    # TODO PVE 10 - stop setting the volume name as a tag? It was initially used for (de)activation,
+    # but there are false positives if there are multiple storages, see bug #7143. Including the
+    # storage ID as a fix would've made moving disks more involved and break manual moving or
+    # renaming. Instead (de)activation switched to using '--select'. The rename_volume() method
+    # needs to be adapted as well.
+    # tags all snapshots volumes with the main volume tag
     push @$tags, "\@pve-$name" if $fmt eq 'qcow2';
     lvcreate($vg, $name, $lvmsize, $tags);
 
@@ -952,21 +957,53 @@ sub deactivate_storage {
     run_command($cmd, errmsg => "can't deactivate VG '$scfg->{vgname}'");
 }
 
+=head3 get_activate_volume_target_opts()
+
+    my ($target_opts, $target_str) =
+        get_activate_volume_target_opts($class, $scfg, $path, $volname);
+
+Returns C<$target_opts>, which is an array reference with parameters for C<lvchange> for selecting
+the volume given by C<($path,$volname)>, or volume chain for C<qcow2>. Also returns a description of
+the target C<$target_str> that can be used for log or error messages.
+
+=cut
+
+my sub get_activate_volume_target_opts {
+    my ($class, $scfg, $path, $volname) = @_;
+
+    my ($name, $format) = ($class->parse_volname($volname))[1,6];
+
+    my ($target_opts, $target_str);
+
+    if ($format eq 'qcow2') {
+        my $vg = $scfg->{vgname};
+        $name =~ s/\.qcow2$//;
+
+        my $filter = "vg_name = $vg";
+        $filter .= "&& lv_name =~ '^(${name}|snap_${name}_.+).qcow2\$'";
+
+        $target_opts = ['--select', $filter];
+        $target_str = "volume chain for LV '$path'";
+    } else {
+        $target_opts = [$path];
+        $target_str = "LV '$path'";
+    }
+
+    return ($target_opts, $target_str);
+}
+
 sub activate_volume {
     my ($class, $storeid, $scfg, $volname, $snapname, $cache) = @_;
 
     my $path = $class->path($scfg, $volname, $storeid, $snapname);
 
-    #activate volume && all snapshots volumes by tag
-    my ($vtype, $name, $vmid, $basename, $basevmid, $isBase, $format) =
-        $class->parse_volname($volname);
+    my ($target_opts, $target_str) =
+        get_activate_volume_target_opts($class, $scfg, $path, $volname);
 
-    $path = "\@pve-$name" if $format eq 'qcow2';
-
-    my $cmd = ['/sbin/lvchange', '-aey', $path];
-    run_command($cmd, errmsg => "can't activate LV '$path'");
-    $cmd = ['/sbin/lvchange', '--refresh', $path];
-    run_command($cmd, errmsg => "can't refresh LV '$path' for activation");
+    my $cmd = ['/sbin/lvchange', '-aey', $target_opts->@*];
+    run_command($cmd, errmsg => "can't activate $target_str");
+    $cmd = ['/sbin/lvchange', '--refresh', $target_opts->@*];
+    run_command($cmd, errmsg => "can't refresh $target_str for activation");
 }
 
 sub deactivate_volume {
@@ -975,12 +1012,11 @@ sub deactivate_volume {
     my $path = $class->path($scfg, $volname, $storeid, $snapname);
     return if !-b $path;
 
-    my ($vtype, $name, $vmid, $basename, $basevmid, $isBase, $format) =
-        $class->parse_volname($volname);
-    $path = "\@pve-$name" if $format eq 'qcow2';
+    my ($target_opts, $target_str) =
+        get_activate_volume_target_opts($class, $scfg, $path, $volname);
 
-    my $cmd = ['/sbin/lvchange', '-aln', $path];
-    run_command($cmd, errmsg => "can't deactivate LV '$path'");
+    my $cmd = ['/sbin/lvchange', '-aln', $target_opts->@*];
+    run_command($cmd, errmsg => "can't deactivate $target_str");
 }
 
 sub volume_resize {
-- 
2.47.3