From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 364F47297A for ; Fri, 2 Jul 2021 15:38:53 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 277209EFB for ; Fri, 2 Jul 2021 15:38:53 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 0612A9EEE for ; Fri, 2 Jul 2021 15:38:52 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id C5C6940551 for ; Fri, 2 Jul 2021 15:38:51 +0200 (CEST) To: Fabian Ebner , pve-devel@lists.proxmox.com References: <20210601161025.32024-1-a.lauterer@proxmox.com> <20210601161025.32024-2-a.lauterer@proxmox.com> <1e42feae-5362-1efb-a8d2-5ee425f17d67@proxmox.com> From: Aaron Lauterer Message-ID: <7bc5f848-8891-0b1f-39c1-4eab330907cd@proxmox.com> Date: Fri, 2 Jul 2021 15:38:50 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: <1e42feae-5362-1efb-a8d2-5ee425f17d67@proxmox.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.661 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -0.001 Looks like a legit reply (A) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [plugin.pm, storage.pm] Subject: Re: [pve-devel] [RFC storage 1/7] storage: expose find_free_diskname X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 02 Jul 2021 13:38:53 -0000 On 6/2/21 10:29 AM, Fabian Ebner wrote: > Am 01.06.21 um 18:10 schrieb Aaron Lauterer: >> Signed-off-by: Aaron Lauterer >> --- >>   PVE/Storage.pm | 8 ++++++++ >>   1 file changed, 8 insertions(+) >> >> diff --git a/PVE/Storage.pm b/PVE/Storage.pm >> index aa36bad..93d09ce 100755 >> --- a/PVE/Storage.pm >> +++ b/PVE/Storage.pm >> @@ -201,6 +201,14 @@ sub storage_can_replicate { >>       return $plugin->storage_can_replicate($scfg, $storeid, $format); >>   } >> +sub find_free_diskname { >> +    my ($cfg, $storeid, $vmid, $fmt, $add_fmt_suffix) = @_; > > Nit: Ideally, the $add_fmt_suffix should be decided by the plugin, as an outside caller cannot know what a plugin wants/expects. Don't know if that's easy to do the way things are though. Yeah, I am not sure how to handle that... most of our storage plugins don't use it at all (ZFS, RBD, LVM). That leaves the Plugin.pm (storage based) and potential 3rd party plugins that have their own implementation. Depending on where you would use the now public `find_free_diskname`, it could be possible to know the format and if the suffix should already be added. For the move-disk to other guest (disk reassign) stuff I cannot do that and leave it undefined. In the Plugin.pm implementation of `rename_volume` I added a check if a suffix is present, and if not, take the one from the source volume. Thinking about it, I might even strip any potential suffixes and always use the one from the source as that should be kept since the file format probably should not change with a rename. > >> + >> +    my $scfg = storage_config($cfg, $storeid); >> +    my $plugin = PVE::Storage::Plugin->lookup($scfg->{type}); >> +    return $plugin->find_free_diskname($storeid, $scfg, $vmid, $fmt, $add_fmt_suffix); >> +} >> + >>   sub storage_ids { >>       my ($cfg) = @_; >>