[pve-devel] [PATCH storage v2 2/2] BTRFSPlugin: reuse DirPlugin update/get_volume_attribute

[Thomas Lamprecht]

Am 27/05/2022 um 14:31 schrieb Dominik Csapak:
> this allows setting notes+protected for backups on btrfs
> 
> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
> ---
>  PVE/Storage/BTRFSPlugin.pm | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)

looks OK, but the whole situation with that and often also btrfs is sometimes
a bit weird in subtle ways, so a quick look from you wolfgang would be appreciated.

Also, one style nit inline.

> 
> diff --git a/PVE/Storage/BTRFSPlugin.pm b/PVE/Storage/BTRFSPlugin.pm
> index be613f4..55f3c05 100644
> --- a/PVE/Storage/BTRFSPlugin.pm
> +++ b/PVE/Storage/BTRFSPlugin.pm
> @@ -138,9 +138,16 @@ sub status {
>      return PVE::Storage::DirPlugin::status($class, $storeid, $scfg, $cache);
>  }
>  
> -# TODO: sub get_volume_attribute {}
> +sub get_volume_attribute {
> +    my ($class, $scfg, $storeid, $volname, $attribute) = @_;
> +    return PVE::Storage::DirPlugin::get_volume_attribute($class, $scfg, $storeid, $volname, $attribute);
> +}
>  
> -# TODO: sub update_volume_attribute {}
> +sub update_volume_attribute {
> +    my ($class, $scfg, $storeid, $volname, $attribute, $value) = @_;
> +    return PVE::Storage::DirPlugin::update_volume_attribute($class, $scfg, $storeid, $volname,
> +	$attribute, $value);

I'd actually prefer the rustfmt style for wrapping simple lists of parameters, in that
case the following should make it happy, well theoretically as it would choke on perl,
as lots others do:

return PVE::Storage::DirPlugin::update_volume_attribute(
    $class, $scfg, $storeid, $volname, $attribute, $value);

And if that'd still require to much width it'd go one param per line.
I'll see that I update the style guide with that.

> +}
>  
>  # croak would not include the caller from within this module
>  sub __error {