[pve-devel] [PATCH v1 pve-storage 7/8] pluginbase: document volume operations

Fabian Grünbichler f.gruenbichler at proxmox.com
Mon Mar 31 17:12:49 CEST 2025


On March 26, 2025 3:20 pm, Max Carrara wrote:
> Add docstrings for the following methods:
> - list_volumes
> - get_volume_attribute
> - update_volume_attribute
> - volume_size_info
> - volume_resize
> - volume_snapshot
> - volume_snapshot_info
> - volume_rollback_is_possible
> - volume_snapshot_rollback
> - volume_snapshot_delete
> - volume_snapshot_needs_fsfreeze
> - storage_can_replicate
> - volume_has_feature
> - map_volume
> - unmap_volume
> - activate_volume
> - deactivate_volume
> - rename_volume
> - prune_backups
> 
> Signed-off-by: Max Carrara <m.carrara at proxmox.com>
> Co-authored-by: Maximiliano Sandoval <m.sandoval at proxmox.com>
> ---
>  src/PVE/Storage/PluginBase.pm | 401 ++++++++++++++++++++++++++++++++--
>  1 file changed, 388 insertions(+), 13 deletions(-)
> 
> diff --git a/src/PVE/Storage/PluginBase.pm b/src/PVE/Storage/PluginBase.pm
> index 37b1471..a1bfc8d 100644
> --- a/src/PVE/Storage/PluginBase.pm
> +++ b/src/PVE/Storage/PluginBase.pm
> @@ -861,108 +861,483 @@ sub free_image {
>  
>  =cut
>  
> +=head3 $plugin->list_volumes($storeid, \%scfg, $vmid, \@content_types)
> +
> +B<REQUIRED:> Must be implemented in every storage plugin.
> +
> +Returns a list of volumes for the given guest whose content type is within the

this is wrong - what if $vmid is not set? or if content type is snippets
or one of other ones that don't have an associated guest/owner? or what
if there is no $vmid given, as in the example below?

> +given C<\@content_types>. If C<\@content_types> is empty, no volumes will be
> +returned. See C<L<< plugindata()|/"$plugin->plugindata()" >>> for all content types.

here we are actually talking about content types for once (and this also
translates from content type "images" and "rootdir" to volume type
"images"! in PVE::Plugin!).

> +
> +    # List all backups for a guest
> +    my $backups = $plugin->list_volumes($storeid, $scfg, $vmid, ['backup']);
> +
> +    # List all containers and virtual machines on the storage
> +    my $guests = $plugin->list_volumes($storeid, $scfg, undef, ['rootdir', 'images'])

eh, this is a bad example? it doesn't list the guests, it lists their
volumes..

> +
> +The returned listref of hashrefs has the following structure:
> +
> +    [
> +	{
> +	    content => "images",
> +	    ctime => "1702298038", # creation time as unix timestamp
> +	    format => "raw",
> +	    size => 9663676416, # in bytes!
> +	    vmid => 102,
> +	    volid => "local-lvm:vm-102-disk-0",
> +	},
> +	# [...]
> +    ]
> +
> +Backups will also contain additional keys:
> +
> +    [
> +	{
> +	    content => "backup",
> +	    ctime => 1742405070, # creation time as unix timestamp
> +	    format => "tar.zst",
> +	    notes => "...", # comment that was entered when backup was created
> +	    size => 328906840, # in bytes!
> +	    subtype => "lxc", # "lxc" for containers, "qemu" for VMs
> +	    vmid => 106,
> +	    volid => "local:backup/vzdump-lxc-106-2025_03_19-18_24_30.tar.zst",
> +	},
> +	# [...]
> +    ]

soooo.. what is the interface here again? -> needs a (complete) schema
for the returned type, else how am I supposed to implement this?

> +
> +=cut
> +
>  sub list_volumes {
>      my ($class, $storeid, $scfg, $vmid, $content_types) = @_;
>      croak "implement me in sub-class\n";
>  }
>  
> -# Returns undef if the attribute is not supported for the volume.
> -# Should die if there is an error fetching the attribute.
> -# Possible attributes:
> -# notes     - user-provided comments/notes.
> -# protected - not to be removed by free_image, and for backups, ignored when pruning.
> +=head3 $plugin->get_volume_attribute(\%scfg, $storeid, $volname, $attribute)
> +
> +=head3 $plugin->get_volume_attribute(...)
> +
> +B<REQUIRED:> Must be implemented in every storage plugin.
> +
> +Returns the value of the given C<$attribute> for a volume. If the attribute isn't
> +supported for the volume, returns C<undef>.
> +
> +C<die>s if there is an error fetching the attribute.
> +
> +B<Possible attributes:>
> +
> +=over
> +
> +=item * C<notes> (returns scalar)

scalar *string* ?
> +
> +User-provided comments / notes.
> +
> +=item * C<protected> (returns scalar)

scalar *boolean* ?

> +
> +When set to C<1>, the volume must not be removed by C<L<< free_image()|/"$plugin->free_image(...)" >>>.
> +Backups with C<protected> set to C<1> are ignored when pruning.

Backup volumes .. when calculating which backup volumes to prune.

> +
> +=back
> +
> +=cut
> +
>  sub get_volume_attribute {
>      my ($class, $scfg, $storeid, $volname, $attribute) = @_;
>      croak "implement me in sub-class\n";
>  }
>  
> -# Dies if the attribute is not supported for the volume.
> +=head3 $plugin->update_volume_attribute(\%scfg, $storeid, $volname, $attribute, $value)
> +
> +=head3 $plugin->update_volume_attribute(...)
> +
> +B<REQUIRED:> Must be implemented in every storage plugin.
> +
> +Sets the value of the given C<$attribute> for a volume to C<$value>.
> +
> +C<die>s if the attribute isn't supported for the volume (or storage).
> +
> +For a list of supported attributes, see C<L<< get_volume_attribute()|/"$plugin->get_volume_attribute(...)" >>>.
> +
> +=cut
> +
>  sub update_volume_attribute {
>      my ($class, $scfg, $storeid, $volname, $attribute, $value) = @_;
>      croak "implement me in sub-class\n";
>  }
>  
> +=head3 $plugin->volume_size_info(\%scfg, $storeid, $volname [, $timeout])
> +
> +B<REQUIRED:> Must be implemented in every storage plugin.
> +
> +Returns information about the given volume's size. In scalar context, this returns
> +just the volume's size in bytes:
> +
> +    my $size = $plugin->volume_size_info($scfg, $storeid, $volname, $timeout)
> +
> +In list context, returns an array with the following structure:
> +
> +    my ($size, $format, $used, $parent, $ctime) = $plugin->volume_size_info(
> +	$scfg, $storeid, $volname, $timeout
> +    )
> +
> +where C<$size> is the size of the volume in bytes, C<$format> one of the possible
> +L<< formats listed in C<plugindata()>|/"$plugin->plugindata()" >>, C<$used> the
> +amount of space used in bytes, C<$parent> the (optional) name of the base volume
> +and C<$ctime> the creation time as unix timestamp.
> +
> +Optionally, a C<$timeout> may be provided after which the method should C<die>.
> +This timeout is best passed to other helpers which already support timeouts,
> +such as C<L<< PVE::Tools::run_command|PVE::Tools/run_command >>>.
> +
> +See also the C<L<< PVE::Storage::Plugin::file_size_info|PVE::Storage::Plugin/file_size_info >>> helper.

PVE::Storage::file_size_info (without Plugin::)!

> +
> +=cut
> +
>  sub volume_size_info {
>      my ($class, $scfg, $storeid, $volname, $timeout) = @_;
>      croak "implement me in sub-class\n";
>  }
>  
> +=head3 $plugin->volume_resize(\%scfg, $storeid, $volname, $size [, $running])
> +
> +B<REQUIRED:> Must be implemented in every storage plugin.
> +
> +Resizes a volume to the new C<$size> in bytes. Optionally, the guest that owns
> +the volume may be C<$running> (= C<1>).

the second sentence doesn't make any sense.

C<$running> indicates the guest is currently running.

but what does that mean/why should a plugin care?

> +
> +C<die>s in case of errors, or if the underlying storage implementation or the
> +volume's format doesn't support resizing.
> +
> +This function should not return any value. In previous versions the returned
> +value would be used to determine the new size of the volume or whether the
> +operation succeeded.

so since this documents the new version.. shouldn't we just drop the
last part?

> +
> +=cut
> +
>  sub volume_resize {
>      my ($class, $scfg, $storeid, $volname, $size, $running) = @_;
>      croak "implement me in sub-class\n";
>  }
>  
> +=head3 $plugin->volume_snapshot(\%scfg, $storeid, $volname, $snap)
> +
> +B<OPTIONAL:> May be implemented if the underlying storage supports snapshots.
> +
> +Takes a snapshot of a volume and gives it the name provided by C<$snap>.

this sounds like this is a two-step process..

Takes a snapshot called C<$snap> of the volume C<$volname>.

> +
> +C<die>s if the underlying storrage doesn't support snapshots or an error
> +occurs while taking a snapshot.

s/a/the
s/storrage/storage

> +
> +=cut
> +
>  sub volume_snapshot {
>      my ($class, $scfg, $storeid, $volname, $snap) = @_;
>      croak "implement me in sub-class\n";
>  }
>  
> -# Returns a hash with the snapshot names as keys and the following data:
> -# id        - Unique id to distinguish different snapshots even if the have the same name.
> -# timestamp - Creation time of the snapshot (seconds since epoch).
> -# Returns an empty hash if the volume does not exist.
> +=head3 $plugin->volume_snapshot_info(\%scfg, $storeid, $volname)
> +
> +Returns a hashref with the snapshot names as keys and the following structure:
> +
> +    {
> +        my_snapshot => {
> +            id => "01b7e668-58b4-6f42-9a5e-1944c2855c84",  # Unique id to distinguish different snapshots with the same name.
> +            timestamp => "1729841807",  # Creation time of the snapshot (seconds since epoch).
> +        },
> +	# [...]
> +    }
> +
> +Returns an empty hashref if the volume does not exist.

or dies if retrieving the snapshot information for the given volume
failed. ?

> +
> +=cut
> +
>  sub volume_snapshot_info {
>      my ($class, $scfg, $storeid, $volname) = @_;
>      croak "implement me in sub-class\n";
>  }
>  
> -# Asserts that a rollback to $snap on $volname is possible.
> -# If certain snapshots are preventing the rollback and $blockers is an array
> -# reference, the snapshot names can be pushed onto $blockers prior to dying.
> +=head3 $plugin->volume_rollback_is_possible(\%scfg, $storeid, $volname, $snap [, \@blockers])
> +
> +=head3 $plugin->volume_rollback_is_possible(...)
> +
> +B<OPTIONAL:> May be implemented if the underlying storage supports snapshots.
> +
> +Asserts whether a rollback to C<$snap> on C<$volname> is possible, C<die>s if
> +it isn't.
> +
> +Optionally, C<\@blockers> may be provided, which may contain the names of
> +snapshots that are preventing the rollback. Should any such snapshots exist,
> +they should be pushed to this listref pior to C<die>-ing. The caller may then
> +use this listref when handling errors.

If the optional paramater C<\@blockers> is provided, the names of any
snapshots blocking the rollback should be added to this listref prior to
C<die>-ing.

> +
> +=cut
> +
>  sub volume_rollback_is_possible {
>      my ($class, $scfg, $storeid, $volname, $snap, $blockers) = @_;
>      croak "implement me in sub-class\n";
>  }
>  
> +=head3 $plugin->volume_snapshot_rollback(\%scfg, $storeid, $volname, $snap)
> +
> +Performs a rollback to the given C<$snap>shot on C<$volname>.
> +
> +C<die>s in case of errors.
> +
> +=cut
> +
>  sub volume_snapshot_rollback {
>      my ($class, $scfg, $storeid, $volname, $snap) = @_;
>      croak "implement me in sub-class\n";
>  }
>  
> +=head3 $plugin->volume_snapshot_delete(\%scfg, $storeid, $volname, $snap [, $running])
> +
> +Deletes the C<$snap>shot of C<$volname>.
> +
> +C<die>s in case of errors.
> +
> +Optionally, the guest that owns the given volume may be C<$running> (= C<1>).

Again, this is phrased weird *and* gives no information to a potential
implementor of this API.. why should a plugin care?

> +
> +B<Deprecated:> The C<$running> parameter is deprecated and will be removed on the
> +next C<APIAGE> reset.

and this makes it even more confusing!

> +
> +=cut
> +
>  sub volume_snapshot_delete {
>      my ($class, $scfg, $storeid, $volname, $snap, $running) = @_;
>      croak "implement me in sub-class\n";
>  }
>  
> +=head3 $plugin->volume_snapshot_needs_fsfreeze()
> +
> +Returns whether filesystems on top of the volume need to flush their journal for
> +consistency before a snapshot is taken. See L<fsfreeze(8)>.
> +
> +This is needed for container mountpoints.

this doesn't really tell me what I should do here either.. (and it's
also a really ugly interace that we seemingly introduced because RBD
snapshots cannot be mounted RO otherwise?? does this really need to be
part of the plugin interface? Oo)

> +
> +=cut
> +
>  sub volume_snapshot_needs_fsfreeze {
>      croak "implement me in sub-class\n";
>  }
>  
> +=head3 $plugin->storage_can_replicate(\%scfg, $storeid, $format)
> +
> +Returns whether volumes in a given C<$format> support replication.
> +
> +See C<L<< plugindata()|/"$plugin->plugindata()" >>> for all disk formats.
> +

should probably note that replication is limited to ZFS at the moment in
any case?

> +=cut
> +
>  sub storage_can_replicate {
>      my ($class, $scfg, $storeid, $format) = @_;
>      croak "implement me in sub-class\n";
>  }
>  
> +=head3 $plugin->volume_has_feature(\%scfg, $feature, $storeid, $volname, $snapname [, $running, \%opts])
> +
> +=head3 $plugin->volume_has_feature(...)
> +
> +B<REQUIRED:> Must be implemented in every storage plugin.
> +
> +Checks whether a volume C<$volname> or its snapshot C<$snapname> supports the
> +given C<$feature>, returning C<1> if it does and C<undef> otherwise. The guest
> +owning the volume may optionally be C<$running>.

that last sentence again doesn't make any sense the way it is phrased..

> +
> +C<$feature> may be one of the following:
> +
> +    clone      # linked clone is possible
> +    copy       # full clone is possible

