[pve-devel] [PATCH storage v3] Fix #582: Add DELETE background delay logic

Dominic Jäger d.jaeger at proxmox.com
Thu Jul 4 12:54:11 CEST 2019


Previously, the web gui timed out when removing content (backup, iso,
...) took long. Doing the main part of the API DELETE call in a
fork_worker solves this.

Signed-off-by: Dominic Jäger <d.jaeger at proxmox.com>
---
v2->v3:
1. Fix indentation, vicinity of use and add log line
2. In addition to ownervm, add storeid to the fork id. This is the one of both
options that Thomas proposed, that didn't have assumptions that could be broken.
Putting the volid in the task description at the bottom of the GUI would
look a bit odd IMO, as it is not as concise as e.g. 'VM/CT 103 -
Backup'.
Additionally, the exact volid is now in the task log.

 PVE/API2/Storage/Content.pm | 45 +++++++++++++++++++++++++++++--------
 1 file changed, 36 insertions(+), 9 deletions(-)

diff --git a/PVE/API2/Storage/Content.pm b/PVE/API2/Storage/Content.pm
index cd4746b..9731720 100644
--- a/PVE/API2/Storage/Content.pm
+++ b/PVE/API2/Storage/Content.pm
@@ -285,9 +285,16 @@ __PACKAGE__->register_method ({
 		type => 'string',
 		completion => \&PVE::Storage::complete_volume,
 	    },
+	    delay => {
+		type => 'integer',
+		description => "Time to wait for the task to finish. We return 'null' if the task finish within that time.",
+		minimum => 1,
+		maximum => 30,
+		optional => 1,
+	    },
 	},
     },
-    returns => { type => 'null' },
+    returns => { type => 'string', optional => 1, },
     code => sub {
 	my ($param) = @_;
 
@@ -306,16 +313,36 @@ __PACKAGE__->register_method ({
 	    $rpcenv->check($authuser, "/storage/$storeid", ['Datastore.Allocate']);
 	}
 
-	PVE::Storage::vdisk_free ($cfg, $volid);
+	my $worker = sub {
+	    PVE::Storage::vdisk_free ($cfg, $volid);
+	    print "Removed volume '$volid'\n";
+	    if ($vtype eq 'backup'
+		&& $path =~ /(.*\/vzdump-\w+-\d+-\d{4}_\d{2}_\d{2}-\d{2}_\d{2}_\d{2})[^\/]+$/) {
+		my $logpath = "$1.log";
+		# try to cleanup our backup log file too, if still exisiting, #318
+		unlink($logpath) if -e $logpath;
+	    }
+	};
 
-	if ($vtype eq 'backup'
-	    && $path =~ /(.*\/vzdump-\w+-\d+-\d{4}_\d{2}_\d{2}-\d{2}_\d{2}_\d{2})[^\/]+$/) {
-	    my $logpath = "$1.log";
-	    # try to cleanup our backup log file too, if still exisiting, #318
-	    unlink($logpath) if -e $logpath;
+	my $id = (defined $ownervm ? "$ownervm@" : '') . $storeid;
+	my $upid = $rpcenv->fork_worker('imgdel', $id, $authuser, $worker);
+	my $background_delay = $param->{delay};
+	if ($background_delay) {
+	    my $end_time = time() + $background_delay;
+	    my $currently_deleting; # not necessarily true, e.g. sequential api call from cli
+	    do {
+		my $task = PVE::Tools::upid_decode($upid);
+		$currently_deleting = PVE::ProcFSTools::check_process_running($task->{pid}, $task->{pstart});
+		sleep 1 if $currently_deleting;
+	    } while (time() < $end_time && $currently_deleting);
+
+	    if (!$currently_deleting) {
+		my $status = PVE::Tools::upid_read_status($upid);
+		return undef if $status eq 'OK';
+		die $status;
+	    }
 	}
-
-	return undef;
+	return $upid;
     }});
 
 __PACKAGE__->register_method ({
-- 
2.20.1




More information about the pve-devel mailing list