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 DFC089F564 for ; Mon, 6 Nov 2023 10:22:20 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id BF00013768 for ; Mon, 6 Nov 2023 10:21:50 +0100 (CET) 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 for ; Mon, 6 Nov 2023 10:21:50 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id D482D452A8 for ; Mon, 6 Nov 2023 10:21:49 +0100 (CET) Message-ID: Date: Mon, 6 Nov 2023 10:21:44 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Content-Language: en-US To: Proxmox VE development discussion , Dominik Csapak , Thomas Lamprecht References: <20231103115343.4133611-1-d.csapak@proxmox.com> <2cd139da-fe1b-4709-881a-603ea5de8427@proxmox.com> From: Fiona Ebner In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.080 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 - Subject: Re: [pve-devel] [RFC PATCH cluster/guest-common/qemu-server/container/manager] add backend profile support 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: Mon, 06 Nov 2023 09:22:20 -0000 Am 06.11.23 um 09:17 schrieb Dominik Csapak: > thx for the answer! i'll see that i send a next version soon > > some more comment from me inline > > On 11/4/23 09:34, Thomas Lamprecht wrote: >> On 03/11/2023 12:53, Dominik Csapak wrote: >>> This series aims to provide profile support when creating guests (ct/vm) >>> so that users can reuse options without having to specify them every >>> time. >>> >>> Sending as RFC because I don't quite like some things with the current >>> implementation and I'm not quite sure in which direction I should take >>> this. Also the GUI part isn't done yet and i wanted to see if the >>> direction is OK. (Sorry for the wall of text) >>> >>> The major issues: >>> >>> Using a single section config for both VMs and CTs make handling the >>> properties a bit weird. For now i prefix the options with "$type_" >>> so vm options are e.g. 'vm_ostype', 'vm_name', and so on while container >>> options are 'ct_ostype', 'ct_hostname', etc. >> >> That properties are shared by different sections by default is IMO not >> really ideal in general, it's also making the storage API and its docs >> rather hard to understand & work with. >> >> One option could be to opt-into a newer behavior, e.g., via some >> property, or getter, that the section config implementation needs to >> set, or override and return true, which makes then all properties >> isolated if not explicitly marked as shared (via another new property), >> that way we could use it now, and move over existing ones where it >> makes sense, without risking wide breakage. >> > > ok, so if i'm reading this right, having a config per type is not really > an option for you? (that would be nice and easy, without having > to extend the section config at all, but still get most of the > upsides) > > hard part of extending the section config is how to handle the api > since when we want to have a 'clean' api per type, we then have > to also split the api per type? (like e.g. we do with the mapping > of pci/usb) and then whats the gain of having both types in a single > config (besides a single file instead of two?) > > >>> >>> Using the same config/plugin system also makes using it a bit weird. >>> We have to register/init them in the api where they're used, but for the >>> cli we have to register only the available type and then init. This >>> makes it necessary to always set 'allow_unknown' while parsing the >>> config so that 'qm' doesn't trip over the container profiles... >>> >>> A fix for both could be to separate the ct/vm configs into two files >>> (similar to how we did pci/usb mapping configs), that would fix the >>> prefixing, as well as the register/init issue (We have to have >>> separate APIs then ofc). >>> >>> Another fix would be to extend the section config to allow different >>> properties of the same name for different types. Should be possible, >>> although we have to be careful to not break existing ones, and also >>> the API interface has to be separate for each type then (cannot really >>> have confiflicting api parameter schemas for the same name?) >>> >>> We could also go in a completely different direction and create a config >>> per profile? (like we handle vm configs). Downside of that is, that the >>> current guest config handling part is partly in pmxcfs, so we'd have to >>> make that either more generic, or duplicate it for profiles. >> >> I don't see how this would have anything to do with pmxcfs and VMIDs, >> that map to an actual guest instance and thus needs special treatment >> compared to just some profile that can, e.g., live in a >> /etc/pve/guest-profiles/  as .profile (the extension just an >> example), and be handled only via perl – profiles are only used in >> the profile management API, where it's ok to fully parse one, and >> in guest creation POST calls, so even cfs_register* would be probably >> overkill. > > you're right, i did not explain right what i meant. We currently do > somethings in pmxcfs for vm configs (e.g. read/write/list) and > we'd have to use the filesystem layer for the profiles if done in perl > (which i meant with 'duplicating') > >> >> Having a file per profile makes some things relatively easy, but doesn't >> fits the commonly used section config, let's see if others have input >> (i.e., actively ask one/some of fabian/wolfgang/fiona. > > i agree, just mentioned it for completeness sake, but yes, lets wait > for another opinion > Yes, then it's just two different section config files with a single type, which is rather unusual. Compared to that, having a dedicated config file per profile akin to templates would seem more natural to me. Don't get me wrong, I have no problem with the section config approach. Ideally, we would be able to avoid the type prefixing of properties and still have it all in one config. And having a way to do per-type declaration of properties should solve that. Do we have the same issue in our PBS/Rust section configs? Or how is it solved there? And what is the situation for third-party storage plugins? Are they already "broken" in that regard, i.e. if a third-party plugin declares fancy-new-property and we later add fancy-new-property to our schema with a different format, will it clash?