actually, this is "bit-wise copying from" is possible, right?

> +    replicate  # replication is possible
> +    snapshot   # taking a snapshot is possible
> +    sparseinit # volume is sparsely initialized
> +    template   # conversion to base image is possible
> +    rename     # renaming volumes is possible
> +
> +Which features are available under which circumstances depends on multiple
> +factors, such as the underlying storage implementation, the format used, etc.

*and whether the corresponding guest is currently running*, which is the
whole point of that parameter ;)

this really needs to explain what the values for those feature keys
mean..

> +It's best to check out C<L<PVE::Storage::Plugin>> or C<L<PVE::Storage::ZFSPoolPlugin>>
> +for examples on how to handle features.
> +
> +Additional keys are given in C<\%opts>:
> +
> +=over
> +
> +=item * C<< valid_target_formats => [...] >>
> +
> +Listref of formats for the target of a copy/clone operation that the caller
> +could work with. The format of the given volume is always considered valid and
> +if no list is specified, all formats are considered valid.
> +
> +=back
> +
> +=cut
> +
>  sub volume_has_feature {
>      my ($class, $scfg, $feature, $storeid, $volname, $snapname, $running, $opts) = @_;
>      croak "implement me in sub-class\n";
>  }
>  
> +=head3 $plugin->map_volume($storeid, \%scfg, $volname [, $snapname])
> +
> +B<OPTIONAL:> May be implemented in a storage plugin.
> +
> +Maps the device asscoiated with a volume or a volume's snapshot to a filesystem

