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 399BF1FF183 for ; Wed, 2 Jul 2025 10:12:29 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 31E1B14786; Wed, 2 Jul 2025 10:13:07 +0200 (CEST) Message-ID: Date: Wed, 2 Jul 2025 10:12:33 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Beta To: Fiona Ebner , Proxmox VE development discussion References: <20250626144644.279679-1-f.ebner@proxmox.com> <20250626144644.279679-2-f.ebner@proxmox.com> <99f545d0-b00b-4216-803b-51fe5eac17f3@proxmox.com> <52125694-2521-4e90-84d8-0a4b96990820@proxmox.com> Content-Language: de-AT, en-US From: Thomas Lamprecht Autocrypt: addr=t.lamprecht@proxmox.com; keydata= xsFNBFsLjcYBEACsaQP6uTtw/xHTUCKF4VD4/Wfg7gGn47+OfCKJQAD+Oyb3HSBkjclopC5J uXsB1vVOfqVYE6PO8FlD2L5nxgT3SWkc6Ka634G/yGDU3ZC3C/7NcDVKhSBI5E0ww4Qj8s9w OQRloemb5LOBkJNEUshkWRTHHOmk6QqFB/qBPW2COpAx6oyxVUvBCgm/1S0dAZ9gfkvpqFSD 90B5j3bL6i9FIv3YGUCgz6Ue3f7u+HsEAew6TMtlt90XV3vT4M2IOuECG/pXwTy7NtmHaBQ7 UJBcwSOpDEweNob50+9B4KbnVn1ydx+K6UnEcGDvUWBkREccvuExvupYYYQ5dIhRFf3fkS4+ wMlyAFh8PQUgauod+vqs45FJaSgTqIALSBsEHKEs6IoTXtnnpbhu3p6XBin4hunwoBFiyYt6 YHLAM1yLfCyX510DFzX/Ze2hLqatqzY5Wa7NIXqYYelz7tXiuCLHP84+sV6JtEkeSUCuOiUY virj6nT/nJK8m0BzdR6FgGtNxp7RVXFRz/+mwijJVLpFsyG1i0Hmv2zTn3h2nyGK/I6yhFNt dX69y5hbo6LAsRjLUvZeHXpTU4TrpN/WiCjJblbj5um5eEr4yhcwhVmG102puTtuCECsDucZ jpKpUqzXlpLbzG/dp9dXFH3MivvfuaHrg3MtjXY1i+/Oxyp5iwARAQABzTNUaG9tYXMgTGFt cHJlY2h0IChBdXRoLTQpIDx0LmxhbXByZWNodEBwcm94bW94LmNvbT7CwY4EEwEIADgWIQQO R4qbEl/pah9K6VrTZCM6gDZWBgUCWwuNxgIbAwULCQgHAgYVCAkKCwIEFgIDAQIeAQIXgAAK CRDTZCM6gDZWBm/jD/4+6JB2s67eaqoP6x9VGaXNGJPCscwzLuxDTCG90G9FYu29VcXtubH/ bPwsyBbNUQpqTm/s4XboU2qpS5ykCuTjqavrcP33tdkYfGcItj2xMipJ1i3TWvpikQVsX42R G64wovLs/dvpTYphRZkg5DwhgTmy3mRkmofFCTa+//MOcNOORltemp984tWjpR3bUJETNWpF sKGZHa3N4kCNxb7A+VMsJZ/1gN3jbQbQG7GkJtnHlWkw9rKCYqBtWrnrHa4UAvSa9M/XCIAB FThFGqZI1ojdVlv5gd6b/nWxfOPrLlSxbUo5FZ1i/ycj7/24nznW1V4ykG9iUld4uYUY86bB UGSjew1KYp9FmvKiwEoB+zxNnuEQfS7/Bj1X9nxizgweiHIyFsRqgogTvLh403QMSGNSoArk tqkorf1U+VhEncIn4H3KksJF0njZKfilrieOO7Vuot1xKr9QnYrZzJ7m7ZxJ/JfKGaRHXkE1 feMmrvZD1AtdUATZkoeQtTOpMu4r6IQRfSdwm/CkppZXfDe50DJxAMDWwfK2rr2bVkNg/yZI tKLBS0YgRTIynkvv0h8d9dIjiicw3RMeYXyqOnSWVva2r+tl+JBaenr8YTQw0zARrhC0mttu cIZGnVEvQuDwib57QLqMjQaC1gazKHvhA15H5MNxUhwm229UmdH3KM7BTQRbC43GARAAyTkR D6KRJ9Xa2fVMh+6f186q0M3ni+5tsaVhUiykxjsPgkuWXWW9MbLpYXkzX6h/RIEKlo2BGA95 QwG5+Ya2Bo3g7FGJHAkXY6loq7DgMp5/TVQ8phsSv3WxPTJLCBq6vNBamp5hda4cfXFUymsy HsJy4dtgkrPQ/bnsdFDCRUuhJHopnAzKHN8APXpKU6xV5e3GE4LwFsDhNHfH/m9+2yO/trcD txSFpyftbK2gaMERHgA8SKkzRhiwRTt9w5idOfpJVkYRsgvuSGZ0pcD4kLCOIFrer5xXudk6 NgJc36XkFRMnwqrL/bB4k6Pi2u5leyqcXSLyBgeHsZJxg6Lcr2LZ35+8RQGPOw9C0ItmRjtY ZpGKPlSxjxA1WHT2YlF9CEt3nx7c4C3thHHtqBra6BGPyW8rvtq4zRqZRLPmZ0kt/kiMPhTM 8wZAlObbATVrUMcZ/uNjRv2vU9O5aTAD9E5r1B0dlqKgxyoImUWB0JgpILADaT3VybDd3C8X s6Jt8MytUP+1cEWt9VKo4vY4Jh5vwrJUDLJvzpN+TsYCZPNVj18+jf9uGRaoK6W++DdMAr5l gQiwsNgf9372dbMI7pt2gnT5/YdG+ZHnIIlXC6OUonA1Ro/Itg90Q7iQySnKKkqqnWVc+qO9 GJbzcGykxD6EQtCSlurt3/5IXTA7t6sAEQEAAcLBdgQYAQgAIBYhBA5HipsSX+lqH0rpWtNk IzqANlYGBQJbC43GAhsMAAoJENNkIzqANlYGD1sP/ikKgHgcspEKqDED9gQrTBvipH85si0j /Jwu/tBtnYjLgKLh2cjv1JkgYYjb3DyZa1pLsIv6rGnPX9bH9IN03nqirC/Q1Y1lnbNTynPk IflgvsJjoTNZjgu1wUdQlBgL/JhUp1sIYID11jZphgzfDgp/E6ve/8xE2HMAnf4zAfJaKgD0 F+fL1DlcdYUditAiYEuN40Ns/abKs8I1MYx7Yglu3RzJfBzV4t86DAR+OvuF9v188WrFwXCS RSf4DmJ8tntyNej+DVGUnmKHupLQJO7uqCKB/1HLlMKc5G3GLoGqJliHjUHUAXNzinlpE2Vj C78pxpwxRNg2ilE3AhPoAXrY5qED5PLE9sLnmQ9AzRcMMJUXjTNEDxEYbF55SdGBHHOAcZtA kEQKub86e+GHA+Z8oXQSGeSGOkqHi7zfgW1UexddTvaRwE6AyZ6FxTApm8wq8NT2cryWPWTF BDSGB3ujWHMM8ERRYJPcBSjTvt0GcEqnd+OSGgxTkGOdufn51oz82zfpVo1t+J/FNz6MRMcg 8nEC+uKvgzH1nujxJ5pRCBOquFZaGn/p71Yr0oVitkttLKblFsqwa+10Lt6HBxm+2+VLp4Ja 0WZNncZciz3V3cuArpan/ZhhyiWYV5FD0pOXPCJIx7WS9PTtxiv0AOS4ScWEUmBxyhFeOpYa DrEx In-Reply-To: <52125694-2521-4e90-84d8-0a4b96990820@proxmox.com> X-SPAM-LEVEL: Spam detection results: 0 AWL -0.151 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.232 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 v3 storage 1/9] plugin: add method to get qemu blockdevice options for volume 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 01.07.25 um 13:01 schrieb Fiona Ebner: > Am 01.07.25 um 11:28 schrieb Thomas Lamprecht: >> It might be potentially enough to have the QEMU version and machine version >> passed here, as then one can generate block dev options that can be understood >> by qemu(-server) and are also compatible with the target machine version. > > It would seem pretty strange to me that an option for accessing an image > would depend on the machine version. How so? New QEMU learns a new option for accessing images that we want to adopt but can't do so transparently because it's not backwards compatible? > We already rely on having an image accessed via different drivers/options > to be fully equivalent for migration (or we couldn't combine block mirror > and migration). Well, that's the current assumption that worked out mostly by luck and because we kept to standard options, that doesn't have to stay that way ant this interface would allow. > OTOH, the machine version is the natural guard and there is less potential > for a plugin to break live migration by accidentally setting or changing an > option it shouldn't have. And if a plugin depends on a specific binary > version for a feature, it can just also guard with the machine version > corresponding to that binary version. So I'd just pass along the machine > version and not the binary version. Yeah, just the machine version would be probably enough. >> And are we sure that we want to allow passing arbitrary options here? >> Could we at least generate some simple schema automatically from the QAPI or >> the like? Could be also a specially prepared JSON file just for this purpose >> shipped by our qemu package, or tracked separately so that we can track the >> version in which a option first appeared or became obsolete. > > Sorry, I don't understand what you mean here. You talk about this being QEMU block dev options, so are they or are they mapped by us in qemu-server? If the former I'd need a more specific question, with the latter this is still not ideal but doesn't matter much for now. > Verify the block device options in the return value that the plugin > implementation has given us? I don't think verifying would give us much, > as QEMU will already complain the same way. What QEMU understands and what we want to expose and allow might be very different things. > Plugins should just use the most basic options to access/open the image. > Other options will be set by qemu-server. The POD mentions this, but > maybe it should be emphasized more? Currently, plugins cannot set options directly at all, with this series, they can set anything we do not explicitly overwrite afterward in qemu-server IIUC. Going back from that would need a major break of the storage API. And given that we would like to minify the attack surface of QEMU/KVM in the future (not having all mounts/blockdevs/..) accessible, I could easily imagine that we do not want to expose all blockdev options just because they are there, starting out with a smaller set would easily allow this, especially if it's available for plugins to check if using an option is supported/allowed; that would have little implementation cost while reducing friction in managing this API surface thus reducing also maintenance cost for both us and those maintaining plugins. If unsure, and I really have not seen any actual justification presented why such an API should be required, let's please start out with a more limiting API first, as for APIs allowing more flexibility is always simpler than restricting it later on. >>> + my $blockdev = {}; >>> + >>> + my ($path) = $class->filesystem_path($scfg, $volname); >>> + >>> + if ($path =~ m|^/|) { >>> + # The 'file' driver only works for regular files. The check below is taken from >>> + # block/file-posix.c:hdev_probe_device() in QEMU. Do not bother with detecting 'host_cdrom' >>> + # devices here, those are not managed by the storage layer. >>> + my $st = File::stat::stat($path) or die "stat for '$path' failed - $!\n"; >>> + my $driver = (S_ISCHR($st->mode) || S_ISBLK($st->mode)) ? 'host_device' : 'file'; >>> + $blockdev = { driver => $driver, filename => $path }; >>> + } else { >>> + die "storage plugin doesn't implement qemu_blockdev_options() method\n"; >>> + } >> Should we rather default to an empty set of extra options? At least for external >> plugins that would be the safer choice for upgrading, might not always work but >> as is users can only loose FWICT? > What extra options do you mean? The default implementation here only > sets driver and filename which is the most minimal possible. > The default implementation die's if it's not a path based.. So any existing external plugin that isn't path based will break with this; I said it a lot of times already, I do not want unnecessary breaking changes for the storage API.. _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel