From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <pve-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
	by lore.proxmox.com (Postfix) with ESMTPS id 4798D1FF391
	for <inbox@lore.proxmox.com>; Wed, 12 Jun 2024 17:57:04 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
	by firstgate.proxmox.com (Proxmox) with ESMTP id 453A91488D;
	Wed, 12 Jun 2024 17:57:41 +0200 (CEST)
Date: Wed, 12 Jun 2024 17:57:08 +0200
Message-Id: <D1Y5UBG6O7S5.1XSWQ3WD7OI21@proxmox.com>
From: "Max Carrara" <m.carrara@proxmox.com>
To: "Proxmox VE development discussion" <pve-devel@lists.proxmox.com>
Mime-Version: 1.0
X-Mailer: aerc 0.17.0-72-g6a84f1331f1c
References: <20240524132209.703402-1-d.csapak@proxmox.com>
 <20240524132209.703402-11-d.csapak@proxmox.com>
In-Reply-To: <20240524132209.703402-11-d.csapak@proxmox.com>
X-SPAM-LEVEL: Spam detection results:  0
 AWL -0.216 Adjusted score from AWL reputation of From: address
 BAYES_00                 -1.9 Bayes spam probability is 0 to 1%
 DMARC_MISSING             0.1 Missing DMARC policy
 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
 T_SCC_BODY_TEXT_LINE    -0.01 -
 URI_NOVOWEL               0.5 URI hostname has long non-vowel sequence
Subject: Re: [pve-devel] [PATCH storage v4 10/12] plugin: enable import for
 nfs/btrfs/cifs/cephfs/glusterfs
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pve-devel-bounces@lists.proxmox.com
Sender: "pve-devel" <pve-devel-bounces@lists.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