Re: [pve-devel] [PATCH storage v4 10/12] plugin: enable import for nfs/btrfs/cifs/cephfs/glusterfs

["Max Carrara" <m.carrara@proxmox.com>]

On Fri May 24, 2024 at 3:21 PM CEST, Dominik Csapak wrote:
> and reuse the DirPlugin implementation
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  src/PVE/Storage/BTRFSPlugin.pm     | 5 +++++
>  src/PVE/Storage/CIFSPlugin.pm      | 6 +++++-
>  src/PVE/Storage/CephFSPlugin.pm    | 6 +++++-
>  src/PVE/Storage/GlusterfsPlugin.pm | 6 +++++-
>  src/PVE/Storage/NFSPlugin.pm       | 6 +++++-
>  5 files changed, 25 insertions(+), 4 deletions(-)
>
> diff --git a/src/PVE/Storage/BTRFSPlugin.pm b/src/PVE/Storage/BTRFSPlugin.pm
> index 42815cb..b7e3f82 100644
> --- a/src/PVE/Storage/BTRFSPlugin.pm
> +++ b/src/PVE/Storage/BTRFSPlugin.pm
> @@ -40,6 +40,7 @@ sub plugindata {
>  		backup => 1,
>  		snippets => 1,
>  		none => 1,
> +		import => 1,
>  	    },
>  	    { images => 1, rootdir => 1 },
>  	],
> @@ -930,4 +931,8 @@ sub volume_import {
>      return "$storeid:$volname";
>  }
>  
> +sub get_import_metadata {
> +    return PVE::Storage::DirPlugin::get_import_metadata(@_);

Not _reaaaaally_ a fan of one plugin having to rely on another one, as
that introduces (another) interdependency (-> higher coupling). But I
would leave this as it is, because I'm working on a series that aims to
resolve stuff like that (and it's IMO better to just tackle this all in
one series at once anyway). So this is IMO fine and gets an OK from me.
Just wanted to note.

> +}
> +
>  1
> diff --git a/src/PVE/Storage/CIFSPlugin.pm b/src/PVE/Storage/CIFSPlugin.pm
> index 2184471..475065a 100644
> --- a/src/PVE/Storage/CIFSPlugin.pm
> +++ b/src/PVE/Storage/CIFSPlugin.pm
> @@ -99,7 +99,7 @@ sub type {
>  sub plugindata {
>      return {
>  	content => [ { images => 1, rootdir => 1, vztmpl => 1, iso => 1,
> -		   backup => 1, snippets => 1}, { images => 1 }],
> +		   backup => 1, snippets => 1, import => 1}, { images => 1 }],
>  	format => [ { raw => 1, qcow2 => 1, vmdk => 1 } , 'raw' ],
>      };
>  }
> @@ -314,4 +314,8 @@ sub update_volume_attribute {
>      return PVE::Storage::DirPlugin::update_volume_attribute(@_);
>  }
>  
> +sub get_import_metadata {
> +    return PVE::Storage::DirPlugin::get_import_metadata(@_);
> +}
> +
>  1;
> diff --git a/src/PVE/Storage/CephFSPlugin.pm b/src/PVE/Storage/CephFSPlugin.pm
> index 8aad518..36c64ea 100644
> --- a/src/PVE/Storage/CephFSPlugin.pm
> +++ b/src/PVE/Storage/CephFSPlugin.pm
> @@ -116,7 +116,7 @@ sub type {
>  
>  sub plugindata {
>      return {
> -	content => [ { vztmpl => 1, iso => 1, backup => 1, snippets => 1},
> +	content => [ { vztmpl => 1, iso => 1, backup => 1, snippets => 1, import => 1 },
>  		     { backup => 1 }],
>      };
>  }
> @@ -261,4 +261,8 @@ sub update_volume_attribute {
>      return PVE::Storage::DirPlugin::update_volume_attribute(@_);
>  }
>  
> +sub get_import_metadata {
> +    return PVE::Storage::DirPlugin::get_import_metadata(@_);
> +}
> +
>  1;
> diff --git a/src/PVE/Storage/GlusterfsPlugin.pm b/src/PVE/Storage/GlusterfsPlugin.pm
> index 2b7f9e1..9d17180 100644
> --- a/src/PVE/Storage/GlusterfsPlugin.pm
> +++ b/src/PVE/Storage/GlusterfsPlugin.pm
> @@ -97,7 +97,7 @@ sub type {
>  
>  sub plugindata {
>      return {
> -	content => [ { images => 1, vztmpl => 1, iso => 1, backup => 1, snippets => 1},
> +	content => [ { images => 1, vztmpl => 1, iso => 1, backup => 1, snippets => 1, import => 1},
>  		     { images => 1 }],
>  	format => [ { raw => 1, qcow2 => 1, vmdk => 1 } , 'raw' ],
>      };
> @@ -352,4 +352,8 @@ sub check_connection {
>      return defined($server) ? 1 : 0;
>  }
>  
> +sub get_import_metadata {
> +    return PVE::Storage::DirPlugin::get_import_metadata(@_);
> +}
> +
>  1;
> diff --git a/src/PVE/Storage/NFSPlugin.pm b/src/PVE/Storage/NFSPlugin.pm
> index f2e4c0d..72e9c6d 100644
> --- a/src/PVE/Storage/NFSPlugin.pm
> +++ b/src/PVE/Storage/NFSPlugin.pm
> @@ -53,7 +53,7 @@ sub type {
>  
>  sub plugindata {
>      return {
> -	content => [ { images => 1, rootdir => 1, vztmpl => 1, iso => 1, backup => 1, snippets => 1 },
> +	content => [ { images => 1, rootdir => 1, vztmpl => 1, iso => 1, backup => 1, snippets => 1, import => 1 },

This hash exceeds our line length limit, but I assume that's because it's
easier to read in diffs ... Can IMO be also left like that and changed
later or be adapted in a follow-up patch if so desired.

>  		     { images => 1 }],
>  	format => [ { raw => 1, qcow2 => 1, vmdk => 1 } , 'raw' ],
>      };
> @@ -223,4 +223,8 @@ sub update_volume_attribute {
>      return PVE::Storage::DirPlugin::update_volume_attribute(@_);
>  }
>  
> +sub get_import_metadata {
> +    return PVE::Storage::DirPlugin::get_import_metadata(@_);
> +}
> +
>  1;



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