On September 5, 2019 4:11 pm, Oguz Bektas wrote:
> 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.bek...@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);
this call to update_lxc_config gets dropped without any comment - is it
not longer needed? was the removal accidental?
> };
>
> 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();
missing use statement for PVE::RPCEnvironment
> + 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 = {};
why this completely different handling of revert and delete?
> + 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);
this also gets called for container creation/restore, which triggers
this die in case of an in-place restore.
> +
> + PVE::LXC::check_ct_modify_config_perm($rpcenv, $authuser, $vmid, undef,
> {}, [@delete]);
what about reverting a pending change? e.g., admin changes something
non-hotpluggable, non-privileged user reverts it?
> +
> + 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});
wrongly indented
also, style: two conditional raise_param_exc right next to eachother,
one with regular, one with postfix if..
> +
> + $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};
does this make sense? it should at best be a nop..
> + } elsif (defined($conf->{pending}->{$opt})) {
> + push @delete, $opt;
why not simply 'delete $conf->{pending}->{$opt}' ? a pending change
should by definition not yet have any effect..
> }
> - };
> + }
>
> - if (defined($delete)) {
> - foreach my $opt (@$delete) {
> - if (!exists($conf->{$opt})) {
> - # silently ignore
> - next;
> - }
>
superfluous blank line
> - 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});
wrongly indented
this is a bit misleading, since @delete at this point potentially also
contains -revert options
> +
> + raise_param_exc({ delete => "you can't use '-delete $opt' and " .
> + "'-revert $opt' at the same time" })
> + if $revert->{$opt};
same as above, but this time it's not just misleading but will trigger
an exception for any revert of a pending change where the key in
question is absent from the currently active config..
$ cat /etc/pve/lxc/123123.conf
[PENDING]
net0: bridge=vmbr0,name=eth1
$ pct set 123123 -revert net0
400 Parameter verification failed.
delete: you can't use '-delete net0' and '-revert net0' at the same time
pct set <vmid> [OPTIONS]
> +
> +
> + if (!PVE::LXC::Config->option_exists($opt)) {
> + raise_param_exc({ delete => "unknown option '$opt'" });
> + }
style: the two preceding raise_param_exc had postfix, this one doesn't
> + }
> +
> + PVE::LXC::check_ct_modify_config_perm($rpcenv, $authuser, $vmid, undef,
> $param, []);
I am not convinced whether the code up to this point should not stay in
the API endpoint, and update_pct_config keep a $delete and get a $revert
hash as parameter..
> +
> + my $storage_cfg = PVE::Storage::config();
> +
> + my $repl_conf = PVE::ReplicationConfig->new();
missing use statement
> + my $is_replicated = $repl_conf->check_for_existing_jobs($vmid, 1);
> + if ($is_replicated) {
> + PVE::LXC::Config->foreach_mountpoint_full($param, 0, sub {
this is now in PVE::LXC::Config, so should be $class-> (this is repeated
quite often below for various methods)
> + 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";
> + }
unused variable/code?
>
> - $wanted_memory //= $old_memory;
> - $wanted_swap //= $old_swap;
> + # write updates to pending section
I don't understand this comment? the code below does not only modify the
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;
move this below the if
> + $conf = PVE::LXC::Config->load_config($vmid); # update/reload
should not be needed, this needs to be called while holding an flock
anyway..
> + if (!defined($conf->{$opt}) && !defined($conf->{pending}->{$opt})) {
> + warn "cannot delete '$opt' - not set in current configuration!\n";
> + $modified->{$opt} = 0;
then you can skip this line
> + next;
> }
the old code ignored deletion of absent options silently btw, is this
change intended or just copied from qemu?
> - $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";
just moved, but memory is not actually a required option? the only
required ones are 'arch', 'ostype' and 'rootfs'..
> + } 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
there already is a write_config call after this if/elsif/else
> + } 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});
I'd only do this in apply_pending, and not here.. (see below for the
non-deletion case with details)
> + 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);
there already is a write_config call after this if/elsif/else
> + }
> + PVE::LXC::Config->write_config($vmid, $conf);
this should be enough ;)
> }
>
> my $storecfg = PVE::Storage::config();
we already have $storage_cfg a little bit above..
>
> - 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) {
> +
> +
this makes three blank lines now ;)
> + 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);
broken for the special storage:size syntax
> + $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});
> - }
we still need handling of adding the old volume as unused for regular
mps as well - but maybe in apply_pending for now, until we actually
hotplug the disk here? e.g., if I do
--------
$ pct start 66667
$ cat /etc/pve/lxc/66667.conf
arch: amd64
mp1: local-zfs:subvol-66667-disk-0,mp=/test,size=1G
ostype: debian
rootfs: fastzfs:subvol-66667-disk-0,acl=1
$ pct set 66667 -mp1 local-zfs:1,mp=/test,acl=1 ✔
Could not rescan volume size - unable to parse zfs volume name '1'
$ cat /etc/pve/lxc/66667.conf
arch: amd64
mp1: local-zfs:subvol-66667-disk-0,mp=/test,size=1G
ostype: debian
rootfs: fastzfs:subvol-66667-disk-0,acl=1
[PENDING]
mp1: local-zfs:subvol-66667-disk-1,mp=/test,acl=1,size=1G
--------
no unused added for the current/old mp1 volume, since it is still in
use. but now, when I stop/start the container:
$ pct stop 66667; pct start 66667
$ cat /etc/pve/lxc/66667.conf
arch: amd64
mp1: local-zfs:subvol-66667-disk-1,mp=/test,acl=1,size=1G
ostype: debian
rootfs: fastzfs:subvol-66667-disk-0,acl=1
the old one is no longer referenced in the config, but still exists on
the storage. regular users cannot delete this volume.
> + $conf->{pending}->{$opt} = $param->{$opt};
> }
> $new_disks = 1;
wouldn't this be sufficient for volume type mountpoints?
> } 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;
wouldn't this be sufficient for volume type mountpoints?
> 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+)$/) {
missing FIXME?
> } 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});
this is broken for unprivileged containers, both regular and with custom
maps. $conf->{pending} does not contain the information, and thus disks
are allocated with the wrong 'root' user..
> + $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
redundant comment
> + $conf = PVE::LXC::Config->load_config($vmid); # update/reload
should not be needed, this needs to be called while holding an flock
anyway..
> + my $changes = PVE::LXC::Config->vmconfig_cleanup_pending($conf);
> + PVE::LXC::Config->write_config($vmid, $conf) if $changes;
> +
> + # apply pending changes
redundant comment
> + $conf = PVE::LXC::Config->load_config($vmid); # update/reload
should not be needed, this needs to be called while holding an flock
anyway..
> +
> + 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
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
