On 06.09.19 14:24, 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
>
> includes some refactoring of the 'vm_stop' call, so that
> we can have a clean 'vm_reboot' sub without the many parameters
> of 'vm_stop'
>
> Signed-off-by: Dominik Csapak <d.csa...@proxmox.com>
> ---
> better view with '-w' as it is mostly code shift
>
> changes from v2:
> * refactored vm_stop so that the code can be reused in vm_reboot
looks better, but I'd done the refactoring in a separate no-semantic-change
patch first.
> * better function signature
> * add vm running check in api, such that it cannot be called on
> stopped vms (which would not start the vm)
>
> PVE/API2/Qemu.pm | 60 +++++++++++++++++
> PVE/CLI/qm.pm | 2 +
> PVE/QemuServer.pm | 162 ++++++++++++++++++++++++++--------------------
> 3 files changed, 153 insertions(+), 71 deletions(-)
>
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index 9db8967..fb63cab 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -1910,6 +1910,7 @@ __PACKAGE__->register_method({
> { subdir => 'reset' },
> { subdir => 'shutdown' },
> { subdir => 'suspend' },
> + { subdir => 'reboot' },
> ];
>
> return $res;
> @@ -2333,6 +2334,65 @@ __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.",
we could add a hint that thus pending changes get applied? Or do you think
that this is clear and expected?
> + 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 $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";
> + }
> +
> + die "VM $vmid not running\n" if !PVE::QemuServer::check_running($vmid);
> +
> + my $realcmd = sub {
> + my $upid = shift;
> +
> + syslog('info', "requesting reboot of VM $vmid: $upid\n");
> + my $storecfg = PVE::Storage::config();
hmm, I'd maybe move the storecfg read into vm_reboot, removing it from the
method parameter signature? As nothing else is done with it here anyway?
> + PVE::QemuServer::vm_reboot($storecfg, $vmid, $param->{timeout});
> + return;
> + };
> +
> + return $rpcenv->fork_worker('qmreboot', $vmid, $authuser, $realcmd);
> + }});
> +
> __PACKAGE__->register_method({
> name => 'vm_suspend',
> path => '{vmid}/status/suspend',
> diff --git a/PVE/CLI/qm.pm b/PVE/CLI/qm.pm
> index dea3deb..3ec8a8c 100755
> --- a/PVE/CLI/qm.pm
> +++ b/PVE/CLI/qm.pm
> @@ -1000,6 +1000,8 @@ our $cmddef = {
>
> shutdown => [ "PVE::API2::Qemu", 'vm_shutdown', ['vmid'], { node =>
> $nodename }, $upid_exit ],
>
> + reboot => [ "PVE::API2::Qemu", 'vm_reboot', ['vmid'], { node =>
> $nodename }, $upid_exit ],
> +
> suspend => [ "PVE::API2::Qemu", 'vm_suspend', ['vmid'], { node =>
> $nodename }, $upid_exit ],
>
> resume => [ "PVE::API2::Qemu", 'vm_resume', ['vmid'], { node =>
> $nodename }, $upid_exit ],
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index ba83b10..a1d4969 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -5773,85 +5773,42 @@ sub vm_stop_cleanup {
> warn $@ if $@; # avoid errors - just warn
> }
>
> -# Note: use $nockeck to skip tests if VM configuration file exists.
> -# 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) = @_;
> +# call only in locked context
> +sub _do_vm_stop {
> + my ($storecfg, $vmid, $skiplock, $nocheck, $timeout, $shutdown, $force,
> $keepActive) = @_;
>
> - $force = 1 if !defined($force) && !$shutdown;
> + my $pid = check_running($vmid, $nocheck);
> + return if !$pid;
>
> - if ($migratedfrom){
> - my $pid = check_running($vmid, $nocheck, $migratedfrom);
> - kill 15, $pid if $pid;
> - my $conf = PVE::QemuConfig->load_config($vmid, $migratedfrom);
> - vm_stop_cleanup($storecfg, $vmid, $conf, $keepActive, 0);
> - return;
> - }
> -
> - PVE::QemuConfig->lock_config($vmid, sub {
> -
> - my $pid = check_running($vmid, $nocheck);
> - return if !$pid;
> -
> - my $conf;
> - if (!$nocheck) {
> - $conf = PVE::QemuConfig->load_config($vmid);
> - PVE::QemuConfig->check_lock($conf) if !$skiplock;
> - if (!defined($timeout) && $shutdown && $conf->{startup}) {
> - my $opts =
> PVE::JSONSchema::pve_parse_startup_order($conf->{startup});
> - $timeout = $opts->{down} if $opts->{down};
> - }
> - PVE::GuestHelpers::exec_hookscript($conf, $vmid, 'pre-stop');
> + my $conf;
> + if (!$nocheck) {
> + $conf = PVE::QemuConfig->load_config($vmid);
> + PVE::QemuConfig->check_lock($conf) if !$skiplock;
> + if (!defined($timeout) && $shutdown && $conf->{startup}) {
> + my $opts =
> PVE::JSONSchema::pve_parse_startup_order($conf->{startup});
> + $timeout = $opts->{down} if $opts->{down};
> }
> + PVE::GuestHelpers::exec_hookscript($conf, $vmid, 'pre-stop');
> + }
>
> - eval {
> - if ($shutdown) {
> - if (defined($conf) && parse_guest_agent($conf)->{enabled}) {
> - vm_qmp_command($vmid, {
> + eval {
> + if ($shutdown) {
> + if (defined($conf) && parse_guest_agent($conf)->{enabled}) {
> + vm_qmp_command($vmid, {
> execute => "guest-shutdown",
> arguments => { timeout => $timeout }
> }, $nocheck);
> - } else {
> - vm_qmp_command($vmid, { execute => "system_powerdown" },
> $nocheck);
> - }
> - } else {
> - vm_qmp_command($vmid, { execute => "quit" }, $nocheck);
> - }
> - };
> - my $err = $@;
> -
> - if (!$err) {
> - $timeout = 60 if !defined($timeout);
> -
> - my $count = 0;
> - while (($count < $timeout) && check_running($vmid, $nocheck)) {
> - $count++;
> - sleep 1;
> - }
> -
> - if ($count >= $timeout) {
> - if ($force) {
> - warn "VM still running - terminating now with SIGTERM\n";
> - kill 15, $pid;
> - } else {
> - die "VM quit/powerdown failed - got timeout\n";
> - }
> } else {
> - vm_stop_cleanup($storecfg, $vmid, $conf, $keepActive, 1) if
> $conf;
> - return;
> + vm_qmp_command($vmid, { execute => "system_powerdown" },
> $nocheck);
> }
> } else {
> - if ($force) {
> - warn "VM quit/powerdown failed - terminating now with
> SIGTERM\n";
> - kill 15, $pid;
> - } else {
> - die "VM quit/powerdown failed\n";
> - }
> + vm_qmp_command($vmid, { execute => "quit" }, $nocheck);
> }
> + };
> + my $err = $@;
>
> - # wait again
> - $timeout = 10;
> + if (!$err) {
> + $timeout = 60 if !defined($timeout);
>
> my $count = 0;
> while (($count < $timeout) && check_running($vmid, $nocheck)) {
> @@ -5860,12 +5817,75 @@ sub vm_stop {
> }
>
> if ($count >= $timeout) {
> - warn "VM still running - terminating now with SIGKILL\n";
> - kill 9, $pid;
> - sleep 1;
> + if ($force) {
> + warn "VM still running - terminating now with SIGTERM\n";
> + kill 15, $pid;
> + } else {
> + die "VM quit/powerdown failed - got timeout\n";
> + }
> + } else {
> + vm_stop_cleanup($storecfg, $vmid, $conf, $keepActive, 1) if $conf;
> + return;
> }
> + } else {
> + if ($force) {
> + warn "VM quit/powerdown failed - terminating now with SIGTERM\n";
> + kill 15, $pid;
> + } else {
> + die "VM quit/powerdown failed\n";
> + }
> + }
> +
> + # wait again
> + $timeout = 10;
> +
> + my $count = 0;
> + while (($count < $timeout) && check_running($vmid, $nocheck)) {
> + $count++;
> + sleep 1;
> + }
> +
> + if ($count >= $timeout) {
> + warn "VM still running - terminating now with SIGKILL\n";
> + kill 9, $pid;
> + sleep 1;
> + }
> +
> + vm_stop_cleanup($storecfg, $vmid, $conf, $keepActive, 1) if $conf;
> +}
> +
> +# Note: use $nockeck to skip tests if VM configuration file exists.
s/nockeck/nocheck
> +# 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) = @_;
> +
> + $force = 1 if !defined($force) && !$shutdown;
> +
> + if ($migratedfrom){
> + my $pid = check_running($vmid, $nocheck, $migratedfrom);
> + kill 15, $pid if $pid;
> + my $conf = PVE::QemuConfig->load_config($vmid, $migratedfrom);
> + vm_stop_cleanup($storecfg, $vmid, $conf, $keepActive, 0);
> + return;
> + }
> +
> + PVE::QemuConfig->lock_config($vmid, sub {
> + _do_vm_stop($storecfg, $vmid, $skiplock, $nocheck, $timeout, $shutdown,
> $force, $keepActive);
> + });
> +}
> +
> +sub vm_reboot {
> + my ($storecfg, $vmid, $timeout) = @_;
> +
> + PVE::QemuConfig->lock_config($vmid, sub {
> + # do not request reboot if it is not running, since
> + # it can only start again if it qmeventd detects that it stopped
double negation, maybe:
# only reboot if running, as a qmeventd stop event starts it again
> + return if !check_running($vmid);
> +
> + create_reboot_request($vmid);
>
> - vm_stop_cleanup($storecfg, $vmid, $conf, $keepActive, 1) if $conf;
> + _do_vm_stop($storecfg, $vmid, undef, undef, $timeout, 1);
> });
> }
>
>
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
