commit 0497d05764dcae38e7a7419a2f25eff043172850
author Alex Deymo <deymo@google.com> Wed Mar 23 09:16:59 2016 -0700
committer Alex Deymo <deymo@google.com> Fri Mar 25 18:58:45 2016 -0700
tree 4f6677f68144fd83dd733869c5138d9ae63d543f
parent d41606fb3c25f6cb8191e154c87391ab285a8a2f

Fix ZERO and DISCARD operations.

These operations were broken (and not used in the generator) because
the code expects them to have a blob offset, but they don't. This bug
prevents them from being used in full payloads, but can still be used
in delta payloads.

Bug: 27858697
TEST=Deployed a full payload with ZERO operations to the new update_engine.

Change-Id: Ifc67bd7c3b2a81593d0347a345723cc34eaaf6b5

payload_consumer/delta_performer.cc

diff --git a/payload_consumer/delta_performer.cc b/payload_consumer/delta_performer.cc
index 074e724..1c6a862 100644
--- a/payload_consumer/delta_performer.cc
+++ b/payload_consumer/delta_performer.cc
@@ -831,10 +831,8 @@
 
 bool DeltaPerformer::CanPerformInstallOperation(
     const chromeos_update_engine::InstallOperation& operation) {
-  // Move and source_copy operations don't require any data blob, so they can
-  // always be performed.
-  if (operation.type() == InstallOperation::MOVE ||
-      operation.type() == InstallOperation::SOURCE_COPY)
+  // If we don't have a data blob we can apply it right away.
+  if (!operation.has_data_offset() && !operation.has_data_length())
     return true;
 
   // See if we have the entire data blob in the buffer

payload_generator/full_update_generator.cc

diff --git a/payload_generator/full_update_generator.cc b/payload_generator/full_update_generator.cc
index af01e63..6d419c3 100644
--- a/payload_generator/full_update_generator.cc
+++ b/payload_generator/full_update_generator.cc
@@ -103,6 +103,16 @@
                                         &bytes_read));
   TEST_AND_RETURN_FALSE(bytes_read == static_cast<ssize_t>(size_));
 
+  if (version_.OperationAllowed(InstallOperation::ZERO) &&
+      std::all_of(buffer_in_.begin(),
+                  buffer_in_.end(),
+                  [](uint8_t x) {return x == 0;})) {
+    // The read buffer is all zeros, so produce a ZERO operation. No need to
+    // check other types of operations in this case.
+    aop_->op.set_type(InstallOperation::ZERO);
+    return true;
+  }
+
   TEST_AND_RETURN_FALSE(BzipCompress(buffer_in_, &op_blob));
   InstallOperation_Type op_type = InstallOperation::REPLACE_BZ;
 

payload_generator/payload_generation_config.cc

diff --git a/payload_generator/payload_generation_config.cc b/payload_generator/payload_generation_config.cc
index 07cf07e..c705f60 100644
--- a/payload_generator/payload_generation_config.cc
+++ b/payload_generator/payload_generation_config.cc
@@ -131,14 +131,19 @@
       // These operations were included in the original payload format.
       return true;
 
-    case InstallOperation::ZERO:
-    case InstallOperation::DISCARD:
     case InstallOperation::REPLACE_XZ:
       // These operations are included in the major version used in Brillo, but
       // can also be used with minor version 3 or newer.
       return major == kBrilloMajorPayloadVersion ||
              minor >= kOpSrcHashMinorPayloadVersion;
 
+    case InstallOperation::ZERO:
+    case InstallOperation::DISCARD:
+      // The implementation of these operations had a bug in earlier versions
+      // that prevents them from being used in any payload. We will enable
+      // them for delta payloads for now.
+      return minor >= kImgdiffMinorPayloadVersion;
+
     // Delta operations:
     case InstallOperation::MOVE:
     case InstallOperation::BSDIFF: