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) server-digest SHA256) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id C7F82986C8 for ; Wed, 15 Nov 2023 15:15:23 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id A35AD74CE for ; Wed, 15 Nov 2023 15:14:53 +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) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Wed, 15 Nov 2023 15:14:52 +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 1BF324316E for ; Wed, 15 Nov 2023 15:14:52 +0100 (CET) Message-ID: <29c00d02-deeb-4563-ab01-639c939b6307@proxmox.com> Date: Wed, 15 Nov 2023 15:14:50 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Beta Content-Language: en-GB, de-AT To: Proxmox VE development discussion , Filip Schauer References: <20231113103037.38313-1-f.schauer@proxmox.com> <20231113103037.38313-4-f.schauer@proxmox.com> 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: <20231113103037.38313-4-f.schauer@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.116 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 PROLO_LEO1 0.1 Meta Catches all Leo drug variations so far 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 - URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [config.pm, tools.pm, lxc.pm, mount.auto] 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: Wed, 15 Nov 2023 14:15:23 -0000 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: > Add a dev[n] argument to the container config to pass devices through to > a container. A device can be passed by its path. Additionally the access > mode, uid and gid can be specified through their respective properties. > > Signed-off-by: Filip Schauer > --- > src/PVE/LXC.pm | 57 +++++++++++++++++++++++- > src/PVE/LXC/Config.pm | 93 +++++++++++++++++++++++++++++++++++++++ > src/PVE/LXC/Tools.pm | 23 +++++++--- > src/lxc-pve-autodev-hook | 18 +++++++- > src/lxc-pve-prestart-hook | 60 ++++++++++++++++++++++++- > 5 files changed, 242 insertions(+), 9 deletions(-) > > diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm > index 7ec816b..242b2a0 100644 > --- a/src/PVE/LXC.pm > +++ b/src/PVE/LXC.pm > @@ -5,7 +5,7 @@ use warnings; > > use Cwd qw(); > use Errno qw(ELOOP ENOTDIR EROFS ECONNREFUSED EEXIST); > -use Fcntl qw(O_RDONLY O_WRONLY O_NOFOLLOW O_DIRECTORY); > +use Fcntl qw(O_RDONLY O_WRONLY O_NOFOLLOW O_DIRECTORY :mode); > use File::Path; > use File::Spec; > use IO::Poll qw(POLLIN POLLHUP); > @@ -639,6 +639,31 @@ sub update_lxc_config { > $raw .= "lxc.mount.auto = sys:mixed\n"; > } > > + PVE::LXC::Config->foreach_passthrough_device($conf, sub { > + my ($key, $device) = @_; > + > + die "Path is not defined for passthrough device $key" > + unless (defined($device->{path})); > + > + my $absolute_path = $device->{path}; > + > + die "Device $absolute_path does not exist\n" > + unless (-e $absolute_path); > + > + die "$absolute_path is not a device\n" > + unless (-b $absolute_path || -c $absolute_path); Those are basically doing a stat each, but you do one below anyway, so maybe use that one for the checks > + > + my ($mode, $rdev) = (stat($absolute_path))[2, 6]; > + > + die "Could not get mode or device ID of $absolute_path\n" > + if (!defined($mode) || !defined($rdev)); > + > + my $major = PVE::Tools::dev_t_major($rdev); > + my $minor = PVE::Tools::dev_t_minor($rdev); > + my $device_type_char = S_ISBLK($mode) ? 'b' : 'c'; > + $raw .= "lxc.cgroup2.devices.allow = $device_type_char $major:$minor rw\n"; > + }); > + > # WARNING: DO NOT REMOVE this without making sure that loop device nodes > # cannot be exposed to the container with r/w access (cgroup perms). > # When this is enabled mounts will still remain in the monitor's namespace > @@ -1318,6 +1343,8 @@ sub check_ct_modify_config_perm { > $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.Network']); > check_bridge_access($rpcenv, $authuser, $oldconf->{$opt}) if $oldconf->{$opt}; > check_bridge_access($rpcenv, $authuser, $newconf->{$opt}) if $newconf->{$opt}; > + } elsif ($opt =~ m/^dev\d+$/) { > + raise_perm_exc("configuring device passthrough is only allowed for root\@pam"); > } elsif ($opt eq 'nameserver' || $opt eq 'searchdomain' || $opt eq 'hostname') { > $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.Network']); > } elsif ($opt eq 'features') { > @@ -2367,6 +2394,34 @@ sub validate_id_maps { > } > } > > +sub map_ct_id_to_host { > + my ($id, $id_map, $id_type) = @_; > + > + foreach my $mapping (@$id_map) { use for over foreach > + my ($type, $ct, $host, $length) = @$mapping; > + > + next if ($type ne $id_type); > + > + if ($id >= int($ct) && $id < ($ct + $length)) { > + return $host - $ct + $id; do we want to throw around some int() here too for good measure? > + } > + } > + > + return $id; > +} > + > +sub map_ct_uid_to_host { > + my ($uid, $id_map) = @_; > + > + return map_ct_id_to_host($uid, $id_map, 'u'); > +} > + > +sub map_ct_gid_to_host { > + my ($gid, $id_map) = @_; > + > + return map_ct_id_to_host($gid, $id_map, 'g'); > +} > + > sub userns_command { > my ($id_map) = @_; > if (@$id_map) { > 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 ? > + 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', > + 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', > + }, > +}; > + > +for (my $i = 0; $i < $MAX_DEVICES; $i++) { > + $confdesc->{"dev$i"} = { > + optional => 1, > + type => 'string', format => $dev_desc, > + description => "Device to pass through to the container", > + } > +} > + > sub parse_pct_config { > my ($filename, $raw, $strict) = @_; > > @@ -1255,6 +1321,21 @@ sub parse_volume { > return; > } > > +sub parse_device { > + my ($class, $device_string, $noerr) = @_; > + > + my $res; > + eval { $res = PVE::JSONSchema::parse_property_string($dev_desc, $device_string) }; you can write above as: my $res = eval { PVE::JSONSchema::parse_property_string($dev_desc, $device_string) }; which makes it slightly nicer > + if ($@) { > + return undef if $noerr; > + die $@; > + } > + > + die "Path has to be defined" unless (defined($res->{path})); We do not use `unless`, it's basically one of the few things our code universally (well almost, nobody is perfect..) agrees too, so lets keep it that way and use ... if !defined(...); also, shouldn't a set $noerr also disarm this die? I.e.: die "..." if !$noerr && !defined(...); > + > + return $res; > +} > + > sub print_volume { > my ($class, $key, $volume) = @_; > > @@ -1762,4 +1843,16 @@ sub get_derived_property { > } > } > > +sub foreach_passthrough_device { > + my ($class, $conf, $func, @param) = @_; > + > + foreach my $key (keys %$conf) { prefer for over foreach for new code (no semantic reason, just for consistency) > + next if $key !~ m/^dev(\d+)$/; > + > + my $device = $class->parse_device($conf->{$key}); > + > + $func->($key, $device, @param); > + } > +} > + > 1; > diff --git a/src/PVE/LXC/Tools.pm b/src/PVE/LXC/Tools.pm > index 62cdbc1..c9fb834 100644 > --- a/src/PVE/LXC/Tools.pm > +++ b/src/PVE/LXC/Tools.pm > @@ -81,10 +81,9 @@ sub lxc_hook($$&) { > $code->($ct_name, $common_vars, $namespaces, $args); > } > > -sub for_current_devices($&) { > - my ($vmid, $code) = @_; > +sub for_devices { > + my ($devlist_file, $vmid, $code) = @_; > > - my $devlist_file = "/var/lib/lxc/$vmid/devices"; > my $fd; > > if (! open $fd, '<', $devlist_file) { > @@ -93,8 +92,8 @@ sub for_current_devices($&) { > } > > while (defined(my $line = <$fd>)) { > - if ($line !~ m@^(b):(\d+):(\d+):/dev/(\S+)\s*$@) { > - warn "invalid .pve-devices entry: $line\n"; > + if ($line !~ m@^(b|c):(\d+):(\d+):/dev/(\S+)\s*$@) { > + warn "invalid $devlist_file entry: $line\n"; > next; > } > > @@ -117,6 +116,20 @@ sub for_current_devices($&) { > close $fd; > } > > +sub for_current_devices($&) { > + my ($vmid, $code) = @_; > + > + my $devlist_file = "/var/lib/lxc/$vmid/devices"; > + for_devices($devlist_file, $vmid, $code); > +} > + > +sub for_passthrough_devices($&) { > + my ($vmid, $code) = @_; > + > + my $passthrough_devlist_file = "/var/lib/lxc/$vmid/passthrough_devices"; > + for_devices($passthrough_devlist_file, $vmid, $code); > +} > + > sub cgroup_do_write($$) { > my ($path, $value) = @_; > my $fd; > diff --git a/src/lxc-pve-autodev-hook b/src/lxc-pve-autodev-hook > index 3c45949..105465f 100755 > --- a/src/lxc-pve-autodev-hook > +++ b/src/lxc-pve-autodev-hook > @@ -3,17 +3,31 @@ > use strict; > use warnings; > > -use File::Path; > +use Fcntl qw(S_IFREG); > use File::Basename; > +use File::Path; > > use PVE::LXC::Tools; > -use PVE::Tools; > +use PVE::Tools qw(MS_BIND); > > PVE::LXC::Tools::lxc_hook('autodev', 'lxc', sub { > my ($vmid, $vars, undef, undef) = @_; > > my $root = $vars->{ROOTFS_MOUNT}; > > + PVE::LXC::Tools::for_passthrough_devices($vmid, sub { > + my ($type, $major, $minor, $dev) = @_; > + > + my $rel_devpath = "/dev/$dev"; > + my $rel_dir = dirname($rel_devpath); > + File::Path::mkpath("$root/$rel_dir"); > + PVE::Tools::mknod("$root/dev/$dev", S_IFREG, 0) > + or die("Could not mknod $root/dev/$dev"); Please include $! in the error > + > + PVE::Tools::mount("/var/lib/lxc/$vmid/passthrough/dev/$dev", "$root/dev/$dev", 0, MS_BIND, 0) > + or die("Bind mount of device $dev into container failed\n"); same here, add $! in error > + }); > + > PVE::LXC::Tools::for_current_devices($vmid, sub { > my ($type, $major, $minor, $dev) = @_; > > diff --git a/src/lxc-pve-prestart-hook b/src/lxc-pve-prestart-hook > index 936d0bf..d8b0106 100755 > --- a/src/lxc-pve-prestart-hook > +++ b/src/lxc-pve-prestart-hook > @@ -6,6 +6,7 @@ use strict; > use warnings; > > use Fcntl qw(O_DIRECTORY :mode); > +use File::Basename; > use File::Path; > use POSIX; > > @@ -62,7 +63,7 @@ PVE::LXC::Tools::lxc_hook('pre-start', 'lxc', sub { > unlink $devlist_file; > my $devices = []; > > - my (undef, $rootuid, $rootgid) = PVE::LXC::parse_id_maps($conf); > + my ($id_map, $rootuid, $rootgid) = PVE::LXC::parse_id_maps($conf); > > # Unmount first when the user mounted the container with "pct mount". > eval { > @@ -118,6 +119,51 @@ PVE::LXC::Tools::lxc_hook('pre-start', 'lxc', sub { > > PVE::LXC::Config->foreach_volume($conf, $setup_mountpoint); > > + # Device passthrough > + my $passthrough_devlist_file = "/var/lib/lxc/$vmid/passthrough_devices"; would favor kebab-case for filenames too > + unlink $passthrough_devlist_file; Would be good to have error handling there: unlink or $!{ENOENT} or die "failed to clean-up passthrough device list file - $!\n" > + my $passthrough_devices = []; > + > + my $passthrough_dir = "/var/lib/lxc/$vmid/passthrough"; no hard feelings, but what about using a common directory for the device list file and the mountpoint: /var/lib/lxc/$vmid/passthrough/devices /var/lib/lxc/$vmid/passthrough/mount But I did not really check the overall picture, so this can be fine as is (well, with snake_case -> kebab-case) > + File::Path::make_path($passthrough_dir); > + PVE::Tools::mount("none", $passthrough_dir, "tmpfs", 0, "size=8k") > + or die ("Could not mount tmpfs for device passthrough at $passthrough_dir"); please include $! in the error message so that we have a better idea what may be going on if a user reports running into this error. > + > + my $setup_passthrough_device = sub { > + my ($key, $device) = @_; > + > + my $absolute_path = $device->{path}; > + my ($mode, $rdev) = (stat($absolute_path))[2, 6]; > + > + die "Could not get mode or device ID of $absolute_path\n" > + if (!defined($mode) || !defined($rdev)); > + > + my $passthrough_device_path = $passthrough_dir . $absolute_path; > + File::Path::make_path(dirname($passthrough_device_path)); > + PVE::Tools::mknod($passthrough_device_path, $mode, $rdev) > + or die("failed to mknod $passthrough_device_path\n"); please include $! in the error > + > + # Use chmod because umask could mess with the access mode on mknod > + my $passthrough_mode = 0660; > + $passthrough_mode = oct($device->{mode}) if (defined($device->{mode})); > + chmod $passthrough_mode, $passthrough_device_path > + or die "failed to chmod $passthrough_mode $passthrough_device_path: $!\n"; > + > + # Set uid and gid of the device node > + my $uid = 0; > + my $gid = 0; > + $uid = $device->{uid} if (defined($device->{uid})); > + $gid = $device->{gid} if (defined($device->{gid})); you can drop the outer parenthesis in the post-if's expression for both of above lines for consistency. > + $uid = PVE::LXC::map_ct_uid_to_host($uid, $id_map); > + $gid = PVE::LXC::map_ct_gid_to_host($gid, $id_map); > + chown $uid, $gid, $passthrough_device_path > + or die("failed to chown $uid:$gid $passthrough_device_path: $!\n"); > + > + push @$passthrough_devices, [$absolute_path, $mode, $rdev]; > + }; > + > + PVE::LXC::Config->foreach_passthrough_device($conf, $setup_passthrough_device); > + > my $lxc_setup = PVE::LXC::Setup->new($conf, $rootdir); > $lxc_setup->pre_start_hook(); > > @@ -140,6 +186,18 @@ PVE::LXC::Tools::lxc_hook('pre-start', 'lxc', sub { > } > PVE::Tools::file_set_contents($devlist_file, $devlist); > } > + > + if (@$passthrough_devices) { > + my $devlist = ''; > + foreach my $dev (@$passthrough_devices) { s/foreach/for/ > + my ($path, $mode, $rdev) = @$dev; > + my $major = PVE::Tools::dev_t_major($rdev); > + my $minor = PVE::Tools::dev_t_minor($rdev); > + my $device_type_char = S_ISBLK($mode) ? 'b' : 'c'; > + $devlist .= "$device_type_char:$major:$minor:$path\n"; > + } > + PVE::Tools::file_set_contents($passthrough_devlist_file, $devlist); > + } > }); > > # Leftover cgroups prevent lxc from starting without any useful information