Relanding 'update_payload: Add XZ compression support'
This patch adds support for checking a payload that has REPLACE_XZ
operations. REPLACE_XZ was added in minor version 3.
BUG=chromium:758792
TEST=unittests pass; paycheck.py with a xz generated payload pass;
CQ-DEPEND=CL:823234
Change-Id: I6ec8068e233f2d595fda93a985923d85c59f150e
Reviewed-on: https://chromium-review.googlesource.com/872124
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Amin Hassani <ahassani@chromium.org>
Reviewed-by: Ben Chan <benchan@chromium.org>
Reviewed-by: Sen Jiang <senj@chromium.org>
diff --git a/scripts/update_payload/applier.py b/scripts/update_payload/applier.py
index e470ac4..8af89e7 100644
--- a/scripts/update_payload/applier.py
+++ b/scripts/update_payload/applier.py
@@ -18,6 +18,20 @@
import bz2
import hashlib
import itertools
+# Not everywhere we can have the lzma library so we ignore it if we didn't have
+# it because it is not going to be used. For example, 'cros flash' uses
+# devserver code which eventually loads this file, but the lzma library is not
+# included in the client test devices, and it is not necessary to do so. But
+# lzma is not used in 'cros flash' so it should be fine. Python 3.x include
+# lzma, but for backward compatibility with Python 2.7, backports-lzma is
+# needed.
+try:
+ import lzma
+except ImportError:
+ try:
+ from backports import lzma
+ except ImportError:
+ pass
import os
import shutil
import subprocess
@@ -216,7 +230,7 @@
self.truncate_to_expected_size = truncate_to_expected_size
def _ApplyReplaceOperation(self, op, op_name, out_data, part_file, part_size):
- """Applies a REPLACE{,_BZ} operation.
+ """Applies a REPLACE{,_BZ,_XZ} operation.
Args:
op: the operation object
@@ -235,6 +249,10 @@
if op.type == common.OpType.REPLACE_BZ:
out_data = bz2.decompress(out_data)
data_length = len(out_data)
+ elif op.type == common.OpType.REPLACE_XZ:
+ # pylint: disable=no-member
+ out_data = lzma.decompress(out_data)
+ data_length = len(out_data)
# Write data to blocks specified in dst extents.
data_start = 0
@@ -508,7 +526,8 @@
# Read data blob.
data = self.payload.ReadDataBlob(op.data_offset, op.data_length)
- if op.type in (common.OpType.REPLACE, common.OpType.REPLACE_BZ):
+ if op.type in (common.OpType.REPLACE, common.OpType.REPLACE_BZ,
+ common.OpType.REPLACE_XZ):
self._ApplyReplaceOperation(op, op_name, data, new_part_file, part_size)
elif op.type == common.OpType.MOVE:
self._ApplyMoveOperation(op, op_name, new_part_file)
diff --git a/scripts/update_payload/checker.py b/scripts/update_payload/checker.py
index e4cb845..8e04f1e 100644
--- a/scripts/update_payload/checker.py
+++ b/scripts/update_payload/checker.py
@@ -322,6 +322,10 @@
self.new_rootfs_fs_size = 0
self.new_kernel_fs_size = 0
self.minor_version = None
+ # TODO(*): When fixing crbug.com/794404, the major version should be
+ # correclty handled in update_payload scripts. So stop forcing
+ # major_verions=1 here and set it to the correct value.
+ self.major_version = 1
@staticmethod
def _CheckElem(msg, name, report, is_mandatory, is_submsg, convert=str,
@@ -701,7 +705,7 @@
return total_num_blocks
def _CheckReplaceOperation(self, op, data_length, total_dst_blocks, op_name):
- """Specific checks for REPLACE/REPLACE_BZ operations.
+ """Specific checks for REPLACE/REPLACE_BZ/REPLACE_XZ operations.
Args:
op: The operation object from the manifest.
@@ -996,6 +1000,9 @@
# Type-specific checks.
if op.type in (common.OpType.REPLACE, common.OpType.REPLACE_BZ):
self._CheckReplaceOperation(op, data_length, total_dst_blocks, op_name)
+ elif op.type == common.OpType.REPLACE_XZ and (self.minor_version >= 3 or
+ self.major_version >= 2):
+ self._CheckReplaceOperation(op, data_length, total_dst_blocks, op_name)
elif op.type == common.OpType.MOVE and self.minor_version == 1:
self._CheckMoveOperation(op, data_offset, total_src_blocks,
total_dst_blocks, op_name)
@@ -1068,6 +1075,7 @@
op_counts = {
common.OpType.REPLACE: 0,
common.OpType.REPLACE_BZ: 0,
+ common.OpType.REPLACE_XZ: 0,
common.OpType.MOVE: 0,
common.OpType.ZERO: 0,
common.OpType.BSDIFF: 0,
@@ -1080,6 +1088,7 @@
op_blob_totals = {
common.OpType.REPLACE: 0,
common.OpType.REPLACE_BZ: 0,
+ common.OpType.REPLACE_XZ: 0,
# MOVE operations don't have blobs.
common.OpType.BSDIFF: 0,
# SOURCE_COPY operations don't have blobs.
diff --git a/scripts/update_payload/checker_unittest.py b/scripts/update_payload/checker_unittest.py
index 974519d..cc080b4 100755
--- a/scripts/update_payload/checker_unittest.py
+++ b/scripts/update_payload/checker_unittest.py
@@ -620,6 +620,41 @@
PayloadError, payload_checker._CheckReplaceOperation,
op, data_length, (data_length + block_size - 1) / block_size, 'foo')
+ def testCheckReplaceXzOperation(self):
+ """Tests _CheckReplaceOperation() where op.type == REPLACE_XZ."""
+ payload_checker = checker.PayloadChecker(self.MockPayload())
+ block_size = payload_checker.block_size
+ data_length = block_size * 3
+
+ op = self.mox.CreateMock(
+ update_metadata_pb2.InstallOperation)
+ op.type = common.OpType.REPLACE_XZ
+
+ # Pass.
+ op.src_extents = []
+ self.assertIsNone(
+ payload_checker._CheckReplaceOperation(
+ op, data_length, (data_length + block_size - 1) / block_size + 5,
+ 'foo'))
+
+ # Fail, src extents founds.
+ op.src_extents = ['bar']
+ self.assertRaises(
+ PayloadError, payload_checker._CheckReplaceOperation,
+ op, data_length, (data_length + block_size - 1) / block_size + 5, 'foo')
+
+ # Fail, missing data.
+ op.src_extents = []
+ self.assertRaises(
+ PayloadError, payload_checker._CheckReplaceOperation,
+ op, None, (data_length + block_size - 1) / block_size, 'foo')
+
+ # Fail, too few blocks to justify XZ.
+ op.src_extents = []
+ self.assertRaises(
+ PayloadError, payload_checker._CheckReplaceOperation,
+ op, data_length, (data_length + block_size - 1) / block_size, 'foo')
+
def testCheckMoveOperation_Pass(self):
"""Tests _CheckMoveOperation(); pass case."""
payload_checker = checker.PayloadChecker(self.MockPayload())
@@ -792,8 +827,8 @@
"""Parametric testing of _CheckOperation().
Args:
- op_type_name: 'REPLACE', 'REPLACE_BZ', 'MOVE', 'BSDIFF', 'SOURCE_COPY',
- 'SOURCE_BSDIFF', BROTLI_BSDIFF or 'PUFFDIFF'.
+ op_type_name: 'REPLACE', 'REPLACE_BZ', 'REPLACE_XZ', 'MOVE', 'BSDIFF',
+ 'SOURCE_COPY', 'SOURCE_BSDIFF', BROTLI_BSDIFF or 'PUFFDIFF'.
is_last: Whether we're testing the last operation in a sequence.
allow_signature: Whether we're testing a signature-capable operation.
allow_unhashed: Whether we're allowing to not hash the data.
@@ -848,6 +883,8 @@
payload_checker.minor_version = 2 if fail_bad_minor_version else 1
elif op_type in (common.OpType.SOURCE_COPY, common.OpType.SOURCE_BSDIFF):
payload_checker.minor_version = 1 if fail_bad_minor_version else 2
+ if op_type == common.OpType.REPLACE_XZ:
+ payload_checker.minor_version = 2 if fail_bad_minor_version else 3
elif op_type in (common.OpType.ZERO, common.OpType.DISCARD,
common.OpType.PUFFDIFF, common.OpType.BROTLI_BSDIFF):
payload_checker.minor_version = 3 if fail_bad_minor_version else 4
@@ -1167,10 +1204,13 @@
"""Returns True iff the combination of arguments represents a valid test."""
op_type = _OpTypeByName(op_type_name)
- # REPLACE/REPLACE_BZ operations don't read data from src partition. They are
- # compatible with all valid minor versions, so we don't need to check that.
- if (op_type in (common.OpType.REPLACE, common.OpType.REPLACE_BZ) and (
- fail_src_extents or fail_src_length or fail_bad_minor_version)):
+ # REPLACE/REPLACE_BZ/REPLACE_XZ operations don't read data from src
+ # partition. They are compatible with all valid minor versions, so we don't
+ # need to check that.
+ if (op_type in (common.OpType.REPLACE, common.OpType.REPLACE_BZ,
+ common.OpType.REPLACE_XZ) and (fail_src_extents or
+ fail_src_length or
+ fail_bad_minor_version)):
return False
# MOVE and SOURCE_COPY operations don't carry data.
@@ -1256,8 +1296,8 @@
# Add all _CheckOperation() test cases.
AddParametricTests('CheckOperation',
- {'op_type_name': ('REPLACE', 'REPLACE_BZ', 'MOVE',
- 'BSDIFF', 'SOURCE_COPY',
+ {'op_type_name': ('REPLACE', 'REPLACE_BZ', 'REPLACE_XZ',
+ 'MOVE', 'BSDIFF', 'SOURCE_COPY',
'SOURCE_BSDIFF', 'PUFFDIFF',
'BROTLI_BSDIFF'),
'is_last': (True, False),
diff --git a/scripts/update_payload/test_utils.py b/scripts/update_payload/test_utils.py
index 38712fb..030f29f 100644
--- a/scripts/update_payload/test_utils.py
+++ b/scripts/update_payload/test_utils.py
@@ -276,7 +276,7 @@
Args:
is_kernel: whether this is a kernel (True) or rootfs (False) operation
- op_type: one of REPLACE, REPLACE_BZ, MOVE or BSDIFF
+ op_type: one of REPLACE, REPLACE_BZ, REPLACE_XZ, MOVE or BSDIFF
src_extents: list of (start, length) pairs indicating src block ranges
src_length: size of the src data in bytes (needed for BSDIFF)
dst_extents: list of (start, length) pairs indicating dst block ranges