From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id E3C1A1FF16B for ; Thu, 20 Feb 2025 09:50:34 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 38EE3B97F; Thu, 20 Feb 2025 09:50:28 +0100 (CET) Message-ID: <4e80e33c-caa2-4d8a-9f30-db82521dec08@proxmox.com> Date: Thu, 20 Feb 2025 09:49:50 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: Proxmox VE development discussion , Daniel Kral References: <20250211160825.254167-1-d.kral@proxmox.com> <20250211160825.254167-5-d.kral@proxmox.com> Content-Language: en-US From: Fiona Ebner In-Reply-To: <20250211160825.254167-5-d.kral@proxmox.com> X-SPAM-LEVEL: Spam detection results: 0 AWL -0.046 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 RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pve-devel] [PATCH pve-storage v2 4/5] vdisk_alloc: factor out optional parameters in options hash argument 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: , Reply-To: Proxmox VE development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" Am 11.02.25 um 17:07 schrieb Daniel Kral: > Moves the optional parameter `name` into an optional hash argument at > the end of the function. > > This allows to add more optional arguments in the future and makes the > function call easier to follow when a name is not required. > > Signed-off-by: Daniel Kral > --- While the cover letter already talks about it, it never hurts to explicitly mention that this requires a versioned breaks for qemu-server and pve-container again in the patch itself. > changes since v1: > - new! > > src/PVE/API2/Storage/Content.pm | 6 +++--- > src/PVE/GuestImport.pm | 2 +- > src/PVE/Storage.pm | 30 ++++++++++++++++++++++++++++-- > src/test/run_test_zfspoolplugin.pl | 8 ++++---- > 4 files changed, 36 insertions(+), 10 deletions(-) > > diff --git a/src/PVE/API2/Storage/Content.pm b/src/PVE/API2/Storage/Content.pm > index fe0ad4a..77924a6 100644 > --- a/src/PVE/API2/Storage/Content.pm > +++ b/src/PVE/API2/Storage/Content.pm > @@ -220,9 +220,9 @@ __PACKAGE__->register_method ({ > > my $cfg = PVE::Storage::config(); > > - my $volid = PVE::Storage::vdisk_alloc ($cfg, $storeid, $param->{vmid}, > - $param->{format}, > - $name, $size); > + my $volid = PVE::Storage::vdisk_alloc($cfg, $storeid, $param->{vmid}, $param->{format}, $size, { > + name => $name, > + }); Style nit: the first line is still too long and just having the last param split out like this looks kinda wonky here. I'd go for this instead: > my $volid = PVE::Storage::vdisk_alloc( > $cfg, $storeid, $param->{vmid}, $param->{format}, $size, { name => $name }); > > return $volid; > }}); ---snip 8<--- > diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm > index 0134893..3776565 100755 > --- a/src/PVE/Storage.pm > +++ b/src/PVE/Storage.pm > @@ -1041,8 +1041,34 @@ sub unmap_volume { > return $plugin->unmap_volume($storeid, $scfg, $volname, $snapname); > } > > -sub vdisk_alloc { > - my ($cfg, $storeid, $vmid, $fmt, $name, $size) = @_; > +=head3 vdisk_alloc($cfg, $storeid, $vmid, $size, %opts) It's missing $fmt here. > + > +Allocates a volume on the storage with the identifier C<$storeid> (defined in the storage config > +C<$cfg>) for the VM with the id C<$vmid> with format C<$fmt> and a size of C<$size> kilobytes. > + > +The format C<$fmt> can be C<'raw'>, C<'qcow2'>, C<'subvol'> or C<'vmdk'>. If C<$fmt> is left > +undefined, it will fallback on the default format of the storage type of C<$storeid>. > + > +Optionally, the following key-value pairs are available for C<%opts>: > + > +=over > + > +=item * C<< $name => $string >> > + > +Specifies the name of the new volume. > + > +If undefined, the name will be generated with C. This might be true for our plugins, but other plugins might not use this method. I'd just mention that it will be generated, but not how. > + > +=back > + > +Returns the identifier for the new volume in the format C<"$storeid:$volname">. > + > +=cut > + > +sub vdisk_alloc : prototype($$$$$;%) { > + my ($cfg, $storeid, $vmid, $fmt, $size, $opts) = @_; > + > + my $name = $opts->{name}; > > die "no storage ID specified\n" if !$storeid; > _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel