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 8EB6A99558 for ; Thu, 16 Nov 2023 12:52:33 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 71EB2125B8 for ; Thu, 16 Nov 2023 12:52:33 +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 ; Thu, 16 Nov 2023 12:52:32 +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 7E6A34380B for ; Thu, 16 Nov 2023 12:52:32 +0100 (CET) Message-ID: <97c2f797-2a8b-431b-84d2-47678c72fd5b@proxmox.com> Date: Thu, 16 Nov 2023 12:52:31 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Content-Language: en-US To: Wolfgang Bumiller , Thomas Lamprecht Cc: Proxmox VE development discussion References: <20231113103037.38313-1-f.schauer@proxmox.com> <20231113103037.38313-4-f.schauer@proxmox.com> <29c00d02-deeb-4563-ab01-639c939b6307@proxmox.com> From: Filip Schauer In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.225 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] [PATCH v4 container 1/1] Add device passthrough 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: Thu, 16 Nov 2023 11:52:33 -0000 Patch v5 available https://lists.proxmox.com/pipermail/pve-devel/2023-November/060285.html On 16/11/2023 09:32, Wolfgang Bumiller wrote: > On Wed, Nov 15, 2023 at 03:14:50PM +0100, Thomas Lamprecht wrote: >> concept wise this looks pretty much OK, but a few (mostly code-style) comments in line >> >> Am 13/11/2023 um 11:30 schrieb Filip Schauer: >>> diff --git a/src/PVE/LXC/Config.pm b/src/PVE/LXC/Config.pm >>> index 56e1f10..9f325f2 100644 >>> --- a/src/PVE/LXC/Config.pm >>> +++ b/src/PVE/LXC/Config.pm >>> @@ -29,6 +29,7 @@ mkdir $lockdir; >>> mkdir "/etc/pve/nodes/$nodename/lxc"; >>> my $MAX_MOUNT_POINTS = 256; >>> my $MAX_UNUSED_DISKS = $MAX_MOUNT_POINTS; >>> +my $MAX_DEVICES = 256; >>> >>> # BEGIN implemented abstract methods from PVE::AbstractConfig >>> >>> @@ -908,6 +909,71 @@ for (my $i = 0; $i < $MAX_UNUSED_DISKS; $i++) { >>> } >>> } >>> >>> +PVE::JSONSchema::register_format('pve-lxc-dev-string', \&verify_lxc_dev_string); >>> +sub verify_lxc_dev_string { >>> + my ($dev, $noerr) = @_; >>> + >>> + if ( >>> + $dev =~ m@/\.\.?/@ || >>> + $dev =~ m@/\.\.?$@ || >> could be a single regex: >> >> $dev =~ @/\.\.?(?:/|$)@ >> >> but no hard feelings, all variant are not easily readable and need close >> checking anyway (iow. like most regexes) >> >>> + $dev !~ m!^/dev/! >>> + ) { >>> + return undef if $noerr; >>> + die "$dev is not a valid device path\n"; >>> + } >>> + >>> + return $dev; >>> +} >>> + >>> +PVE::JSONSchema::register_format('file-access-mode-string', \&verify_file_access_mode); >>> +sub verify_file_access_mode { >>> + my ($mode, $noerr) = @_; >>> + >>> + if ($mode !~ /^[0-7]*$/) { >> this would allow an empty mode though? Also, not sure if we want to allow >> partial modes like 77 ? > Yeah, an empty mode should not be allowed. Not sure what you mean by > partial though, other than the missing leading zero. > > For octal we should definitely enforce the leading zero. > >>> + return undef if $noerr; >>> + die "$mode is not a valid file access mode\n"; >>> + } >>> + >>> + return $mode; >>> +} >>> + >>> +my $dev_desc = { >>> + path => { >>> + optional => 1, >>> + type => 'string', >>> + default_key => 1, >>> + format => 'pve-lxc-dev-string', >>> + format_description => 'Path', >>> + description => 'Device to pass through to the container', >>> + verbose_description => 'Path to the device to pass through to the container', >>> + }, >>> + mode => { >>> + optional => 1, >>> + type => 'integer', >>> + format => 'file-access-mode-string', > ... this should be `type => 'string'` (integer just doesn't enforce a > format), > the `format` should be dropped (including the registered sub above), and > instead use 'pattern' as: > > pattern => '0[0-7]*', > > JSONSchema anchors the pattern, so no need to include '^' and '$', > although I also wouldn't mind using > > pattern => qr/^0[0-7]*$/, > >>> + format_description => 'Octal access mode', >>> + description => 'Access mode to be set on the device node', >>> + }, >>> + uid => { >>> + optional => 1, >>> + type => 'integer', >>> + description => 'User ID to be assigned to the device node', >>> + }, >>> + gid => { >>> + optional => 1, >>> + type => 'integer', >>> + description => 'Group ID to be assigned to the device node', > Add `minimum => 0`, to both uid and gid.