[pve-devel] [PATCH storage v2 3/3] fix #6224: disks: get: set timeout for retrieval of SMART stat data

2025-04-15 Thread Daniel Kral
In rare scenarios, `smartctl` takes up to 60 seconds to timeout for SCSI commands to be completed, as reported in our user forum [0] and bugzilla [1]. It seems that USB drives handled by the USB Attached SCSI (UAS) kernel module are more likely to be affected by this [2], but is more of a case-by-c

Re: [pve-devel] [PATCH container v2 03/11] alloc_disk: fail fast if storage does not support content type rootdir

2025-04-15 Thread Daniel Kral
On 2/20/25 13:15, Fiona Ebner wrote: I also noticed that we have no check against starting a container with volumes on a storage that does not support 'rootdir'. We have such a check for VMs IIRC. Prohibiting that would also be good, but maybe something for PVE 9 where we can also check for misco

Re: [pve-devel] [PATCH manager 1/1] fix #6311: ui: prevent consent text from showing twice for oidc

2025-04-15 Thread Thomas Lamprecht
On Sun, 13 Apr 2025 21:18:36 -0500, Thomas Skinner wrote: > Applied, thanks! For future UI patches please follow our JavaScript style guide, e.g. for casing we prefer camelCase there [0]. [0]: https://pve.proxmox.com/wiki/Javascript_Style_Guide#Casing [1/1] fix #6311: ui: prevent consent text

[pve-devel] [PATCH storage v3 3/4] vdisk_alloc: factor out optional parameters in options hash argument

2025-04-15 Thread Daniel Kral
Move the optional parameter `name` into an optional hash argument at the end of the subroutine signature of vdisk_alloc(). This allows to add more optional arguments in the future and makes the function call easier to follow when a name is not required. This requires a versioned break for package

[pve-devel] [PATCH container v3 10/11] api: update_vm: use volume content type assertion helpers

2025-04-15 Thread Daniel Kral
Make use of the newly introduced content type assertion helpers at verifying the added and/or changed container config values, which is called e.g. in the API handler for updating the container config. Signed-off-by: Daniel Kral --- changes since v2: * rename helper from *_supported to *_availa

[pve-devel] [PATCH qemu-server v3 10/12] api: qmrestore: use volume content type assertion helpers

2025-04-15 Thread Daniel Kral
Make use of the newly introduced content type assertion helpers in the preconditions check paths and the call to `vdisk_alloc` in the API and CLI handler for restoring VMs to a specified target storage. Since the preconditions existed before, adding a content type safeguard at `vdisk_alloc` does n

[pve-devel] [PATCH qemu-server v3 07/12] api: {clone, move}_vm: use volume content type assertion helpers

2025-04-15 Thread Daniel Kral
Make use of the newly introduced content type assertion helpers in the preconditions check paths and the call to `vdisk_alloc` in the API handlers for cloning VMs to another storage and moving a VM disk to another storage with the newly introduced helper. Since the preconditions existed before, ad

[pve-devel] [PATCH qemu-server v3 09/12] api: import{disk, ovf}: use volume content type assertion helpers

2025-04-15 Thread Daniel Kral
Make use of the newly introduced content type assertion helpers in the preconditions check paths and the call to `vdisk_alloc` in the API handlers for importing VM disks or importing OVF manifests as VMs. Since the preconditions existed before, adding a content type safeguard at `vdisk_alloc` does

[pve-devel] [PATCH qemu-server v3 06/12] cfg2cmd: improve error message for invalid volume content type

2025-04-15 Thread Daniel Kral
Improve the error message when a VM start fails due to a volume storage not supporting the volume's required content type by prefixing it with the disk handle (e.g. scsi0) and using the new helper. Signed-off-by: Daniel Kral --- changes since v2: * rename helper from *_supported to *_available

[pve-devel] [PATCH container v3 08/11] api: create: use volume content type assertion helpers

