[pve-devel] [RFC pve-container 1/4] get rid of most of the loop-devices code

Wolfgang Bumiller w.bumiller at proxmox.com
Mon Sep 7 12:27:13 CEST 2015


-) loop devices are now attached in mountpoint_mount, and
immediately detached in order to set the auto-clear flag

Keeping track of loop-devices is otherweise next to
impossible and a security concern.
We mount the filesystems for the container. We do not
support full loop device access for containers for a simple
reason: once a container detached a loop device, the
startup of another container might reuse it, exposing its
devices to the first container, generating unwatned cross
container access permissions.

Loop devices are also set to auto-clear, so that we do not
need to worry about detaching them when stopping the
container.
---
 src/PVE/LXC.pm            | 134 ++++++++--------------------------------------
 src/PVE/LXC/Create.pm     |   7 ++-
 src/PVE/VZDump/LXC.pm     |  21 +-------
 src/lxc-pve-mount-hook    |   5 +-
 src/lxc-pve-prestart-hook |   1 -
 5 files changed, 29 insertions(+), 139 deletions(-)

diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
index 7b91906..216c3cf 100644
--- a/src/PVE/LXC.pm
+++ b/src/PVE/LXC.pm
@@ -1286,9 +1286,6 @@ sub vm_stop_cleanup {
 	if (!$keepActive) {
 
             my $vollist = get_vm_volumes($conf);
-            my $loopdevlist = get_vm_volumes($conf, 'rootfs');
-
-	    detach_loops($storage_cfg, $loopdevlist);
 	    PVE::Storage::deactivate_volumes($storage_cfg, $vollist);
 	}
     };
@@ -1821,21 +1818,6 @@ sub foreach_mountpoint_reverse {
     foreach_mountpoint_full($conf, 1, $func);
 }
 
-sub loopdevices_list {
-
-    my $loopdev = {};
-    my $parser = sub {
-	my $line = shift;
-	if ($line =~ m/^(\/dev\/loop\d+)\s+\d\s+\d\s+\d\s+\d\s(\S+)$/) {
-	    $loopdev->{$1} = $2;
-	}
-    };
-
-    PVE::Tools::run_command(['losetup'], outfunc => $parser);
-
-    return $loopdev;
-}
-
 sub blockdevices_list {
 
     my $bdevs = {};
@@ -1849,14 +1831,6 @@ sub blockdevices_list {
     return $bdevs;
 }
 
-sub find_loopdev {
-    my ($loopdevs, $path) = @_;
-
-    foreach my $dev (keys %$loopdevs){
-	return $dev if $loopdevs->{$dev} eq $path;
-    }
-}
-
 sub check_ct_modify_config_perm {
     my ($rpcenv, $authuser, $vmid, $pool, $key_list) = @_;
 
@@ -1881,62 +1855,8 @@ sub check_ct_modify_config_perm {
     return 1;
 }
 
-sub attach_loops {
-    my ($storage_cfg, $vollist, $snapname) = @_;
-
-    my $loopdevs = {};
-
-    foreach my $volid (@$vollist) {
-
-	my ($storage, $volname) = PVE::Storage::parse_volume_id($volid);
-	my $scfg = PVE::Storage::storage_config($storage_cfg, $storage);
-
-	my ($vtype, undef, undef, undef, undef, $isBase, $format) =
-	    PVE::Storage::parse_volname($storage_cfg, $volid);
-
-	if ($format eq 'raw' && $scfg->{path}) {
-	    my $path = PVE::Storage::path($storage_cfg, $volid, $snapname);
-	    my $loopdev;
-
-	    my $parser = sub {
-		my $line = shift;
-		$loopdev = $line if $line =~m|^/dev/loop\d+$|;
-		$loopdevs->{$loopdev} = $path;
-	    };
-
-	    PVE::Tools::run_command(['losetup', '--find', '--show', $path], outfunc => $parser);
-	}
-    }
-
-    return $loopdevs;
-}
-
-sub detach_loops {
-    my ($storage_cfg, $vollist, $snapname) = @_;
-
-    my $loopdevs = loopdevices_list();
-
-    foreach my $volid (@$vollist) {
-
-	my ($storage, $volname) = PVE::Storage::parse_volume_id($volid);
-	my $scfg = PVE::Storage::storage_config($storage_cfg, $storage);
-
-	my ($vtype, undef, undef, undef, undef, $isBase, $format) =
-	    PVE::Storage::parse_volname($storage_cfg, $volid);
-
-	if ($format eq 'raw' && $scfg->{path}) {
-	    my $path = PVE::Storage::path($storage_cfg, $volid, $snapname);
-            foreach my $dev (keys %$loopdevs){
-		PVE::Tools::run_command(['losetup', '-d', $dev]) if $loopdevs->{$dev} eq $path;
-	    }
-	}
-    }
-}
-
 sub umount_all {
-    my ($vmid, $storage_cfg, $conf, $noerr, $loopdevs) = @_;
-
-    $loopdevs ||= loopdevices_list();
+    my ($vmid, $storage_cfg, $conf, $noerr) = @_;
 
     my $rootdir = "/var/lib/lxc/$vmid/rootfs";
     my $volid_list = get_vm_volumes($conf);
@@ -1965,8 +1885,6 @@ sub umount_all {
 	    }
 	}
     });
-
-    PVE::LXC::detach_loops($storage_cfg, $volid_list);
 }
 
 sub mount_all {
@@ -1977,10 +1895,7 @@ sub mount_all {
     my $volid_list = get_vm_volumes($conf);
     PVE::Storage::activate_volumes($storage_cfg, $volid_list);
 
-    my $loopdevs;
     eval {
-	$loopdevs = attach_loops($storage_cfg, $volid_list);
-
 	foreach_mountpoint($conf, sub {
 	    my ($ms, $mountpoint) = @_;
 
@@ -1996,7 +1911,7 @@ sub mount_all {
 	    die "unable to mount base volume - internal error" if $isBase;
 
 	    File::Path::make_path "$rootdir/$mount" if $mkdirs;
-	    mountpoint_mount($mountpoint, $rootdir, $storage_cfg, $loopdevs);
+	    mountpoint_mount($mountpoint, $rootdir, $storage_cfg);
         });
     };
     if (my $err = $@) {
@@ -2004,19 +1919,19 @@ sub mount_all {
 	umount_all($vmid, $storage_cfg, $conf, 1);
     }
 
-    return wantarray ? ($rootdir, $loopdevs) : $rootdir;
+    return $rootdir;
 }
 
 
 sub mountpoint_mount_path {
-    my ($mountpoint, $storage_cfg, $loopdevs, $snapname) = @_;
+    my ($mountpoint, $storage_cfg, $snapname) = @_;
 
-    return mountpoint_mount($mountpoint, undef, $storage_cfg, $loopdevs, $snapname);
+    return mountpoint_mount($mountpoint, undef, $storage_cfg, $snapname);
 }
 
 # use $rootdir = undef to just return the corresponding mount path
 sub mountpoint_mount {
-    my ($mountpoint, $rootdir, $storage_cfg, $loopdevs, $snapname) = @_;
+    my ($mountpoint, $rootdir, $storage_cfg, $snapname) = @_;
 
     my $volid = $mountpoint->{volume};
     my $mount = $mountpoint->{mp};
@@ -2040,42 +1955,37 @@ sub mountpoint_mount {
 
 	my $scfg = PVE::Storage::storage_config($storage_cfg, $storage);
 	my $path = PVE::Storage::path($storage_cfg, $volid, $snapname);
+	return $path if !$mount_path;
 
 	my ($vtype, undef, undef, undef, undef, $isBase, $format) =
 	    PVE::Storage::parse_volname($storage_cfg, $volid);
 
 	if ($format eq 'subvol') {
-	    
-	    if ($mount_path) {
-		if ($snapname) {
-		    if ($scfg->{type} eq 'zfspool') {
-			my $path_arg = $path;
-			$path_arg =~ s!^/+!!;
-			PVE::Tools::run_command(['mount', '-o', 'ro', '-t', 'zfs', $path_arg, $mount_path]);
-		    } else {
-			die "cannot mount subvol snapshots for storage type '$scfg->{type}'\n";
-		    }		
+	    if ($snapname) {
+		if ($scfg->{type} eq 'zfspool') {
+		    my $path_arg = $path;
+		    $path_arg =~ s!^/+!!;
+		    PVE::Tools::run_command(['mount', '-o', 'ro', '-t', 'zfs', $path_arg, $mount_path]);
 		} else {
-		    PVE::Tools::run_command(['mount', '-o', 'bind', $path, $mount_path]);
-		}
+		    die "cannot mount subvol snapshots for storage type '$scfg->{type}'\n";
+		}		
+	    } else {
+		PVE::Tools::run_command(['mount', '-o', 'bind', $path, $mount_path]);
 	    }
 	    return $path;
-	    
 	} elsif ($format eq 'raw') {
-
+	    my @extra_opts;
 	    if ($scfg->{path}) {
-		$path = find_loopdev($loopdevs, $path) if $loopdevs;
+		push @extra_opts, '-o', 'loop';
 	    } elsif ($scfg->{type} eq 'drbd' || $scfg->{type} eq 'rbd') {
 		# do nothing
 	    } else {
 		die "unsupported storage type '$scfg->{type}'\n";
 	    }
-	    if ($mount_path) {
-		if ($isBase || defined($snapname)) {
-		    PVE::Tools::run_command(['mount', '-o', 'ro', $path, $mount_path]);
-		} else {
-		    PVE::Tools::run_command(['mount', $path, $mount_path]);
-		}
+	    if ($isBase || defined($snapname)) {
+		PVE::Tools::run_command(['mount', '-o', "ro", @extra_opts, $path, $mount_path]);
+	    } else {
+		PVE::Tools::run_command(['mount', @extra_opts, $path, $mount_path]);
 	    }
 	    return $path;
 	} else {
diff --git a/src/PVE/LXC/Create.pm b/src/PVE/LXC/Create.pm
index 9a63083..463af0b 100644
--- a/src/PVE/LXC/Create.pm
+++ b/src/PVE/LXC/Create.pm
@@ -205,16 +205,15 @@ sub create_rootfs {
 	PVE::LXC::create_config($vmid, $conf);
     }
 
-    my $loopdevs;
     eval {
-	my ($rootdir, $loopdevs) = PVE::LXC::mount_all($vmid, $storage_cfg, $conf);
+	my $rootdir = PVE::LXC::mount_all($vmid, $storage_cfg, $conf);
         restore_and_configure($vmid, $archive, $rootdir, $conf, $password, $restore);
     };
     if (my $err = $@) {
 	warn $err;
-	PVE::LXC::umount_all($vmid, $storage_cfg, $conf, 1, $loopdevs);
+	PVE::LXC::umount_all($vmid, $storage_cfg, $conf, 1);
     } else {
-	PVE::LXC::umount_all($vmid, $storage_cfg, $conf, 0, $loopdevs);
+	PVE::LXC::umount_all($vmid, $storage_cfg, $conf, 0);
     }
 
     PVE::Storage::deactivate_volumes($storage_cfg, PVE::LXC::get_vm_volumes($conf));
diff --git a/src/PVE/VZDump/LXC.pm b/src/PVE/VZDump/LXC.pm
index b99cd62..77cd9cf 100644
--- a/src/PVE/VZDump/LXC.pm
+++ b/src/PVE/VZDump/LXC.pm
@@ -130,10 +130,8 @@ sub prepare {
 	&$check_mountpoint_empty($mountpoint);
 
 	my $volid_list = [$diskinfo->{volid}];
-	$task->{cleanup}->{detach_loops} = $volid_list;
-	my $loopdevs = PVE::LXC::attach_loops($self->{storecfg}, $volid_list);
 	my $mp = { volume => $diskinfo->{volid}, mp => "/" };
-	PVE::LXC::mountpoint_mount($mp, $mountpoint, $self->{storecfg}, $loopdevs);
+	PVE::LXC::mountpoint_mount($mp, $mountpoint, $self->{storecfg});
 	$diskinfo->{dir} = $diskinfo->{mountpoint} = $mountpoint;
 	$task->{snapdir} = $diskinfo->{dir};
     } elsif ($mode eq 'suspend') {
@@ -177,13 +175,10 @@ sub snapshot {
     # my $volid_list = PVE::LXC::get_vm_volumes($snapconf);
     my $volid_list = [$diskinfo->{volid}];
 
-    $task->{cleanup}->{detach_loops} = $volid_list;
-    my $loopdevs = PVE::LXC::attach_loops($self->{storecfg}, $volid_list, 'vzdump');
-
     my $mountpoint = $default_mount_point;
 	
     my $mp = { volume => $diskinfo->{volid}, mp => "/" };
-    PVE::LXC::mountpoint_mount($mp, $mountpoint, $self->{storecfg}, $loopdevs, 'vzdump');
+    PVE::LXC::mountpoint_mount($mp, $mountpoint, $self->{storecfg}, 'vzdump');
  
     $diskinfo->{dir} = $diskinfo->{mountpoint} = $mountpoint;
     $task->{snapdir} = $diskinfo->{dir};
@@ -293,18 +288,6 @@ sub cleanup {
 	PVE::Tools::run_command(['umount', '-l', '-d', $mountpoint]);
     };
 
-    if (my $volid_list = $task->{cleanup}->{detach_vzdump_snapshot_loops}) {
-	PVE::LXC::detach_loops($self->{storecfg}, $volid_list, 'vzdump');
-    }
-
-    if (my $volid_list = $task->{cleanup}->{detach_loops}) {
-	if ($task->{cleanup}->{remove_snapshot}) {
-	    PVE::LXC::detach_loops($self->{storecfg}, $volid_list, 'vzdump');
-	} else {
-	    PVE::LXC::detach_loops($self->{storecfg}, $volid_list);
-	}
-    }
-
     if ($task->{cleanup}->{remove_snapshot}) {
 	$self->loginfo("remove vzdump snapshot");
 	PVE::LXC::snapshot_delete($vmid, 'vzdump', 0);
diff --git a/src/lxc-pve-mount-hook b/src/lxc-pve-mount-hook
index c4bccae..b7d84ed 100755
--- a/src/lxc-pve-mount-hook
+++ b/src/lxc-pve-mount-hook
@@ -89,13 +89,12 @@ __PACKAGE__->register_method ({
 	my $storage_cfg = PVE::Storage::Plugin->parse_config($fn, $raw);
 
 	my $bdevs = PVE::LXC::blockdevices_list();
-	my $loopdevs =  PVE::LXC::loopdevices_list();
 
 	my $setup_mountpoint = sub {
 	    my ($ms, $mountpoint) = @_;
 
 	    return if $ms eq 'rootfs';
-	    PVE::LXC::mountpoint_mount($mountpoint, $rootdir, $storage_cfg, $loopdevs);
+	    PVE::LXC::mountpoint_mount($mountpoint, $rootdir, $storage_cfg);
 	};
 
 	my $setup_cgroup_device = sub {
@@ -104,7 +103,7 @@ __PACKAGE__->register_method ({
 	    my $volid = $mountpoint->{volume};
 	    return if !$volid || $volid =~ m|^/dev/.+|;
 
-	    my $path = PVE::LXC::mountpoint_mount_path($mountpoint, $storage_cfg, $loopdevs);
+	    my $path = PVE::LXC::mountpoint_mount_path($mountpoint, $storage_cfg);
 
 	    if (-l $path) {
 		$path = readlink($path);
diff --git a/src/lxc-pve-prestart-hook b/src/lxc-pve-prestart-hook
index 04caff3..d2691eb 100755
--- a/src/lxc-pve-prestart-hook
+++ b/src/lxc-pve-prestart-hook
@@ -85,7 +85,6 @@ __PACKAGE__->register_method ({
 	my $loopdevlist = PVE::LXC::get_vm_volumes($conf, 'rootfs');
 
 	PVE::Storage::activate_volumes($storage_cfg, $vollist);
-	PVE::LXC::attach_loops($storage_cfg, $loopdevlist);
 	return undef;
     }});
 
-- 
2.1.4




More information about the pve-devel mailing list