On 19.03.20 09:01, Fabian Grünbichler wrote:
On March 18, 2020 2:02 pm, Fabian Ebner wrote:Signed-off-by: Fabian Ebner <f.eb...@proxmox.com> ---For VMs this already happens. When adding volumes to templates no such conversion to base images happens yet (affects both VM/LXC). Because templates are more-or-less supposed to be read-only it probably makes sense to disallow adding volumes altogether. Or should we bother and do conversion instead? When a volume with linked clones is moved with 'delete source' then the volume will be copied over and the source volume does not get deleted, but the source volume disappears from the config. With the second patch it gets added as an unused volume instead. src/PVE/API2/LXC.pm | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm index a5aa5fc..9eb52dc 100644 --- a/src/PVE/API2/LXC.pm +++ b/src/PVE/API2/LXC.pm @@ -468,6 +468,13 @@ __PACKAGE__->register_method({ return $rpcenv->fork_worker($workername, $vmid, $authuser, $realcmd); }});+sub assert_storeid_supports_templates {+ my ($scfg, $sid) = @_; + + die "Warning: Directory storage '$sid' does not support container templates!\n" + if $scfg->{ids}->{$sid}->{path}; +} +so this lead me down quite a rabbit hole back to https://bugzilla.proxmox.com/show_bug.cgi?id=1778 I think the original fix that lead to this "dir storages don't support CT templates" was wrong - what we should do is error out early on attempts to do a linked clone of a volume on a directory storage (as that would create a qcow2 image, which containers don't support). having a template on directory storage is perfectly fine - you just can only do full clones, since .raw does not support anything else. could you take a look at how involved cleaning this up would be?
I think the problem is that volume_has_feature cannot distinguish between a raw volume of a VM and a raw volume of a container. And the clone feature is not actually present in the latter case.
I see two approaches:One is to extend volume_has_feature with $vmtype (would be a non-breaking API change). The other is doing an additional check in LXC's clone_vm and not only rely on volume_has_feature.
sub check_storage_supports_templates { my ($conf) = @_;@@ -477,8 +484,7 @@ sub check_storage_supports_templates {my ($ms, $mp) = @_;my ($sid) = PVE::Storage::parse_volume_id($mp->{volume}, 0);- die "Warning: Directory storage '$sid' does not support container templates!\n" - if $scfg->{ids}->{$sid}->{path}; + assert_storeid_supports_templates($scfg, $sid); }); }; return $@ @@ -1805,6 +1811,8 @@ __PACKAGE__->register_method({my ($mpdata, $old_volid); + my $storage_cfg = PVE::Storage::config();+ PVE::LXC::Config->lock_config($vmid, sub { my $conf = PVE::LXC::Config->load_config($vmid); PVE::LXC::Config->check_lock($conf); @@ -1823,6 +1831,8 @@ __PACKAGE__->register_method({ die "you can't move a volume with snapshots and delete the source\n" if $param->{delete} && PVE::LXC::Config->is_volume_in_use_by_snapshots($conf, $old_volid);+ assert_storeid_supports_templates($storage_cfg, $storage) if PVE::LXC::Config->is_template($conf);+ PVE::Tools::assert_if_modified($param->{digest}, $conf->{digest});PVE::LXC::Config->set_lock($vmid, $lockname);@@ -1833,7 +1843,6 @@ __PACKAGE__->register_method({ PVE::Cluster::log_msg('info', $authuser, "move volume CT $vmid: move --volume $mpkey --storage $storage");my $conf = PVE::LXC::Config->load_config($vmid);- my $storage_cfg = PVE::Storage::config();my $new_volid; @@ -1843,7 +1852,13 @@ __PACKAGE__->register_method({my $source_storage = PVE::Storage::parse_volume_id($old_volid); my $movelimit = PVE::Storage::get_bandwidth_limit('move', [$source_storage, $storage], $bwlimit); $new_volid = PVE::LXC::copy_volume($mpdata, $vmid, $storage, $storage_cfg, $conf, undef, $movelimit); - $mpdata->{volume} = $new_volid; + if (PVE::LXC::Config->is_template($conf)) { + PVE::Storage::activate_volumes($storage_cfg, [$new_volid]); + my $template_volid = PVE::Storage::vdisk_create_base($storage_cfg, $new_volid); + $mpdata->{volume} = $template_volid; + } else { + $mpdata->{volume} = $new_volid; + }PVE::LXC::Config->lock_config($vmid, sub {my $digest = $conf->{digest}; -- 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
_______________________________________________ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