2025-04-15 Thread Daniel Kral
Make use of newly introduced content type assertion helpers in the preconditions check path in the API handler for creating a container. Signed-off-by: Daniel Kral --- changes since v2: * rename helper from *_supported to *_available * lower/remove storage_check_enabled to storage_config wher

[pve-devel] [PATCH container v3 02/11] tests: add tests for expected behavior of alloc_disk wrapper

2025-04-15 Thread Daniel Kral
Adds test cases for the alloc_disk wrapper subroutine to ensure that: - zero-sized volumes are allocated as subvols on path-based storages - non-zero-sized volumes are allocated as raw on path-based storages - volumes are allocated as raw on btrfs storages without quotas - volumes are allocated as

[pve-devel] [PATCH container v3 11/11] mount: raw/iso: use volume content type assertion helpers

2025-04-15 Thread Daniel Kral
Make use of the newly introduced content type assertion helpers at verifying the content type when mounting mountpoints, which have the format "raw" or "iso". Signed-off-by: Daniel Kral --- changes since v2: * rename helper from *_supported to *_available I already pointed that out in a respon

[pve-devel] [PATCH container v3 07/11] alloc_disk: use volume content type assertion helpers

2025-04-15 Thread Daniel Kral
Make use of the newly introduced content type assertion helpers in the precondition check path and the call to `vdisk_alloc` in the wrapper for allocating container's disks. Signed-off-by: Daniel Kral --- changes since v2: * rename helper from *_supported to *_available * lower/remove storage

[pve-devel] [PATCH qemu-server v3 11/12] api: migrate_vm: use volume content type assertion helpers

2025-04-15 Thread Daniel Kral
Make use of the newly introduced content type assertion helpers in the preconditions check paths and the call to `vdisk_alloc` in the API handler for migrating VMs (and its subcommands and `mtunnel` endpoint). Since the preconditions existed before, adding a content type safeguard at `vdisk_alloc`

[pve-devel] [PATCH container v3 05/11] alloc_disk: simplify storage-specific logic for vdisk_alloc arguments

2025-04-15 Thread Daniel Kral
Since there are only two distinct outcomes, i.e. raw format + do_format and subvol format + needs_chown, break down the conditions to reduce the branching depth. Drop the check for $scfg->{path} for BTRFS and ZFS pool storages, since BTRFS must always have a path and ZFS pool can never have a path

[pve-devel] [PATCH storage v3 1/4] introduce helpers for content type assertions of storages and volumes

2025-04-15 Thread Daniel Kral
Add subroutines for asserting the content types of storages and volumes to reduce code duplication, e.g. when implementing preconditions in an API handler before calling vdisk_alloc(). Signed-off-by: Daniel Kral --- changes since v2: * split documentation back to be above each helper * change

[pve-devel] [PATCH qemu-server v3 12/12] tree-wide: add todos for breaking content type assertions

2025-04-15 Thread Daniel Kral
Snapshot state files and fleecing images are currently allowed to be put on storages, which do not support VM images. For consistency's sake, these should also only be created on storages supporting VM images, but this would break the API for users now, so postpone this until PVE 9. Signed-off-by:

Re: [pve-devel] iscsi and multipathing

2025-04-15 Thread Timo Veith via pve-devel
--- Begin Message --- Hello Mira, thank you very much for your reply. > Am 15.04.2025 um 11:09 schrieb Mira Limbeck : > > Hi Timo, > > At the moment I'm working on storage mapping support for iSCSI. > This would allow one to configure different portals on each of the hosts > that are logically th

Re: [pve-devel] [PATCH container v2 03/11] alloc_disk: fail fast if storage does not support content type rootdir

2025-04-15 Thread Wolfgang Bumiller
On Tue, Apr 15, 2025 at 02:27:24PM +0200, Daniel Kral wrote: > On 2/20/25 13:15, Fiona Ebner wrote: > > I also noticed that we have no check against starting a container with > > volumes on a storage that does not support 'rootdir'. We have such a > > check for VMs IIRC. Prohibiting that would also

[pve-devel] [PATCH storage v3 4/4] vdisk_alloc: add assertion for volume's content type

