On 19.03.20 16:53, Fabian Grünbichler wrote:
On March 19, 2020 1:37 pm, Fabian Ebner wrote:
Relevant for the 'clone' feature, because Plugin.pm's clone_image
always produces qcow2. Also fixed style for neighboring if/else block.

Signed-off-by: Fabian Ebner <f.eb...@proxmox.com>
---

Previous discussion: 
https://pve.proxmox.com/pipermail/pve-devel/2020-March/042472.html

Changes from v1:
     * As Fabian G. pointed out, templates are not impossible
       on directory based storages, but linked cloning (currently)
       is. So fix the storage backend and get rid of the wrong checks.

This solution doesn't need an API change. It does need
PVE::Cluster which is used by list_images already.

I really don't like this - hence my comment on v1. I also did not like
it for list_images ;) pve-storage should not be concerned with which
guests support which formats.

if we don't want to extend has_feature with a target_format for
copy/clone, then I'd rather we work around this in pve-container. it's a
single call site there anyway.


Ok, I hadn't caught your last reply before sending out this patch.

There's actually two places that would need an extra check in LXC:
1. In clone_vm in PVE/API2/LXC.pm
2. In the has_feature function in PVE/LXC/Config.pm
(AFAIU clone_vm doesn't use has_feature, since it does other things while iterating the mount points)

I'm thinking about adding an $opts parameter to volume_has_feature and using the option valid_target_formats (as a list of acceptable formats for the target of the clone operation) as you suggested.


  PVE/Storage/Plugin.pm | 14 ++++++++++----
  1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
index 2232261..8baa410 100644
--- a/PVE/Storage/Plugin.pm
+++ b/PVE/Storage/Plugin.pm
@@ -844,13 +844,19 @@ sub volume_has_feature {
      my ($vtype, $name, $vmid, $basename, $basevmid, $isBase, $format) =
        $class->parse_volname($volname);
+ my $vmlist = PVE::Cluster::get_vmlist();
+    my $vminfo = $vmlist->{ids}->{$vmid};
+
      my $key = undef;
-    if($snapname){
-        $key = 'snap';
-    }else{
-        $key =  $isBase ? 'base' : 'current';
+    if ($snapname) {
+       $key = 'snap';
+    } else {
+       $key = $isBase ? 'base' : 'current';
      }
+ # clone_images produces a qcow2 image
+    return 0 if defined($vminfo) && $vminfo->{type} eq 'lxc' && $feature eq 
'clone';
+
      return 1 if defined($features->{$feature}->{$key}->{$format});
return undef;
--
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

Reply via email to