On 23.08.19 10:55, Dominik Csapak wrote:
> this creates a reboot trigger file (inspired by pve-container)
> and relies on the 'qm cleanup' call by the qmeventd to detect
> and restart the vm afterwards
>
> Signed-off-by: Dominik Csapak <d.csa...@proxmox.com>
> ---
> changes from rfc:
> * use PVE::QemuServer:vm_stop instead of the api call, to prevent
> the ha manager to change the target state
> * create the trigger only during the lock in vm_stop, to be sure we are
> actually shutting down
>
> PVE/API2/Qemu.pm | 63 +++++++++++++++++++++++++++++++++++++++++++++++
> PVE/QemuServer.pm | 4 ++-
> 2 files changed, 66 insertions(+), 1 deletion(-)
>
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index b30931d..5094721 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -2339,6 +2339,69 @@ __PACKAGE__->register_method({
> }
> }});
>
> +__PACKAGE__->register_method({
> + name => 'vm_reboot',
> + path => '{vmid}/status/reboot',
> + method => 'POST',
> + protected => 1,
> + proxyto => 'node',
> + description => "Reboot the VM by shutting it down, and starting it
> again.",
> + permissions => {
> + check => ['perm', '/vms/{vmid}', [ 'VM.PowerMgmt' ]],
> + },
> + parameters => {
> + additionalProperties => 0,
> + properties => {
> + node => get_standard_option('pve-node'),
> + vmid => get_standard_option('pve-vmid',
> + { completion =>
> \&PVE::QemuServer::complete_vmid_running }),
> + timeout => {
> + description => "Wait maximal timeout seconds for the shutdown.",
> + type => 'integer',
> + minimum => 0,
> + optional => 1,
> + },
> + },
> + },
> + returns => {
> + type => 'string',
> + },
> + code => sub {
> + my ($param) = @_;
> +
> + my $rpcenv = PVE::RPCEnvironment::get();
> + my $authuser = $rpcenv->get_user();
> +
> + my $node = extract_param($param, 'node');
> + my $vmid = extract_param($param, 'vmid');
> +
> + my $storecfg = PVE::Storage::config();
> +
> + my $shutdown = 1;
> + my $reboot = 1;
variable declaration vs use. locality is not too good here, IMO.
$storecfg could be moved below; above the vm_stop call, the two
variables which try to improve readability could be also moved there.
> +
> + my $qmpstatus = eval {
> + PVE::QemuServer::vm_qmp_command($vmid, { execute => "query-status"
> }, 0);
> + };
> + my $err = $@ if $@;
> +
> + if (!$err && $qmpstatus->{status} eq "paused") {
> + die "VM is paused - cannot shutdown\n";
> + }
> +
> + my $realcmd = sub {
> + my $upid = shift;
> +
> + syslog('info', "shutdown VM $vmid: $upid\n");
s/shutdown/reboot/
> +
> + PVE::QemuServer::vm_stop($storecfg, $vmid, undef, 0,
> $param->{timeout},
> + $shutdown, undef, undef, undef, $reboot);
> + return;
> + };
> +
> + return $rpcenv->fork_worker('qmshutdown', $vmid, $authuser, $realcmd);
> + }});
> +
> __PACKAGE__->register_method({
> name => 'vm_suspend',
> path => '{vmid}/status/suspend',
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index cc0c95e..ab4ee3e 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -5739,7 +5739,7 @@ sub vm_stop_cleanup {
> # We need that when migration VMs to other nodes (files already moved)
> # Note: we set $keepActive in vzdump stop mode - volumes need to stay active
> sub vm_stop {
> - my ($storecfg, $vmid, $skiplock, $nocheck, $timeout, $shutdown, $force,
> $keepActive, $migratedfrom) = @_;
> + my ($storecfg, $vmid, $skiplock, $nocheck, $timeout, $shutdown, $force,
> $keepActive, $migratedfrom, $reboot) = @_;
I'd like to rather avoid adding such a parameter to this mess of
a method signature..
Also, your call to this above has 4 undefs and a 0 which could be undef,
so 5 fixed params..
Maybe move the actual anonymous sub here doing the shutdown in locked
context to a "_do_vm_stop method" (with comment that it's required to call
it in locked context) and add your own "vm_reboot" method with a reduced
method-signature, which just calls "_do_vm_stop" and the "create_reboot_trigger"
(or "create_reboot_request", if you agree with the review of 1/5) methods.
would be IMO much nicer.
>
> $force = 1 if !defined($force) && !$shutdown;
>
> @@ -5756,6 +5756,8 @@ sub vm_stop {
> my $pid = check_running($vmid, $nocheck);
> return if !$pid;
>
> + create_reboot_trigger($vmid) if $reboot;
maybe a comment to the create_reboot... call that it's handled by the qm eventd
called post stop hooks would be great, else people reading this will question
where
or how it happens.
> +
> my $conf;
> if (!$nocheck) {
> $conf = PVE::QemuConfig->load_config($vmid);
>
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