2025-04-15 Thread Daniel Kral
Require a caller of vdisk_alloc() 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. The 'any' volume type is for backwards compatibility of external callers, whose API would break if cont

[pve-devel] [PATCH qemu-server v3 02/12] api: remove unusable default storage parameter in check_storage_access

2025-04-15 Thread Daniel Kral
Since 0541eeb8 ("use property strings for drive options") the user input of a volume with allocation support must be a pair of a PVE-managed storage and an "arbitrary" string, even though only a volume name or the size of a new disk in GB is supported [0]. This means the $volid must always be the

[pve-devel] [PATCH container v3 09/11] migration: prepare: use volume content type assertion helpers

2025-04-15 Thread Daniel Kral
Make use of newly introduced content type assertion helpers in the preconditions check and preparation path of the migration API handlers. Signed-off-by: Daniel Kral --- changes since v2: * rename helper from *_supported to *_available * lower/remove storage_check_enabled to storage_config wh

[pve-devel] [PATCH qemu-server v3 04/12] tree-wide: update vdisk_alloc optional arguments signature

2025-04-15 Thread Daniel Kral
Update any callers of `PVE::Storage::vdisk_alloc` to the updated function signature, which moves the optional argument `$name` to the optional hash at the end of the argument list. Signed-off-by: Daniel Kral --- changes since v2: * fix lines which are 100+ chars * fix some wrappings to meet t

[pve-devel] [PATCH container v3 03/11] alloc disk: fix content type check for ZFS storages

2025-04-15 Thread Daniel Kral
There were existing assertions whether the underlying storage supports the content type 'rootdir' for the case where volumes are created on ZFS storages in the create_vm API handler and update_pct_config(). This assertion was missing from the case where volumes are copied with copy_volume(), so al

[pve-devel] [PATCH qemu-server v3 08/12] api: {create, update}_vm: use volume content type assertion helpers

2025-04-15 Thread Daniel Kral
Make use of the newly introduced content type assertion helpers in the preconditions check paths and the call to `vdisk_alloc` in the API handlers for creating a VM or updating a VM with newly allocated VM, EFI, TPM and/or cloudinit images. Since the preconditions existed before, adding a content

[pve-devel] [PATCH container v3 01/11] migration: prepare: factor out common read-only variables

2025-04-15 Thread Daniel Kral
Factor out the common read-only variables to allow the second call to `storage_check_enabled` to be below 100 characters. Signed-off-by: Daniel Kral --- changes since v2: * none src/PVE/LXC/Migrate.pm | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/PVE/LXC/Migrate

[pve-devel] superseded: [PATCH container/qemu-server/storage v2 00/31] consistent assertions for volume's content types

2025-04-15 Thread Daniel Kral
Sent a v3: https://lore.proxmox.com/pve-devel/20250415135045.255272-1-d.k...@proxmox.com/#u ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH container v2 03/11] alloc_disk: fail fast if storage does not support content type rootdir

2025-04-15 Thread Fabian Grünbichler
On April 15, 2025 3:31 pm, Wolfgang Bumiller wrote: > On Tue, Apr 15, 2025 at 02:27:24PM +0200, Daniel Kral wrote: >> On 2/20/25 13:15, Fiona Ebner wrote: >> > I also noticed that we have no check against starting a container with >> > volumes on a storage that does not support 'rootdir'. We have s

[pve-devel] [PATCH storage/qemu-server/container v3 00/27] consistent assertions for volume's content types

2025-04-15 Thread Daniel Kral
== Changelog since v2 == Thanks @Fiona for the feedback and help on this! v1: https://lore.proxmox.com/pve-devel/20240916163839.236908-1-d.k...@proxmox.com/ v2: https://lore.proxmox.com/pve-devel/20250211160825.254167-1-d.k...@proxmox.com/ - improve some commit messages and clarify some non-tr

[pve-devel] [PATCH manager 1/1] fix #6311: ui: prevent consent text from showing twice for oidc

