[pve-devel] [RFC_V2 manager] Storage: remove guest images in storage contendview

Wolfgang Link w.link at proxmox.com
Thu Nov 8 10:34:36 CET 2018


If an image on storage has not referenced to any guest or
replication config, we can safely delete it on the GUI.
Also, if a config exists on another node, we can delete it too.

But if an image has a <vmid> encoded in the image name and a guest
exists in the cluster with this VMID then it must a lost image of the VM.
In this case, a rescan will add it back to the config.

This follows the logic of 'qm rescan',
what assume if an image exists on a node,
Images that are marked as unused in the config are referred.

This call can't be in the store because of cycles dependencies.
The extra Subclass is necessary for the "fragmentDelimiter"
which is used for forwarding the correct volume name for dir storages.

---
 PVE/API2/DeleteImage.pm             | 185 ++++++++++++++++++++++++++++++++++++
 PVE/API2/Makefile                   |   1 +
 PVE/API2/Nodes.pm                   |   7 ++
 www/manager6/storage/ContentView.js |  45 ++++++---
 4 files changed, 227 insertions(+), 11 deletions(-)
 create mode 100644 PVE/API2/DeleteImage.pm

[V1 -> V2]
Do not remove images if a config exists.
Check replication config if the image is used by it.

diff --git a/PVE/API2/DeleteImage.pm b/PVE/API2/DeleteImage.pm
new file mode 100644
index 00000000..55fc9fe2
--- /dev/null
+++ b/PVE/API2/DeleteImage.pm
@@ -0,0 +1,185 @@
+package PVE::API2::DeleteImage;
+
+use strict;
+use warnings;
+
+use PVE::JSONSchema qw(get_standard_option);
+use PVE::Tools;
+use PVE::RPCEnvironment;
+use PVE::Storage;
+use PVE::Cluster;
+use PVE::QemuServer;
+use PVE::LXC;
+use PVE::QemuConfig;
+use PVE::LXC::Config;
+
+use base qw(PVE::RESTHandler);
+
+__PACKAGE__->register_method ({
+    name => 'index',
+    path => '',
+    method => 'GET',
+    description => "Pool index.",
+    permissions => {
+	description => "List all pools where you have Pool.Allocate or VM.Allocate permissions on /pool/<pool>.",
+	user => 'all',
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => {},
+    },
+    returns => {
+	type => 'array',
+	items => {
+	    type => "object",
+	    properties => {
+		poolid => { type => 'string' },
+	    },
+	},
+	links => [ { rel => 'child', href => "{poolid}" } ],
+    },
+    code => sub {
+	my ($param) = @_;
+
+	my $res = [
+	    { id => '' },
+	];
+
+	return $res;
+    }});
+
+__PACKAGE__->register_method ({
+    name => 'deletimage',
+    path => '{volume}',
+    method => 'DELETE',
+    protected => 1,
+    proxyto => 'node',
+    permissions => {
+	check => ['perm', '/storage/{storage}', ['Datastore.Allocate'], any => 1],
+    },
+    description => "Remove image which is not referred to any VM or replication config.",
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    node => get_standard_option('pve-node'),
+	    volume => {
+		description => "Volume identifier",
+		type => 'string',
+	    },
+	},
+    },
+    returns => {
+	type => "string",
+    },
+    code => sub {
+	my ($param) = @_;
+
+	my $rpcenv = PVE::RPCEnvironment::get();
+	my $authuser = $rpcenv->get_user();
+
+	my $volume = PVE::Tools::extract_param($param, 'volume');
+	my $node = PVE::Tools::extract_param($param, 'node');
+
+	my $storagecfg = PVE::Storage::config();
+	my $vmid = (PVE::Storage::parse_volname($storagecfg, $volume))[2];
+	my $vms = PVE::Cluster::get_vmlist();
+
+	# This function removes only guest images.
+	return 1 if !defined($vmid);
+
+	# If guest config exists on this node, we assume the unrefered image belongs to it.
+	if (defined($vms->{ids}->{$vmid}) && $vms->{ids}->{$vmid}->{node} eq $node) {
+	    my $vm = $vms->{ids}->{$vmid};
+	    my $vmtype = $vm->{type};
+
+	    my $guest_class = &$PVE::API2::Replication::lookup_guest_class($vmtype);
+	    my $guest_conf = $guest_class->load_config($vmid, $vm->{node});
+
+	    foreach my $key (keys %$guest_conf) {
+		if ($key =~ /^((unused|ide|sata|scsi|virtio|mp)\d+|rootfs)$/) {
+		    die "Image: $volume is referred to guest config VMID: $vmid\n"
+			if $guest_conf->{$key} =~ /\Q$volume\E/;
+		}
+	    }
+	    # ToDo include automatic rescan instead if recommand to do it.
+	    if ($vmtype eq 'qemu') {
+		PVE::QemuServer::rescan($vmid);
+		die "Image: $volume belongs to VM: $vmid.\n"
+	    } elsif ($vmtype eq 'lxc') {
+		# ToDo remove if LXC rescan is commited;
+		# PVE::LXC::rescan($vmid);
+		die "Image: $volume belongs to CT: $vmid.\n"
+	    } else {
+		die "VMtype: $vmtype of VM: $vmid unknown."; 
+	    }
+
+	}
+
+	my ($sid, $volname) = PVE::Storage::parse_volume_id($volume);
+
+	$rpcenv->check($authuser, "/storage/$sid", ['Datastore.Allocate']);
+
+	my $image_exist_in_replica = sub {
+	    my ($vmid, $target) = @_;
+
+	    my $rep_cfg = PVE::ReplicationConfig->new();
+	    foreach my $jobid (keys %{$rep_cfg->{ids}}) {
+		my $jobcfg = $rep_cfg->{ids}->{$jobid};
+
+		return 1 if ($jobcfg->{guest} == $vmid
+			     && $jobcfg->{target} eq $target);
+	    }
+	    return 0;
+	};
+
+	foreach my $key_vmid (keys %{$vms->{ids}}) {
+
+	    my $vm = $vms->{ids}->{$key_vmid};
+	    my $vmtype = $vm->{type};
+
+	    my $guest_class = &$PVE::API2::Replication::lookup_guest_class($vmtype);
+	    my $guest_conf = $guest_class->load_config($key_vmid, $vm->{node});
+
+	    foreach my $key (keys %$guest_conf) {
+		if ($key =~ /^((unused|ide|sata|scsi|virtio|mp)\d+|rootfs)$/) {
+		    my $volid = $guest_conf->{$key};
+
+		    next if $volid =~ /media=cdrom/ && $volid !~ /cloudinit/;
+		    # we ignore bindmounts and direct path to storages
+		    my ($config_sid, $config_volname);
+		    eval {
+			($config_sid, $config_volname) =
+			    PVE::Storage::parse_volume_id($volid);
+		    };
+		    next if $@;
+
+		    my $config_node = $vm->{node};
+		    if (&$image_exist_in_replica($vmid, $node)) {
+			die "Image: $volume is referred in replication config\n";
+		    }
+
+		    # Check if image is used in another guest config,
+		    # than the image VMID of the image is.
+		    my $error = "Image: $volume is referred in config: $key_vmid\n";
+		    if ($config_sid eq $sid && $config_volname =~ /\Q$volname\E/ &&
+			$vms->{ids}->{$vmid}->{node} eq $node) {
+			die $error;
+		    }
+
+		    my $shared = $storagecfg->{ids}->{$sid}->{shared};
+		    $shared = !defined($shared) ? 0 : $shared;
+		    if ($config_sid eq $sid && $config_volname =~ /\Q$volname\E/ &&
+			$shared) {
+			$error .= "and storage: $sid is shared";
+			die $error;
+		    }
+		}
+	    }
+	}
+
+	eval {
+	    PVE::Storage::vdisk_free ($storagecfg, $volume);
+	};
+	return @$;
+    }});
+1;
diff --git a/PVE/API2/Makefile b/PVE/API2/Makefile
index 9862e498..5ba78657 100644
--- a/PVE/API2/Makefile
+++ b/PVE/API2/Makefile
@@ -18,6 +18,7 @@ PERLSOURCE = 			\
 	ACME.pm			\
 	ACMEAccount.pm		\
 	NodeConfig.pm		\
+	DeleteImage.pm		\
 	Services.pm
 
 all:
diff --git a/PVE/API2/Nodes.pm b/PVE/API2/Nodes.pm
index 49502830..55b6a30b 100644
--- a/PVE/API2/Nodes.pm
+++ b/PVE/API2/Nodes.pm
@@ -43,6 +43,7 @@ use PVE::API2::Firewall::Host;
 use PVE::API2::Replication;
 use PVE::API2::Certificates;
 use PVE::API2::NodeConfig;
+use PVE::API2::DeleteImage;
 use Digest::MD5;
 use Digest::SHA;
 use PVE::API2::Disks;
@@ -125,6 +126,11 @@ __PACKAGE__->register_method ({
     path => 'certificates',
 });
 
+__PACKAGE__->register_method ({
+    subclass => "PVE::API2::DeleteImage",
+    path => 'deleteimage',
+    fragmentDelimiter => '',
+});
 
 __PACKAGE__->register_method ({
     subclass => "PVE::API2::NodeConfig",
@@ -187,6 +193,7 @@ __PACKAGE__->register_method ({
 	    { name => 'certificates' },
 	    { name => 'config' },
 	    { name => 'hosts' },
+	    { name => 'deleteimage' },
 	    ];
 
 	return $result;
diff --git a/www/manager6/storage/ContentView.js b/www/manager6/storage/ContentView.js
index 0661065d..67b980a4 100644
--- a/www/manager6/storage/ContentView.js
+++ b/www/manager6/storage/ContentView.js
@@ -373,6 +373,38 @@ Ext.define('PVE.storage.ContentView', {
 
 	Proxmox.Utils.monStoreErrors(me, store);
 
+	var removeButton = Ext.create('Proxmox.button.StdRemoveButton',{
+	    selModel: sm,
+	    enableFn: function(rec) {
+		if (rec && rec.data.content !== 'images') {
+		    removeButton.setVisible(true);
+		    imageRemoveButton.setVisible(false);
+		    return true;
+		}
+		return false;
+	    },
+	    callback: function() {
+		reload();
+	    },
+	    baseurl: baseurl + '/'
+	});
+
+	var imageRemoveButton = Ext.create('Proxmox.button.StdRemoveButton',{
+	    selModel: sm,
+	    enableFn: function(rec) {
+		if (rec && rec.data.content === 'images') {
+		    removeButton.setVisible(false);
+		    imageRemoveButton.setVisible(true);
+		    return true;
+		}
+		return false;
+	    },
+	    callback: function() {
+		reload();
+	    },
+	    baseurl: '/nodes/' + nodename + '/deleteimage',
+	});
+
 	var templateButton = Ext.create('Proxmox.button.Button',{
 	    itemId: 'tmpl-btn',
 	    text: gettext('Templates'),
@@ -437,17 +469,8 @@ Ext.define('PVE.storage.ContentView', {
 			win.on('destroy', reload);
 		    }
 		},
-		{
-		    xtype: 'proxmoxStdRemoveButton',
-		    selModel: sm,
-		    enableFn: function(rec) {
-			return rec && rec.data.content !== 'images';
-		    },
-		    callback: function() {
-			reload();
-		    },
-		    baseurl: baseurl + '/'
-		},
+		removeButton,
+		imageRemoveButton,
 		templateButton,
 		uploadButton,
 		{
-- 
2.11.0





More information about the pve-devel mailing list