Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add --force argument to the stop command #1946

Merged
merged 53 commits into from
Jul 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
53 commits
Select commit Hold shift + click to select a range
aaf8b20
[rpc] Add option to force stopping an instance.
luis4a0 Jan 29, 2021
abbfbfe
[vm] Add bool to force shutdown.
luis4a0 Jan 29, 2021
27b5447
[cli] Handle command.
luis4a0 Jan 29, 2021
a9ac053
[tests] Adapt stub and mock to new vm interface.
luis4a0 Jan 29, 2021
610aab7
[daemon] Handle forced stop command
luis4a0 May 28, 2021
46bda20
[tests] Use the new virtual_machine.h interface.
luis4a0 May 28, 2021
1803c9f
[qemu] Implement forced stop.
luis4a0 May 28, 2021
d0444be
[lxd] Implement forced stop.
luis4a0 May 28, 2021
3ac6b53
[libvirt] Implement forced stop.
luis4a0 May 28, 2021
d063e76
refactor with main
sharder996 Aug 19, 2022
661f84c
[cli/stop] Make force a long option only
Mar 21, 2024
bcb050e
[virtual_machine] Make force shutdown bool const
Mar 26, 2024
8c060e0
[tests] Fix QEMU test for setting bool for shutdown()
Apr 4, 2024
63ee64a
[tests/cli] Test the --force option for stop command
Apr 5, 2024
7e52303
[tests/qemu] Add tests for the force shutdown handling
Apr 5, 2024
9dd92b0
Update src/platform/backends/qemu/qemu_virtual_machine.cpp
Apr 11, 2024
bb49b88
[tests] Fix tests due to change in log string
Apr 12, 2024
3c048d0
[cli/stop] Be more specific with force stop help
Apr 15, 2024
aefb733
[utils/qemu_img] Add general function for deleting a snapshot image
Apr 15, 2024
e484f7c
[backend/qemu] Delete suspend image on force shutdown
Apr 15, 2024
a67e2b2
[tests] Add qemu test for deleting suspend on force shutdown
Apr 15, 2024
bdbb952
[qemu] Check if there is a suspend image in force shutdown
Apr 17, 2024
c4884b2
[backend/qemu] Redo the shutdown logic
Apr 18, 2024
15aee25
[qemu] Redo some logic in how force shutdown is handled
Apr 26, 2024
8b1e3cc
[vm] Don't enclose BaseVirtualMachine impl in multipass scope
Apr 26, 2024
c05b950
[exceptions] Add new exception types for VirtualMachine states
May 2, 2024
d4cd079
[virtual_machine] Add method in base to check for states when stopping
May 2, 2024
629efe9
[backend/qemu] Add base VM state check handling for shutdown
May 2, 2024
06f740c
[tests] Fix/modify QEMU backend tests
May 2, 2024
38f670d
[qemu] Redo locking logic a bit to be more safe
May 5, 2024
1613141
[libvirt] Redo shutdown logic based on new base method
May 5, 2024
7f324b7
[tests] Fix/modify libvirt tests
May 5, 2024
08e9553
[tests] Add tests for libvirt shutdown
May 5, 2024
4e80935
[lxd] Add call to base class for checkoing shutdown states
May 6, 2024
46c39a0
[tests] Adapt LXD backend tests for changes in shutdown
May 6, 2024
9ceab96
[daemon] using lambda as opposed to std::bind to adapt functor.
georgeliao Jun 24, 2024
bcd0351
[qemu vm] use waiting for asynchronous process to finish as opposed t…
georgeliao Jun 24, 2024
7401440
[qemu vm] move the lock into the check_state_for_shutdown function si…
georgeliao Jun 24, 2024
c691e77
[daemon] small renaming for better clarity.
georgeliao Jun 24, 2024
0c867b3
[qemu] remove the state check and added the missing lock on state var…
georgeliao Jun 24, 2024
73430f5
[virtual machine] reverted back the commit "move the lock into the ch…
georgeliao Jun 25, 2024
bc50a1e
[qemu vm] removed unnecessary condition variable notification since w…
georgeliao Jun 25, 2024
072842c
[lxd vm] small polishing of LXDVirtualMachine::shutdown code.
georgeliao Jun 25, 2024
58ad18f
[qemu vm] added if empty check to pass unit tests and check the retur…
georgeliao Jun 26, 2024
7b24055
[cmd] update the stop command description.
georgeliao Jun 28, 2024
fa3dfca
[cmd] refined the force option help text based on the advice of the t…
georgeliao Jul 1, 2024
c5b1aa7
[lxd] address the review comment, change the function parameter to co…
georgeliao Jul 1, 2024
c9b3e3c
[qemu] refine the timeout values of wait_for_finished calls.
georgeliao Jul 1, 2024
2aab1f9
[qemu] change log to throw exception in the case of killing process c…
georgeliao Jul 1, 2024
d9fde4f
[virtual machine] change the const bool to bool.
georgeliao Jul 1, 2024
b2c5eea
Update src/platform/backends/libvirt/libvirt_virtual_machine.cpp
georgeliao Jul 2, 2024
985dff2
[qemu] refined the qemu process killing can not finish error message.
georgeliao Jul 2, 2024
5173700
[daemon] changed adapter from std::bind to lambda.
georgeliao Jul 2, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions include/multipass/exceptions/virtual_machine_state_exceptions.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
* Copyright (C) Canonical, Ltd.
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; version 3.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/

