From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <f.schauer@proxmox.com>
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 <pve-devel@lists.proxmox.com>; 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 <pve-devel@lists.proxmox.com>; 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 <pve-devel@lists.proxmox.com>; 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 <pve-devel@lists.proxmox.com>; 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 <w.bumiller@proxmox.com>,
Thomas Lamprecht <t.lamprecht@proxmox.com>
Cc: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
References: <20231113103037.38313-1-f.schauer@proxmox.com>
<20231113103037.38313-4-f.schauer@proxmox.com>
<29c00d02-deeb-4563-ab01-639c939b6307@proxmox.com>
<ijtfmcmi3sb37jtknsrp3s4c3raosw67k7zdufu5pfxtmb3jiv@ayt62obbll6f>
From: Filip Schauer <f.schauer@proxmox.com>
In-Reply-To: <ijtfmcmi3sb37jtknsrp3s4c3raosw67k7zdufu5pfxtmb3jiv@ayt62obbll6f>
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 <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>
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.