update_engine: Use GetDlcsToUpdate() instead of GetInstalled()
The meaning of GetInstalled() DBus in dlcservice have changed. So we need
to get the list of DLCs that ought to be updated from the new DBus
GetDlcsToUpdate().
Also rename all dlc_module_ids to dlc_ids.
BUG=chromium:1071654
TEST=cros_workon_make --board reef --test update_engine
Cq-Depend: chromium:2157669
Change-Id: I02e450a1fd75f8b387eb8a107c9c8a32f3e01e6e
Reviewed-on: https://chromium-review.googlesource.com/c/aosp/platform/system/update_engine/+/2163441
Tested-by: Amin Hassani <ahassani@chromium.org>
Reviewed-by: Andrew Lassalle <andrewlassalle@chromium.org>
Reviewed-by: Jae Hoon Kim <kimjae@chromium.org>
Commit-Queue: Amin Hassani <ahassani@chromium.org>
diff --git a/client_library/client_dbus.h b/client_library/client_dbus.h
index a032d21..74fcce3 100644
--- a/client_library/client_dbus.h
+++ b/client_library/client_dbus.h
@@ -44,7 +44,7 @@
bool at_user_request) override;
bool AttemptInstall(const std::string& omaha_url,
- const std::vector<std::string>& dlc_module_ids) override;
+ const std::vector<std::string>& dlc_ids) override;
bool SetDlcActiveValue(bool is_active, const std::string& dlc_id) override;
diff --git a/client_library/include/update_engine/client.h b/client_library/include/update_engine/client.h
index 9bda0b9..f734733 100644
--- a/client_library/include/update_engine/client.h
+++ b/client_library/include/update_engine/client.h
@@ -54,11 +54,10 @@
// empty indicates update_engine should use its default value. Note that
// update_engine will ignore this parameter in production mode to avoid
// pulling untrusted updates.
- // |dlc_module_ids|
+ // |dlc_ids|
// A list of DLC module IDs.
- virtual bool AttemptInstall(
- const std::string& omaha_url,
- const std::vector<std::string>& dlc_module_ids) = 0;
+ virtual bool AttemptInstall(const std::string& omaha_url,
+ const std::vector<std::string>& dlc_ids) = 0;
// Same as above but return the entire struct instead.
virtual bool GetStatus(UpdateEngineStatus* out_status) const = 0;
diff --git a/common/dlcservice_interface.h b/common/dlcservice_interface.h
index 3524d50..70b74ab 100644
--- a/common/dlcservice_interface.h
+++ b/common/dlcservice_interface.h
@@ -30,17 +30,17 @@
public:
virtual ~DlcServiceInterface() = default;
- // Returns true and a list of installed DLC module ids in |dlc_module_ids|.
+ // Returns true and a list of installed DLC ids in |dlc_ids|.
// On failure it returns false.
- virtual bool GetInstalled(std::vector<std::string>* dlc_module_ids) = 0;
+ virtual bool GetDlcsToUpdate(std::vector<std::string>* dlc_ids) = 0;
// Returns true if dlcservice successfully handled the install completion
// method call, otherwise false.
- virtual bool InstallCompleted(const std::vector<std::string>& ids) = 0;
+ virtual bool InstallCompleted(const std::vector<std::string>& dlc_ids) = 0;
// Returns true if dlcservice successfully handled the update completion
// method call, otherwise false.
- virtual bool UpdateCompleted(const std::vector<std::string>& ids) = 0;
+ virtual bool UpdateCompleted(const std::vector<std::string>& dlc_ids) = 0;
protected:
DlcServiceInterface() = default;
diff --git a/common/dlcservice_stub.cc b/common/dlcservice_stub.cc
index 3dcb2e0..2447147 100644
--- a/common/dlcservice_stub.cc
+++ b/common/dlcservice_stub.cc
@@ -27,16 +27,16 @@
return std::make_unique<DlcServiceStub>();
}
-bool DlcServiceStub::GetInstalled(std::vector<std::string>* dlc_module_ids) {
- if (dlc_module_ids)
- dlc_module_ids->clear();
+bool DlcServiceStub::GetDlcsToUpdate(vector<string>* dlc_ids) {
+ if (dlc_ids)
+ dlc_ids->clear();
return true;
}
-bool DlcServiceStub::InstallCompleted(const vector<string>& ids) {
+bool DlcServiceStub::InstallCompleted(const vector<string>& dlc_ids) {
return true;
}
-bool DlcServiceStub::UpdateCompleted(const vector<string>& ids) {
+bool DlcServiceStub::UpdateCompleted(const vector<string>& dlc_ids) {
return true;
}
diff --git a/common/dlcservice_stub.h b/common/dlcservice_stub.h
index 9b27971..bc803e8 100644
--- a/common/dlcservice_stub.h
+++ b/common/dlcservice_stub.h
@@ -31,9 +31,9 @@
~DlcServiceStub() = default;
// BootControlInterface overrides.
- bool GetInstalled(std::vector<std::string>* dlc_module_ids) override;
- bool InstallCompleted(const std::vector<std::string>& ids) override;
- bool UpdateCompleted(const std::vector<std::string>& ids) override;
+ bool GetDlcsToUpdate(std::vector<std::string>* dlc_ids) override;
+ bool InstallCompleted(const std::vector<std::string>& dlc_ids) override;
+ bool UpdateCompleted(const std::vector<std::string>& dlc_ids) override;
private:
DISALLOW_COPY_AND_ASSIGN(DlcServiceStub);
diff --git a/common_service.cc b/common_service.cc
index 347833b..85fb9e4 100644
--- a/common_service.cc
+++ b/common_service.cc
@@ -105,9 +105,8 @@
bool UpdateEngineService::AttemptInstall(brillo::ErrorPtr* error,
const string& omaha_url,
- const vector<string>& dlc_module_ids) {
- if (!system_state_->update_attempter()->CheckForInstall(dlc_module_ids,
- omaha_url)) {
+ const vector<string>& dlc_ids) {
+ if (!system_state_->update_attempter()->CheckForInstall(dlc_ids, omaha_url)) {
// TODO(xiaochu): support more detailed error messages.
LogAndSetError(error, FROM_HERE, "Could not schedule install operation.");
return false;
diff --git a/common_service.h b/common_service.h
index 6c742a5..cfcece5 100644
--- a/common_service.h
+++ b/common_service.h
@@ -55,10 +55,10 @@
// Attempts a DLC module install operation.
// |omaha_url|: the URL to query for update.
- // |dlc_module_ids|: a list of DLC module IDs.
+ // |dlc_ids|: a list of DLC module IDs.
bool AttemptInstall(brillo::ErrorPtr* error,
const std::string& omaha_url,
- const std::vector<std::string>& dlc_module_ids);
+ const std::vector<std::string>& dlc_ids);
bool AttemptRollback(brillo::ErrorPtr* error, bool in_powerwash);
diff --git a/dlcservice_chromeos.cc b/dlcservice_chromeos.cc
index 3c76b2a..08482ee 100644
--- a/dlcservice_chromeos.cc
+++ b/dlcservice_chromeos.cc
@@ -23,7 +23,6 @@
#include "update_engine/dbus_connection.h"
-using dlcservice::DlcModuleList;
using std::string;
using std::vector;
@@ -39,25 +38,25 @@
return std::make_unique<DlcServiceChromeOS>();
}
-bool DlcServiceChromeOS::GetInstalled(vector<string>* dlc_module_ids) {
- if (!dlc_module_ids)
+bool DlcServiceChromeOS::GetDlcsToUpdate(vector<string>* dlc_ids) {
+ if (!dlc_ids)
return false;
- dlc_module_ids->clear();
+ dlc_ids->clear();
- dlcservice::DlcModuleList dlc_module_list;
- if (!GetDlcServiceProxy().GetInstalled(&dlc_module_list, nullptr)) {
- LOG(ERROR) << "dlcservice does not return installed DLC module list.";
+ brillo::ErrorPtr err;
+ if (!GetDlcServiceProxy().GetDlcsToUpdate(dlc_ids, &err)) {
+ LOG(ERROR) << "dlcservice failed to return DLCs that need to be updated. "
+ << "ErrorCode=" << err->GetCode()
+ << ", ErrMsg=" << err->GetMessage();
+ dlc_ids->clear();
return false;
}
- for (const auto& dlc_module_info : dlc_module_list.dlc_module_infos()) {
- dlc_module_ids->emplace_back(dlc_module_info.dlc_id());
- }
return true;
}
-bool DlcServiceChromeOS::InstallCompleted(const std::vector<std::string>& ids) {
+bool DlcServiceChromeOS::InstallCompleted(const vector<string>& dlc_ids) {
brillo::ErrorPtr err;
- if (!GetDlcServiceProxy().InstallCompleted(ids, &err)) {
+ if (!GetDlcServiceProxy().InstallCompleted(dlc_ids, &err)) {
LOG(ERROR) << "dlcservice failed to complete install. ErrCode="
<< err->GetCode() << ", ErrMsg=" << err->GetMessage();
return false;
@@ -65,9 +64,9 @@
return true;
}
-bool DlcServiceChromeOS::UpdateCompleted(const std::vector<std::string>& ids) {
+bool DlcServiceChromeOS::UpdateCompleted(const vector<string>& dlc_ids) {
brillo::ErrorPtr err;
- if (!GetDlcServiceProxy().UpdateCompleted(ids, &err)) {
+ if (!GetDlcServiceProxy().UpdateCompleted(dlc_ids, &err)) {
LOG(ERROR) << "dlcservice failed to complete updated. ErrCode="
<< err->GetCode() << ", ErrMsg=" << err->GetMessage();
return false;
diff --git a/dlcservice_chromeos.h b/dlcservice_chromeos.h
index b56b495..8828e1a 100644
--- a/dlcservice_chromeos.h
+++ b/dlcservice_chromeos.h
@@ -34,17 +34,17 @@
// DlcServiceInterface overrides.
- // Will clear the |dlc_module_ids|, passed to be modified. Clearing by
- // default has the added benefit of avoiding indeterminate behavior in the
- // case that |dlc_module_ids| wasn't empty to begin which would lead to
- // possible duplicates and cases when error was not checked it's still safe.
- bool GetInstalled(std::vector<std::string>* dlc_module_ids) override;
+ // Will clear the |dlc_ids|, passed to be modified. Clearing by default has
+ // the added benefit of avoiding indeterminate behavior in the case that
+ // |dlc_ids| wasn't empty to begin which would lead to possible duplicates and
+ // cases when error was not checked it's still safe.
+ bool GetDlcsToUpdate(std::vector<std::string>* dlc_ids) override;
// Call into dlcservice for it to mark the DLC IDs as being installed.
- bool InstallCompleted(const std::vector<std::string>& ids) override;
+ bool InstallCompleted(const std::vector<std::string>& dlc_ids) override;
// Call into dlcservice for it to mark the DLC IDs as being updated.
- bool UpdateCompleted(const std::vector<std::string>& ids) override;
+ bool UpdateCompleted(const std::vector<std::string>& dlc_ids) override;
private:
DISALLOW_COPY_AND_ASSIGN(DlcServiceChromeOS);
diff --git a/mock_update_attempter.h b/mock_update_attempter.h
index 9d966d7..fdeba52 100644
--- a/mock_update_attempter.h
+++ b/mock_update_attempter.h
@@ -55,7 +55,7 @@
UpdateAttemptFlags flags));
MOCK_METHOD2(CheckForInstall,
- bool(const std::vector<std::string>& dlc_module_ids,
+ bool(const std::vector<std::string>& dlc_ids,
const std::string& omaha_url));
MOCK_METHOD2(SetDlcActiveValue, bool(bool, const std::string&));
diff --git a/omaha_request_builder_xml.cc b/omaha_request_builder_xml.cc
index e2bf307..097b9f1 100644
--- a/omaha_request_builder_xml.cc
+++ b/omaha_request_builder_xml.cc
@@ -421,13 +421,13 @@
app_xml += GetApp(system_app);
}
for (const auto& it : params_->dlc_apps_params()) {
- OmahaAppData dlc_module_app = {
+ OmahaAppData dlc_app_data = {
.id = it.first,
.version = params_->is_install() ? kNoVersion : params_->app_version(),
.skip_update = false,
.is_dlc = true,
.app_params = it.second};
- app_xml += GetApp(dlc_module_app);
+ app_xml += GetApp(dlc_app_data);
}
return app_xml;
}
diff --git a/omaha_request_builder_xml_unittest.cc b/omaha_request_builder_xml_unittest.cc
index 3cf5cc0..017acec 100644
--- a/omaha_request_builder_xml_unittest.cc
+++ b/omaha_request_builder_xml_unittest.cc
@@ -99,14 +99,14 @@
0,
fake_system_state_.prefs(),
""};
- OmahaAppData dlc_module_app = {.id = "XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX",
- .version = "",
- .skip_update = false,
- .is_dlc = false};
+ OmahaAppData dlc_app_data = {.id = "XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX",
+ .version = "",
+ .skip_update = false,
+ .is_dlc = false};
// Verify that the attributes that shouldn't be missing for Platform AppID are
// in fact present in the <app ...></app>.
- const string app = omaha_request.GetApp(dlc_module_app);
+ const string app = omaha_request.GetApp(dlc_app_data);
EXPECT_NE(string::npos, app.find("lang="));
EXPECT_NE(string::npos, app.find("fw_version="));
EXPECT_NE(string::npos, app.find("ec_version="));
@@ -125,12 +125,12 @@
0,
fake_system_state_.prefs(),
""};
- OmahaAppData dlc_module_app = {
+ OmahaAppData dlc_app_data = {
.id = "_dlc_id", .version = "", .skip_update = false, .is_dlc = true};
// Verify that the attributes that should be missing for DLC AppIDs are in
// fact not present in the <app ...></app>.
- const string app = omaha_request.GetApp(dlc_module_app);
+ const string app = omaha_request.GetApp(dlc_app_data);
EXPECT_EQ(string::npos, app.find("lang="));
EXPECT_EQ(string::npos, app.find("fw_version="));
EXPECT_EQ(string::npos, app.find("ec_version="));
diff --git a/update_attempter.cc b/update_attempter.cc
index c45fe4f..ae7f71e 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -717,10 +717,10 @@
}
void UpdateAttempter::CalculateDlcParams() {
- // Set the |dlc_module_ids_| only for an update. This is required to get the
+ // Set the |dlc_ids_| only for an update. This is required to get the
// currently installed DLC(s).
if (!is_install_ &&
- !system_state_->dlcservice()->GetInstalled(&dlc_module_ids_)) {
+ !system_state_->dlcservice()->GetDlcsToUpdate(&dlc_ids_)) {
LOG(INFO) << "Failed to retrieve DLC module IDs from dlcservice. Check the "
"state of dlcservice, will not update DLC modules.";
}
@@ -730,8 +730,7 @@
base::FileEnumerator dir_enum(metadata_root_path,
false /* recursive */,
base::FileEnumerator::DIRECTORIES);
- std::unordered_set<string> dlc_ids(dlc_module_ids_.begin(),
- dlc_module_ids_.end());
+ std::unordered_set<string> dlc_ids(dlc_ids_.begin(), dlc_ids_.end());
for (base::FilePath name = dir_enum.Next(); !name.empty();
name = dir_enum.Next()) {
string id = name.BaseName().value();
@@ -742,7 +741,7 @@
}
}
std::map<std::string, OmahaRequestParams::AppParams> dlc_apps_params;
- for (const auto& dlc_id : dlc_module_ids_) {
+ for (const auto& dlc_id : dlc_ids_) {
OmahaRequestParams::AppParams dlc_params{
.active_counting_type = OmahaRequestParams::kDateBased,
.name = dlc_id,
@@ -1012,7 +1011,7 @@
return true;
}
-bool UpdateAttempter::CheckForInstall(const vector<string>& dlc_module_ids,
+bool UpdateAttempter::CheckForInstall(const vector<string>& dlc_ids,
const string& omaha_url) {
if (status_ != UpdateStatus::IDLE) {
LOG(INFO) << "Refusing to do an install as there is an "
@@ -1021,7 +1020,7 @@
return false;
}
- dlc_module_ids_ = dlc_module_ids;
+ dlc_ids_ = dlc_ids;
is_install_ = true;
forced_omaha_url_.clear();
diff --git a/update_attempter.h b/update_attempter.h
index 3c6f4a1..9e48179 100644
--- a/update_attempter.h
+++ b/update_attempter.h
@@ -141,7 +141,7 @@
UpdateAttemptFlags flags);
// This is the version of CheckForUpdate called by AttemptInstall API.
- virtual bool CheckForInstall(const std::vector<std::string>& dlc_module_ids,
+ virtual bool CheckForInstall(const std::vector<std::string>& dlc_ids,
const std::string& omaha_url);
// This is the internal entry point for going through a rollback. This will
@@ -447,7 +447,7 @@
int64_t GetPingMetadata(const PrefsInterface& prefs,
const std::string& metadata_name) const;
- // Calculates the update parameters for DLCs. Sets the |dlc_modules_|
+ // Calculates the update parameters for DLCs. Sets the |dlc_ids_|
// parameter on the |omaha_request_params_| object.
void CalculateDlcParams();
@@ -555,7 +555,7 @@
std::string forced_omaha_url_;
// A list of DLC module IDs.
- std::vector<std::string> dlc_module_ids_;
+ std::vector<std::string> dlc_ids_;
// Whether the operation is install (write to the current slot not the
// inactive slot).
bool is_install_;
diff --git a/update_attempter_unittest.cc b/update_attempter_unittest.cc
index 5849c38..56665ad 100644
--- a/update_attempter_unittest.cc
+++ b/update_attempter_unittest.cc
@@ -153,7 +153,7 @@
class MockDlcService : public DlcServiceInterface {
public:
- MOCK_METHOD1(GetInstalled, bool(vector<string>*));
+ MOCK_METHOD1(GetDlcsToUpdate, bool(vector<string>*));
MOCK_METHOD1(InstallCompleted, bool(const vector<string>&));
MOCK_METHOD1(UpdateCompleted, bool(const vector<string>&));
};
@@ -2360,7 +2360,7 @@
ASSERT_TRUE(base::CreateDirectory(metadata_path_dlc0));
attempter_.is_install_ = true;
- attempter_.dlc_module_ids_ = {dlc_id};
+ attempter_.dlc_ids_ = {dlc_id};
attempter_.CalculateDlcParams();
OmahaRequestParams* params = fake_system_state_.request_params();
@@ -2387,7 +2387,7 @@
base::FilePath(fake_system_state_.request_params()->dlc_prefs_root())
.Append(dlc_id);
ASSERT_TRUE(base::CreateDirectory(metadata_path_dlc0));
- EXPECT_CALL(mock_dlcservice_, GetInstalled(_))
+ EXPECT_CALL(mock_dlcservice_, GetDlcsToUpdate(_))
.WillOnce(
DoAll(SetArgPointee<0>(std::vector<string>({dlc_id})), Return(true)));
@@ -2416,7 +2416,7 @@
base::FilePath(fake_system_state_.request_params()->dlc_prefs_root())
.Append(dlc_id);
ASSERT_TRUE(base::CreateDirectory(metadata_path_dlc0));
- EXPECT_CALL(mock_dlcservice_, GetInstalled(_))
+ EXPECT_CALL(mock_dlcservice_, GetDlcsToUpdate(_))
.WillOnce(
DoAll(SetArgPointee<0>(std::vector<string>({dlc_id})), Return(true)));
@@ -2449,7 +2449,7 @@
base::FilePath(fake_system_state_.request_params()->dlc_prefs_root())
.Append(dlc_id);
ASSERT_TRUE(base::CreateDirectory(metadata_path_dlc0));
- EXPECT_CALL(mock_dlcservice_, GetInstalled(_))
+ EXPECT_CALL(mock_dlcservice_, GetDlcsToUpdate(_))
.WillOnce(
DoAll(SetArgPointee<0>(std::vector<string>({dlc_id})), Return(true)));
@@ -2485,7 +2485,7 @@
base::FilePath(fake_system_state_.request_params()->dlc_prefs_root())
.Append("stale");
ASSERT_TRUE(base::CreateDirectory(metadata_path_dlc_stale));
- EXPECT_CALL(mock_dlcservice_, GetInstalled(_))
+ EXPECT_CALL(mock_dlcservice_, GetDlcsToUpdate(_))
.WillOnce(
DoAll(SetArgPointee<0>(std::vector<string>({dlc_id})), Return(true)));