[PATCH-SERIES storage 0/2] fix #7280: lvm plugin: rename: activate qcow2 volume to read snapshot info

[PATCH-SERIES storage 0/2] fix #7280: lvm plugin: rename: activate qcow2 volume to read snapshot info

[Fiona Ebner]

See patch 1/2 for more information.
Patch 2/2 just improves an error message.

Storage:

Fiona Ebner (2):
  fix #7280: lvm plugin: rename: activate qcow2 volume to read snapshot
    info
  lvm plugin: rename: improve error message

 src/PVE/Storage/LVMPlugin.pm | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)


Summary over all repositories:
  1 files changed, 3 insertions(+), 1 deletions(-)

-- 
Generated by git-murpp 0.5.0

[PATCH storage 1/2] fix #7280: lvm plugin: rename: activate qcow2 volume to read snapshot info

[Fiona Ebner]

Other callers of volume_snapshot_info() already do this. For other
storages and formats, activation is not necessary to rename it, so the
activation is only added here, rather than in the qemu-server, and, for
consistency, container API endpoints.

There is no need for deactivation after the rename operation. In fact,
in case of reassign to a running target VM with SCSI or VirtIO block,
the renamed disk is even hotplugged, so deactivation would be wrong.

Note that the order of $scfg and $storeid really is switched when
comparing volume_snapshot_info() and activate_volume().

Technically, the check for snapshots would not be needed, as
reassigning a volume referenced in a snapshot is already prohibited by
the guest API endpoints. But additional checks on the storage layer
don't hurt and there might be other users of rename_volume() in the
future.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/Storage/LVMPlugin.pm | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm
index 32a8339..f9349de 100644
--- a/src/PVE/Storage/LVMPlugin.pm
+++ b/src/PVE/Storage/LVMPlugin.pm
@@ -1439,6 +1439,7 @@ sub rename_volume {
     ) = $class->parse_volname($source_volname);
 
     if ($format eq 'qcow2') {
+        $class->activate_volume($storeid, $scfg, $source_volname);
         my $snapshots = $class->volume_snapshot_info($scfg, $storeid, $source_volname);
         die "we can't rename volume if external snapshot exists" if $snapshots->{current}->{parent};
     }
-- 
2.47.3

[PATCH storage 2/2] lvm plugin: rename: improve error message

[Fiona Ebner]

Avoid the use of 'we', mention the volume in question and add a
newline.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/Storage/LVMPlugin.pm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm
index f9349de..8392c89 100644
--- a/src/PVE/Storage/LVMPlugin.pm
+++ b/src/PVE/Storage/LVMPlugin.pm
@@ -1441,7 +1441,8 @@ sub rename_volume {
     if ($format eq 'qcow2') {
         $class->activate_volume($storeid, $scfg, $source_volname);
         my $snapshots = $class->volume_snapshot_info($scfg, $storeid, $source_volname);
-        die "we can't rename volume if external snapshot exists" if $snapshots->{current}->{parent};
+        die "can't rename volume '$source_volname' - external snapshot exists\n"
+            if $snapshots->{current}->{parent};
     }
 
     $target_volname = $class->find_free_diskname($storeid, $scfg, $target_vmid, $format)
-- 
2.47.3

superseded: [PATCH-SERIES storage 0/2] fix #7280: lvm plugin: rename: activate qcow2 volume to read snapshot info

[Fiona Ebner]

Superseded by a larger series fixing more issues:
https://lore.proxmox.com/pve-devel/20260331115011.102276-1-f.ebner@proxmox.com/T/

Am 06.02.26 um 12:43 PM schrieb Fiona Ebner:
> See patch 1/2 for more information.
> Patch 2/2 just improves an error message.
> 
> Storage:
> 
> Fiona Ebner (2):
>   fix #7280: lvm plugin: rename: activate qcow2 volume to read snapshot
>     info
>   lvm plugin: rename: improve error message
> 
>  src/PVE/Storage/LVMPlugin.pm | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> 
> Summary over all repositories:
>   1 files changed, 3 insertions(+), 1 deletions(-)
>