update_engine: Refactor OperationsGenerator into a base class.
This refactor cleans up the interface of the algorithms that generate
the list of rootfs and kernel operations removing the mention of a
Graph from it. The Graph class is only used by the in-place generator
because it requires to keep track of dependencies between operations
reading or writting the same block. The full update generator, using
only REPLACE or REPLACE_BZ doesn't need to use a graph to do that, but
in order to reuse some code, the interface was hacked that way.
This patch now uses two vectors of "AnnotatedOperations", which are
a mere InstallOperation as defined by the .proto file plus a name
used for logging purposes only. Both rootfs and kernel operations
have now the same type on the interface, allowing to share common
functions handling those.
BUG=chromium:331965
TEST=FEATURES=test emerge-link update_engine
Change-Id: I78566bbecb948634b7ecc8d086766ce67a79b43e
Reviewed-on: https://chromium-review.googlesource.com/262281
Reviewed-by: Alex Vakulenko <avakulenko@chromium.org>
Reviewed-by: Don Garrett <dgarrett@chromium.org>
Commit-Queue: Alex Deymo <deymo@chromium.org>
Trybot-Ready: Alex Deymo <deymo@chromium.org>
Tested-by: Alex Deymo <deymo@chromium.org>
diff --git a/update_engine/delta_performer_unittest.cc b/update_engine/delta_performer_unittest.cc
index fc2ac7e..e0bd95d 100644
--- a/update_engine/delta_performer_unittest.cc
+++ b/update_engine/delta_performer_unittest.cc
@@ -525,7 +525,7 @@
EXPECT_TRUE(payload_config.Validate());
EXPECT_TRUE(
- DeltaDiffGenerator::GenerateDeltaUpdateFile(
+ GenerateUpdatePayloadFile(
payload_config,
state->delta_path,
private_key,
diff --git a/update_engine/payload_generator/annotated_operation.cc b/update_engine/payload_generator/annotated_operation.cc
new file mode 100644
index 0000000..0e73479
--- /dev/null
+++ b/update_engine/payload_generator/annotated_operation.cc
@@ -0,0 +1,80 @@
+// Copyright 2015 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "update_engine/payload_generator/annotated_operation.h"
+
+#include <base/format_macros.h>
+#include <base/strings/string_number_conversions.h>
+#include <base/strings/stringprintf.h>
+
+using std::string;
+
+namespace chromeos_update_engine {
+
+namespace {
+// Output the list of extents as (start_block, num_blocks) in the passed output
+// stream.
+void OutputExtents(std::ostream* os,
+ const google::protobuf::RepeatedPtrField<Extent>& extents) {
+ for (const auto& extent : extents) {
+ *os << " (" << extent.start_block() << ", " << extent.num_blocks() << ")";
+ }
+}
+} // namespace
+
+void AnnotatedOperation::SetNameFromFileAndChunk(
+ const string& filename, off_t chunk_offset, off_t chunk_size) {
+ name = filename;
+ if (chunk_offset != 0 || chunk_size != -1) {
+ if (chunk_size != -1) {
+ base::StringAppendF(&name, " [%" PRId64 ", %" PRId64 ")",
+ chunk_offset, chunk_offset + chunk_size);
+ } else {
+ base::StringAppendF(&name, " [%" PRId64 ", end)", chunk_offset);
+ }
+ }
+}
+
+string InstallOperationTypeName(
+ DeltaArchiveManifest_InstallOperation_Type op_type) {
+ switch (op_type) {
+ case DeltaArchiveManifest_InstallOperation_Type_BSDIFF:
+ return "BSDIFF";
+ case DeltaArchiveManifest_InstallOperation_Type_MOVE:
+ return "MOVE";
+ case DeltaArchiveManifest_InstallOperation_Type_REPLACE:
+ return "REPLACE";
+ case DeltaArchiveManifest_InstallOperation_Type_REPLACE_BZ:
+ return "REPLACE_BZ";
+ case DeltaArchiveManifest_InstallOperation_Type_SOURCE_COPY:
+ return "SOURCE_COPY";
+ case DeltaArchiveManifest_InstallOperation_Type_SOURCE_BSDIFF:
+ return "SOURCE_BSDIFF";
+ }
+ return "UNK";
+}
+
+std::ostream& operator<<(std::ostream& os, const AnnotatedOperation& aop) {
+ // For example, this prints:
+ // REPLACE_BZ 500 @3000
+ // name: /foo/bar
+ // dst: (123, 3) (127, 2)
+ os << InstallOperationTypeName(aop.op.type()) << " " << aop.op.data_length();
+ if (aop.op.data_length() > 0)
+ os << " @" << aop.op.data_offset();
+ if (!aop.name.empty()) {
+ os << std::endl << " name: " << aop.name;
+ }
+ if (aop.op.src_extents_size() != 0) {
+ os << std::endl << " src:";
+ OutputExtents(&os, aop.op.src_extents());
+ }
+ if (aop.op.dst_extents_size() != 0) {
+ os << std::endl << " dst:";
+ OutputExtents(&os, aop.op.dst_extents());
+ }
+ return os;
+}
+
+} // namespace chromeos_update_engine
diff --git a/update_engine/payload_generator/annotated_operation.h b/update_engine/payload_generator/annotated_operation.h
new file mode 100644
index 0000000..e370cfe
--- /dev/null
+++ b/update_engine/payload_generator/annotated_operation.h
@@ -0,0 +1,36 @@
+// Copyright 2015 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef UPDATE_ENGINE_PAYLOAD_GENERATOR_ANNOTATED_OPERATION_H_
+#define UPDATE_ENGINE_PAYLOAD_GENERATOR_ANNOTATED_OPERATION_H_
+
+#include <ostream> // NOLINT(readability/streams)
+#include <string>
+
+#include "update_engine/update_metadata.pb.h"
+
+namespace chromeos_update_engine {
+
+struct AnnotatedOperation {
+ // The name given to the operation, for logging and debugging purposes only.
+ // This normally includes the path to the file and the chunk used, if any.
+ std::string name;
+
+ // The InstallOperation, as defined by the protobuf.
+ DeltaArchiveManifest_InstallOperation op;
+
+ // Sets |name| to a human readable representation of a chunk in a file.
+ void SetNameFromFileAndChunk(const std::string& filename,
+ off_t chunk_offset, off_t chunk_size);
+};
+
+// For logging purposes.
+std::ostream& operator<<(std::ostream& os, const AnnotatedOperation& aop);
+
+std::string InstallOperationTypeName(
+ DeltaArchiveManifest_InstallOperation_Type op_type);
+
+} // namespace chromeos_update_engine
+
+#endif // UPDATE_ENGINE_PAYLOAD_GENERATOR_ANNOTATED_OPERATION_H_
diff --git a/update_engine/payload_generator/delta_diff_generator.cc b/update_engine/payload_generator/delta_diff_generator.cc
index 6527bff..725442a 100644
--- a/update_engine/payload_generator/delta_diff_generator.cc
+++ b/update_engine/payload_generator/delta_diff_generator.cc
@@ -21,7 +21,6 @@
#include <base/files/file_util.h>
#include <base/logging.h>
#include <base/strings/stringprintf.h>
-#include <base/strings/string_number_conversions.h>
#include <base/strings/string_util.h>
#include <bzlib.h>
@@ -114,47 +113,28 @@
return true;
}
-// Adds each operation from |graph| to |out_manifest| in the order specified by
-// |order| while building |out_op_name_map| with operation to name
-// mappings. Adds all |kernel_ops| to |out_manifest|. Filters out no-op
-// operations.
+// Adds each operation from |rootfs_ops| and |kernel_ops| to |out_manifest| in
+// the order they come in those vectors. reports the operations names
void InstallOperationsToManifest(
- const Graph& graph,
- const vector<Vertex::Index>& order,
- const vector<DeltaArchiveManifest_InstallOperation>& kernel_ops,
+ const vector<AnnotatedOperation>& rootfs_ops,
+ const vector<AnnotatedOperation>& kernel_ops,
DeltaArchiveManifest* out_manifest,
OperationNameMap* out_op_name_map) {
- for (Vertex::Index vertex_index : order) {
- const Vertex& vertex = graph[vertex_index];
- const DeltaArchiveManifest_InstallOperation& add_op = vertex.op;
- if (DeltaDiffGenerator::IsNoopOperation(add_op)) {
+ for (const AnnotatedOperation& aop : rootfs_ops) {
+ if (DeltaDiffGenerator::IsNoopOperation(aop.op))
continue;
- }
- DeltaArchiveManifest_InstallOperation* op =
+ DeltaArchiveManifest_InstallOperation* new_op =
out_manifest->add_install_operations();
- *op = add_op;
- string name = vertex.file_name;
- if (vertex.chunk_offset || vertex.chunk_size != -1) {
- string offset = base::Int64ToString(vertex.chunk_offset);
- if (vertex.chunk_size != -1) {
- name += " [" + offset + ", " +
- base::Int64ToString(vertex.chunk_offset + vertex.chunk_size - 1) +
- "]";
- } else {
- name += " [" + offset + ", end]";
- }
- }
- (*out_op_name_map)[op] = name;
+ (*out_op_name_map)[new_op] = aop.name;
+ *new_op = aop.op;
}
- for (vector<DeltaArchiveManifest_InstallOperation>::const_iterator it =
- kernel_ops.begin(); it != kernel_ops.end(); ++it) {
- const DeltaArchiveManifest_InstallOperation& add_op = *it;
- if (DeltaDiffGenerator::IsNoopOperation(add_op)) {
+ for (const AnnotatedOperation& aop : kernel_ops) {
+ if (DeltaDiffGenerator::IsNoopOperation(aop.op))
continue;
- }
- DeltaArchiveManifest_InstallOperation* op =
+ DeltaArchiveManifest_InstallOperation* new_op =
out_manifest->add_kernel_install_operations();
- *op = add_op;
+ (*out_op_name_map)[new_op] = aop.name;
+ *new_op = aop.op;
}
}
@@ -317,12 +297,6 @@
} // namespace
-void DeltaDiffGenerator::CheckGraph(const Graph& graph) {
- for (const Vertex& v : graph) {
- CHECK(v.op.has_type());
- }
-}
-
bool DeltaDiffGenerator::DeltaReadFiles(Graph* graph,
vector<Block>* blocks,
const string& old_root,
@@ -634,7 +608,7 @@
bool DeltaDiffGenerator::DeltaCompressKernelPartition(
const string& old_kernel_part,
const string& new_kernel_part,
- vector<DeltaArchiveManifest_InstallOperation>* ops,
+ vector<AnnotatedOperation>* kernel_ops,
int blobs_fd,
off_t* blobs_length,
bool src_ops_allowed) {
@@ -673,8 +647,10 @@
}
// Add the new install operation.
- ops->clear();
- ops->push_back(op);
+ kernel_ops->clear();
+ kernel_ops->emplace_back();
+ kernel_ops->back().op = op;
+ kernel_ops->back().name = "<kernel-delta-operation>";
TEST_AND_RETURN_FALSE(utils::WriteAll(blobs_fd, data.data(), data.size()));
*blobs_length += data.size();
@@ -684,6 +660,8 @@
return true;
}
+// TODO(deymo): Replace Vertex with AnnotatedOperation. This requires to move
+// out the code that adds the reader dependencies on the new vertex.
bool DeltaDiffGenerator::ReadUnwrittenBlocks(
const vector<Block>& blocks,
int blobs_fd,
@@ -893,6 +871,14 @@
ExpandExtents(op.src_extents()) == ExpandExtents(op.dst_extents()));
}
+void DeltaDiffGenerator::FilterNoopOperations(vector<AnnotatedOperation>* ops) {
+ ops->erase(
+ std::remove_if(
+ ops->begin(), ops->end(),
+ [](const AnnotatedOperation& aop){return IsNoopOperation(aop.op);}),
+ ops->end());
+}
+
bool DeltaDiffGenerator::ReorderDataBlobs(
DeltaArchiveManifest* manifest,
const string& data_blobs_path,
@@ -948,25 +934,22 @@
return true;
}
-vector<Vertex::Index> DeltaDiffGenerator::OrderIndices(const Graph& graph) {
- vector<Vertex::Index> order(graph.size());
- std::iota(order.begin(), order.end(), 0);
- return order;
-}
-
-bool DeltaDiffGenerator::GenerateDeltaWithSourceOperations(
+bool DeltaDiffGenerator::GenerateOperations(
const PayloadGenerationConfig& config,
int data_file_fd,
off_t* data_file_size,
- Graph* graph,
- vector<DeltaArchiveManifest_InstallOperation>* kernel_ops,
- vector<Vertex::Index>* final_order) {
+ vector<AnnotatedOperation>* rootfs_ops,
+ vector<AnnotatedOperation>* kernel_ops) {
// List of blocks in the target partition, with the operation that needs to
// write it and the operation that needs to read it. This is used here to
// keep track of the blocks that no operation is writting it.
vector<Block> blocks(config.target.rootfs_size / config.block_size);
- TEST_AND_RETURN_FALSE(DeltaReadFiles(graph,
+ // TODO(deymo): DeltaReadFiles() should not use a graph to generate the
+ // operations, either in the in-place or source uprate. Split out the
+ // graph dependency generation.
+ Graph graph;
+ TEST_AND_RETURN_FALSE(DeltaReadFiles(&graph,
&blocks,
config.source.rootfs_mountpt,
config.target.rootfs_mountpt,
@@ -974,9 +957,15 @@
data_file_fd,
data_file_size,
true)); // src_ops_allowed
+ rootfs_ops->clear();
+ for (const Vertex& v : graph) {
+ rootfs_ops->emplace_back();
+ AnnotatedOperation& aop = rootfs_ops->back();
+ aop.op = v.op;
+ aop.SetNameFromFileAndChunk(v.file_name, v.chunk_offset, v.chunk_size);
+ }
LOG(INFO) << "done reading normal files";
- CheckGraph(*graph);
// Read kernel partition
TEST_AND_RETURN_FALSE(
@@ -987,25 +976,25 @@
data_file_size,
true)); // src_ops_allowed
LOG(INFO) << "done reading kernel";
- CheckGraph(*graph);
- graph->emplace_back();
+ Vertex unwritten_vertex;
TEST_AND_RETURN_FALSE(ReadUnwrittenBlocks(blocks,
data_file_fd,
data_file_size,
config.source.rootfs_part,
config.target.rootfs_part,
- &graph->back()));
- if (graph->back().op.data_length() == 0) {
+ &unwritten_vertex));
+ if (unwritten_vertex.op.data_length() == 0) {
LOG(INFO) << "No unwritten blocks to write, omitting operation";
- graph->pop_back();
+ } else {
+ rootfs_ops->emplace_back();
+ rootfs_ops->back().op = unwritten_vertex.op;
+ rootfs_ops->back().name = unwritten_vertex.file_name;
}
-
- *final_order = OrderIndices(*graph);
return true;
}
-bool DeltaDiffGenerator::GenerateDeltaUpdateFile(
+bool GenerateUpdatePayloadFile(
const PayloadGenerationConfig& config,
const string& output_path,
const string& private_key_path,
@@ -1038,29 +1027,26 @@
DeltaArchiveManifest manifest;
manifest.set_minor_version(config.minor_version);
- vector<DeltaArchiveManifest_InstallOperation> kernel_ops;
-
- Graph graph;
- CheckGraph(graph);
- vector<Vertex::Index> final_order;
+ vector<AnnotatedOperation> rootfs_ops;
+ vector<AnnotatedOperation> kernel_ops;
// Select payload generation strategy based on the config.
- OperationsGenerator* strategy = nullptr;
+ unique_ptr<OperationsGenerator> strategy;
if (config.is_delta) {
// We don't efficiently support deltas on squashfs. For now, we will
// produce full operations in that case.
if (utils::IsSquashfsFilesystem(config.target.rootfs_part)) {
LOG(INFO) << "Using generator FullUpdateGenerator::Run for squashfs "
"deltas";
- strategy = &FullUpdateGenerator::Run;
+ strategy.reset(new FullUpdateGenerator());
} else if (utils::IsExtFilesystem(config.target.rootfs_part)) {
// Delta update (with possibly a full kernel update).
if (config.minor_version == kInPlaceMinorPayloadVersion) {
LOG(INFO) << "Using generator InplaceGenerator::GenerateInplaceDelta";
- strategy = &InplaceGenerator::GenerateInplaceDelta;
+ strategy.reset(new InplaceGenerator());
} else if (config.minor_version == kSourceMinorPayloadVersion) {
LOG(INFO) << "Using generator DeltaDiffGenerator::GenerateSourceDelta";
- strategy = &DeltaDiffGenerator::GenerateDeltaWithSourceOperations;
+ strategy.reset(new DeltaDiffGenerator());
} else {
LOG(ERROR) << "Unsupported minor version given for delta payload: "
<< config.minor_version;
@@ -1074,7 +1060,7 @@
} else {
// Full update.
LOG(INFO) << "Using generator FullUpdateGenerator::Run";
- strategy = &FullUpdateGenerator::Run;
+ strategy.reset(new FullUpdateGenerator());
}
{
@@ -1086,12 +1072,11 @@
ScopedFdCloser data_file_fd_closer(&data_file_fd);
// Generate the operations using the strategy we selected above.
- TEST_AND_RETURN_FALSE((*strategy)(config,
- data_file_fd,
- &data_file_size,
- &graph,
- &kernel_ops,
- &final_order));
+ TEST_AND_RETURN_FALSE(strategy->GenerateOperations(config,
+ data_file_fd,
+ &data_file_size,
+ &rootfs_ops,
+ &kernel_ops));
}
if (!config.source.ImageInfoIsEmpty())
@@ -1100,26 +1085,27 @@
if (!config.target.ImageInfoIsEmpty())
*(manifest.mutable_new_image_info()) = config.target.image_info;
+ // Filter the no-operations. OperationsGenerators should not output this kind
+ // of operations normally, but this is an extra step to fix that if
+ // happened.
+ DeltaDiffGenerator::FilterNoopOperations(&rootfs_ops);
+ DeltaDiffGenerator::FilterNoopOperations(&kernel_ops);
+
OperationNameMap op_name_map;
- CheckGraph(graph);
- InstallOperationsToManifest(graph,
- final_order,
- kernel_ops,
- &manifest,
- &op_name_map);
- CheckGraph(graph);
+ InstallOperationsToManifest(rootfs_ops, kernel_ops, &manifest, &op_name_map);
manifest.set_block_size(config.block_size);
- // Reorder the data blobs with the newly ordered manifest
+ // Reorder the data blobs with the newly ordered manifest.
string ordered_blobs_path;
TEST_AND_RETURN_FALSE(utils::MakeTempFile(
"CrAU_temp_data.ordered.XXXXXX",
&ordered_blobs_path,
nullptr));
ScopedPathUnlinker ordered_blobs_unlinker(ordered_blobs_path);
- TEST_AND_RETURN_FALSE(ReorderDataBlobs(&manifest,
- temp_file_path,
- ordered_blobs_path));
+ TEST_AND_RETURN_FALSE(
+ DeltaDiffGenerator::ReorderDataBlobs(&manifest,
+ temp_file_path,
+ ordered_blobs_path));
temp_file_unlinker.reset();
// Check that install op blobs are in order.
@@ -1149,7 +1135,8 @@
TEST_AND_RETURN_FALSE(
PayloadSigner::SignatureBlobLength(vector<string>(1, private_key_path),
&signature_blob_length));
- AddSignatureOp(next_blob_offset, signature_blob_length, &manifest);
+ DeltaDiffGenerator::AddSignatureOp(
+ next_blob_offset, signature_blob_length, &manifest);
}
TEST_AND_RETURN_FALSE(InitializePartitionInfos(config, &manifest));
@@ -1157,9 +1144,7 @@
// Serialize protobuf
string serialized_manifest;
- CheckGraph(graph);
TEST_AND_RETURN_FALSE(manifest.AppendToString(&serialized_manifest));
- CheckGraph(graph);
LOG(INFO) << "Writing final delta file header...";
DirectFileWriter writer;
diff --git a/update_engine/payload_generator/delta_diff_generator.h b/update_engine/payload_generator/delta_diff_generator.h
index 299b793..6004d31 100644
--- a/update_engine/payload_generator/delta_diff_generator.h
+++ b/update_engine/payload_generator/delta_diff_generator.h
@@ -15,6 +15,7 @@
#include "update_engine/payload_constants.h"
#include "update_engine/payload_generator/graph_types.h"
#include "update_engine/payload_generator/graph_utils.h"
+#include "update_engine/payload_generator/operations_generator.h"
#include "update_engine/payload_generator/payload_generation_config.h"
#include "update_engine/update_metadata.pb.h"
@@ -31,34 +32,10 @@
extern const size_t kBlockSize;
extern const size_t kRootFSPartitionSize;
-// The payload generation strategy prototype. This is the function that does
-// all the work to generate the operations for the rootfs and the kernel.
-// Given the |config|, generates the payload by populating the |graph| with
-// the operations required to update the rootfs when applied in the order
-// specified by |final_order|, and populating |kernel_ops| with the list of
-// kernel operations required to update the kernel.
-// The operations returned will refer to offsets in the file |data_file_fd|,
-// where this function stores the output, but not necessarily in the same
-// order as they appear in the |final_order| and |kernel_ops|.
-// This function stores the amount of data written to |data_file_fd| in
-// |data_file_size|.
-// TODO(deymo): Replace |graph|, |kernel_ops| and |final_order| by two vectors
-// of annotated InstallOperation (InstallOperation plus a name for logging
-// purposes). At this level, there's no need to have a graph.
-// TODO(deymo): Convert this type alias into a base class, so other parameter
-// can be passed to the particular generators while keeping the same interface
-// for operation generation.
-using OperationsGenerator = bool(
- const PayloadGenerationConfig& /* config */,
- int /* data_file_fd */,
- off_t* /* data_file_size */,
- Graph* /* graph */,
- std::vector<DeltaArchiveManifest_InstallOperation>* /* kernel_ops */,
- std::vector<Vertex::Index>* /* final_order */);
-
-
-class DeltaDiffGenerator {
+class DeltaDiffGenerator : public OperationsGenerator {
public:
+ DeltaDiffGenerator() = default;
+
// Represents a disk block on the install partition.
struct Block {
// During install, each block on the install partition will be written
@@ -74,22 +51,6 @@
Vertex::Index writer;
};
- // This is the only function that external users of the class should call.
- // The |config| describes the payload generation request, describing both
- // old and new images for delta payloads and only the new image for full
- // payloads.
- // For delta payloads, the images should be already mounted read-only at
- // the respective rootfs_mountpt.
- // |private_key_path| points to a private key used to sign the update.
- // Pass empty string to not sign the update.
- // |output_path| is the filename where the delta update should be written.
- // Returns true on success. Also writes the size of the metadata into
- // |metadata_size|.
- static bool GenerateDeltaUpdateFile(const PayloadGenerationConfig& config,
- const std::string& output_path,
- const std::string& private_key_path,
- uint64_t* metadata_size);
-
// These functions are public so that the unit tests can access them:
// Generate the update payload operations for the kernel and rootfs using
@@ -97,18 +58,17 @@
// kSourceMinorPayloadVersion. This function will generate operations in the
// rootfs that will read blocks from the source partition in random order and
// write the new image on the target partition, also possibly in random order.
- // The rootfs operations are stored in |graph| and should be executed in the
- // |final_order| order. The kernel operations are stored in |kernel_ops|. All
+ // The rootfs operations are stored in |rootfs_ops| and should be executed in
+ // that order. The kernel operations are stored in |kernel_ops|. All
// the offsets in the operations reference the data written to |data_file_fd|.
// The total amount of data written to that file is stored in
// |data_file_size|.
- static bool GenerateDeltaWithSourceOperations(
+ bool GenerateOperations(
const PayloadGenerationConfig& config,
int data_file_fd,
off_t* data_file_size,
- Graph* graph,
- std::vector<DeltaArchiveManifest_InstallOperation>* kernel_ops,
- std::vector<Vertex::Index>* final_order);
+ std::vector<AnnotatedOperation>* rootfs_ops,
+ std::vector<AnnotatedOperation>* kernel_ops) override;
// For each regular file within new_root, creates a node in the graph,
// determines the best way to compress it (REPLACE, REPLACE_BZ, COPY, BSDIFF),
@@ -170,7 +130,7 @@
static bool DeltaCompressKernelPartition(
const std::string& old_kernel_part,
const std::string& new_kernel_part,
- std::vector<DeltaArchiveManifest_InstallOperation>* ops,
+ std::vector<AnnotatedOperation>* ops,
int blobs_fd,
off_t* blobs_length,
bool src_ops_allowed);
@@ -218,6 +178,10 @@
// (e.g., a move operation that copies blocks onto themselves).
static bool IsNoopOperation(const DeltaArchiveManifest_InstallOperation& op);
+ // Filters all the operations that are no-op, maintaining the relative order
+ // of the rest of the operations.
+ static void FilterNoopOperations(std::vector<AnnotatedOperation>* ops);
+
static bool InitializePartitionInfo(bool is_kernel,
const std::string& partition,
PartitionInfo* info);
@@ -234,10 +198,6 @@
uint64_t signature_blob_length,
DeltaArchiveManifest* manifest);
- // Takes |graph| and returns a vertex order with the vertex indices of
- // |graph|, in the order they appear. Returns |true| on success.
- static std::vector<Vertex::Index> OrderIndices(const Graph& graph);
-
// Takes a collection (vector or RepeatedPtrField) of Extent and
// returns a vector of the blocks referenced, in order.
template<typename T>
@@ -257,13 +217,27 @@
return ret;
}
- static void CheckGraph(const Graph& graph);
-
private:
- // This should never be constructed.
- DISALLOW_IMPLICIT_CONSTRUCTORS(DeltaDiffGenerator);
+ DISALLOW_COPY_AND_ASSIGN(DeltaDiffGenerator);
};
+// This is the only function that external users of this module should call.
+// The |config| describes the payload generation request, describing both
+// old and new images for delta payloads and only the new image for full
+// payloads.
+// For delta payloads, the images should be already mounted read-only at
+// the respective rootfs_mountpt.
+// |private_key_path| points to a private key used to sign the update.
+// Pass empty string to not sign the update.
+// |output_path| is the filename where the delta update should be written.
+// Returns true on success. Also writes the size of the metadata into
+// |metadata_size|.
+bool GenerateUpdatePayloadFile(const PayloadGenerationConfig& config,
+ const std::string& output_path,
+ const std::string& private_key_path,
+ uint64_t* metadata_size);
+
+
}; // namespace chromeos_update_engine
#endif // UPDATE_ENGINE_PAYLOAD_GENERATOR_DELTA_DIFF_GENERATOR_H_
diff --git a/update_engine/payload_generator/delta_diff_generator_unittest.cc b/update_engine/payload_generator/delta_diff_generator_unittest.cc
index 9f938be..22d9c12 100644
--- a/update_engine/payload_generator/delta_diff_generator_unittest.cc
+++ b/update_engine/payload_generator/delta_diff_generator_unittest.cc
@@ -26,7 +26,6 @@
#include "update_engine/payload_generator/extent_mapper.h"
#include "update_engine/payload_generator/graph_types.h"
#include "update_engine/payload_generator/graph_utils.h"
-#include "update_engine/payload_generator/topological_sort.h"
#include "update_engine/subprocess.h"
#include "update_engine/test_utils.h"
#include "update_engine/utils.h"
@@ -556,14 +555,26 @@
EXPECT_FALSE(DeltaDiffGenerator::IsNoopOperation(op));
}
-TEST_F(DeltaDiffGeneratorTest, OrderIndicesTest) {
- Graph graph(3);
- vector<Vertex::Index> order;
- order = DeltaDiffGenerator::OrderIndices(graph);
- EXPECT_EQ(order.size(), 3);
- EXPECT_EQ(order[0], 0);
- EXPECT_EQ(order[1], 1);
- EXPECT_EQ(order[2], 2);
+TEST_F(DeltaDiffGeneratorTest, FilterNoopOperations) {
+ AnnotatedOperation aop1;
+ aop1.op.set_type(DeltaArchiveManifest_InstallOperation_Type_REPLACE_BZ);
+ *(aop1.op.add_dst_extents()) = ExtentForRange(3, 2);
+ aop1.name = "aop1";
+
+ AnnotatedOperation aop2 = aop1;
+ aop2.name = "aop2";
+
+ AnnotatedOperation noop;
+ noop.op.set_type(DeltaArchiveManifest_InstallOperation_Type_MOVE);
+ *(noop.op.add_src_extents()) = ExtentForRange(3, 2);
+ *(noop.op.add_dst_extents()) = ExtentForRange(3, 2);
+ noop.name = "noop";
+
+ vector<AnnotatedOperation> ops = {noop, aop1, noop, noop, aop2, noop};
+ DeltaDiffGenerator::FilterNoopOperations(&ops);
+ EXPECT_EQ(2u, ops.size());
+ EXPECT_EQ("aop1", ops[0].name);
+ EXPECT_EQ("aop2", ops[1].name);
}
} // namespace chromeos_update_engine
diff --git a/update_engine/payload_generator/full_update_generator.cc b/update_engine/payload_generator/full_update_generator.cc
index 0259b7d..3513746 100644
--- a/update_engine/payload_generator/full_update_generator.cc
+++ b/update_engine/payload_generator/full_update_generator.cc
@@ -110,16 +110,17 @@
} // namespace
-bool FullUpdateGenerator::Run(
+bool FullUpdateGenerator::GenerateOperations(
const PayloadGenerationConfig& config,
int fd,
off_t* data_file_size,
- Graph* graph,
- vector<DeltaArchiveManifest_InstallOperation>* kernel_ops,
- vector<Vertex::Index>* final_order) {
+ vector<AnnotatedOperation>* rootfs_ops,
+ vector<AnnotatedOperation>* kernel_ops) {
TEST_AND_RETURN_FALSE(config.Validate());
+ rootfs_ops->clear();
+ kernel_ops->clear();
- // FullUpdateGenerator requires a positive chuck_size, otherwise there will
+ // FullUpdateGenerator requires a positive chunk_size, otherwise there will
// be only one operation with the whole partition which should not be allowed.
size_t full_chunk_size = kDefaultFullChunkSize;
if (config.chunk_size >= 0) {
@@ -167,14 +168,15 @@
DeltaArchiveManifest_InstallOperation* op = nullptr;
if (partition == 0) {
- graph->emplace_back();
- graph->back().file_name =
+ rootfs_ops->emplace_back();
+ rootfs_ops->back().name =
base::StringPrintf("<rootfs-operation-%" PRIuS ">", counter++);
- op = &graph->back().op;
- final_order->push_back(graph->size() - 1);
+ op = &rootfs_ops->back().op;
} else {
- kernel_ops->resize(kernel_ops->size() + 1);
- op = &kernel_ops->back();
+ kernel_ops->emplace_back();
+ kernel_ops->back().name =
+ base::StringPrintf("<kernel-operation-%" PRIuS ">", counter++);
+ op = &kernel_ops->back().op;
}
const bool compress = processor->ShouldCompress();
diff --git a/update_engine/payload_generator/full_update_generator.h b/update_engine/payload_generator/full_update_generator.h
index b741163..b4d1135 100644
--- a/update_engine/payload_generator/full_update_generator.h
+++ b/update_engine/payload_generator/full_update_generator.h
@@ -8,29 +8,31 @@
#include <string>
#include <vector>
-#include "update_engine/payload_generator/graph_types.h"
+#include <base/macros.h>
+
+#include "update_engine/payload_generator/operations_generator.h"
#include "update_engine/payload_generator/payload_generation_config.h"
namespace chromeos_update_engine {
-class FullUpdateGenerator {
+class FullUpdateGenerator : public OperationsGenerator {
public:
+ FullUpdateGenerator() = default;
+
// Creates a full update for the target image defined in |config|. |config|
// must be a valid payload generation configuration for a full payload.
- // Populates |graph|, |kernel_ops|, and |final_order|, with data about the
- // update operations, and writes relevant data to |data_file_fd|, updating
+ // Populates |rootfs_ops| and |kernel_ops|, with data about the update
+ // operations, and writes relevant data to |data_file_fd|, updating
// |data_file_size| as it does.
- static bool Run(
+ bool GenerateOperations(
const PayloadGenerationConfig& config,
int data_file_fd,
off_t* data_file_size,
- Graph* graph,
- std::vector<DeltaArchiveManifest_InstallOperation>* kernel_ops,
- std::vector<Vertex::Index>* final_order);
+ std::vector<AnnotatedOperation>* rootfs_ops,
+ std::vector<AnnotatedOperation>* kernel_ops) override;
private:
- // This should never be constructed.
- DISALLOW_IMPLICIT_CONSTRUCTORS(FullUpdateGenerator);
+ DISALLOW_COPY_AND_ASSIGN(FullUpdateGenerator);
};
} // namespace chromeos_update_engine
diff --git a/update_engine/payload_generator/full_update_generator_unittest.cc b/update_engine/payload_generator/full_update_generator_unittest.cc
index 33b219e..c69f2ee 100644
--- a/update_engine/payload_generator/full_update_generator_unittest.cc
+++ b/update_engine/payload_generator/full_update_generator_unittest.cc
@@ -65,32 +65,30 @@
ScopedFdCloser out_blobs_fd_closer(&out_blobs_fd);
off_t out_blobs_length = 0;
- Graph graph;
- vector<DeltaArchiveManifest_InstallOperation> kernel_ops;
- vector<Vertex::Index> final_order;
+ vector<AnnotatedOperation> rootfs_ops;
+ vector<AnnotatedOperation> kernel_ops;
- EXPECT_TRUE(FullUpdateGenerator::Run(config_,
- out_blobs_fd,
- &out_blobs_length,
- &graph,
- &kernel_ops,
- &final_order));
- int64_t target_rootfs_chucks =
+ FullUpdateGenerator generator;
+
+ EXPECT_TRUE(generator.GenerateOperations(config_,
+ out_blobs_fd,
+ &out_blobs_length,
+ &rootfs_ops,
+ &kernel_ops));
+ int64_t target_rootfs_chunks =
config_.target.rootfs_size / config_.chunk_size;
- EXPECT_EQ(target_rootfs_chucks, graph.size());
- EXPECT_EQ(target_rootfs_chucks, final_order.size());
+ EXPECT_EQ(target_rootfs_chunks, rootfs_ops.size());
EXPECT_EQ(new_kern.size() / config_.chunk_size, kernel_ops.size());
- for (off_t i = 0; i < target_rootfs_chucks; ++i) {
- EXPECT_EQ(i, final_order[i]);
- EXPECT_EQ(1, graph[i].op.dst_extents_size());
+ for (off_t i = 0; i < target_rootfs_chunks; ++i) {
+ EXPECT_EQ(1, rootfs_ops[i].op.dst_extents_size());
EXPECT_EQ(i * config_.chunk_size / config_.block_size,
- graph[i].op.dst_extents(0).start_block()) << "i = " << i;
+ rootfs_ops[i].op.dst_extents(0).start_block()) << "i = " << i;
EXPECT_EQ(config_.chunk_size / config_.block_size,
- graph[i].op.dst_extents(0).num_blocks());
- if (graph[i].op.type() !=
+ rootfs_ops[i].op.dst_extents(0).num_blocks());
+ if (rootfs_ops[i].op.type() !=
DeltaArchiveManifest_InstallOperation_Type_REPLACE) {
EXPECT_EQ(DeltaArchiveManifest_InstallOperation_Type_REPLACE_BZ,
- graph[i].op.type());
+ rootfs_ops[i].op.type());
}
}
}
diff --git a/update_engine/payload_generator/generate_delta_main.cc b/update_engine/payload_generator/generate_delta_main.cc
index 1b558be..38e2c21 100644
--- a/update_engine/payload_generator/generate_delta_main.cc
+++ b/update_engine/payload_generator/generate_delta_main.cc
@@ -418,11 +418,10 @@
}
uint64_t metadata_size;
- if (!DeltaDiffGenerator::GenerateDeltaUpdateFile(
- payload_config,
- FLAGS_out_file,
- FLAGS_private_key,
- &metadata_size)) {
+ if (!GenerateUpdatePayloadFile(payload_config,
+ FLAGS_out_file,
+ FLAGS_private_key,
+ &metadata_size)) {
return 1;
}
diff --git a/update_engine/payload_generator/graph_utils.cc b/update_engine/payload_generator/graph_utils.cc
index 2d8c051..3f9b28f 100644
--- a/update_engine/payload_generator/graph_utils.cc
+++ b/update_engine/payload_generator/graph_utils.cc
@@ -12,6 +12,7 @@
#include <base/macros.h>
#include "update_engine/payload_constants.h"
+#include "update_engine/payload_generator/annotated_operation.h"
using std::make_pair;
using std::pair;
@@ -138,32 +139,11 @@
void DumpGraph(const Graph& graph) {
LOG(INFO) << "Graph length: " << graph.size();
for (Graph::size_type i = 0, e = graph.size(); i != e; ++i) {
- string type_str = "UNK";
- switch (graph[i].op.type()) {
- case DeltaArchiveManifest_InstallOperation_Type_BSDIFF:
- type_str = "BSDIFF";
- break;
- case DeltaArchiveManifest_InstallOperation_Type_MOVE:
- type_str = "MOVE";
- break;
- case DeltaArchiveManifest_InstallOperation_Type_REPLACE:
- type_str = "REPLACE";
- break;
- case DeltaArchiveManifest_InstallOperation_Type_REPLACE_BZ:
- type_str = "REPLACE_BZ";
- break;
- case DeltaArchiveManifest_InstallOperation_Type_SOURCE_COPY:
- type_str = "SOURCE_COPY";
- break;
- case DeltaArchiveManifest_InstallOperation_Type_SOURCE_BSDIFF:
- type_str = "SOURCE_BSDIFF";
- break;
- }
LOG(INFO) << i
<< (graph[i].valid ? "" : "-INV")
<< ": " << graph[i].file_name
<< " " << graph[i].chunk_size << "@" << graph[i].chunk_offset
- << ": " << type_str;
+ << ": " << InstallOperationTypeName(graph[i].op.type());
LOG(INFO) << " src_extents:";
DumpExtents(graph[i].op.src_extents(), 4);
LOG(INFO) << " dst_extents:";
diff --git a/update_engine/payload_generator/inplace_generator.cc b/update_engine/payload_generator/inplace_generator.cc
index ca2d398..0e3284c 100644
--- a/update_engine/payload_generator/inplace_generator.cc
+++ b/update_engine/payload_generator/inplace_generator.cc
@@ -61,6 +61,12 @@
return new_extents;
}
+void InplaceGenerator::CheckGraph(const Graph& graph) {
+ for (const Vertex& v : graph) {
+ CHECK(v.op.has_type());
+ }
+}
+
void InplaceGenerator::SubstituteBlocks(
Vertex* vertex,
const vector<Extent>& remove_extents,
@@ -565,7 +571,7 @@
set<Edge> cut_edges;
cycle_breaker.BreakCycles(*graph, &cut_edges);
LOG(INFO) << "done finding cycles";
- DeltaDiffGenerator::CheckGraph(*graph);
+ CheckGraph(*graph);
// Calculate number of scratch blocks needed
@@ -574,12 +580,12 @@
TEST_AND_RETURN_FALSE(CutEdges(graph, cut_edges, &cuts));
LOG(INFO) << "done cutting cycles";
LOG(INFO) << "There are " << cuts.size() << " cuts.";
- DeltaDiffGenerator::CheckGraph(*graph);
+ CheckGraph(*graph);
LOG(INFO) << "Creating initial topological order...";
TopologicalSort(*graph, final_order);
LOG(INFO) << "done with initial topo order";
- DeltaDiffGenerator::CheckGraph(*graph);
+ CheckGraph(*graph);
LOG(INFO) << "Moving full ops to the back";
MoveFullOpsToBack(graph, final_order);
@@ -626,13 +632,6 @@
extent->set_num_blocks(num_blocks);
}
-// The |blocks| vector contains a reader and writer for each block on the
-// filesystem that's being in-place updated. We populate the reader/writer
-// fields of |blocks| by calling this function.
-// For each block in |operation| that is read or written, find that block
-// in |blocks| and set the reader/writer field to the vertex passed.
-// |graph| is not strictly necessary, but useful for printing out
-// error messages.
bool InplaceGenerator::AddInstallOpToBlocksVector(
const DeltaArchiveManifest_InstallOperation& operation,
const Graph& graph,
@@ -676,16 +675,18 @@
return true;
}
-bool InplaceGenerator::GenerateInplaceDelta(
+bool InplaceGenerator::GenerateOperations(
const PayloadGenerationConfig& config,
int data_file_fd,
off_t* data_file_size,
- Graph* graph,
- vector<DeltaArchiveManifest_InstallOperation>* kernel_ops,
- vector<Vertex::Index>* final_order) {
+ vector<AnnotatedOperation>* rootfs_ops,
+ vector<AnnotatedOperation>* kernel_ops) {
+ Graph graph;
+ CheckGraph(graph);
vector<Block> blocks(config.target.rootfs_size / config.block_size);
+
TEST_AND_RETURN_FALSE(
- DeltaDiffGenerator::DeltaReadFiles(graph,
+ DeltaDiffGenerator::DeltaReadFiles(&graph,
&blocks,
config.source.rootfs_mountpt,
config.target.rootfs_mountpt,
@@ -694,41 +695,41 @@
data_file_size,
false)); // src_ops_allowed
LOG(INFO) << "done reading normal files";
- DeltaDiffGenerator::CheckGraph(*graph);
+ CheckGraph(graph);
LOG(INFO) << "Starting metadata processing";
TEST_AND_RETURN_FALSE(Metadata::DeltaReadMetadata(
- graph,
+ &graph,
&blocks,
config.source.rootfs_part,
config.target.rootfs_part,
data_file_fd,
data_file_size));
LOG(INFO) << "Done metadata processing";
- DeltaDiffGenerator::CheckGraph(*graph);
+ CheckGraph(graph);
- graph->emplace_back();
+ graph.emplace_back();
TEST_AND_RETURN_FALSE(
DeltaDiffGenerator::ReadUnwrittenBlocks(blocks,
data_file_fd,
data_file_size,
config.source.rootfs_part,
config.target.rootfs_part,
- &graph->back()));
- if (graph->back().op.data_length() == 0) {
+ &graph.back()));
+ if (graph.back().op.data_length() == 0) {
LOG(INFO) << "No unwritten blocks to write, omitting operation";
- graph->pop_back();
+ graph.pop_back();
}
// Final scratch block (if there's space)
Vertex::Index scratch_vertex = Vertex::kInvalidIndex;
if (blocks.size() < (config.rootfs_partition_size / kBlockSize)) {
- scratch_vertex = graph->size();
- graph->emplace_back();
+ scratch_vertex = graph.size();
+ graph.emplace_back();
CreateScratchNode(
blocks.size(),
(config.rootfs_partition_size / kBlockSize) - blocks.size(),
- &graph->back());
+ &graph.back());
}
// Read kernel partition
@@ -742,20 +743,33 @@
false)); // src_ops_allowed
LOG(INFO) << "done reading kernel";
- DeltaDiffGenerator::CheckGraph(*graph);
+ CheckGraph(graph);
LOG(INFO) << "Creating edges...";
- CreateEdges(graph, blocks);
+ CreateEdges(&graph, blocks);
LOG(INFO) << "Done creating edges";
- DeltaDiffGenerator::CheckGraph(*graph);
+ CheckGraph(graph);
+ vector<Vertex::Index> final_order;
TEST_AND_RETURN_FALSE(ConvertGraphToDag(
- graph,
+ &graph,
config.target.rootfs_mountpt,
data_file_fd,
data_file_size,
- final_order,
+ &final_order,
scratch_vertex));
+
+ // Copy operations over to the rootfs_ops in the final_order generated by the
+ // topological sort.
+ rootfs_ops->clear();
+ for (const Vertex::Index vertex_index : final_order) {
+ const Vertex& vertex = graph[vertex_index];
+ rootfs_ops->emplace_back();
+ rootfs_ops->back().op = vertex.op;
+ rootfs_ops->back().SetNameFromFileAndChunk(
+ vertex.file_name, vertex.chunk_offset, vertex.chunk_size);
+ }
+
return true;
}
diff --git a/update_engine/payload_generator/inplace_generator.h b/update_engine/payload_generator/inplace_generator.h
index 01d1e3a..5799c0b 100644
--- a/update_engine/payload_generator/inplace_generator.h
+++ b/update_engine/payload_generator/inplace_generator.h
@@ -11,6 +11,7 @@
#include "update_engine/payload_generator/delta_diff_generator.h"
#include "update_engine/payload_generator/graph_types.h"
+#include "update_engine/payload_generator/operations_generator.h"
// InplaceGenerator contains all functionality related to the inplace algorithm
// for generating update payloads. These are the functions used when delta minor
@@ -34,8 +35,13 @@
std::vector<Extent> tmp_extents;
};
-class InplaceGenerator {
+class InplaceGenerator : public OperationsGenerator {
public:
+ InplaceGenerator() = default;
+
+ // Checks all the operations in the graph have a type assigned.
+ static void CheckGraph(const Graph& graph);
+
// Modifies blocks read by 'op' so that any blocks referred to by
// 'remove_extents' are replaced with blocks from 'replace_extents'.
// 'remove_extents' and 'replace_extents' must be the same number of blocks.
@@ -165,17 +171,15 @@
// the offsets in the operations reference the data written to |data_file_fd|.
// The total amount of data written to that file is stored in
// |data_file_size|.
- static bool GenerateInplaceDelta(
+ bool GenerateOperations(
const PayloadGenerationConfig& config,
int data_file_fd,
off_t* data_file_size,
- Graph* graph,
- std::vector<DeltaArchiveManifest_InstallOperation>* kernel_ops,
- std::vector<Vertex::Index>* final_order);
+ std::vector<AnnotatedOperation>* rootfs_ops,
+ std::vector<AnnotatedOperation>* kernel_ops) override;
private:
- // This should never be constructed.
- DISALLOW_IMPLICIT_CONSTRUCTORS(InplaceGenerator);
+ DISALLOW_COPY_AND_ASSIGN(InplaceGenerator);
};
}; // namespace chromeos_update_engine
diff --git a/update_engine/payload_generator/operations_generator.h b/update_engine/payload_generator/operations_generator.h
new file mode 100644
index 0000000..31d57b3
--- /dev/null
+++ b/update_engine/payload_generator/operations_generator.h
@@ -0,0 +1,48 @@
+// Copyright 2015 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef UPDATE_ENGINE_PAYLOAD_GENERATOR_OPERATIONS_GENERATOR_H_
+#define UPDATE_ENGINE_PAYLOAD_GENERATOR_OPERATIONS_GENERATOR_H_
+
+#include <vector>
+
+#include <base/macros.h>
+
+#include "update_engine/payload_generator/annotated_operation.h"
+#include "update_engine/payload_generator/payload_generation_config.h"
+
+namespace chromeos_update_engine {
+
+class OperationsGenerator {
+ public:
+ virtual ~OperationsGenerator() = default;
+
+ // This method generates two lists of operations, one for the rootfs and one
+ // for the kernel and stores the generated operations in |rootfs_ops| and
+ // |kernel_ops| respectivelly. These operations are generated based on the
+ // given |config|. The operations should be applied in the order specified in
+ // the list, and they respect the payload version and type (delta or full)
+ // specified in |config|.
+ // The operations generated will refer to offsets in the file |data_file_fd|,
+ // where this function stores the output, but not necessarily in the same
+ // order as they appear in the |rootfs_ops| and |kernel_ops|.
+ // This function stores the amount of data written to |data_file_fd| in
+ // |data_file_size|.
+ virtual bool GenerateOperations(
+ const PayloadGenerationConfig& config,
+ int data_file_fd,
+ off_t* data_file_size,
+ std::vector<AnnotatedOperation>* rootfs_ops,
+ std::vector<AnnotatedOperation>* kernel_ops) = 0;
+
+ protected:
+ OperationsGenerator() = default;
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(OperationsGenerator);
+};
+
+} // namespace chromeos_update_engine
+
+#endif // UPDATE_ENGINE_PAYLOAD_GENERATOR_OPERATIONS_GENERATOR_H_
diff --git a/update_engine/update_engine.gyp b/update_engine/update_engine.gyp
index e8db1ed..6cbf8a7 100644
--- a/update_engine/update_engine.gyp
+++ b/update_engine/update_engine.gyp
@@ -278,6 +278,7 @@
],
},
'sources': [
+ 'payload_generator/annotated_operation.cc',
'payload_generator/cycle_breaker.cc',
'payload_generator/delta_diff_generator.cc',
'payload_generator/extent_mapper.cc',