associated

> +path, returning the path on completion. This method is used by containers.

returning the path. (obviously on completion, how else?)

it's not only used by containers, so maybe drop that part and instead
note that it only needs to be implemented if `path` doesn't (always)
return such a path anyway.

> +
> +C<die>s in case of errors.
> +
> +C<L<< unmap_volume()|/"$plugin->unmap_volume(...)" >>> can be used to declare
> +how the device should be unmapped.

maybe just say that if you implement map, you should also implement
unmap?

> +
> +=cut
> +
>  sub map_volume {
>      my ($class, $storeid, $scfg, $volname, $snapname) = @_;
>      croak "implement me in sub-class\n";
>  }
>  
> +=head3 $plugin->unmap_volume($storeid, \%scfg, $volname [, $snapname])
> +
> +B<OPTIONAL:> May be implemented in a storage plugin.
> +
> +Unmaps the device associated to a volume or a volume's snapshot.
> +
> +C<die>s in case of errors.
> +
> +=cut
> +
>  sub unmap_volume {
>      my ($class, $storeid, $scfg, $volname, $snapname) = @_;
>      croak "implement me in sub-class\n";
>  }
>  
> +=head3 $plugin->activate_volume($storeid, \%scfg, $volname, $snapname [, \%cache])
> +
> +=head3 $plugin->activate_volume(...)
> +
> +B<REQUIRED:> Must be implemented in every storage plugin.
> +
> +Activates a volume or its associated snapshot, making it available to the
> +system for further use. For example, this could mean activating an LVM volume,
> +mounting a ZFS dataset, checking whether the volume's file path exists, etc.
> +
> +C<die>s in case of errors or if an operation is not supported.
> +
> +If this isn't needed, the method should simply be a no-op.

If no activation is needed, 