2025-04-15 Thread Thomas Skinner
Signed-off-by: Thomas Skinner --- www/manager6/window/LoginWindow.js | 15 +-- 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/www/manager6/window/LoginWindow.js b/www/manager6/window/LoginWindow.js index b4dff0c1..a1ba3cdb 100644 --- a/www/manager6/window/LoginWindow.j

Re: [pve-devel] Storage plugin questions

2025-04-15 Thread Max Schettler via pve-devel
--- Begin Message --- On 21.03.25 15:45, Fiona Ebner wrote: Am 18.03.25 um 11:32 schrieb Max Schettler via pve-devel: - is it possible to integrate with the webinterface, to allow creation of a custom storage provider from there, instead of the CLI? Not yet and we can't give a timeline/promis

[pve-devel] [RFC storage v2 1/3] api: smart: return unknown health instead of error message

2025-04-15 Thread Daniel Kral
In case of an error, the WebGUI expects the SMART data API endpoint to return a health value, but it will return an error message directly. To make this more user friendly, mask the error in the API handler. Signed-off-by: Daniel Kral --- This could also have been intentional as the error message

Re: [pve-devel] [PATCH storage 1/2] disks: get: separate error path for retrieving SMART data

2025-04-15 Thread Daniel Kral
On 4/11/25 17:52, Max Carrara wrote: On Fri Apr 11, 2025 at 5:08 PM CEST, Daniel Kral wrote: Make the subroutine get_smart_data() die with the error message from running the `smartctl` command before. This is in preparation for the next patch, which makes that command fail in certain scenarios.

[pve-devel] [PATCH storage v2 2/3] disks: get: separate error path for retrieving SMART data

