From b8e03b20520c37c8a97b0fd26a3abe7cb51fc088 Mon Sep 17 00:00:00 2001 From: Marek Blaha Date: Fri, 30 Aug 2024 12:47:42 +0200 Subject: [PATCH 1/2] DownloadCallbacks: Enum for possible return codes Instead of hard coded constants use enum as callbacks return values. --- dnf5/download_callbacks.cpp | 6 +++--- dnf5daemon-server/callbacks.cpp | 6 +++--- include/libdnf5/repo/download_callbacks.hpp | 4 ++++ libdnf5/repo/download_callbacks.cpp | 6 +++--- 4 files changed, 13 insertions(+), 9 deletions(-) diff --git a/dnf5/download_callbacks.cpp b/dnf5/download_callbacks.cpp index 2123f1928..18405a974 100644 --- a/dnf5/download_callbacks.cpp +++ b/dnf5/download_callbacks.cpp @@ -60,7 +60,7 @@ int DownloadCallbacks::progress(void * user_cb_data, double total_to_download, d if (is_time_to_print()) { print(); } - return 0; + return ReturnCode::OK; } int DownloadCallbacks::end(void * user_cb_data, TransferStatus status, const char * msg) { @@ -87,7 +87,7 @@ int DownloadCallbacks::end(void * user_cb_data, TransferStatus status, const cha break; } print(); - return 0; + return ReturnCode::OK; } int DownloadCallbacks::mirror_failure(void * user_cb_data, const char * msg, const char * url, const char * metadata) { @@ -98,7 +98,7 @@ int DownloadCallbacks::mirror_failure(void * user_cb_data, const char * msg, con } progress_bar->add_message(libdnf5::cli::progressbar::MessageType::ERROR, message); print(); - return 0; + return ReturnCode::OK; } void DownloadCallbacks::reset_progress_bar() { diff --git a/dnf5daemon-server/callbacks.cpp b/dnf5daemon-server/callbacks.cpp index 7b20f1931..726dfb904 100644 --- a/dnf5daemon-server/callbacks.cpp +++ b/dnf5daemon-server/callbacks.cpp @@ -75,7 +75,7 @@ int DownloadCB::progress(void * user_cb_data, double total_to_download, double d } } catch (...) { } - return 0; + return ReturnCode::OK; } int DownloadCB::end(void * user_cb_data, TransferStatus status, const char * msg) { @@ -86,7 +86,7 @@ int DownloadCB::end(void * user_cb_data, TransferStatus status, const char * msg dbus_object->emitSignal(signal); } catch (...) { } - return 0; + return ReturnCode::OK; } int DownloadCB::mirror_failure(void * user_cb_data, const char * msg, const char * url, const char * metadata) { @@ -98,7 +98,7 @@ int DownloadCB::mirror_failure(void * user_cb_data, const char * msg, const char dbus_object->emitSignal(signal); } catch (...) { } - return 0; + return ReturnCode::OK; } diff --git a/include/libdnf5/repo/download_callbacks.hpp b/include/libdnf5/repo/download_callbacks.hpp index 883107f16..8fc4f27e6 100644 --- a/include/libdnf5/repo/download_callbacks.hpp +++ b/include/libdnf5/repo/download_callbacks.hpp @@ -67,6 +67,10 @@ class LIBDNF_API DownloadCallbacks { /// Transfer status codes. enum class TransferStatus { SUCCESSFUL, ALREADYEXISTS, ERROR }; + /// Download callbacks return values. + /// RETURN_OK - all is fine, RETURN_ABORT - abort just the current download, RETURN_ERROR - abort all downloading + enum ReturnCode : int { OK = 0, ABORT = 1, ERROR = 2 }; + DownloadCallbacks() = default; DownloadCallbacks(const DownloadCallbacks &) = delete; DownloadCallbacks(DownloadCallbacks &&) = delete; diff --git a/libdnf5/repo/download_callbacks.cpp b/libdnf5/repo/download_callbacks.cpp index 4ebb5ac6d..c12760603 100644 --- a/libdnf5/repo/download_callbacks.cpp +++ b/libdnf5/repo/download_callbacks.cpp @@ -30,14 +30,14 @@ void * DownloadCallbacks::add_new_download( int DownloadCallbacks::end( [[maybe_unused]] void * user_cb_data, [[maybe_unused]] TransferStatus status, [[maybe_unused]] const char * msg) { - return 0; + return ReturnCode::OK; } int DownloadCallbacks::progress( [[maybe_unused]] void * user_cb_data, [[maybe_unused]] double total_to_download, [[maybe_unused]] double downloaded) { - return 0; + return ReturnCode::OK; } int DownloadCallbacks::mirror_failure( @@ -45,7 +45,7 @@ int DownloadCallbacks::mirror_failure( [[maybe_unused]] const char * msg, [[maybe_unused]] const char * url, [[maybe_unused]] const char * metadata) { - return 0; + return ReturnCode::OK; } void DownloadCallbacks::fastest_mirror( From aec787f542c2c65d96b35168056826aa14f6b3fb Mon Sep 17 00:00:00 2001 From: Marek Blaha Date: Fri, 30 Aug 2024 12:48:50 +0200 Subject: [PATCH 2/2] dnfdaemon: API to cancel current running transaction New API `Goal.cancel()` cancels the transaction that was initiated by `do_transaction()`. The transaction can only be canceled during the package download phase. Once the RPM transaction has begun, cancellation is no longer permitted. --- dnf5daemon-server/callbacks.cpp | 3 ++ .../dbus/interfaces/org.rpm.dnf.v0.Goal.xml | 12 ++++++ dnf5daemon-server/services/goal/goal.cpp | 40 +++++++++++++++++++ dnf5daemon-server/services/goal/goal.hpp | 1 + dnf5daemon-server/session.cpp | 1 + dnf5daemon-server/session.hpp | 7 ++++ 6 files changed, 64 insertions(+) diff --git a/dnf5daemon-server/callbacks.cpp b/dnf5daemon-server/callbacks.cpp index 726dfb904..d6797f5cb 100644 --- a/dnf5daemon-server/callbacks.cpp +++ b/dnf5daemon-server/callbacks.cpp @@ -66,6 +66,9 @@ void * DownloadCB::add_new_download(void * user_data, const char * description, } int DownloadCB::progress(void * user_cb_data, double total_to_download, double downloaded) { + if (session.get_cancel_download() == Session::CancelDownload::REQUESTED) { + return ReturnCode::ERROR; + } try { if (is_time_to_print()) { auto signal = create_signal_download(dnfdaemon::SIGNAL_DOWNLOAD_PROGRESS, user_cb_data); diff --git a/dnf5daemon-server/dbus/interfaces/org.rpm.dnf.v0.Goal.xml b/dnf5daemon-server/dbus/interfaces/org.rpm.dnf.v0.Goal.xml index 56014b7bb..c94804c54 100644 --- a/dnf5daemon-server/dbus/interfaces/org.rpm.dnf.v0.Goal.xml +++ b/dnf5daemon-server/dbus/interfaces/org.rpm.dnf.v0.Goal.xml @@ -94,6 +94,18 @@ along with libdnf. If not, see . + + + + + + diff --git a/dnf5daemon-server/services/goal/goal.cpp b/dnf5daemon-server/services/goal/goal.cpp index 58797100c..0052441b8 100644 --- a/dnf5daemon-server/services/goal/goal.cpp +++ b/dnf5daemon-server/services/goal/goal.cpp @@ -104,6 +104,16 @@ void Goal::dbus_register() { [this](sdbus::MethodCall call) -> void { session.get_threads_manager().handle_method(*this, &Goal::do_transaction, call, session.session_locale); }); + dbus_object->registerMethod( + dnfdaemon::INTERFACE_GOAL, + "cancel", + "", + {}, + "bs", + {"success", "error_msg"}, + [this](sdbus::MethodCall call) -> void { + session.get_threads_manager().handle_method(*this, &Goal::cancel, call, session.session_locale); + }); } sdbus::MethodReply Goal::resolve(sdbus::MethodCall & call) { @@ -269,6 +279,7 @@ sdbus::MethodReply Goal::do_transaction(sdbus::MethodCall & call) { if (!session.check_authorization(dnfdaemon::POLKIT_EXECUTE_RPM_TRANSACTION, call.getSender())) { throw std::runtime_error("Not authorized"); } + session.set_cancel_download(Session::CancelDownload::NOT_REQUESTED); // read options from dbus call dnfdaemon::KeyValueMap options; @@ -281,6 +292,7 @@ sdbus::MethodReply Goal::do_transaction(sdbus::MethodCall & call) { // TODO(mblaha): signalize reboot? } else { session.download_transaction_packages(); + session.set_cancel_download(Session::CancelDownload::NOT_ALLOWED); std::string comment; if (options.find("comment") != options.end()) { @@ -306,3 +318,31 @@ sdbus::MethodReply Goal::do_transaction(sdbus::MethodCall & call) { auto reply = call.createReply(); return reply; } + +sdbus::MethodReply Goal::cancel(sdbus::MethodCall & call) { + bool success{true}; + std::string error_msg; + + switch (session.get_cancel_download()) { + case Session::CancelDownload::NOT_REQUESTED: + session.set_cancel_download(Session::CancelDownload::REQUESTED); + break; + case Session::CancelDownload::REQUESTED: + success = false; + error_msg = "Cancellation was already requested."; + break; + case Session::CancelDownload::NOT_ALLOWED: + success = false; + error_msg = "Cancellation is not allowed after the RPM transaction has started."; + break; + case Session::CancelDownload::NOT_RUNNING: + success = false; + error_msg = "No transaction is running."; + break; + } + + auto reply = call.createReply(); + reply << success; + reply << error_msg; + return reply; +} diff --git a/dnf5daemon-server/services/goal/goal.hpp b/dnf5daemon-server/services/goal/goal.hpp index f9f66c1fe..d19b6001e 100644 --- a/dnf5daemon-server/services/goal/goal.hpp +++ b/dnf5daemon-server/services/goal/goal.hpp @@ -36,6 +36,7 @@ class Goal : public IDbusSessionService { sdbus::MethodReply get_transaction_problems_string(sdbus::MethodCall & call); sdbus::MethodReply get_transaction_problems(sdbus::MethodCall & call); sdbus::MethodReply do_transaction(sdbus::MethodCall & call); + sdbus::MethodReply cancel(sdbus::MethodCall & call); }; #endif diff --git a/dnf5daemon-server/session.cpp b/dnf5daemon-server/session.cpp index 1869dd492..1a31530e0 100644 --- a/dnf5daemon-server/session.cpp +++ b/dnf5daemon-server/session.cpp @@ -313,6 +313,7 @@ void Session::store_transaction_offline() { std::filesystem::create_directories(dest_dir); base->get_config().get_destdir_option().set(dest_dir); download_transaction_packages(); + set_cancel_download(Session::CancelDownload::NOT_ALLOWED); const auto & offline_datadir = installroot / libdnf5::offline::DEFAULT_DATADIR.relative_path(); std::filesystem::create_directories(offline_datadir); diff --git a/dnf5daemon-server/session.hpp b/dnf5daemon-server/session.hpp index c0672877c..de46d5ed7 100644 --- a/dnf5daemon-server/session.hpp +++ b/dnf5daemon-server/session.hpp @@ -52,6 +52,7 @@ class IDbusSessionService { class Session { public: + enum class CancelDownload { NOT_RUNNING, NOT_REQUESTED, REQUESTED, NOT_ALLOWED }; Session( std::vector> && loggers, sdbus::IConnection & connection, @@ -93,6 +94,11 @@ class Session { /// prepare the current transaction to run during the next reboot void store_transaction_offline(); + /// Getter for download cancel request flag. + CancelDownload get_cancel_download() { return cancel_download.load(); } + /// Setter for download cancel request flag. + void set_cancel_download(CancelDownload value) { cancel_download.store(value); } + private: sdbus::IConnection & connection; std::unique_ptr base; @@ -110,6 +116,7 @@ class Session { std::mutex key_import_mutex; std::condition_variable key_import_condition; std::map key_import_status{}; // map key_id: confirmation status + std::atomic cancel_download{CancelDownload::NOT_RUNNING}; }; #endif