> +
> +This method may reuse L<< cached information via C<\%cache>|/"CACHING EXPENSIVE OPERATIONS" >>.
> +
> +=cut
> +
>  sub activate_volume {
>      my ($class, $storeid, $scfg, $volname, $snapname, $cache) = @_;
>      croak "implement me in sub-class\n";
>  }
>  
> +=head3 $plugin->deactivate_volume($storeid, \%scfg, $volname, $snapname [, \%cache])
> +
> +=head3 deactivate_volume(...)
> +
> +B<REQUIRED:> Must be implemented in every storage plugin.
> +
> +Deactivates a volume or its associated snapshot, making it unavailable to
> +the system. For example, this could mean deactivating an LVM volume,
> +unmapping a Ceph/RBD device, etc.
> +
> +If this isn't needed, the method should simply be a no-op.

If deactivation is not needed/not possible,

should we note here that deactivation will not happen for every
activation, but only when we know for sure that the volume is no longer
needed, such as before it is removed or when its owner is migrated to
another node?

> +
> +This method may reuse L<< cached information via C<\%cache>|/"CACHING EXPENSIVE OPERATIONS" >>.
> +
> +=cut
> +
>  sub deactivate_volume {
>      my ($class, $storeid, $scfg, $volname, $snapname, $cache) = @_;
>      croak "implement me in sub-class\n";
>  }
>  
> +=head3 $plugin->rename_volume(...)
> +
> +=head3 $plugin->rename_volume(\%scfg, $storeid, $source_volname, $target_vmid, $target_volname)
> +
> +B<OPTIONAL:> May be implemented in a storage plugin.
> +
> +Renames the volume given by C<$source_volname> to C<$target_volname> and assigns
> +it to the guest C<$target_vmid>. Returns the volume ID of the renamed volume.
> +
> +This method is needed for the I<Change Owner> feature.
> +
> +C<die>s if the rename failed.
> +
> +=cut
> +
>  sub rename_volume {
>      my ($class, $scfg, $storeid, $source_volname, $target_vmid, $target_volname) = @_;
>      croak "implement me in sub-class\n";
>  }
>  
> +=head3 $plugin->prune_backups(\%scfg, $storeid [, \%keep, $vmid, $type, $dryrun, \&logfunc])
> +
> +=head3 $plugin->prune_backups(...)
> +
> +Export a volume into a file handle as a stream with a desired format.

copy-paste mistake? ;)

> +
> +C<die>s if there are (grave) problems while pruning.
> +
> +This method may take several optional parameters:
> +
> +=over
> +
> +=item * C<< \%keep >>
> +
> +A hashref containing backup retention policies. It has the following structure:
> +
> +    {
> +	'keep-all'     => 1 # (optional) Whether to keep all backups.
> +	                    # Conflicts with the other options when true.
> +	'keep-last'    => N # (optional) Keep the last N backups.
> +	'keep-hourly'  => N # (optional) Keep backups for the last N different hours.
> +			    # If there is more than one backup for a single
> +			    # hour, only the latest one is kept.
> +	'keep-daily'   => N # (optional) Keep backups for the last N different days.
> +			    # If there is more than one backup for a single
> +			    # day, only the latest one is kept.
> +	'keep-weekly'  => N # (optional) Keep backups for the last N different weeks.
> +			    # If there is more than one backup for a single
> +			    # week, only the latest one is kept.
> +	'keep-monthly' => N # (optional) Keep backups for the last N different months.
> +			    # If there is more than one backup for a single
> +			    # month, only the latest one is kept.
> +	'keep-yearly'  => N # (optional) Keep backups for the last N different years.
> +			    # If there is more than one backup for a single
> +			    # year, only the latest one is kept.
> +    }

should we add a pointer to docs here? or to
PVE::Storage::prune_mark_backup_group? and should that or its body be
moved somewhere else?

> +
> +=item * C<< $vmid >>
> +
> +The guest's ID.
> +
> +=item * C<< $type >>
> +
> +The type of guest. When C<defined>, it can be one of C<"qemu"> or C<"lxc">.
> +If C<undefined>, both types of backups will be pruned.
> +
> +=item * C<< $dry_run >>
> +
> +Whether this is a dry run. If set to C<1> there won't be any change on the
> +underlying storage.
> +
> +=item * C<< \&logfunc >>
> +
> +A subroutine ref that can be used to log messages with the following signature:
> +
> +    $logfunc->($severity, $message)
> +
> +where $severity can be one of C<"info">, C<"err">, or C<"warn">.
> +
> +=back
> +
> +=cut
> +
>  sub prune_backups {
>      my ($class, $scfg, $storeid, $keep, $vmid, $type, $dryrun, $logfunc) = @_;
>      croak "implement me in sub-class\n";
> -- 
> 2.39.5
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 




More information about the pve-devel mailing list