Re: [pve-devel] [PATCH pve-storage 07/10] storage: volume_snapshot: add $running param

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

> Alexandre Derumier via pve-devel <pve-devel@lists.proxmox.com> hat am 04.07.2025 08:45 CEST geschrieben:
> This add a $running param to volume_snapshot,
> it can be used if some extra actions need to be done at the storage
> layer when the snapshot has already be done at qemu level.
> 
> Note: zfs && rbd plugins already used this param in create_base,
> but it was not implemented in volume_snapshot.

it used to be[0], but got dropped. it served as an early return and got
replaced by the "do snapshot with qemu" logic in qemu-server that we are
now improving once more ;)

those two sites have FIXMEs, and since create_base is only called
- for disks newly added to templates
- when creating a template (via conversion, cloning, restoring, ..)

and by definition template guests are never running and never write
to their disks/mountpoints, those $running parameters there can be
dropped.

0: https://git.proxmox.com/?p=pve-storage.git;a=commit;h=f5640e7d3be3068d34599512435954276d6e27f0
> 
> Signed-off-by: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
> ---
>  src/PVE/Storage.pm                   | 4 ++--
>  src/PVE/Storage/ESXiPlugin.pm        | 2 +-
>  src/PVE/Storage/ISCSIDirectPlugin.pm | 2 +-
>  src/PVE/Storage/LVMPlugin.pm         | 2 +-
>  src/PVE/Storage/LvmThinPlugin.pm     | 2 +-
>  src/PVE/Storage/PBSPlugin.pm         | 2 +-
>  src/PVE/Storage/Plugin.pm            | 2 +-
>  src/PVE/Storage/RBDPlugin.pm         | 2 +-
>  src/PVE/Storage/ZFSPoolPlugin.pm     | 2 +-
>  9 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
> index fe6eaf7..0396160 100755
> --- a/src/PVE/Storage.pm
> +++ b/src/PVE/Storage.pm
> @@ -449,13 +449,13 @@ sub volume_rollback_is_possible {
>  }
>  
>  sub volume_snapshot {
> -    my ($cfg, $volid, $snap) = @_;
> +    my ($cfg, $volid, $snap, $running) = @_;
>  
>      my ($storeid, $volname) = parse_volume_id($volid, 1);
>      if ($storeid) {
>          my $scfg = storage_config($cfg, $storeid);
>          my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
> -        return $plugin->volume_snapshot($scfg, $storeid, $volname, $snap);
> +        return $plugin->volume_snapshot($scfg, $storeid, $volname, $snap, $running);
>      } elsif ($volid =~ m|^(/.+)$| && -e $volid) {
>          die "snapshot file/device '$volid' is not possible\n";
>      } else {
> diff --git a/src/PVE/Storage/ESXiPlugin.pm b/src/PVE/Storage/ESXiPlugin.pm
> index ab5242d..e655d7b 100644
> --- a/src/PVE/Storage/ESXiPlugin.pm
> +++ b/src/PVE/Storage/ESXiPlugin.pm
> @@ -555,7 +555,7 @@ sub volume_size_info {
>  }
>  
>  sub volume_snapshot {
> -    my ($class, $scfg, $storeid, $volname, $snap) = @_;
> +    my ($class, $scfg, $storeid, $volname, $snap, $running) = @_;
>  
>      die "creating snapshots is not supported for $class\n";
>  }
> diff --git a/src/PVE/Storage/ISCSIDirectPlugin.pm b/src/PVE/Storage/ISCSIDirectPlugin.pm
> index 62e9026..93cfd3c 100644
> --- a/src/PVE/Storage/ISCSIDirectPlugin.pm
> +++ b/src/PVE/Storage/ISCSIDirectPlugin.pm
> @@ -232,7 +232,7 @@ sub volume_resize {
>  }
>  
>  sub volume_snapshot {
> -    my ($class, $scfg, $storeid, $volname, $snap) = @_;
> +    my ($class, $scfg, $storeid, $volname, $snap, $running) = @_;
>      die "volume snapshot is not possible on iscsi device\n";
>  }
>  
> diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm
> index 0b506c7..3d07260 100644
> --- a/src/PVE/Storage/LVMPlugin.pm
> +++ b/src/PVE/Storage/LVMPlugin.pm
> @@ -691,7 +691,7 @@ sub volume_size_info {
>  }
>  
>  sub volume_snapshot {
> -    my ($class, $scfg, $storeid, $volname, $snap) = @_;
> +    my ($class, $scfg, $storeid, $volname, $snap, $running) = @_;
>  
>      die "lvm snapshot is not implemented";
>  }
> diff --git a/src/PVE/Storage/LvmThinPlugin.pm b/src/PVE/Storage/LvmThinPlugin.pm
> index c244c91..e5df0b4 100644
> --- a/src/PVE/Storage/LvmThinPlugin.pm
> +++ b/src/PVE/Storage/LvmThinPlugin.pm
> @@ -339,7 +339,7 @@ sub create_base {
>  # sub volume_resize {} reuse code from parent class
>  
>  sub volume_snapshot {
> -    my ($class, $scfg, $storeid, $volname, $snap) = @_;
> +    my ($class, $scfg, $storeid, $volname, $snap, $running) = @_;
>  
>      my $vg = $scfg->{vgname};
>      my $snapvol = "snap_${volname}_$snap";
> diff --git a/src/PVE/Storage/PBSPlugin.pm b/src/PVE/Storage/PBSPlugin.pm
> index 00170f5..45edc46 100644
> --- a/src/PVE/Storage/PBSPlugin.pm
> +++ b/src/PVE/Storage/PBSPlugin.pm
> @@ -966,7 +966,7 @@ sub volume_resize {
>  }
>  
>  sub volume_snapshot {
> -    my ($class, $scfg, $storeid, $volname, $snap) = @_;
> +    my ($class, $scfg, $storeid, $volname, $snap, $running) = @_;
>      die "volume snapshot is not possible on pbs device";
>  }
>  
> diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
> index 81443aa..88c30c2 100644
> --- a/src/PVE/Storage/Plugin.pm
> +++ b/src/PVE/Storage/Plugin.pm
> @@ -1155,7 +1155,7 @@ sub volume_resize {
>  }
>  
>  sub volume_snapshot {
> -    my ($class, $scfg, $storeid, $volname, $snap) = @_;
> +    my ($class, $scfg, $storeid, $volname, $snap, $running) = @_;
>  
>      die "can't snapshot this image format\n" if $volname !~ m/\.(qcow2|qed)$/;
>  
> diff --git a/src/PVE/Storage/RBDPlugin.pm b/src/PVE/Storage/RBDPlugin.pm
> index ce7db50..883b0e4 100644
> --- a/src/PVE/Storage/RBDPlugin.pm
> +++ b/src/PVE/Storage/RBDPlugin.pm
> @@ -868,7 +868,7 @@ sub volume_resize {
>  }
>  
>  sub volume_snapshot {
> -    my ($class, $scfg, $storeid, $volname, $snap) = @_;
> +    my ($class, $scfg, $storeid, $volname, $snap, $running) = @_;
>  
>      my ($vtype, $name, $vmid) = $class->parse_volname($volname);
>  
> diff --git a/src/PVE/Storage/ZFSPoolPlugin.pm b/src/PVE/Storage/ZFSPoolPlugin.pm
> index 979cf2c..9cdfa68 100644
> --- a/src/PVE/Storage/ZFSPoolPlugin.pm
> +++ b/src/PVE/Storage/ZFSPoolPlugin.pm
> @@ -480,7 +480,7 @@ sub volume_size_info {
>  }
>  
>  sub volume_snapshot {
> -    my ($class, $scfg, $storeid, $volname, $snap) = @_;
> +    my ($class, $scfg, $storeid, $volname, $snap, $running) = @_;
>  
>      my $vname = ($class->parse_volname($volname))[1];
>  
> -- 
> 2.39.5


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel