[pve-devel] [PATCH container 3/4] Refactor mountpoint and general conf methods

Fabian Grünbichler f.gruenbichler at proxmox.com
Wed Mar 2 14:03:51 CET 2016


Move add_unused_volume into abstract
pve-common/src/PVE/AbstractConfig.pm, because it is
identical for LXC and Qemu.

Move classify_mountpoint, is_volume_in_use, has_dev_console,
mountpoint_names, foreach_mountpoint_XX and get_vm_volumes
to PVE::LXC::Config because they only deal with config
related matters.

(Some of) the latter methods might get moved to or become
implementations of methods in PVE::AbstractConfig in the
future.
---
 src/PVE/API2/LXC.pm       |   4 +-
 src/PVE/CLI/pct.pm        |   2 +-
 src/PVE/LXC.pm            | 148 +++++-----------------------------------------
 src/PVE/LXC/Config.pm     |  98 +++++++++++++++++++++++++++++-
 src/PVE/LXC/Create.pm     |   2 +-
 src/PVE/LXC/Migrate.pm    |   8 +--
 src/PVE/VZDump/LXC.pm     |   2 +-
 src/lxc-pve-prestart-hook |   6 +-
 8 files changed, 123 insertions(+), 147 deletions(-)

diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
index 428f2c1..6cd754f 100644
--- a/src/PVE/API2/LXC.pm
+++ b/src/PVE/API2/LXC.pm
@@ -258,7 +258,7 @@ __PACKAGE__->register_method({
 	}
 
 	# check storage access, activate storage
-	PVE::LXC::foreach_mountpoint($param, sub {
+	PVE::LXC::Config->foreach_mountpoint($param, sub {
 	    my ($ms, $mountpoint) = @_;
 
 	    my $volid = $mountpoint->{volume};
@@ -1208,7 +1208,7 @@ __PACKAGE__->register_method({
 	    disk => {
 		type => 'string',
 		description => "The disk you want to resize.",
-		enum => [PVE::LXC::mountpoint_names()],
+		enum => [PVE::LXC::Config->mountpoint_names()],
 	    },
 	    size => {
 		type => 'string',
diff --git a/src/PVE/CLI/pct.pm b/src/PVE/CLI/pct.pm
index 1fde039..c7adfc4 100755
--- a/src/PVE/CLI/pct.pm
+++ b/src/PVE/CLI/pct.pm
@@ -151,7 +151,7 @@ __PACKAGE__->register_method ({
 		optional => 1,
 		type => 'string',
 		description => "A volume on which to run the filesystem check",
-		enum => [PVE::LXC::mountpoint_names()],
+		enum => [PVE::LXC::Config->mountpoint_names()],
 	    },
 	},
     },
diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
index 0144861..913d68b 100644
--- a/src/PVE/LXC.pm
+++ b/src/PVE/LXC.pm
@@ -433,7 +433,7 @@ sub write_pct_config {
     my ($filename, $conf) = @_;
 
     delete $conf->{snapstate}; # just to be sure
-    my $volidlist = get_vm_volumes($conf);
+    my $volidlist = PVE::LXC::Config->get_vm_volumes($conf);
     my $used_volids = {};
     foreach my $vid (@$volidlist) {
 	$used_volids->{$vid} = 1;
@@ -825,15 +825,6 @@ sub vmstatus {
     return $list;
 }
 
-sub classify_mountpoint {
-    my ($vol) = @_;
-    if ($vol =~ m!^/!) {
-	return 'device' if $vol =~ m!^/dev/!;
-	return 'bind';
-    }
-    return 'volume';
-}
-
 my $parse_ct_mountpoint_full = sub {
     my ($desc, $data, $noerr) = @_;
 
@@ -855,7 +846,7 @@ my $parse_ct_mountpoint_full = sub {
 	$res->{size} = $size;
     }
 
-    $res->{type} = classify_mountpoint($res->{volume});
+    $res->{type} = PVE::LXC::Config->classify_mountpoint($res->{volume});
 
     return $res;
 };
@@ -1024,7 +1015,7 @@ sub update_lxc_config {
 	$raw .= "lxc.id_map = g 0 100000 65536\n";
     }
 
-    if (!has_dev_console($conf)) {
+    if (!PVE::LXC::Config->has_dev_console($conf)) {
 	$raw .= "lxc.console = none\n";
 	$raw .= "lxc.cgroup.devices.deny = c 5:1 rwm\n";
     }
@@ -1113,48 +1104,6 @@ sub verify_searchdomain_list {
     return join(' ', @list);
 }
 
-sub is_volume_in_use {
-    my ($config, $volid, $include_snapshots) = @_;
-    my $used = 0;
-
-    foreach_mountpoint($config, sub {
-	my ($ms, $mountpoint) = @_;
-	return if $used;
-	if ($mountpoint->{type} eq 'volume' && $mountpoint->{volume} eq $volid) {
-	    $used = 1;
-	}
-    });
-
-    my $snapshots = $config->{snapshots};
-    if ($include_snapshots && $snapshots) {
-	foreach my $snap (keys %$snapshots) {
-	    $used ||= is_volume_in_use($snapshots->{$snap}, $volid);
-	}
-    }
-
-    return $used;
-}
-
-sub add_unused_volume {
-    my ($config, $volid) = @_;
-
-    my $key;
-    for (my $ind = $MAX_UNUSED_DISKS - 1; $ind >= 0; $ind--) {
-	my $test = "unused$ind";
-	if (my $vid = $config->{$test}) {
-	    return if $vid eq $volid; # do not add duplicates
-	} else {
-	    $key = $test;
-	}
-    }
-
-    die "Too many unused volumes - please delete them first.\n" if !$key;
-
-    $config->{$key} = $volid;
-
-    return $key;
-}
-
 sub update_pct_config {
     my ($vmid, $conf, $running, $param, $delete) = @_;
 
@@ -1214,7 +1163,7 @@ sub update_pct_config {
 		my $mp = parse_ct_mountpoint($conf->{$opt});
 		delete $conf->{$opt};
 		if ($mp->{type} eq 'volume') {
-		    add_unused_volume($conf, $mp->{volume});
+		    PVE::LXC::Config->add_unused_volume($conf, $mp->{volume});
 		}
 	    } elsif ($opt eq 'unprivileged') {
 		die "unable to delete read-only option: '$opt'\n";
@@ -1301,7 +1250,7 @@ sub update_pct_config {
 	    if (defined($old)) {
 		my $mp = parse_ct_mountpoint($old);
 		if ($mp->{type} eq 'volume') {
-		    add_unused_volume($conf, $mp->{volume});
+		    PVE::LXC::Config->add_unused_volume($conf, $mp->{volume});
 		}
 	    }
 	    $new_disks = 1;
@@ -1315,7 +1264,7 @@ sub update_pct_config {
 	    if (defined($old)) {
 		my $mp = parse_ct_rootfs($old);
 		if ($mp->{type} eq 'volume') {
-		    add_unused_volume($conf, $mp->{volume});
+		    PVE::LXC::Config->add_unused_volume($conf, $mp->{volume});
 		}
 	    }
 	    my $mp = parse_ct_rootfs($value);
@@ -1337,7 +1286,7 @@ sub update_pct_config {
 	foreach my $volume (@deleted_volumes) {
 	    next if $used_volids->{$volume}; # could have been re-added, too
 	    # also check for references in snapshots
-	    next if is_volume_in_use($conf, $volume, 1);
+	    next if PVE::LXC::Config->is_volume_in_use($conf, $volume, 1);
 	    delete_mountpoint_volume($storage_cfg, $vmid, $volume);
 	}
     }
@@ -1353,12 +1302,6 @@ sub update_pct_config {
     }
 }
 
-sub has_dev_console {
-    my ($conf) = @_;
-
-    return !(defined($conf->{console}) && !$conf->{console});
-}
-	
 sub get_tty_count {
     my ($conf) = @_;
 
@@ -1418,7 +1361,7 @@ sub get_primary_ips {
 sub delete_mountpoint_volume {
     my ($storage_cfg, $vmid, $volume) = @_;
 
-    return if classify_mountpoint($volume) ne 'volume';
+    return if PVE::LXC::Config->classify_mountpoint($volume) ne 'volume';
 
     my ($vtype, $name, $owner) = PVE::Storage::parse_volname($storage_cfg, $volume);
     PVE::Storage::vdisk_free($storage_cfg, $volume) if $vmid == $owner;
@@ -1427,7 +1370,7 @@ sub delete_mountpoint_volume {
 sub destroy_lxc_container {
     my ($storage_cfg, $vmid, $conf) = @_;
 
-    foreach_mountpoint($conf, sub {
+    PVE::LXC::Config->foreach_mountpoint($conf, sub {
 	my ($ms, $mountpoint) = @_;
 	delete_mountpoint_volume($storage_cfg, $vmid, $mountpoint->{volume});
     });
@@ -1447,7 +1390,7 @@ sub vm_stop_cleanup {
     eval {
 	if (!$keepActive) {
 
-            my $vollist = get_vm_volumes($conf);
+            my $vollist = PVE::LXC::Config->get_vm_volumes($conf);
 	    PVE::Storage::deactivate_volumes($storage_cfg, $vollist);
 	}
     };
@@ -1750,44 +1693,6 @@ sub template_create {
     PVE::LXC::Config->write_config($vmid, $conf);
 }
 
-sub mountpoint_names {
-    my ($reverse) = @_;
-
-    my @names = ('rootfs');
-
-    for (my $i = 0; $i < $MAX_MOUNT_POINTS; $i++) {
-	push @names, "mp$i";
-    }
-
-    return $reverse ? reverse @names : @names;
-}
-
-
-sub foreach_mountpoint_full {
-    my ($conf, $reverse, $func) = @_;
-
-    foreach my $key (mountpoint_names($reverse)) {
-	my $value = $conf->{$key};
-	next if !defined($value);
-	my $mountpoint = $key eq 'rootfs' ? parse_ct_rootfs($value, 1) : parse_ct_mountpoint($value, 1);
-	next if !defined($mountpoint);
-
-	&$func($key, $mountpoint);
-    }
-}
-
-sub foreach_mountpoint {
-    my ($conf, $func) = @_;
-
-    foreach_mountpoint_full($conf, 0, $func);
-}
-
-sub foreach_mountpoint_reverse {
-    my ($conf, $func) = @_;
-
-    foreach_mountpoint_full($conf, 1, $func);
-}
-
 sub check_ct_modify_config_perm {
     my ($rpcenv, $authuser, $vmid, $pool, $newconf, $delete) = @_;
 
@@ -1827,9 +1732,9 @@ sub umount_all {
     my ($vmid, $storage_cfg, $conf, $noerr) = @_;
 
     my $rootdir = "/var/lib/lxc/$vmid/rootfs";
-    my $volid_list = get_vm_volumes($conf);
+    my $volid_list = PVE::LXC::Config->get_vm_volumes($conf);
 
-    foreach_mountpoint_reverse($conf, sub {
+    PVE::LXC::Config->foreach_mountpoint_reverse($conf, sub {
 	my ($ms, $mountpoint) = @_;
 
 	my $volid = $mountpoint->{volume};
@@ -1861,11 +1766,11 @@ sub mount_all {
     my $rootdir = "/var/lib/lxc/$vmid/rootfs";
     File::Path::make_path($rootdir);
 
-    my $volid_list = get_vm_volumes($conf);
+    my $volid_list = PVE::LXC::Config->get_vm_volumes($conf);
     PVE::Storage::activate_volumes($storage_cfg, $volid_list);
 
     eval {
-	foreach_mountpoint($conf, sub {
+	PVE::LXC::Config->foreach_mountpoint($conf, sub {
 	    my ($ms, $mountpoint) = @_;
 
 	    mountpoint_mount($mountpoint, $rootdir, $storage_cfg);
@@ -2060,29 +1965,6 @@ sub mountpoint_mount {
     die "unsupported storage";
 }
 
-sub get_vm_volumes {
-    my ($conf, $excludes) = @_;
-
-    my $vollist = [];
-
-    foreach_mountpoint($conf, sub {
-	my ($ms, $mountpoint) = @_;
-
-	return if $excludes && $ms eq $excludes;
-
-	my $volid = $mountpoint->{volume};
-
-        return if !$volid || $mountpoint->{type} ne 'volume';
-
-        my ($sid, $volname) = PVE::Storage::parse_volume_id($volid, 1);
-        return if !$sid;
-
-        push @$vollist, $volid;
-    });
-
-    return $vollist;
-}
-
 sub mkfs {
     my ($dev, $rootuid, $rootgid) = @_;
 
@@ -2134,7 +2016,7 @@ sub create_disks {
 	my (undef, $rootuid, $rootgid) = PVE::LXC::parse_id_maps($conf);
 	my $chown_vollist = [];
 
-	foreach_mountpoint($settings, sub {
+	PVE::LXC::Config->foreach_mountpoint($settings, sub {
 	    my ($ms, $mountpoint) = @_;
 
 	    my $volid = $mountpoint->{volume};
diff --git a/src/PVE/LXC/Config.pm b/src/PVE/LXC/Config.pm
index f041450..4426e59 100644
--- a/src/PVE/LXC/Config.pm
+++ b/src/PVE/LXC/Config.pm
@@ -48,7 +48,7 @@ sub has_feature {
     my ($class, $feature, $conf, $storecfg, $snapname, $running, $backup_only) = @_;
     my $err;
 
-    PVE::LXC::foreach_mountpoint($conf, sub {
+    $class->foreach_mountpoint($conf, sub {
 	my ($ms, $mountpoint) = @_;
 
 	return if $err; # skip further test
@@ -157,9 +157,103 @@ sub __snapshot_rollback_vm_start {
 sub __snapshot_foreach_volume {
     my ($class, $conf, $func) = @_;
 
-    PVE::LXC::foreach_mountpoint($conf, $func);
+    $class->foreach_mountpoint($conf, $func);
 }
 
 # END implemented abstract methods from PVE::AbstractConfig
 
+sub classify_mountpoint {
+    my ($class, $vol) = @_;
+    if ($vol =~ m!^/!) {
+	return 'device' if $vol =~ m!^/dev/!;
+	return 'bind';
+    }
+    return 'volume';
+}
+
+sub is_volume_in_use {
+    my ($class, $config, $volid, $include_snapshots) = @_;
+    my $used = 0;
+
+    $class->foreach_mountpoint($config, sub {
+	my ($ms, $mountpoint) = @_;
+	return if $used;
+	$used = $mountpoint->{type} eq 'volume' && $mountpoint->{volume} eq $volid;
+    });
+
+    my $snapshots = $config->{snapshots};
+    if ($include_snapshots && $snapshots) {
+	foreach my $snap (keys %$snapshots) {
+	    $used ||= $class->is_volume_in_use($snapshots->{$snap}, $volid);
+	}
+    }
+
+    return $used;
+}
+
+sub has_dev_console {
+    my ($class, $conf) = @_;
+
+    return !(defined($conf->{console}) && !$conf->{console});
+}
+
+sub mountpoint_names {
+    my ($class, $reverse) = @_;
+
+    my @names = ('rootfs');
+
+    for (my $i = 0; $i < $MAX_MOUNT_POINTS; $i++) {
+	push @names, "mp$i";
+    }
+
+    return $reverse ? reverse @names : @names;
+}
+
+sub foreach_mountpoint_full {
+    my ($class, $conf, $reverse, $func) = @_;
+
+    foreach my $key ($class->mountpoint_names($reverse)) {
+	my $value = $conf->{$key};
+	next if !defined($value);
+	my $mountpoint = $key eq 'rootfs' ? PVE::LXC::parse_ct_rootfs($value, 1) : PVE::LXC::parse_ct_mountpoint($value, 1);
+	next if !defined($mountpoint);
+
+	&$func($key, $mountpoint);
+    }
+}
+
+sub foreach_mountpoint {
+    my ($class, $conf, $func) = @_;
+
+    $class->foreach_mountpoint_full($conf, 0, $func);
+}
+
+sub foreach_mountpoint_reverse {
+    my ($class, $conf, $func) = @_;
+
+    $class->foreach_mountpoint_full($conf, 1, $func);
+}
+
+sub get_vm_volumes {
+    my ($class, $conf, $excludes) = @_;
+
+    my $vollist = [];
+
+    $class->foreach_mountpoint($conf, sub {
+	my ($ms, $mountpoint) = @_;
+
+	return if $excludes && $ms eq $excludes;
+
+	my $volid = $mountpoint->{volume};
+	return if !$volid || $mountpoint->{type} ne 'volume';
+
+	my ($sid, $volname) = PVE::Storage::parse_volume_id($volid, 1);
+	return if !$sid;
+
+	push @$vollist, $volid;
+    });
+
+    return $vollist;
+}
+
 return 1;
diff --git a/src/PVE/LXC/Create.pm b/src/PVE/LXC/Create.pm
index a7c66a1..931cbae 100644
--- a/src/PVE/LXC/Create.pm
+++ b/src/PVE/LXC/Create.pm
@@ -225,7 +225,7 @@ sub create_rootfs {
     };
     my $err = $@;
     PVE::LXC::umount_all($vmid, $storage_cfg, $conf, $err ? 1 : 0);
-    PVE::Storage::deactivate_volumes($storage_cfg, PVE::LXC::get_vm_volumes($conf));
+    PVE::Storage::deactivate_volumes($storage_cfg, PVE::LXC::Config->get_vm_volumes($conf));
     die $err if $err;
 }
 
diff --git a/src/PVE/LXC/Migrate.pm b/src/PVE/LXC/Migrate.pm
index ba23f1a..61e0806 100644
--- a/src/PVE/LXC/Migrate.pm
+++ b/src/PVE/LXC/Migrate.pm
@@ -39,7 +39,7 @@ sub prepare {
 	$running = 1;
     }
 
-    PVE::LXC::foreach_mountpoint($conf, sub {
+    PVE::LXC::Config->foreach_mountpoint($conf, sub {
 	my ($ms, $mountpoint) = @_;
 
 	my $volid = $mountpoint->{volume};
@@ -55,7 +55,7 @@ sub prepare {
 
     });
 
-    my $volid_list = PVE::LXC::get_vm_volumes($conf);
+    my $volid_list = PVE::LXC::Config->get_vm_volumes($conf);
     PVE::Storage::activate_volumes($self->{storecfg}, $volid_list);
 
     # todo: test if VM uses local resources
@@ -83,7 +83,7 @@ sub phase1 {
 
     $self->{volumes} = [];
 
-    PVE::LXC::foreach_mountpoint($conf, sub {
+    PVE::LXC::Config->foreach_mountpoint($conf, sub {
 	my ($ms, $mountpoint) = @_;
 
 	my $volid = $mountpoint->{volume};
@@ -114,7 +114,7 @@ sub phase1 {
     PVE::LXC::umount_all($vmid, $self->{storecfg}, $conf);
 
     #to be sure there are no active volumes
-    my $vollist = PVE::LXC::get_vm_volumes($conf);
+    my $vollist = PVE::LXC::Config->get_vm_volumes($conf);
     PVE::Storage::deactivate_volumes($self->{storecfg}, $vollist);
 
     # move config
diff --git a/src/PVE/VZDump/LXC.pm b/src/PVE/VZDump/LXC.pm
index a91fabd..396a94d 100644
--- a/src/PVE/VZDump/LXC.pm
+++ b/src/PVE/VZDump/LXC.pm
@@ -111,7 +111,7 @@ sub prepare {
     $task->{userns_cmd} = PVE::LXC::userns_command($id_map);
 
     my $volids = $task->{volids} = [];
-    PVE::LXC::foreach_mountpoint($conf, sub {
+    PVE::LXC::Config->foreach_mountpoint($conf, sub {
 	my ($name, $data) = @_;
 	my $volid = $data->{volume};
 	my $mount = $data->{mp};
diff --git a/src/lxc-pve-prestart-hook b/src/lxc-pve-prestart-hook
index 7cb9cb6..9835b1c 100755
--- a/src/lxc-pve-prestart-hook
+++ b/src/lxc-pve-prestart-hook
@@ -69,8 +69,8 @@ __PACKAGE__->register_method ({
 
 	my $storage_cfg = PVE::Storage::config();
 
-	my $vollist = PVE::LXC::get_vm_volumes($conf);
-	my $loopdevlist = PVE::LXC::get_vm_volumes($conf, 'rootfs');
+	my $vollist = PVE::LXC::Config->get_vm_volumes($conf);
+	my $loopdevlist = PVE::LXC::Config->get_vm_volumes($conf, 'rootfs');
 
 	PVE::Storage::activate_volumes($storage_cfg, $vollist);
 
@@ -88,7 +88,7 @@ __PACKAGE__->register_method ({
 	    push @$devices, $dev if $dev && $mountpoint->{quota};
 	};
 
-	PVE::LXC::foreach_mountpoint($conf, $setup_mountpoint);
+	PVE::LXC::Config->foreach_mountpoint($conf, $setup_mountpoint);
 
 	my $lxc_setup = PVE::LXC::Setup->new($conf, $rootdir);
 	$lxc_setup->pre_start_hook();
-- 
2.1.4





More information about the pve-devel mailing list