#ifndef MULTIPASS_VIRTUAL_MACHINE_STATE_EXCEPTIONS_H
#define MULTIPASS_VIRTUAL_MACHINE_STATE_EXCEPTIONS_H

#include <stdexcept>
#include <string>

namespace multipass
{
class VMStateIdempotentException : public std::runtime_error
{
public:
explicit VMStateIdempotentException(const std::string& msg) : runtime_error{msg}
{
}
};

class VMStateInvalidException : public std::runtime_error
{
public:
explicit VMStateInvalidException(const std::string& msg) : runtime_error{msg}
{
}
};

} // namespace multipass

#endif // MULTIPASS_VIRTUAL_MACHINE_STATE_EXCEPTIONS_H
2 changes: 1 addition & 1 deletion include/multipass/virtual_machine.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class VirtualMachine : private DisabledCopyMove

virtual ~VirtualMachine() = default;
virtual void start() = 0;
virtual void shutdown() = 0;
virtual void shutdown(bool force = false) = 0;
virtual void suspend() = 0;
virtual State current_state() = 0;
virtual int ssh_port() = 0;
Expand Down
17 changes: 14 additions & 3 deletions src/client/cli/cmd/stop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ QString cmd::Stop::short_help() const

QString cmd::Stop::description() const
{
return QStringLiteral("Stop the named instances, if running. Exits with\n"
"return code 0 if successful.");
return QStringLiteral("Stop the named instances. Exits with return code 0 \n"
"if successful.");
}

mp::ParseCode cmd::Stop::parse_args(mp::ArgParser* parser)
Expand All @@ -84,7 +84,10 @@ mp::ParseCode cmd::Stop::parse_args(mp::ArgParser* parser)
QCommandLineOption time_option({"t", "time"}, "Time from now, in minutes, to delay shutdown of the instance",
"time", "0");
QCommandLineOption cancel_option({"c", "cancel"}, "Cancel a pending delayed shutdown");
parser->addOptions({all_option, time_option, cancel_option});
QCommandLineOption force_option("force",
"Force the instance to shut down immediately. Warning: This could potentially "
"corrupt a running instance, so use with caution.");
parser->addOptions({all_option, time_option, cancel_option, force_option});

auto status = parser->commandParse(this);
if (status != ParseCode::Ok)
Expand All @@ -105,6 +108,14 @@ mp::ParseCode cmd::Stop::parse_args(mp::ArgParser* parser)
return ParseCode::CommandLineError;
}

if (parser->isSet(force_option) && (parser->isSet(time_option) || parser->isSet(cancel_option)))
{
cerr << "Cannot set \'force\' along with \'time\' or \'cancel\' options at the same time\n";
return ParseCode::CommandLineError;
}

request.set_force_stop(parser->isSet(force_option));

auto time = parser->value(time_option);

if (time.startsWith("+"))
Expand Down
39 changes: 33 additions & 6 deletions src/daemon/daemon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2132,10 +2132,13 @@

std::function<grpc::Status(VirtualMachine&)> operation;
if (request->cancel_shutdown())
operation = std::bind(&Daemon::cancel_vm_shutdown, this, std::placeholders::_1);
operation = [this](const VirtualMachine& vm) { return this->cancel_vm_shutdown(vm); };
else if (request->force_stop())
operation = [this](VirtualMachine& vm) { return this->switch_off_vm(vm); };

Check warning on line 2137 in src/daemon/daemon.cpp

View check run for this annotation

Codecov / codecov/patch

src/daemon/daemon.cpp#L2135-L2137

Added lines #L2135 - L2137 were not covered by tests
else
operation = std::bind(&Daemon::shutdown_vm, this, std::placeholders::_1,
std::chrono::minutes(request->time_minutes()));
operation = [this, delay_minutes = std::chrono::minutes(request->time_minutes())](VirtualMachine& vm) {
return this->shutdown_vm(vm, delay_minutes);
};

Check warning on line 2141 in src/daemon/daemon.cpp

View check run for this annotation

Codecov / codecov/patch

src/daemon/daemon.cpp#L2139-L2141

Added lines #L2139 - L2141 were not covered by tests

status = cmd_vms(instance_selection.operative_selection, operation);
}
Expand Down Expand Up @@ -3071,7 +3074,7 @@
delayed_shutdown_instances.erase(name);

mounts[name].clear();
instance->shutdown();
instance->shutdown(purge);

