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?
> 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