2025-04-15 Thread Daniel Kral
Make the subroutine get_smart_data() die with the error message from running the `smartctl` command before. This is in preparation for the next patch, which makes that command fail in certain scenarios. Signed-off-by: Daniel Kral --- src/PVE/Diskmanage.pm | 4 ++-- 1 file changed, 2 insertions(+

Re: [pve-devel] [PATCH proxmox-ve-rs v4 1/1] ve-config: move types to proxmox-network-types

2025-04-15 Thread Christoph Heiss
FYI: Doesn't apply anymore on latest proxmox-ve-rs master and needs a rebase, probably due to 746d7a0be "partially fix #6176: config: guest: change default for firewall key" On Fri Apr 4, 2025 at 3:55 PM CEST, Stefan Hanreich wrote: > Some of the types defined in this crate have be

[pve-devel] [PATCH manager] pveproxy: create log directory when starting

2025-04-15 Thread Maximiliano Sandoval
We only create this directory while installing the package. If a user deletes /var/log then they will lose access to the web UI. Signed-off-by: Maximiliano Sandoval --- bin/pveproxy | 18 +++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/bin/pveproxy b/bin/pveprox

Re: [pve-devel] iscsi and multipathing

2025-04-15 Thread Mira Limbeck
Hi Timo, At the moment I'm working on storage mapping support for iSCSI. This would allow one to configure different portals on each of the hosts that are logically the same storage. If you tried setting up a storage via iSCSI where each host can only access a part of the portals which are announ

[pve-devel] [PATCH frr 2/2] debian: bump version to 10.2.2-1+pve1

2025-04-15 Thread Stefan Hanreich
Signed-off-by: Stefan Hanreich --- debian/changelog | 9 + 1 file changed, 9 insertions(+) diff --git a/debian/changelog b/debian/changelog index a458d64..6328fbc 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,12 @@ +frr (10.2.2-1+pve1) bookworm; urgency=medium + + * up

[pve-devel] [PATCH frr 1/2] update to 10.2.2

2025-04-15 Thread Stefan Hanreich
When rebooting single nodes, BFD is not always able to re-establish its sessions, as reported by users in our forum [1] [2]. The corresponding FRR issue seems to be found in [3]. The issue mainly surfaced when updating nodes one-by-one and then rebooting, leading some hosts to not be able to re-es

Re: [pve-devel] [PATCH frr 1/2] update to 10.2.2

2025-04-15 Thread Thomas Lamprecht
On Tue, 15 Apr 2025 12:23:00 +0200, Stefan Hanreich wrote: > When rebooting single nodes, BFD is not always able to re-establish > its sessions, as reported by users in our forum [1] [2]. The > corresponding FRR issue seems to be found in [3]. > > The issue mainly surfaced when updating nodes one-

[pve-devel] Migration of VM/Container on custom storage type

2025-04-15 Thread Max Schettler via pve-devel
--- Begin Message --- Hi, I'm developing a Proxmox storage plugin. Trying to use the high-availability features or migrating VMs/Containers to another hypervisor in the cluster I get error messages about the storage type I'm providing not being supported. Looking at the source, it seems like

[pve-devel] [PATCH manager 2/2] ui: qemu: make validator warning for serial displays more concrete

2025-04-15 Thread Aaron Lauterer
Signed-off-by: Aaron Lauterer --- feel free to squash this into the first patch if you prefer. www/manager6/qemu/DisplayEdit.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/www/manager6/qemu/DisplayEdit.js b/www/manager6/qemu/DisplayEdit.js index 7a9a5160..7482ca55 100644

[pve-devel] [PATCH manager 1/2] fix #6304 ui: qemu: fix validator in DisplayEdit to match serial displays

2025-04-15 Thread Aaron Lauterer
because the value we get is not the config key, but the displayed text. Therefore we fetch the config key from the PVE.Utils.kvm_vga_drivers object. Signed-off-by: Aaron Lauterer --- We have some options here, we could also match and extract the serial ID from the displayed text, but that will fa

[pve-devel] [PATCH qemu-server v3 05/12] tree-wide: update vdisk_alloc vtype argument signature

2025-04-15 Thread Daniel Kral
Update any callers of `PVE::Storage::vdisk_alloc` to the updated function signature, which adds the required parameter `$vtype` that asserts whether the underlying storage is available and supports the content type. Make all callers assert for 'any' volume type, i.e. skip the check for now, so tha

[pve-devel] [PATCH storage v3 2/4] tree-wide: make use of content type assertion helper

2025-04-15 Thread Daniel Kral
Make any code path with an existent content type assertion use the newly introduced content type assertion helper. Where possible, existing calls to `storage_check_enabled()` can be lowered to calls to `storage_config()` as the former subroutine is already called in the helper already. Also the o

[pve-devel] [PATCH qemu-server v3 01/12] fix #5284: cli: importovf: assert content type support for target storage

2025-04-15 Thread Daniel Kral
Asserts whether the target storage supports storing VM images before importing a OVF manifest as a VM to the target storage. Without the check in place, a VM volume can be imported to a storage, which does not support VM images, but won't be able to start since any attached volume must be stored o

[pve-devel] [PATCH container v3 06/11] alloc_disk: update vdisk_alloc optional arguments signature

2025-04-15 Thread Daniel Kral
Update the call to `PVE::Storage::vdisk_alloc` to the updated function signature, which moves the optional argument `$name` (as it may be passed as undefined), to the optional hash argument at the end of the argument list. Signed-off-by: Daniel Kral --- changes since v2: * rename helper from *_

[pve-devel] [PATCH container v3 04/11] alloc_disk: factor out common arguments for call to vdisk_alloc

2025-04-15 Thread Daniel Kral
Signed-off-by: Daniel Kral --- changes since v2: * none src/PVE/LXC.pm | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm index 8581da6..780a0d4 100644 --- a/src/PVE/LXC.pm +++ b/src/PVE/LXC.pm @@ -2170,6 +2170,7 @@ sub alloc_disk {

[pve-devel] [PATCH qemu-server v3 03/12] fix #5284: api: update-vm: assert content type support for cloudinit images

2025-04-15 Thread Daniel Kral
Asserts whether the target storage supports storing cloudinit images, i.e. VM images, before creating a cloudinit image on the target storage. Without the check in place, a cloudinit image can be created on the storage, which does not support VM images, but won't be able to start since any attache