if (!purge)
{
Expand Down Expand Up @@ -3116,12 +3119,14 @@
grpc::Status mp::Daemon::shutdown_vm(VirtualMachine& vm, const std::chrono::milliseconds delay)
{
const auto& name = vm.vm_name;
const auto& state = vm.current_state();
const auto& current_state = vm.current_state();

Check warning on line 3122 in src/daemon/daemon.cpp

View check run for this annotation

Codecov / codecov/patch

src/daemon/daemon.cpp#L3122

Added line #L3122 was not covered by tests

using St = VirtualMachine::State;
const auto skip_states = {St::off, St::stopped, St::suspended};

if (std::none_of(cbegin(skip_states), cend(skip_states), [&state](const auto& st) { return state == st; }))
if (std::none_of(cbegin(skip_states), cend(skip_states), [&current_state](const auto& skip_state) {
return current_state == skip_state;

Check warning on line 3128 in src/daemon/daemon.cpp

View check run for this annotation

Codecov / codecov/patch

src/daemon/daemon.cpp#L3127-L3128

Added lines #L3127 - L3128 were not covered by tests
}))
{
delayed_shutdown_instances.erase(name);

Expand All @@ -3140,6 +3145,28 @@
return grpc::Status::OK;
}

grpc::Status mp::Daemon::switch_off_vm(VirtualMachine& vm)

Check warning on line 3148 in src/daemon/daemon.cpp

View check run for this annotation

Codecov / codecov/patch

src/daemon/daemon.cpp#L3148

Added line #L3148 was not covered by tests
{
const auto& name = vm.vm_name;
const auto& current_state = vm.current_state();

Check warning on line 3151 in src/daemon/daemon.cpp

View check run for this annotation

Codecov / codecov/patch

src/daemon/daemon.cpp#L3150-L3151

Added lines #L3150 - L3151 were not covered by tests

using St = VirtualMachine::State;
const auto skip_states = {St::off, St::stopped};

Check warning on line 3154 in src/daemon/daemon.cpp

View check run for this annotation

Codecov / codecov/patch

src/daemon/daemon.cpp#L3154

Added line #L3154 was not covered by tests
ricab marked this conversation as resolved.
Show resolved Hide resolved

if (std::none_of(cbegin(skip_states), cend(skip_states), [&current_state](const auto& skip_state) {
return current_state == skip_state;

Check warning on line 3157 in src/daemon/daemon.cpp

View check run for this annotation

Codecov / codecov/patch

src/daemon/daemon.cpp#L3156-L3157

Added lines #L3156 - L3157 were not covered by tests
}))
{
delayed_shutdown_instances.erase(name);

Check warning on line 3160 in src/daemon/daemon.cpp

View check run for this annotation

Codecov / codecov/patch

src/daemon/daemon.cpp#L3160

Added line #L3160 was not covered by tests

vm.shutdown(true);

Check warning on line 3162 in src/daemon/daemon.cpp

View check run for this annotation

Codecov / codecov/patch

src/daemon/daemon.cpp#L3162

Added line #L3162 was not covered by tests
}
else
mpl::log(mpl::Level::debug, category, fmt::format("instance \"{}\" does not need stopping", name));

Check warning on line 3165 in src/daemon/daemon.cpp

View check run for this annotation

Codecov / codecov/patch

src/daemon/daemon.cpp#L3165

Added line #L3165 was not covered by tests

return grpc::Status::OK;

Check warning on line 3167 in src/daemon/daemon.cpp

View check run for this annotation

Codecov / codecov/patch

src/daemon/daemon.cpp#L3167

Added line #L3167 was not covered by tests
}

grpc::Status mp::Daemon::cancel_vm_shutdown(const VirtualMachine& vm)
{
auto it = delayed_shutdown_instances.find(vm.vm_name);
Expand Down
1 change: 1 addition & 0 deletions src/daemon/daemon.h
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ public slots:
bool delete_vm(InstanceTable::iterator vm_it, bool purge, DeleteReply& response);
grpc::Status reboot_vm(VirtualMachine& vm);
grpc::Status shutdown_vm(VirtualMachine& vm, const std::chrono::milliseconds delay);
grpc::Status switch_off_vm(VirtualMachine& vm);
grpc::Status cancel_vm_shutdown(const VirtualMachine& vm);
grpc::Status get_ssh_info_for_vm(VirtualMachine& vm, SSHInfoReply& response);

Expand Down
56 changes: 33 additions & 23 deletions src/platform/backends/libvirt/libvirt_virtual_machine.cpp
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

libvirt does not remove suspended state on force stop

Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "libvirt_virtual_machine.h"

#include <multipass/exceptions/start_exception.h>
#include <multipass/exceptions/virtual_machine_state_exceptions.h>
#include <multipass/format.h>
#include <multipass/logging/log.h>
#include <multipass/memory_size.h>
Expand Down Expand Up @@ -345,41 +346,50 @@ void mp::LibVirtVirtualMachine::start()
monitor->on_resume();
}

void mp::LibVirtVirtualMachine::shutdown()
void mp::LibVirtVirtualMachine::shutdown(bool force)
{
std::unique_lock<decltype(state_mutex)> lock{state_mutex};
auto domain = domain_by_name_for(vm_name, open_libvirt_connection(libvirt_wrapper).get(), libvirt_wrapper);
std::unique_lock<std::mutex> lock{state_mutex};
auto domain = checked_vm_domain();

state = refresh_instance_state_for_domain(domain.get(), state, libvirt_wrapper);
if (state == State::suspended)

try
{
mpl::log(mpl::Level::info, vm_name, fmt::format("Ignoring shutdown issued while suspended"));
check_state_for_shutdown(force);
}
else
catch (const VMStateIdempotentException& e)
{
drop_ssh_session();
mpl::log(mpl::Level::info, vm_name, e.what());
return;
}

if (state == State::running || state == State::delayed_shutdown || state == State::unknown)
{
if (!domain || libvirt_wrapper->virDomainShutdown(domain.get()) == -1)
{
auto warning_string{
fmt::format("Cannot shutdown '{}': {}", vm_name, libvirt_wrapper->virGetLastErrorMessage())};
mpl::log(mpl::Level::warning, vm_name, warning_string);
throw std::runtime_error(warning_string);
}

state = State::off;
update_state();
}
else if (state == State::starting)
if (force) // TODO delete suspension state if it exists
{
mpl::log(mpl::Level::info, vm_name, "Forcing shutdown");

libvirt_wrapper->virDomainDestroy(domain.get());

if (state == State::starting || state == State::restarting)
{
libvirt_wrapper->virDomainDestroy(domain.get());
state_wait.wait(lock, [this] { return shutdown_while_starting; });
update_state();
}
}
else
{
drop_ssh_session();

lock.unlock();
if (libvirt_wrapper->virDomainShutdown(domain.get()) == -1)
{
auto warning_string{
fmt::format("Cannot shutdown '{}': {}", vm_name, libvirt_wrapper->virGetLastErrorMessage())};
mpl::log(mpl::Level::warning, vm_name, warning_string);
throw std::runtime_error(warning_string);
}
}

state = State::off;
update_state();
monitor->on_shutdown();
}

Expand Down
2 changes: 1 addition & 1 deletion src/platform/backends/libvirt/libvirt_virtual_machine.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class LibVirtVirtualMachine final : public BaseVirtualMachine
~LibVirtVirtualMachine();

void start() override;
void shutdown() override;
void shutdown(bool force = false) override;
void suspend() override;
State current_state() override;
int ssh_port() override;
Expand Down
30 changes: 17 additions & 13 deletions src/platform/backends/lxd/lxd_virtual_machine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include <multipass/exceptions/local_socket_connection_exception.h>
#include <multipass/exceptions/snap_environment_exception.h>
#include <multipass/exceptions/start_exception.h>
#include <multipass/exceptions/virtual_machine_state_exceptions.h>
#include <multipass/format.h>
#include <multipass/logging/log.h>
#include <multipass/memory_size.h>
Expand Down Expand Up @@ -261,26 +262,25 @@ void mp::LXDVirtualMachine::start()
update_state();
}

void mp::LXDVirtualMachine::shutdown()
void mp::LXDVirtualMachine::shutdown(bool force)
{
std::unique_lock<decltype(state_mutex)> lock{state_mutex};
auto present_state = current_state();
std::unique_lock<std::mutex> lock{state_mutex};

if (present_state == State::stopped)
const auto present_state = current_state();

try
{
mpl::log(mpl::Level::debug, vm_name, "Ignoring stop request since instance is already stopped");
return;
check_state_for_shutdown(force);
}

if (present_state == State::suspended)
catch (const VMStateIdempotentException& e)
{
mpl::log(mpl::Level::info, vm_name, fmt::format("Ignoring shutdown issued while suspended"));
mpl::log(mpl::Level::info, vm_name, e.what());
return;
}

request_state("stop");
request_state("stop", {{"force", force}});

state = State::stopped;
state = State::off;

if (present_state == State::starting)
{
Expand Down Expand Up @@ -407,9 +407,13 @@ const QUrl mp::LXDVirtualMachine::network_leases_url()
return base_url.toString() + "/networks/" + bridge_name + "/leases";
}

void mp::LXDVirtualMachine::request_state(const QString& new_state)
void mp::LXDVirtualMachine::request_state(const QString& new_state, const QJsonObject& args)
{
const QJsonObject state_json{{"action", new_state}};
QJsonObject state_json{{"action", new_state}};
for (auto it = args.constBegin(); it != args.constEnd(); it++)
{
state_json.insert(it.key(), it.value());
}

auto state_task = lxd_request(manager, "PUT", state_url(), state_json, 5000);

Expand Down
5 changes: 3 additions & 2 deletions src/platform/backends/lxd/lxd_virtual_machine.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,9 @@ class LXDVirtualMachine : public BaseVirtualMachine
const SSHKeyProvider& key_provider,
const Path& instance_dir);
~LXDVirtualMachine() override;

void start() override;
void shutdown() override;
void shutdown(bool force = false) override;
void suspend() override;
State current_state() override;
int ssh_port() override;
Expand Down Expand Up @@ -85,7 +86,7 @@ class LXDVirtualMachine : public BaseVirtualMachine
const QUrl url() const;
const QUrl state_url();
const QUrl network_leases_url();
void request_state(const QString& new_state);
void request_state(const QString& new_state, const QJsonObject& args = {});
};
} // namespace multipass
#endif // MULTIPASS_LXD_VIRTUAL_MACHINE_H
Loading
Loading