[pve-devel] [PATCH pve-manager 0/2] add bulk suspend

[pve-devel] [PATCH pve-manager 0/2] add bulk suspend

[Hannes Laimer]

Adds support for bulk suspending VMs as it already exists for stop.

Hannes Laimer (2):
  api: add suspendall endpoint
  ui: add bulk suspend support

 PVE/API2/Nodes.pm                 | 118 ++++++++++++++++++++++++++++++
 www/manager6/Utils.js             |   1 +
 www/manager6/form/VMSelector.js   |   4 +
 www/manager6/node/CmdMenu.js      |  14 ++++
 www/manager6/node/Config.js       |  14 ++++
 www/manager6/window/BulkAction.js |   2 +-
 6 files changed, 152 insertions(+), 1 deletion(-)

-- 
2.39.2

[pve-devel] [PATCH pve-manager 1/2] api: add suspendall endpoint

[Hannes Laimer]

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---

code is mostly taken from the already existing stopal endpoint, since
all checks are basically the same for both suspend and stop.

 PVE/API2/Nodes.pm | 118 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 118 insertions(+)

diff --git a/PVE/API2/Nodes.pm b/PVE/API2/Nodes.pm
index 0843c3a3..bb77474f 100644
--- a/PVE/API2/Nodes.pm
+++ b/PVE/API2/Nodes.pm
@@ -286,6 +286,7 @@ __PACKAGE__->register_method ({
 	    { name => 'spiceshell' },
 	    { name => 'startall' },
 	    { name => 'status' },
+	    { name => 'suspendall' },
 	    { name => 'stopall' },
 	    { name => 'storage' },
 	    { name => 'subscription' },
@@ -2019,6 +2020,123 @@ __PACKAGE__->register_method ({
 	return $rpcenv->fork_worker('stopall', undef, $authuser, $code);
     }});
 
+my $create_suspend_worker = sub {
+    my ($nodename, $vmid) = @_;
+    return if !PVE::QemuServer::check_running($vmid, 1);
+    print STDERR "Suspending VM $vmid\n";
+    return PVE::API2::Qemu->vm_suspend(
+	{ node => $nodename, vmid => $vmid, todisk => 1 }
+    );
+};
+
+__PACKAGE__->register_method ({
+    name => 'suspendall',
+    path => 'suspendall',
+    method => 'POST',
+    protected => 1,
+    permissions => {
+	description => "The 'VM.PowerMgmt' permission is required on '/' or on '/vms/<ID>' for "
+	    ."each ID passed via the 'vms' parameter.",
+	user => 'all',
+    },
+    proxyto => 'node',
+    description => "Suspend all VMs.",
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    node => get_standard_option('pve-node'),
+	    vms => {
+		description => "Only consider Guests with these IDs.",
+		type => 'string',  format => 'pve-vmid-list',
+		optional => 1,
+	    },
+	},
+    },
+    returns => {
+	type => 'string',
+    },
+    code => sub {
+	my ($param) = @_;
+
+	my $rpcenv = PVE::RPCEnvironment::get();
+	my $authuser = $rpcenv->get_user();
+
+	if (!$rpcenv->check($authuser, "/", [ 'VM.PowerMgmt' ], 1)) {
+	    my @vms = PVE::Tools::split_list($param->{vms});
+	    if (scalar(@vms) > 0) {
+		$rpcenv->check($authuser, "/vms/$_", [ 'VM.PowerMgmt' ]) for @vms;
+	    } else {
+		raise_perm_exc("/, VM.PowerMgmt");
+	    }
+	}
+
+	my $nodename = $param->{node};
+	$nodename = PVE::INotify::nodename() if $nodename eq 'localhost';
+
+	my $code = sub {
+
+	    $rpcenv->{type} = 'priv'; # to start tasks in background
+
+	    my $stopList = $get_start_stop_list->($nodename, undef, $param->{vms});
+
+	    my $cpuinfo = PVE::ProcFSTools::read_cpuinfo();
+	    my $datacenterconfig = cfs_read_file('datacenter.cfg');
+	    # if not set by user spawn max cpu count number of workers
+	    my $maxWorkers =  $datacenterconfig->{max_workers} || $cpuinfo->{cpus};
+
+	    for my $order (sort {$b <=> $a} keys %$stopList) {
+		my $vmlist = $stopList->{$order};
+		my $workers = {};
+
+		my $finish_worker = sub {
+		    my $pid = shift;
+		    my $worker = delete $workers->{$pid} || return;
+
+		    syslog('info', "end task $worker->{upid}");
+		};
+
+		for my $vmid (sort {$b <=> $a} keys %$vmlist) {
+		    my $d = $vmlist->{$vmid};
+		    my $upid = eval {
+			$create_suspend_worker->($nodename, $vmid)
+		    };
+		    warn $@ if $@;
+		    next if !$upid;
+
+		    my $task = PVE::Tools::upid_decode($upid, 1);
+		    next if !$task;
+
+		    my $pid = $task->{pid};
+
+		    $workers->{$pid} = { type => $d->{type}, upid => $upid, vmid => $vmid };
+		    while (scalar(keys %$workers) >= $maxWorkers) {
+			foreach my $p (keys %$workers) {
+			    if (!PVE::ProcFSTools::check_process_running($p)) {
+				$finish_worker->($p);
+			    }
+			}
+			sleep(1);
+		    }
+		}
+		while (scalar(keys %$workers)) {
+		    for my $p (keys %$workers) {
+			if (!PVE::ProcFSTools::check_process_running($p)) {
+			    $finish_worker->($p);
+			}
+		    }
+		    sleep(1);
+		}
+	    }
+
+	    syslog('info', "all VMs suspended");
+
+	    return;
+	};
+
+	return $rpcenv->fork_worker('suspendall', undef, $authuser, $code);
+    }});
+
+
 my $create_migrate_worker = sub {
     my ($nodename, $type, $vmid, $target, $with_local_disks) = @_;
 
-- 
2.39.2

[pve-devel] [PATCH pve-manager 2/2] ui: add bulk suspend support

[Hannes Laimer]

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 www/manager6/Utils.js             |  1 +
 www/manager6/form/VMSelector.js   |  4 ++++
 www/manager6/node/CmdMenu.js      | 14 ++++++++++++++
 www/manager6/node/Config.js       | 14 ++++++++++++++
 www/manager6/window/BulkAction.js |  2 +-
 5 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index 06b63315..e74fd394 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -1991,6 +1991,7 @@ Ext.define('PVE.Utils', {
 	    spiceshell: ['', gettext('Shell') + ' (Spice)'],
 	    startall: ['', gettext('Start all VMs and Containers')],
 	    stopall: ['', gettext('Stop all VMs and Containers')],
+	    suspendall: ['', gettext('Suspend all VMs')],
 	    unknownimgdel: ['', gettext('Destroy image from unknown guest')],
 	    wipedisk: ['Device', gettext('Wipe Disk')],
 	    vncproxy: ['VM/CT', gettext('Console')],
diff --git a/www/manager6/form/VMSelector.js b/www/manager6/form/VMSelector.js
index d59847f2..ad0bfc03 100644
--- a/www/manager6/form/VMSelector.js
+++ b/www/manager6/form/VMSelector.js
@@ -233,6 +233,10 @@ Ext.define('PVE.form.VMSelector', {
 		case 'stopall':
 		    statusfilter = 'running';
 		    break;
+		case 'suspendall':
+		    statusfilter = 'running';
+		    me.getStore().addFilter({ property: 'type', value: /qemu/ });
+		    break;
 	    }
 	    if (statusfilter !== '') {
 		me.getStore().addFilter([{
diff --git a/www/manager6/node/CmdMenu.js b/www/manager6/node/CmdMenu.js
index dc56ef08..c64a54d2 100644
--- a/www/manager6/node/CmdMenu.js
+++ b/www/manager6/node/CmdMenu.js
@@ -56,6 +56,20 @@ Ext.define('PVE.node.CmdMenu', {
 		});
 	    },
 	},
+	{
+	    text: gettext('Bulk Suspend'),
+	    itemId: 'bulkstop',
+	    iconCls: 'fa fa-fw fa-download',
+	    handler: function() {
+		Ext.create('PVE.window.BulkAction', {
+		    nodename: this.up('menu').nodename,
+		    title: gettext('Bulk Suspend'),
+		    btnText: gettext('Suspend'),
+		    action: 'suspendall',
+		    autoShow: true,
+		});
+	    },
+	},
 	{
 	    text: gettext('Bulk Migrate'),
 	    itemId: 'bulkmigrate',
diff --git a/www/manager6/node/Config.js b/www/manager6/node/Config.js
index 6ed2172a..ac5e6b25 100644
--- a/www/manager6/node/Config.js
+++ b/www/manager6/node/Config.js
@@ -65,6 +65,20 @@ Ext.define('PVE.node.Config', {
 			    });
 			},
 		    },
+		    {
+			text: gettext('Bulk Suspend'),
+			iconCls: 'fa fa-fw fa-download',
+			disabled: !caps.vms['VM.PowerMgmt'],
+			handler: function() {
+			    Ext.create('PVE.window.BulkAction', {
+				autoShow: true,
+				nodename: nodename,
+				title: gettext('Bulk Suspend'),
+				btnText: gettext('Suspend'),
+				action: 'suspendall',
+			    });
+			},
+		    },
 		    {
 			text: gettext('Bulk Migrate'),
 			iconCls: 'fa fa-fw fa-send-o',
diff --git a/www/manager6/window/BulkAction.js b/www/manager6/window/BulkAction.js
index 949e167e..1a6d83fd 100644
--- a/www/manager6/window/BulkAction.js
+++ b/www/manager6/window/BulkAction.js
@@ -10,7 +10,7 @@ Ext.define('PVE.window.BulkAction', {
     },
     border: false,
 
-    // the action to set, currently there are: `startall`, `migrateall`, `stopall`
+    // the action to set, currently there are: `startall`, `migrateall`, `stopall`, `suspendall`
     action: undefined,
 
     submit: function(params) {
-- 
2.39.2

Re: [pve-devel] [PATCH pve-manager 1/2] api: add suspendall endpoint

[Thomas Lamprecht]

Am 18/10/2023 um 12:34 schrieb Hannes Laimer:
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
> 
> code is mostly taken from the already existing stopal endpoint, since
> all checks are basically the same for both suspend and stop.
> 
>  PVE/API2/Nodes.pm | 118 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 118 insertions(+)
> 
> diff --git a/PVE/API2/Nodes.pm b/PVE/API2/Nodes.pm
> index 0843c3a3..bb77474f 100644
> --- a/PVE/API2/Nodes.pm
> +++ b/PVE/API2/Nodes.pm
> @@ -286,6 +286,7 @@ __PACKAGE__->register_method ({
>  	    { name => 'spiceshell' },
>  	    { name => 'startall' },
>  	    { name => 'status' },
> +	    { name => 'suspendall' },

this list is alphanumerical sorted, please insert the new entry that way
too.

>  	    { name => 'stopall' },
>  	    { name => 'storage' },
>  	    { name => 'subscription' },
> @@ -2019,6 +2020,123 @@ __PACKAGE__->register_method ({
>  	return $rpcenv->fork_worker('stopall', undef, $authuser, $code);
>      }});
>  
> +my $create_suspend_worker = sub {
> +    my ($nodename, $vmid) = @_;
> +    return if !PVE::QemuServer::check_running($vmid, 1);

what if one passes VMIDs for Containers? should be either checked early and
errored out, or explictily returned early here to avoid strange effects.

> +    print STDERR "Suspending VM $vmid\n";
> +    return PVE::API2::Qemu->vm_suspend(
> +	{ node => $nodename, vmid => $vmid, todisk => 1 }
> +    );
> +};
> +
> +__PACKAGE__->register_method ({
> +    name => 'suspendall',
> +    path => 'suspendall',
> +    method => 'POST',
> +    protected => 1,
> +    permissions => {
> +	description => "The 'VM.PowerMgmt' permission is required on '/' or on '/vms/<ID>' for "
> +	    ."each ID passed via the 'vms' parameter.",

not complete, as we pass todisk to the actual suspend API call, there the access
for 'VM.Config.Disk' and 'Datastore.AllocateSpace' on the resolved storage is also
tested.

I had the following diff as follow-up, but due to the container VMID not being handled
it might make more sense to send a further revision.

diff --git a/PVE/API2/Nodes.pm b/PVE/API2/Nodes.pm
index f2f08dd7..995f0086 100644
--- a/PVE/API2/Nodes.pm
+++ b/PVE/API2/Nodes.pm
@@ -2027,8 +2027,9 @@ __PACKAGE__->register_method ({
     method => 'POST',
     protected => 1,
     permissions => {
-       description => "The 'VM.PowerMgmt' permission is required on '/' or on '/vms/<ID>' for "
-           ."each ID passed via the 'vms' parameter.",
+       description => "The 'VM.PowerMgmt' permission is required on '/' or on '/vms/<ID>' for each"
+           ." ID passed via the 'vms' parameter. Additionally, you need 'VM.Config.Disk' on  the"
+           ." '/vms/{vmid}' path and 'Datastore.AllocateSpace' for the configured state-storage(s)",
        user => 'all',
     },
     proxyto => 'node',
@@ -2053,12 +2054,13 @@ __PACKAGE__->register_method ({
        my $rpcenv = PVE::RPCEnvironment::get();
        my $authuser = $rpcenv->get_user();
 
-       if (!$rpcenv->check($authuser, "/", [ 'VM.PowerMgmt' ], 1)) {
+       # we cannot really check access to the state-storage here, that's happening per worker.
+       if (!$rpcenv->check($authuser, "/", [ 'VM.PowerMgmt', 'VM.Config.Disk' ], 1)) {
            my @vms = PVE::Tools::split_list($param->{vms});
            if (scalar(@vms) > 0) {
-               $rpcenv->check($authuser, "/vms/$_", [ 'VM.PowerMgmt' ]) for @vms;
+               $rpcenv->check($authuser, "/vms/$_", [ 'VM.PowerMgmt', 'VM.Config.Disk' ]) for @vms;
            } else {
-               raise_perm_exc("/, VM.PowerMgmt");
+               raise_perm_exc("/, VM.PowerMgmt && VM.Config.Disk");
            }
        }

Re: [pve-devel] [PATCH pve-manager 2/2] ui: add bulk suspend support

[Thomas Lamprecht]

Am 18/10/2023 um 12:34 schrieb Hannes Laimer:

> diff --git a/www/manager6/form/VMSelector.js b/www/manager6/form/VMSelector.js
> index d59847f2..ad0bfc03 100644
> --- a/www/manager6/form/VMSelector.js
> +++ b/www/manager6/form/VMSelector.js
> @@ -233,6 +233,10 @@ Ext.define('PVE.form.VMSelector', {
>  		case 'stopall':
>  		    statusfilter = 'running';
>  		    break;
> +		case 'suspendall':
> +		    statusfilter = 'running';
> +		    me.getStore().addFilter({ property: 'type', value: /qemu/ });

@Dominik: this (as a whole) clashes a bit with your filter-rework for bulk-actions,
would you prefer to get that in earlier or is adapting to this not much work anyway?

> +		    break;
>  	    }
>  	    if (statusfilter !== '') {
>  		me.getStore().addFilter([{
> diff --git a/www/manager6/node/CmdMenu.js b/www/manager6/node/CmdMenu.js
> index dc56ef08..c64a54d2 100644
> --- a/www/manager6/node/CmdMenu.js
> +++ b/www/manager6/node/CmdMenu.js
> @@ -56,6 +56,20 @@ Ext.define('PVE.node.CmdMenu', {
>  		});
>  	    },
>  	},
> +	{
> +	    text: gettext('Bulk Suspend'),
> +	    itemId: 'bulkstop',

wrong itemId, and the duplication with the one from Stop results in that getting
hidden. FWICT, that id isn't even needed here, so just dropping that would fix
this.

> +	    iconCls: 'fa fa-fw fa-download',
> +	    handler: function() {
> +		Ext.create('PVE.window.BulkAction', {
> +		    nodename: this.up('menu').nodename,
> +		    title: gettext('Bulk Suspend'),
> +		    btnText: gettext('Suspend'),
> +		    action: 'suspendall',
> +		    autoShow: true,
> +		});
> +	    },
> +	},
>  	{
>  	    text: gettext('Bulk Migrate'),
>  	    itemId: 'bulkmigrate',

Re: [pve-devel] [PATCH pve-manager 2/2] ui: add bulk suspend support

[Dominik Csapak]

On 11/6/23 19:32, Thomas Lamprecht wrote:
> Am 18/10/2023 um 12:34 schrieb Hannes Laimer:
> 
>> diff --git a/www/manager6/form/VMSelector.js b/www/manager6/form/VMSelector.js
>> index d59847f2..ad0bfc03 100644
>> --- a/www/manager6/form/VMSelector.js
>> +++ b/www/manager6/form/VMSelector.js
>> @@ -233,6 +233,10 @@ Ext.define('PVE.form.VMSelector', {
>>   		case 'stopall':
>>   		    statusfilter = 'running';
>>   		    break;
>> +		case 'suspendall':
>> +		    statusfilter = 'running';
>> +		    me.getStore().addFilter({ property: 'type', value: /qemu/ });
> 
> @Dominik: this (as a whole) clashes a bit with your filter-rework for bulk-actions,
> would you prefer to get that in earlier or is adapting to this not much work anyway?

would not really mind if this patch gets in first, i can adapt to that ;)