update_engine: Add newer DBus method and signal for GetStatus
The current GetStatus function is pretty non-extendable and there has
been use cases where we wanted to add arguments to it but it was quite
hard to do specially changes in Chrome.
This CL adds a new DBus Method GetStatusAdvanced and Signal
UpdateStatusAdvanced which use a protobuf for communicating dbus
messages. This allows us to extend the protobuf without much effort in
the future.
BUG=chromium:977320
TEST=unittests, precq
Cq-Depend: chromium:1672684, chrome-internal:1424559
Change-Id: Ia93ed189e7561ca18c63b5ded81826bc9b1cff12
Reviewed-on: https://chromium-review.googlesource.com/1669974
Tested-by: Amin Hassani <ahassani@chromium.org>
Commit-Ready: Amin Hassani <ahassani@chromium.org>
Legacy-Commit-Queue: Commit Bot <commit-bot@chromium.org>
Reviewed-by: Amin Hassani <ahassani@chromium.org>
diff --git a/UpdateEngine.conf b/UpdateEngine.conf
index 9490096..42f73fc 100644
--- a/UpdateEngine.conf
+++ b/UpdateEngine.conf
@@ -1,5 +1,20 @@
<!DOCTYPE busconfig PUBLIC "-//freedesktop//DTD D-BUS Bus Configuration 1.0//EN"
"http://www.freedesktop.org/standards/dbus/1.0/busconfig.dtd">
+<!--
+ Copyright (C) 2019 The Android Open Source Project
+
+ Licensed under the Apache License, Version 2.0 (the "License");
+ you may not use this file except in compliance with the License.
+ You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+!-->
<busconfig>
<policy user="root">
<allow own="org.chromium.UpdateEngine" />
@@ -29,6 +44,9 @@
send_member="GetStatus"/>
<allow send_destination="org.chromium.UpdateEngine"
send_interface="org.chromium.UpdateEngineInterface"
+ send_member="GetStatusAdvanced"/>
+ <allow send_destination="org.chromium.UpdateEngine"
+ send_interface="org.chromium.UpdateEngineInterface"
send_member="RebootIfNeeded"/>
<allow send_destination="org.chromium.UpdateEngine"
send_interface="org.chromium.UpdateEngineInterface"
@@ -75,6 +93,9 @@
<allow send_destination="org.chromium.UpdateEngine"
send_interface="org.chromium.UpdateEngineInterface"
send_member="GetStatus"/>
+ <allow send_destination="org.chromium.UpdateEngine"
+ send_interface="org.chromium.UpdateEngineInterface"
+ send_member="GetStatusAdvanced"/>
</policy>
<policy user="dlcservice">
<allow send_destination="org.chromium.UpdateEngine"
@@ -82,6 +103,9 @@
send_member="GetStatus"/>
<allow send_destination="org.chromium.UpdateEngine"
send_interface="org.chromium.UpdateEngineInterface"
+ send_member="GetStatusAdvanced"/>
+ <allow send_destination="org.chromium.UpdateEngine"
+ send_interface="org.chromium.UpdateEngineInterface"
send_member="AttemptInstall"/>
</policy>
</busconfig>
diff --git a/client_library/client_dbus.cc b/client_library/client_dbus.cc
index 809ad13..48a563b 100644
--- a/client_library/client_dbus.cc
+++ b/client_library/client_dbus.cc
@@ -45,6 +45,19 @@
namespace internal {
+namespace {
+// This converts the status from Protobuf |StatusResult| to The internal
+// |UpdateEngineStatus| struct.
+bool ConvertToUpdateEngineStatus(const StatusResult& status,
+ UpdateEngineStatus* out_status) {
+ out_status->last_checked_time = status.last_checked_time();
+ out_status->progress = status.progress();
+ out_status->new_version = status.new_version();
+ out_status->new_size_bytes = status.new_size();
+ return StringToUpdateStatus(status.current_operation(), &out_status->status);
+}
+} // namespace
+
bool DBusUpdateEngineClient::Init() {
Bus::Options options;
options.bus_type = Bus::SYSTEM;
@@ -93,18 +106,25 @@
UpdateStatus* out_update_status,
string* out_new_version,
int64_t* out_new_size) const {
- string status_as_string;
- const bool success = proxy_->GetStatus(out_last_checked_time,
- out_progress,
- &status_as_string,
- out_new_version,
- out_new_size,
- nullptr);
- if (!success) {
+ StatusResult status;
+ if (!proxy_->GetStatusAdvanced(&status, nullptr)) {
return false;
}
- return StringToUpdateStatus(status_as_string, out_update_status);
+ *out_last_checked_time = status.last_checked_time();
+ *out_progress = status.progress();
+ *out_new_version = status.new_version();
+ *out_new_size = status.new_size();
+ return StringToUpdateStatus(status.current_operation(), out_update_status);
+}
+
+bool DBusUpdateEngineClient::GetStatus(UpdateEngineStatus* out_status) const {
+ StatusResult status;
+ if (!proxy_->GetStatusAdvanced(&status, nullptr)) {
+ return false;
+ }
+
+ return ConvertToUpdateEngineStatus(status, out_status);
}
bool DBusUpdateEngineClient::SetCohortHint(const string& cohort_hint) {
@@ -173,40 +193,25 @@
void DBusUpdateEngineClient::StatusUpdateHandlersRegistered(
StatusUpdateHandler* handler) const {
- int64_t last_checked_time;
- double progress;
- UpdateStatus update_status;
- string new_version;
- int64_t new_size;
-
- if (!GetStatus(&last_checked_time,
- &progress,
- &update_status,
- &new_version,
- &new_size)) {
+ UpdateEngineStatus status;
+ if (!GetStatus(&status)) {
handler->IPCError("Could not query current status");
return;
}
std::vector<update_engine::StatusUpdateHandler*> just_handler = {handler};
for (auto h : handler ? just_handler : handlers_) {
- h->HandleStatusUpdate(
- last_checked_time, progress, update_status, new_version, new_size);
+ h->HandleStatusUpdate(status);
}
}
void DBusUpdateEngineClient::RunStatusUpdateHandlers(
- int64_t last_checked_time,
- double progress,
- const string& current_operation,
- const string& new_version,
- int64_t new_size) {
- UpdateStatus status;
- StringToUpdateStatus(current_operation, &status);
+ const StatusResult& status) {
+ UpdateEngineStatus ue_status;
+ ConvertToUpdateEngineStatus(status, &ue_status);
for (auto handler : handlers_) {
- handler->HandleStatusUpdate(
- last_checked_time, progress, status, new_version, new_size);
+ handler->HandleStatusUpdate(ue_status);
}
}
@@ -235,7 +240,7 @@
return true;
}
- proxy_->RegisterStatusUpdateSignalHandler(
+ proxy_->RegisterStatusUpdateAdvancedSignalHandler(
base::Bind(&DBusUpdateEngineClient::RunStatusUpdateHandlers,
base::Unretained(this)),
base::Bind(&DBusUpdateEngineClient::DBusStatusHandlersRegistered,
diff --git a/client_library/client_dbus.h b/client_library/client_dbus.h
index a186d45..1b127e3 100644
--- a/client_library/client_dbus.h
+++ b/client_library/client_dbus.h
@@ -23,6 +23,7 @@
#include <vector>
#include <base/macros.h>
+#include <update_engine/proto_bindings/update_engine.pb.h>
#include "update_engine/client_library/include/update_engine/client.h"
#include "update_engine/dbus-proxies.h"
@@ -50,6 +51,8 @@
std::string* out_new_version,
int64_t* out_new_size) const override;
+ bool GetStatus(UpdateEngineStatus* out_status) const override;
+
bool SetCohortHint(const std::string& cohort_hint) override;
bool GetCohortHint(std::string* cohort_hint) const override;
@@ -93,11 +96,7 @@
// registered handlers receive the event.
void StatusUpdateHandlersRegistered(StatusUpdateHandler* handler) const;
- void RunStatusUpdateHandlers(int64_t last_checked_time,
- double progress,
- const std::string& current_operation,
- const std::string& new_version,
- int64_t new_size);
+ void RunStatusUpdateHandlers(const StatusResult& status);
std::unique_ptr<org::chromium::UpdateEngineInterfaceProxy> proxy_;
std::vector<update_engine::StatusUpdateHandler*> handlers_;
diff --git a/client_library/include/update_engine/client.h b/client_library/include/update_engine/client.h
index 1bc6111..89f36af 100644
--- a/client_library/include/update_engine/client.h
+++ b/client_library/include/update_engine/client.h
@@ -80,6 +80,9 @@
std::string* out_new_version,
int64_t* out_new_size) const = 0;
+ // Same as above but return the entire struct instead.
+ virtual bool GetStatus(UpdateEngineStatus* out_status) const = 0;
+
// Getter and setter for the cohort hint.
virtual bool SetCohortHint(const std::string& cohort_hint) = 0;
virtual bool GetCohortHint(std::string* cohort_hint) const = 0;
diff --git a/client_library/include/update_engine/status_update_handler.h b/client_library/include/update_engine/status_update_handler.h
index d2fad34..238f6bd 100644
--- a/client_library/include/update_engine/status_update_handler.h
+++ b/client_library/include/update_engine/status_update_handler.h
@@ -14,7 +14,9 @@
// limitations under the License.
//
+// NOLINTNEXTLINE(whitespace/line_length)
#ifndef UPDATE_ENGINE_CLIENT_LIBRARY_INCLUDE_UPDATE_ENGINE_STATUS_UPDATE_HANDLER_H_
+// NOLINTNEXTLINE(whitespace/line_length)
#define UPDATE_ENGINE_CLIENT_LIBRARY_INCLUDE_UPDATE_ENGINE_STATUS_UPDATE_HANDLER_H_
#include <string>
@@ -35,13 +37,10 @@
virtual void IPCError(const std::string& error) = 0;
// Runs every time update_engine reports a status change.
- virtual void HandleStatusUpdate(int64_t last_checked_time,
- double progress,
- UpdateStatus current_operation,
- const std::string& new_version,
- int64_t new_size) = 0;
+ virtual void HandleStatusUpdate(const UpdateEngineStatus& status) = 0;
};
} // namespace update_engine
+// NOLINTNEXTLINE(whitespace/line_length)
#endif // UPDATE_ENGINE_CLIENT_LIBRARY_INCLUDE_UPDATE_ENGINE_STATUS_UPDATE_HANDLER_H_
diff --git a/dbus_bindings/org.chromium.UpdateEngineInterface.dbus-xml b/dbus_bindings/org.chromium.UpdateEngineInterface.dbus-xml
index f81d4ed..ef7bea7 100644
--- a/dbus_bindings/org.chromium.UpdateEngineInterface.dbus-xml
+++ b/dbus_bindings/org.chromium.UpdateEngineInterface.dbus-xml
@@ -1,4 +1,19 @@
<?xml version="1.0" encoding="utf-8" ?>
+<!--
+ Copyright (C) 2019 The Android Open Source Project
+
+ Licensed under the Apache License, Version 2.0 (the "License");
+ you may not use this file except in compliance with the License.
+ You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+!-->
<node name="/org/chromium/UpdateEngine">
<interface name="org.chromium.UpdateEngineInterface">
<annotation name="org.freedesktop.DBus.GLib.CSymbol"
@@ -31,12 +46,22 @@
<method name="ResetStatus">
</method>
<method name="GetStatus">
+ <!-- TODO(crbug.com/977320): Deprecate this method. -->
<arg type="x" name="last_checked_time" direction="out" />
<arg type="d" name="progress" direction="out" />
<arg type="s" name="current_operation" direction="out" />
<arg type="s" name="new_version" direction="out" />
<arg type="x" name="new_size" direction="out" />
</method>
+ <method name="GetStatusAdvanced">
+ <arg type="ay" name="status" direction="out">
+ <tp:docstring>
+ The current status serialized in a protobuf.
+ </tp:docstring>
+ <annotation name="org.chromium.DBus.Argument.ProtobufClass"
+ value="update_engine::StatusResult"/>
+ </arg>
+ </method>
<method name="RebootIfNeeded">
</method>
<method name="SetChannel">
@@ -81,12 +106,22 @@
<arg type="x" name="usec_wallclock" direction="out" />
</method>
<signal name="StatusUpdate">
+ <!-- TODO(crbug.com/977320): Deprecate this method. -->
<arg type="x" name="last_checked_time" />
<arg type="d" name="progress" />
<arg type="s" name="current_operation" />
<arg type="s" name="new_version" />
<arg type="x" name="new_size" />
</signal>
+ <signal name="StatusUpdateAdvanced">
+ <arg type="ay" name="status" direction="out">
+ <tp:docstring>
+ The current status serialized in a protobuf.
+ </tp:docstring>
+ <annotation name="org.chromium.DBus.Argument.ProtobufClass"
+ value="update_engine::StatusResult"/>
+ </arg>
+ </signal>
<method name="GetPrevVersion">
<arg type="s" name="prev_version" direction="out" />
</method>
diff --git a/dbus_service.cc b/dbus_service.cc
index 2a5662f..105d581 100644
--- a/dbus_service.cc
+++ b/dbus_service.cc
@@ -23,6 +23,7 @@
#include <update_engine/dbus-constants.h>
#include "update_engine/dbus_connection.h"
+#include "update_engine/proto_bindings/update_engine.pb.h"
#include "update_engine/update_status_utils.h"
namespace chromeos_update_engine {
@@ -31,8 +32,21 @@
using chromeos_update_engine::UpdateEngineService;
using std::string;
using std::vector;
+using update_engine::StatusResult;
using update_engine::UpdateEngineStatus;
+namespace {
+// Converts the internal |UpdateEngineStatus| to the protobuf |StatusResult|.
+void ConvertToStatusResult(const UpdateEngineStatus& ue_status,
+ StatusResult* out_status) {
+ out_status->set_last_checked_time(ue_status.last_checked_time);
+ out_status->set_progress(ue_status.progress);
+ out_status->set_current_operation(UpdateStatusToString(ue_status.status));
+ out_status->set_new_version(ue_status.new_version);
+ out_status->set_new_size(ue_status.new_size_bytes);
+}
+} // namespace
+
DBusUpdateEngineService::DBusUpdateEngineService(SystemState* system_state)
: common_(new UpdateEngineService{system_state}) {}
@@ -116,6 +130,17 @@
return true;
}
+bool DBusUpdateEngineService::GetStatusAdvanced(ErrorPtr* error,
+ StatusResult* out_status) {
+ UpdateEngineStatus status;
+ if (!common_->GetStatus(error, &status)) {
+ return false;
+ }
+
+ ConvertToStatusResult(status, out_status);
+ return true;
+}
+
bool DBusUpdateEngineService::RebootIfNeeded(ErrorPtr* error) {
return common_->RebootIfNeeded(error);
}
@@ -216,11 +241,18 @@
void UpdateEngineAdaptor::SendStatusUpdate(
const UpdateEngineStatus& update_engine_status) {
- SendStatusUpdateSignal(update_engine_status.last_checked_time,
- update_engine_status.progress,
- UpdateStatusToString(update_engine_status.status),
- update_engine_status.new_version,
- update_engine_status.new_size_bytes);
+ StatusResult status;
+ ConvertToStatusResult(update_engine_status, &status);
+
+ // TODO(crbug.com/977320): Deprecate |StatusUpdate| signal.
+ SendStatusUpdateSignal(status.last_checked_time(),
+ status.progress(),
+ status.current_operation(),
+ status.new_version(),
+ status.new_size());
+
+ // Send |StatusUpdateAdvanced| signal.
+ SendStatusUpdateAdvancedSignal(status);
}
} // namespace chromeos_update_engine
diff --git a/dbus_service.h b/dbus_service.h
index 134461b..71a6d2b 100644
--- a/dbus_service.h
+++ b/dbus_service.h
@@ -24,6 +24,7 @@
#include <base/memory/ref_counted.h>
#include <brillo/errors/error.h>
+#include <update_engine/proto_bindings/update_engine.pb.h>
#include "update_engine/common_service.h"
#include "update_engine/service_observer_interface.h"
@@ -72,6 +73,11 @@
std::string* out_new_version,
int64_t* out_new_size) override;
+ // Similar to Above, but returns a protobuffer instead. In the future it will
+ // have more features and is easily extendable.
+ bool GetStatusAdvanced(brillo::ErrorPtr* error,
+ update_engine::StatusResult* out_status) override;
+
// Reboots the device if an update is applied and a reboot is required.
bool RebootIfNeeded(brillo::ErrorPtr* error) override;
diff --git a/update_engine_client.cc b/update_engine_client.cc
index d1b2267..1b680d1 100644
--- a/update_engine_client.cc
+++ b/update_engine_client.cc
@@ -46,6 +46,7 @@
using std::string;
using std::unique_ptr;
using std::vector;
+using update_engine::UpdateEngineStatus;
using update_engine::UpdateStatus;
namespace {
@@ -132,42 +133,24 @@
public:
~WatchingStatusUpdateHandler() override = default;
- void HandleStatusUpdate(int64_t last_checked_time,
- double progress,
- UpdateStatus current_operation,
- const string& new_version,
- int64_t new_size) override;
+ void HandleStatusUpdate(const UpdateEngineStatus& status) override;
};
void WatchingStatusUpdateHandler::HandleStatusUpdate(
- int64_t last_checked_time,
- double progress,
- UpdateStatus current_operation,
- const string& new_version,
- int64_t new_size) {
+ const UpdateEngineStatus& status) {
LOG(INFO) << "Got status update:";
- LOG(INFO) << " last_checked_time: " << last_checked_time;
- LOG(INFO) << " progress: " << progress;
- LOG(INFO) << " current_operation: "
- << UpdateStatusToString(current_operation);
- LOG(INFO) << " new_version: " << new_version;
- LOG(INFO) << " new_size: " << new_size;
+ LOG(INFO) << " last_checked_time: " << status.last_checked_time;
+ LOG(INFO) << " progress: " << status.progress;
+ LOG(INFO) << " current_operation: " << UpdateStatusToString(status.status);
+ LOG(INFO) << " new_version: " << status.new_version;
+ LOG(INFO) << " new_size: " << status.new_size_bytes;
}
bool UpdateEngineClient::ShowStatus() {
- int64_t last_checked_time = 0;
- double progress = 0.0;
- UpdateStatus current_op;
- string new_version;
- int64_t new_size = 0;
-
+ UpdateEngineStatus status;
int retry_count = kShowStatusRetryCount;
while (retry_count > 0) {
- if (client_->GetStatus(&last_checked_time,
- &progress,
- ¤t_op,
- &new_version,
- &new_size)) {
+ if (client_->GetStatus(&status)) {
break;
}
if (--retry_count == 0) {
@@ -181,31 +164,22 @@
printf("LAST_CHECKED_TIME=%" PRIi64
"\nPROGRESS=%f\nCURRENT_OP=%s\n"
"NEW_VERSION=%s\nNEW_SIZE=%" PRIi64 "\n",
- last_checked_time,
- progress,
- UpdateStatusToString(current_op),
- new_version.c_str(),
- new_size);
+ status.last_checked_time,
+ status.progress,
+ UpdateStatusToString(status.status),
+ status.new_version.c_str(),
+ status.new_size_bytes);
return true;
}
int UpdateEngineClient::GetNeedReboot() {
- int64_t last_checked_time = 0;
- double progress = 0.0;
- UpdateStatus current_op;
- string new_version;
- int64_t new_size = 0;
-
- if (!client_->GetStatus(&last_checked_time,
- &progress,
- ¤t_op,
- &new_version,
- &new_size)) {
+ UpdateEngineStatus status;
+ if (!client_->GetStatus(&status)) {
return 1;
}
- if (current_op == UpdateStatus::UPDATED_NEED_REBOOT) {
+ if (status.status == UpdateStatus::UPDATED_NEED_REBOOT) {
return 0;
}
@@ -220,35 +194,26 @@
~UpdateWaitHandler() override = default;
- void HandleStatusUpdate(int64_t last_checked_time,
- double progress,
- UpdateStatus current_operation,
- const string& new_version,
- int64_t new_size) override;
+ void HandleStatusUpdate(const UpdateEngineStatus& status) override;
private:
bool exit_on_error_;
update_engine::UpdateEngineClient* client_;
};
-void UpdateWaitHandler::HandleStatusUpdate(int64_t /* last_checked_time */,
- double /* progress */,
- UpdateStatus current_operation,
- const string& /* new_version */,
- int64_t /* new_size */) {
- if (exit_on_error_ && current_operation == UpdateStatus::IDLE) {
+void UpdateWaitHandler::HandleStatusUpdate(const UpdateEngineStatus& status) {
+ if (exit_on_error_ && status.status == UpdateStatus::IDLE) {
int last_attempt_error;
ErrorCode code = ErrorCode::kSuccess;
if (client_ && client_->GetLastAttemptError(&last_attempt_error))
code = static_cast<ErrorCode>(last_attempt_error);
LOG(ERROR) << "Update failed, current operation is "
- << UpdateStatusToString(current_operation)
- << ", last error code is " << ErrorCodeToString(code) << "("
- << last_attempt_error << ")";
+ << UpdateStatusToString(status.status) << ", last error code is "
+ << ErrorCodeToString(code) << "(" << last_attempt_error << ")";
exit(1);
}
- if (current_operation == UpdateStatus::UPDATED_NEED_REBOOT) {
+ if (status.status == UpdateStatus::UPDATED_NEED_REBOOT) {
LOG(INFO) << "Update succeeded -- reboot needed.";
exit(0);
}