[pve-devel] [PATCH v2 pve-container] unprivileged: remove bad chown -R call

[Wolfgang Bumiller]

This was added before we had bind mounts, instead we now
change ownership when creating disks by passing the
`root_owner` option to mkfs or activating+chown()ing the
paths for subvolumes.
---
 Changes: added chown to non-raw disks.
 Decided against doing it at mount time as that would mean shared
 mounts between priv+unpriv containers would be reowned whenever
 a container starts.

 src/PVE/LXC.pm        | 28 +++++++++++++++++++++-------
 src/PVE/LXC/Create.pm |  3 ---
 2 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
index 7f375b9..c61df23 100644
--- a/src/PVE/LXC.pm
+++ b/src/PVE/LXC.pm
@@ -2213,13 +2213,15 @@ sub get_vm_volumes {
 }
 
 sub mkfs {
-    my ($dev) = @_;
+    my ($dev, $rootuid, $rootgid) = @_;
 
-    PVE::Tools::run_command(['mkfs.ext4', '-O', 'mmp', $dev]);
+    PVE::Tools::run_command(['mkfs.ext4', '-O', 'mmp',
+			     '-E', "root_owner=$rootuid:$rootgid",
+			     $dev]);
 }
 
 sub format_disk {
-    my ($storage_cfg, $volid) = @_;
+    my ($storage_cfg, $volid, $rootuid, $rootgid) = @_;
 
     if ($volid =~ m!^/dev/.+!) {
 	mkfs($volid);
@@ -2240,7 +2242,7 @@ sub format_disk {
     die "cannot format volume '$volid' (format == $format)\n"
 	if $format ne 'raw';
 
-    mkfs($path);
+    mkfs($path, $rootuid, $rootgid);
 }
 
 sub destroy_disks {
@@ -2258,6 +2260,9 @@ sub create_disks {
     my $vollist = [];
 
     eval {
+	my (undef, $rootuid, $rootgid) = PVE::LXC::parse_id_maps($conf);
+	my $chown_vollist = [];
+
 	foreach_mountpoint($settings, sub {
 	    my ($ms, $mountpoint) = @_;
 
@@ -2280,25 +2285,27 @@ sub create_disks {
 		    if ($size_kb > 0) {
 			$volid = PVE::Storage::vdisk_alloc($storecfg, $storage, $vmid, 'raw',
 							   undef, $size_kb);
-			format_disk($storecfg, $volid);
+			format_disk($storecfg, $volid, $rootuid, $rootgid);
 		    } else {
 			$volid = PVE::Storage::vdisk_alloc($storecfg, $storage, $vmid, 'subvol',
 							   undef, 0);
+			push @$chown_vollist, $volid;
 		    }
 		} elsif ($scfg->{type} eq 'zfspool') {
 
 		    $volid = PVE::Storage::vdisk_alloc($storecfg, $storage, $vmid, 'subvol',
 					               undef, $size_kb);
+		    push @$chown_vollist, $volid;
 		} elsif ($scfg->{type} eq 'drbd' || $scfg->{type} eq 'lvm') {
 
 		    $volid = PVE::Storage::vdisk_alloc($storecfg, $storage, $vmid, 'raw', undef, $size_kb);
-		    format_disk($storecfg, $volid);
+		    format_disk($storecfg, $volid, $rootuid, $rootgid);
 
 		} elsif ($scfg->{type} eq 'rbd') {
 
 		    die "krbd option must be enabled on storage type '$scfg->{type}'\n" if !$scfg->{krbd};
 		    $volid = PVE::Storage::vdisk_alloc($storecfg, $storage, $vmid, 'raw', undef, $size_kb);
-		    format_disk($storecfg, $volid);
+		    format_disk($storecfg, $volid, $rootuid, $rootgid);
 		} else {
 		    die "unable to create containers on storage type '$scfg->{type}'\n";
 		}
@@ -2309,6 +2316,13 @@ sub create_disks {
 		# use specified/existing volid
 	    }
 	});
+
+	PVE::Storage::activate_volumes($storecfg, $chown_vollist, undef);
+	foreach my $volid (@$chown_vollist) {
+	    my $path = PVE::Storage::path($storecfg, $volid, undef);
+	    chown($rootuid, $rootgid, $path);
+	}
+	PVE::Storage::deactivate_volumes($storecfg, $chown_vollist, undef);
     };
     # free allocated images on error
     if (my $err = $@) {
diff --git a/src/PVE/LXC/Create.pm b/src/PVE/LXC/Create.pm
index 853a840..48d8843 100644
--- a/src/PVE/LXC/Create.pm
+++ b/src/PVE/LXC/Create.pm
@@ -28,9 +28,6 @@ sub restore_archive {
 #    we always use the same mapping: 'b:0:100000:65536'
     my ($id_map, $rootuid, $rootgid) = PVE::LXC::parse_id_maps($conf);
     my $userns_cmd = PVE::LXC::userns_command($id_map);
-    if (@$id_map) {
-	PVE::Tools::run_command(['chown', '-R', "$rootuid:$rootgid", $rootdir]);
-    }
 
     my $cmd = [@$userns_cmd, 'tar', 'xpf', $archive, '--totals',
                @$PVE::LXC::COMMON_TAR_FLAGS,
-- 
2.1.4