[pve-devel] [PATCH container 8/9] rework update_pct_config to write and apply pending changes

Oguz Bektas o.bektas at proxmox.com
Thu Sep 5 16:11:20 CEST 2019


use vmconfig_hotplug_pending (when ct up) and vmconfig_apply_pending
(when ct down) to apply or write pending changes.

Signed-off-by: Oguz Bektas <o.bektas at proxmox.com>
---
 src/PVE/API2/LXC/Config.pm |  52 +-----
 src/PVE/LXC/Config.pm      | 328 +++++++++++++++++--------------------
 2 files changed, 147 insertions(+), 233 deletions(-)

diff --git a/src/PVE/API2/LXC/Config.pm b/src/PVE/API2/LXC/Config.pm
index 405a079..a31ddd5 100644
--- a/src/PVE/API2/LXC/Config.pm
+++ b/src/PVE/API2/LXC/Config.pm
@@ -156,58 +156,10 @@ __PACKAGE__->register_method({
     code => sub {
 	my ($param) = @_;
 
-	my $rpcenv = PVE::RPCEnvironment::get();
-	my $authuser = $rpcenv->get_user();
-
 	my $node = extract_param($param, 'node');
 	my $vmid = extract_param($param, 'vmid');
-
 	my $digest = extract_param($param, 'digest');
 
-	die "no options specified\n" if !scalar(keys %$param);
-
-	my $delete_str = extract_param($param, 'delete');
-	my @delete = PVE::Tools::split_list($delete_str);
-
-	PVE::LXC::check_ct_modify_config_perm($rpcenv, $authuser, $vmid, undef, {}, [@delete]);
-
-	foreach my $opt (@delete) {
-	    raise_param_exc({ delete => "you can't use '-$opt' and -delete $opt' at the same time" })
-		if defined($param->{$opt});
-
-	    if (!PVE::LXC::Config->option_exists($opt)) {
-		raise_param_exc({ delete => "unknown option '$opt'" });
-	    }
-	}
-
-	PVE::LXC::check_ct_modify_config_perm($rpcenv, $authuser, $vmid, undef, $param, []);
-
-	my $storage_cfg = cfs_read_file("storage.cfg");
-
-	my $repl_conf = PVE::ReplicationConfig->new();
-	my $is_replicated = $repl_conf->check_for_existing_jobs($vmid, 1);
-	if ($is_replicated) {
-	    PVE::LXC::Config->foreach_mountpoint_full($param, 0, sub {
-		my ($opt, $mountpoint) = @_;
-		my $volid = $mountpoint->{volume};
-		return if !$volid || !($mountpoint->{replicate}//1);
-		if ($mountpoint->{type} eq 'volume') {
-		    my ($storeid, $format);
-		    if ($volid =~ $PVE::LXC::NEW_DISK_RE) {
-			$storeid = $1;
-			$format = $mountpoint->{format} || PVE::Storage::storage_default_format($storage_cfg, $storeid);
-		    } else {
-			($storeid, undef) = PVE::Storage::parse_volume_id($volid, 1);
-			$format = (PVE::Storage::parse_volname($storage_cfg, $volid))[6];
-		    }
-		    return if PVE::Storage::storage_can_replicate($storage_cfg, $storeid, $format);
-		    my $scfg = PVE::Storage::storage_config($storage_cfg, $storeid);
-		    return if $scfg->{shared};
-		}
-		die "cannot add non-replicatable volume to a replicated VM\n";
-	    });
-	}
-
 	my $code = sub {
 
 	    my $conf = PVE::LXC::Config->load_config($vmid);
@@ -217,10 +169,8 @@ __PACKAGE__->register_method({
 
 	    my $running = PVE::LXC::check_running($vmid);
 
-	    PVE::LXC::Config->update_pct_config($vmid, $conf, $running, $param, \@delete);
+	    PVE::LXC::Config->update_pct_config($vmid, $conf, $running, $param);
 
-	    PVE::LXC::Config->write_config($vmid, $conf);
-	    PVE::LXC::update_lxc_config($vmid, $conf);
 	};
 
 	PVE::LXC::Config->lock_config($vmid, $code);
diff --git a/src/PVE/LXC/Config.pm b/src/PVE/LXC/Config.pm
index 26c694f..6bec9b7 100644
--- a/src/PVE/LXC/Config.pm
+++ b/src/PVE/LXC/Config.pm
@@ -1080,129 +1080,132 @@ sub write_pct_config {
 }
 
 sub update_pct_config {
-    my ($class, $vmid, $conf, $running, $param, $delete) = @_;
+    my ($class, $vmid, $conf, $running, $param) = @_;
 
-    my @nohotplug;
+    my $rpcenv = PVE::RPCEnvironment::get();
+    my $authuser = $rpcenv->get_user();
 
-    my $new_disks = 0;
-    my @deleted_volumes;
+    my $delete_str = extract_param($param, 'delete');
+    my @delete = PVE::Tools::split_list($delete_str);
+    my $revert_str = extract_param($param, 'revert');
+    my $revert = {};
+    my $force = extract_param($param, 'force');
 
-    my $rootdir;
-    if ($running) {
-	my $pid = PVE::LXC::find_lxc_pid($vmid);
-	$rootdir = "/proc/$pid/root";
+    die "no options specified\n" if !$delete_str && !$revert_str && !scalar(keys %$param);
+
+    PVE::LXC::check_ct_modify_config_perm($rpcenv, $authuser, $vmid, undef, {}, [@delete]);
+
+    foreach my $opt (PVE::Tools::split_list($revert_str)) {
+	if (!PVE::LXC::Config->option_exists($opt)) {
+	    raise_param_exc({ revert => "unknown option '$opt'" });
+	}
+
+	raise_param_exc({ delete => "you can't use '-$opt' and " .
+		"'-revert $opt' at the same time" })
+	if defined($param->{$opt});
+
+	$revert->{$opt} = 1;
     }
 
-    my $hotplug_error = sub {
-	if ($running) {
-	    push @nohotplug, @_;
-	    return 1;
-	} else {
-	    return 0;
+    foreach my $opt (keys %$revert) {
+	if (defined($conf->{$opt})) {
+	    $param->{$opt} = $conf->{$opt};
+	} elsif (defined($conf->{pending}->{$opt})) {
+	    push @delete, $opt;
 	}
-    };
+    }
 
-    if (defined($delete)) {
-	foreach my $opt (@$delete) {
-	    if (!exists($conf->{$opt})) {
-		# silently ignore
-		next;
-	    }
 
-	    if ($opt eq 'memory' || $opt eq 'rootfs') {
-		die "unable to delete required option '$opt'\n";
-	    } elsif ($opt eq 'hostname') {
-		delete $conf->{$opt};
-	    } elsif ($opt eq 'swap') {
-		delete $conf->{$opt};
-		PVE::LXC::write_cgroup_value("memory", $vmid,
-					     "memory.memsw.limit_in_bytes", -1);
-	    } elsif ($opt eq 'description' || $opt eq 'onboot' || $opt eq 'startup' || $opt eq 'hookscript') {
-		delete $conf->{$opt};
-	    } elsif ($opt eq 'nameserver' || $opt eq 'searchdomain' ||
-		     $opt eq 'tty' || $opt eq 'console' || $opt eq 'cmode') {
-		next if $hotplug_error->($opt);
-		delete $conf->{$opt};
-	    } elsif ($opt eq 'cores') {
-		delete $conf->{$opt}; # rest is handled by pvestatd
-	    } elsif ($opt eq 'cpulimit') {
-		PVE::LXC::write_cgroup_value("cpu", $vmid, "cpu.cfs_quota_us", -1);
-		delete $conf->{$opt};
-	    } elsif ($opt eq 'cpuunits') {
-		PVE::LXC::write_cgroup_value("cpu", $vmid, "cpu.shares", $confdesc->{cpuunits}->{default});
-		delete $conf->{$opt};
-	    } elsif ($opt =~ m/^net(\d)$/) {
-		delete $conf->{$opt};
-		next if !$running;
-		my $netid = $1;
-		PVE::Network::veth_delete("veth${vmid}i$netid");
-	    } elsif ($opt eq 'protection') {
-		delete $conf->{$opt};
-	    } elsif ($opt =~ m/^unused(\d+)$/) {
-		next if $hotplug_error->($opt);
-		PVE::LXC::Config->check_protection($conf, "can't remove CT $vmid drive '$opt'");
-		push @deleted_volumes, $conf->{$opt};
-		delete $conf->{$opt};
-	    } elsif ($opt =~ m/^mp(\d+)$/) {
-		next if $hotplug_error->($opt);
-		PVE::LXC::Config->check_protection($conf, "can't remove CT $vmid drive '$opt'");
-		my $mp = PVE::LXC::Config->parse_ct_mountpoint($conf->{$opt});
-		delete $conf->{$opt};
-		if ($mp->{type} eq 'volume') {
-		    PVE::LXC::Config->add_unused_volume($conf, $mp->{volume});
-		}
-	    } elsif ($opt eq 'unprivileged') {
-		die "unable to delete read-only option: '$opt'\n";
-	    } elsif ($opt eq 'features') {
-		next if $hotplug_error->($opt);
-		delete $conf->{$opt};
+    foreach my $opt (@delete) {
+	raise_param_exc({ delete => "you can't use '-$opt' and -delete $opt' at the same time" })
+	if defined($param->{$opt});
+
+	raise_param_exc({ delete => "you can't use '-delete $opt' and " .
+		"'-revert $opt' at the same time" })
+	if $revert->{$opt};
+
+
+	if (!PVE::LXC::Config->option_exists($opt)) {
+	    raise_param_exc({ delete => "unknown option '$opt'" });
+	}
+    }
+
+    PVE::LXC::check_ct_modify_config_perm($rpcenv, $authuser, $vmid, undef, $param, []);
+
+    my $storage_cfg = PVE::Storage::config();
+
+    my $repl_conf = PVE::ReplicationConfig->new();
+    my $is_replicated = $repl_conf->check_for_existing_jobs($vmid, 1);
+    if ($is_replicated) {
+	PVE::LXC::Config->foreach_mountpoint_full($param, 0, sub {
+	my ($opt, $mountpoint) = @_;
+	my $volid = $mountpoint->{volume};
+	return if !$volid || !($mountpoint->{replicate}//1);
+	if ($mountpoint->{type} eq 'volume') {
+	    my ($storeid, $format);
+	    if ($volid =~ $PVE::LXC::NEW_DISK_RE) {
+		$storeid = $1;
+		$format = $mountpoint->{format} || PVE::Storage::storage_default_format($storage_cfg, $storeid);
 	    } else {
-		die "implement me (delete: $opt)"
+		($storeid, undef) = PVE::Storage::parse_volume_id($volid, 1);
+		$format = (PVE::Storage::parse_volname($storage_cfg, $volid))[6];
 	    }
-	    PVE::LXC::Config->write_config($vmid, $conf) if $running;
+	    return if PVE::Storage::storage_can_replicate($storage_cfg, $storeid, $format);
+	    my $scfg = PVE::Storage::storage_config($storage_cfg, $storeid);
+	    return if $scfg->{shared};
 	}
+	die "cannot add non-replicatable volume to a replicated VM\n";
+	});
     }
 
-    # There's no separate swap size to configure, there's memory and "total"
-    # memory (iow. memory+swap). This means we have to change them together.
-    my $wanted_memory = PVE::Tools::extract_param($param, 'memory');
-    my $wanted_swap =  PVE::Tools::extract_param($param, 'swap');
-    if (defined($wanted_memory) || defined($wanted_swap)) {
+    my $used_volids = {};
+    my $new_disks = 0;
+    my @deleted_volumes;
 
-	my $old_memory = ($conf->{memory} || 512);
-	my $old_swap = ($conf->{swap} || 0);
+    my $rootdir;
+    if ($running) {
+	my $pid = PVE::LXC::find_lxc_pid($vmid);
+	$rootdir = "/proc/$pid/root";
+    }
 
-	$wanted_memory //= $old_memory;
-	$wanted_swap //= $old_swap;
+    # write updates to pending section
+    my $modified = {}; # record modified options
 
-	my $total = $wanted_memory + $wanted_swap;
-	if ($running) {
-	    my $old_total = $old_memory + $old_swap;
-	    if ($total > $old_total) {
-		PVE::LXC::write_cgroup_value("memory", $vmid,
-					     "memory.memsw.limit_in_bytes",
-					     int($total*1024*1024));
-		PVE::LXC::write_cgroup_value("memory", $vmid,
-					     "memory.limit_in_bytes",
-					     int($wanted_memory*1024*1024));
-	    } else {
-		PVE::LXC::write_cgroup_value("memory", $vmid,
-					     "memory.limit_in_bytes",
-					     int($wanted_memory*1024*1024));
-		PVE::LXC::write_cgroup_value("memory", $vmid,
-					     "memory.memsw.limit_in_bytes",
-					     int($total*1024*1024));
-	    }
+    foreach my $opt (@delete) {
+	$modified->{$opt} = 1;
+	$conf = PVE::LXC::Config->load_config($vmid); # update/reload
+	if (!defined($conf->{$opt}) && !defined($conf->{pending}->{$opt})) {
+	    warn "cannot delete '$opt' - not set in current configuration!\n";
+	    $modified->{$opt} = 0;
+	    next;
 	}
-	$conf->{memory} = $wanted_memory;
-	$conf->{swap} = $wanted_swap;
 
-	PVE::LXC::Config->write_config($vmid, $conf) if $running;
+	if ($opt eq 'memory' || $opt eq 'rootfs') {
+	    die "unable to delete required option '$opt'\n";
+	} elsif ($opt =~ m/^unused(\d+)$/) {
+	    PVE::LXC::Config->check_protection($conf, "can't remove CT $vmid drive '$opt'");
+	    push @deleted_volumes, $conf->{$opt};
+	    delete $conf->{$opt};
+	    $class->write_config($vmid, $conf); # unused disks can be removed directly
+	} elsif ($opt =~ m/^mp(\d+)$/) {
+	    PVE::LXC::Config->check_protection($conf, "can't remove CT $vmid drive '$opt'");
+	    my $mp = PVE::LXC::Config->parse_ct_mountpoint($conf->{$opt});
+	    PVE::LXC::Config->vmconfig_delete_pending_option($conf, $opt, $force);
+	    if ($mp->{type} eq 'volume') {
+		PVE::LXC::Config->add_unused_volume($conf, $mp->{volume});
+		warn "--force is not yet implemented, will apply change after start" if $force; # FIXME
+	    }
+	} elsif ($opt eq 'unprivileged') {
+	    die "unable to delete read-only option: '$opt'\n";
+	} else {
+	    PVE::LXC::Config->vmconfig_delete_pending_option($conf, $opt, $force);
+	    PVE::LXC::Config->write_config($vmid, $conf);
+	}
+	PVE::LXC::Config->write_config($vmid, $conf);
     }
 
     my $storecfg = PVE::Storage::config();
 
-    my $used_volids = {};
     my $check_content_type = sub {
 	my ($mp) = @_;
 	my $sid = PVE::Storage::parse_volume_id($mp->{volume});
@@ -1220,111 +1223,59 @@ sub update_pct_config {
 	warn "Could not rescan volume size - $@\n" if $@;
     };
 
-    foreach my $opt (keys %$param) {
+
+
+    foreach my $opt (keys %$param) { # add/change
+	$modified->{$opt} = 1;
 	my $value = $param->{$opt};
-	my $check_protection_msg = "can't update CT $vmid drive '$opt'";
-	if ($opt eq 'hostname' || $opt eq 'arch') {
-	    $conf->{$opt} = $value;
-	} elsif ($opt eq 'onboot') {
-	    $conf->{$opt} = $value ? 1 : 0;
-	} elsif ($opt eq 'startup') {
-	    $conf->{$opt} = $value;
-	} elsif ($opt eq 'tty' || $opt eq 'console' || $opt eq 'cmode') {
-	    next if $hotplug_error->($opt);
-	    $conf->{$opt} = $value;
-	} elsif ($opt eq 'nameserver') {
-	    next if $hotplug_error->($opt);
-	    my $list = PVE::LXC::verify_nameserver_list($value);
-	    $conf->{$opt} = $list;
-	} elsif ($opt eq 'searchdomain') {
-	    next if $hotplug_error->($opt);
-	    my $list = PVE::LXC::verify_searchdomain_list($value);
-	    $conf->{$opt} = $list;
-	} elsif ($opt eq 'cores') {
-	    $conf->{$opt} = $value;# rest is handled by pvestatd
-	} elsif ($opt eq 'cpulimit') {
-	    if ($value == 0) {
-		PVE::LXC::write_cgroup_value("cpu", $vmid, "cpu.cfs_quota_us", -1);
-	    } else {
-		PVE::LXC::write_cgroup_value("cpu", $vmid, "cpu.cfs_quota_us", int(100000*$value));
-	    }
-	    $conf->{$opt} = $value;
-	} elsif ($opt eq 'cpuunits') {
-	    $conf->{$opt} = $value;
-	    PVE::LXC::write_cgroup_value("cpu", $vmid, "cpu.shares", $value);
-	} elsif ($opt eq 'description') {
-	    $conf->{$opt} = $value;
-	} elsif ($opt =~ m/^net(\d+)$/) {
-	    my $netid = $1;
-	    my $net = PVE::LXC::Config->parse_lxc_network($value);
-	    if (!$running) {
-		$conf->{$opt} = PVE::LXC::Config->print_lxc_network($net);
-	    } else {
-		PVE::LXC::update_net($vmid, $conf, $opt, $net, $netid, $rootdir);
-	    }
-	} elsif ($opt eq 'protection') {
-	    $conf->{$opt} = $value ? 1 : 0;
-	} elsif ($opt =~ m/^mp(\d+)$/) {
-	    next if $hotplug_error->($opt);
-	    PVE::LXC::Config->check_protection($conf, $check_protection_msg);
+	if ($opt =~ m/^mp(\d+)$/) {
+	    PVE::LXC::Config->check_protection($conf, "can't update CT $vmid drive '$opt'");
 	    my $old = $conf->{$opt};
 	    my $mp = PVE::LXC::Config->parse_ct_mountpoint($value);
 	    if ($mp->{type} eq 'volume') {
-		&$check_content_type($mp);
+		$check_content_type->($mp);
 		$used_volids->{$mp->{volume}} = 1;
-		&$rescan_volume($mp);
-		$conf->{$opt} = PVE::LXC::Config->print_ct_mountpoint($mp);
+		$rescan_volume->($mp);
+		$conf->{pending}->{$opt} = PVE::LXC::Config->print_ct_mountpoint($mp);
 	    } else {
-		$conf->{$opt} = $value;
-	    }
-	    if (defined($old)) {
-		my $mp = PVE::LXC::Config->parse_ct_mountpoint($old);
-		if ($mp->{type} eq 'volume') {
-		    PVE::LXC::Config->add_unused_volume($conf, $mp->{volume});
-		}
+		$conf->{pending}->{$opt} = $param->{$opt};
 	    }
 	    $new_disks = 1;
 	} elsif ($opt eq 'rootfs') {
-	    next if $hotplug_error->($opt);
-	    PVE::LXC::Config->check_protection($conf, $check_protection_msg);
+	    PVE::LXC::Config->check_protection($conf, "can't update CT $vmid drive '$opt'");
 	    my $old = $conf->{$opt};
 	    my $mp = PVE::LXC::Config->parse_ct_rootfs($value);
 	    if ($mp->{type} eq 'volume') {
-		&$check_content_type($mp);
+		$check_content_type->($mp);
 		$used_volids->{$mp->{volume}} = 1;
-		&$rescan_volume($mp);
-		$conf->{$opt} = PVE::LXC::Config->print_ct_mountpoint($mp, 1);
+		$rescan_volume->($mp);
+		$conf->{pending}->{$opt} = PVE::LXC::Config->print_ct_mountpoint($mp, 1);
 	    } else {
-		$conf->{$opt} = $value;
+		$conf->{pending}->{$opt} = $value;
 	    }
+	    $new_disks = 1;
 	    if (defined($old)) {
 		my $mp = PVE::LXC::Config->parse_ct_rootfs($old);
 		if ($mp->{type} eq 'volume') {
 		    PVE::LXC::Config->add_unused_volume($conf, $mp->{volume});
 		}
 	    }
-	    $new_disks = 1;
-	} elsif ($opt eq 'unprivileged') {
-	    die "unable to modify read-only option: '$opt'\n";
-	} elsif ($opt eq 'ostype') {
-	    next if $hotplug_error->($opt);
-	    $conf->{$opt} = $value;
-	} elsif ($opt eq 'features') {
-	    next if $hotplug_error->($opt);
-	    $conf->{$opt} = $value;
+	    $conf->{pending}->{$opt} = $param->{$opt};
+	} elsif ($opt =~ m/^unused(\d+)$/) {
 	} elsif ($opt eq 'hookscript') {
 	    PVE::GuestHelpers::check_hookscript($value);
-	    $conf->{$opt} = $value;
+	    $conf->{pending}->{$opt} = $param->{$opt};
+	} elsif ($opt eq 'unprivileged') {
+	    die "unable to modify read-only option: '$opt'\n";
 	} else {
-	    die "implement me: $opt";
+	    $conf->{pending}->{$opt} = $param->{$opt};
 	}
-
-	PVE::LXC::Config->write_config($vmid, $conf) if $running;
+	$class->vmconfig_undelete_pending_option($conf, $opt);
+	$class->write_config($vmid, $conf);
     }
 
     # Apply deletions and creations of new volumes
     if (@deleted_volumes) {
-	my $storage_cfg = PVE::Storage::config();
 	foreach my $volume (@deleted_volumes) {
 	    next if $used_volids->{$volume}; # could have been re-added, too
 	    # also check for references in snapshots
@@ -1334,13 +1285,26 @@ sub update_pct_config {
     }
 
     if ($new_disks) {
-	my $storage_cfg = PVE::Storage::config();
-	PVE::LXC::create_disks($storage_cfg, $vmid, $conf, $conf);
+	PVE::LXC::create_disks($storage_cfg, $vmid, $conf->{pending}, $conf->{pending});
+	$class->write_config($vmid, $conf);
     }
 
-    # This should be the last thing we do here
-    if ($running && scalar(@nohotplug)) {
-	die "unable to modify " . join(',', @nohotplug) . " while container is running\n";
+
+
+    # remove pending changes when nothing changed
+    $conf = PVE::LXC::Config->load_config($vmid); # update/reload
+    my $changes = PVE::LXC::Config->vmconfig_cleanup_pending($conf);
+    PVE::LXC::Config->write_config($vmid, $conf) if $changes;
+
+    # apply pending changes
+    $conf = PVE::LXC::Config->load_config($vmid); # update/reload
+
+    if ($running) {
+	my $errors = {};
+	PVE::LXC::Config->vmconfig_hotplug_pending($vmid, $conf, $storecfg, $modified, $errors);
+	raise_param_exc($errors) if %$errors;
+    } else {
+	PVE::LXC::Config->vmconfig_apply_pending($vmid, $conf, $storecfg);
     }
 }
 
-- 
2.20.1




More information about the pve-devel mailing list