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 [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id E6CF31FF16B for <inbox@lore.proxmox.com>; Thu, 20 Feb 2025 11:41:26 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id E44C6E98D; Thu, 20 Feb 2025 11:41:19 +0100 (CET) MIME-Version: 1.0 In-Reply-To: <8d062f4c-cd2f-48ae-9aa0-c81cedfd5c84@proxmox.com> References: <20250211160825.254167-1-d.kral@proxmox.com> <20250211160825.254167-6-d.kral@proxmox.com> <8d062f4c-cd2f-48ae-9aa0-c81cedfd5c84@proxmox.com> From: Fabian =?utf-8?q?Gr=C3=BCnbichler?= <f.gruenbichler@proxmox.com> To: Daniel Kral <d.kral@proxmox.com>, Fiona Ebner <f.ebner@proxmox.com>, Proxmox VE development discussion <pve-devel@lists.proxmox.com> Date: Thu, 20 Feb 2025 11:40:41 +0100 Message-ID: <174004804172.338734.4519880313579913500@yuna.proxmox.com> User-Agent: alot/0.10 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.045 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 5/5] vdisk_alloc: add optional assertion for volume's content type 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> Quoting Fiona Ebner (2025-02-20 10:03:09) > Am 11.02.25 um 17:07 schrieb Daniel Kral: > > Allow a caller to specify the volume's intended content type and assert > > whether the specified content type may be stored on the specified > > storage before allocating any volume. > > > > Signed-off-by: Daniel Kral <d.kral@proxmox.com> > > --- > > changes since v1: > > - add assertion at `vdisk_alloc` instead of wrapper `alloc_volume_disk` > > > > src/PVE/Storage.pm | 10 ++++++++++ > > 1 file changed, 10 insertions(+) > > > > diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm > > index 3776565..96d4e41 100755 > > --- a/src/PVE/Storage.pm > > +++ b/src/PVE/Storage.pm > > @@ -1059,6 +1059,13 @@ Specifies the name of the new volume. > > > > If undefined, the name will be generated with C<PVE::Storage::Plugin::find_free_diskname>. > > > > +=item * C<< $vtype => $string >> > > + > > +Specifies the content type of the new volume, which can be one of C<'images'>, C<'rootdir'>, > > +C<'vztmpl'>, C<'iso'>, C<'backup'>, C<'snippets'> or C<'import'>. > > Hmm, vdisk_alloc() is only for allocating guest and import images. So I > think the other content types should be prohibited here (can still be > extended later if it ever comes up). > > We plan to better distinguish between 'rootdir' and 'images' in the > future, so I think we should aim to make the $vtype parameter even > required here. Not quite possible yet, because that would require > breaking 'pvesm alloc', but we can postpone that part for PVE 9 and have > all other callers already use it. > > So my question is if we should rather make it a required parameter > already rather than putting it into $opts? I mean while supporting > passing in undef too, until we are ready to require it for 'pvesm alloc' > too. > > @Fabian sounds good to you too? we could also make it required for real in vdisk_alloc, and make `pvesm alloc` auto-select one of the allowed ones based on the storage config and error out if the storage doesn't support either rootdir or images? or use a different "magic" placeholder value like "any" - using undef is a bit opaque and could happen by accident, although it is not very likely for this hash ;) then when we introduce properly scoped volume names, we can replace that fallback logic in `pvesm alloc` with properly setting *either* rootdir or images, depending on what gets allocated? OTOH, vdisk_alloc doesn't have too call sites anyway, so doing $opts now, and then updating those when it becomes a regular argument would also work fine I think.. the only downside to that approach is that we might miss setting the option for newly introduce callers in the meantime.. _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel