releasetools: Allow generating BBOTA for images with shared blocks.

When target defines 'BOARD_EXT4_SHARE_DUP_BLOCKS := true', the generated
system/vendor images may contain shared blocks (i.e. some blocks will
show up in multiple files' block list), which violates the current
assumptions in BBOTA script.

This CL allows generating BBOTAs by considering the first occurrence as
the "owner" of the shared blocks. All the later users of the shared
blocks will have an incomplete block list, whose RangeSet's will be
tagged with 'uses_shared_blocks'.

Files with 'uses_shared_blocks' tag will not be diff'd with imgdiff,
potentially with patch size penalty. Such files will be accounted for in
imgdiff stats report, where we can revisit for a better solution.

Bug: 64109868
Test: Generate BBOTA full and incremental package with targets defining
      'BOARD_EXT4_SHARE_DUP_BLOCKS := true'.
Change-Id: I87fbc22eef7fafe2a470a03fdcfa1babf088ea8d
This commit is contained in:
Tao Bao 2018-02-07 12:40:00 -08:00
parent 4ccea8549e
commit e709b094e4
4 changed files with 63 additions and 11 deletions

View file

@ -272,6 +272,7 @@ class ImgdiffStats(object):
# Reasons for not applying imgdiff on APKs. # Reasons for not applying imgdiff on APKs.
SKIPPED_TRIMMED = "Not used imgdiff due to trimmed RangeSet" SKIPPED_TRIMMED = "Not used imgdiff due to trimmed RangeSet"
SKIPPED_NONMONOTONIC = "Not used imgdiff due to having non-monotonic ranges" SKIPPED_NONMONOTONIC = "Not used imgdiff due to having non-monotonic ranges"
SKIPPED_SHARED_BLOCKS = "Not used imgdiff due to using shared blocks"
SKIPPED_INCOMPLETE = "Not used imgdiff due to incomplete RangeSet" SKIPPED_INCOMPLETE = "Not used imgdiff due to incomplete RangeSet"
# The list of valid reasons, which will also be the dumped order in a report. # The list of valid reasons, which will also be the dumped order in a report.
@ -280,6 +281,7 @@ class ImgdiffStats(object):
USED_IMGDIFF_LARGE_APK, USED_IMGDIFF_LARGE_APK,
SKIPPED_TRIMMED, SKIPPED_TRIMMED,
SKIPPED_NONMONOTONIC, SKIPPED_NONMONOTONIC,
SKIPPED_SHARED_BLOCKS,
SKIPPED_INCOMPLETE, SKIPPED_INCOMPLETE,
) )
@ -414,6 +416,7 @@ class BlockImageDiff(object):
- The file type is supported by imgdiff; - The file type is supported by imgdiff;
- The source and target blocks are monotonic (i.e. the data is stored with - The source and target blocks are monotonic (i.e. the data is stored with
blocks in increasing order); blocks in increasing order);
- Both files don't contain shared blocks;
- Both files have complete lists of blocks; - Both files have complete lists of blocks;
- We haven't removed any blocks from the source set. - We haven't removed any blocks from the source set.
@ -437,6 +440,11 @@ class BlockImageDiff(object):
self.imgdiff_stats.Log(name, ImgdiffStats.SKIPPED_NONMONOTONIC) self.imgdiff_stats.Log(name, ImgdiffStats.SKIPPED_NONMONOTONIC)
return False return False
if (tgt_ranges.extra.get('uses_shared_blocks') or
src_ranges.extra.get('uses_shared_blocks')):
self.imgdiff_stats.Log(name, ImgdiffStats.SKIPPED_SHARED_BLOCKS)
return False
if tgt_ranges.extra.get('incomplete') or src_ranges.extra.get('incomplete'): if tgt_ranges.extra.get('incomplete') or src_ranges.extra.get('incomplete'):
self.imgdiff_stats.Log(name, ImgdiffStats.SKIPPED_INCOMPLETE) self.imgdiff_stats.Log(name, ImgdiffStats.SKIPPED_INCOMPLETE)
return False return False

View file

@ -625,7 +625,7 @@ def UnzipTemp(filename, pattern=None):
return tmp, zipfile.ZipFile(filename, "r") return tmp, zipfile.ZipFile(filename, "r")
def GetSparseImage(which, tmpdir, input_zip): def GetSparseImage(which, tmpdir, input_zip, allow_shared_blocks):
"""Returns a SparseImage object suitable for passing to BlockImageDiff. """Returns a SparseImage object suitable for passing to BlockImageDiff.
This function loads the specified sparse image from the given path, and This function loads the specified sparse image from the given path, and
@ -637,6 +637,7 @@ def GetSparseImage(which, tmpdir, input_zip):
which: The partition name, which must be "system" or "vendor". which: The partition name, which must be "system" or "vendor".
tmpdir: The directory that contains the prebuilt image and block map file. tmpdir: The directory that contains the prebuilt image and block map file.
input_zip: The target-files ZIP archive. input_zip: The target-files ZIP archive.
allow_shared_blocks: Whether having shared blocks is allowed.
Returns: Returns:
A SparseImage object, with file_map info loaded. A SparseImage object, with file_map info loaded.
@ -655,7 +656,8 @@ def GetSparseImage(which, tmpdir, input_zip):
# unconditionally. Note that they are still part of care_map. (Bug: 20939131) # unconditionally. Note that they are still part of care_map. (Bug: 20939131)
clobbered_blocks = "0" clobbered_blocks = "0"
image = sparse_img.SparseImage(path, mappath, clobbered_blocks) image = sparse_img.SparseImage(path, mappath, clobbered_blocks,
allow_shared_blocks=allow_shared_blocks)
# block.map may contain less blocks, because mke2fs may skip allocating blocks # block.map may contain less blocks, because mke2fs may skip allocating blocks
# if they contain all zeros. We can't reconstruct such a file from its block # if they contain all zeros. We can't reconstruct such a file from its block
@ -669,6 +671,13 @@ def GetSparseImage(which, tmpdir, input_zip):
info = input_zip.getinfo(arcname) info = input_zip.getinfo(arcname)
ranges = image.file_map[entry] ranges = image.file_map[entry]
# If a RangeSet has been tagged as using shared blocks while loading the
# image, its block list must be already incomplete due to that reason. Don't
# give it 'incomplete' tag to avoid messing up the imgdiff stats.
if ranges.extra.get('uses_shared_blocks'):
continue
if RoundUpTo4K(info.file_size) > ranges.size() * 4096: if RoundUpTo4K(info.file_size) > ranges.size() * 4096:
ranges.extra['incomplete'] = True ranges.extra['incomplete'] = True

View file

@ -778,11 +778,15 @@ else if get_stage("%(bcb_dev)s") == "3/3" then
script.ShowProgress(system_progress, 0) script.ShowProgress(system_progress, 0)
# See the notes in WriteBlockIncrementalOTAPackage().
allow_shared_blocks = target_info.get('ext4_share_dup_blocks') == "true"
# Full OTA is done as an "incremental" against an empty source image. This # Full OTA is done as an "incremental" against an empty source image. This
# has the effect of writing new data from the package to the entire # has the effect of writing new data from the package to the entire
# partition, but lets us reuse the updater code that writes incrementals to # partition, but lets us reuse the updater code that writes incrementals to
# do it. # do it.
system_tgt = common.GetSparseImage("system", OPTIONS.input_tmp, input_zip) system_tgt = common.GetSparseImage("system", OPTIONS.input_tmp, input_zip,
allow_shared_blocks)
system_tgt.ResetFileMap() system_tgt.ResetFileMap()
system_diff = common.BlockDifference("system", system_tgt, src=None) system_diff = common.BlockDifference("system", system_tgt, src=None)
system_diff.WriteScript(script, output_zip) system_diff.WriteScript(script, output_zip)
@ -793,7 +797,8 @@ else if get_stage("%(bcb_dev)s") == "3/3" then
if HasVendorPartition(input_zip): if HasVendorPartition(input_zip):
script.ShowProgress(0.1, 0) script.ShowProgress(0.1, 0)
vendor_tgt = common.GetSparseImage("vendor", OPTIONS.input_tmp, input_zip) vendor_tgt = common.GetSparseImage("vendor", OPTIONS.input_tmp, input_zip,
allow_shared_blocks)
vendor_tgt.ResetFileMap() vendor_tgt.ResetFileMap()
vendor_diff = common.BlockDifference("vendor", vendor_tgt) vendor_diff = common.BlockDifference("vendor", vendor_tgt)
vendor_diff.WriteScript(script, output_zip) vendor_diff.WriteScript(script, output_zip)
@ -970,8 +975,16 @@ def WriteBlockIncrementalOTAPackage(target_zip, source_zip, output_zip):
target_recovery = common.GetBootableImage( target_recovery = common.GetBootableImage(
"/tmp/recovery.img", "recovery.img", OPTIONS.target_tmp, "RECOVERY") "/tmp/recovery.img", "recovery.img", OPTIONS.target_tmp, "RECOVERY")
system_src = common.GetSparseImage("system", OPTIONS.source_tmp, source_zip) # When target uses 'BOARD_EXT4_SHARE_DUP_BLOCKS := true', images may contain
system_tgt = common.GetSparseImage("system", OPTIONS.target_tmp, target_zip) # shared blocks (i.e. some blocks will show up in multiple files' block
# list). We can only allocate such shared blocks to the first "owner", and
# disable imgdiff for all later occurrences.
allow_shared_blocks = (source_info.get('ext4_share_dup_blocks') == "true" or
target_info.get('ext4_share_dup_blocks') == "true")
system_src = common.GetSparseImage("system", OPTIONS.source_tmp, source_zip,
allow_shared_blocks)
system_tgt = common.GetSparseImage("system", OPTIONS.target_tmp, target_zip,
allow_shared_blocks)
blockimgdiff_version = max( blockimgdiff_version = max(
int(i) for i in target_info.get("blockimgdiff_versions", "1").split(",")) int(i) for i in target_info.get("blockimgdiff_versions", "1").split(","))
@ -996,8 +1009,10 @@ def WriteBlockIncrementalOTAPackage(target_zip, source_zip, output_zip):
if HasVendorPartition(target_zip): if HasVendorPartition(target_zip):
if not HasVendorPartition(source_zip): if not HasVendorPartition(source_zip):
raise RuntimeError("can't generate incremental that adds /vendor") raise RuntimeError("can't generate incremental that adds /vendor")
vendor_src = common.GetSparseImage("vendor", OPTIONS.source_tmp, source_zip) vendor_src = common.GetSparseImage("vendor", OPTIONS.source_tmp, source_zip,
vendor_tgt = common.GetSparseImage("vendor", OPTIONS.target_tmp, target_zip) allow_shared_blocks)
vendor_tgt = common.GetSparseImage("vendor", OPTIONS.target_tmp, target_zip,
allow_shared_blocks)
# Check first block of vendor partition for remount R/W only if # Check first block of vendor partition for remount R/W only if
# disk type is ext4 # disk type is ext4

View file

@ -33,7 +33,7 @@ class SparseImage(object):
""" """
def __init__(self, simg_fn, file_map_fn=None, clobbered_blocks=None, def __init__(self, simg_fn, file_map_fn=None, clobbered_blocks=None,
mode="rb", build_map=True): mode="rb", build_map=True, allow_shared_blocks=False):
self.simg_f = f = open(simg_fn, mode) self.simg_f = f = open(simg_fn, mode)
header_bin = f.read(28) header_bin = f.read(28)
@ -129,7 +129,8 @@ class SparseImage(object):
self.extended = extended self.extended = extended
if file_map_fn: if file_map_fn:
self.LoadFileBlockMap(file_map_fn, self.clobbered_blocks) self.LoadFileBlockMap(file_map_fn, self.clobbered_blocks,
allow_shared_blocks)
else: else:
self.file_map = {"__DATA": self.care_map} self.file_map = {"__DATA": self.care_map}
@ -209,7 +210,14 @@ class SparseImage(object):
yield fill_data * (this_read * (self.blocksize >> 2)) yield fill_data * (this_read * (self.blocksize >> 2))
to_read -= this_read to_read -= this_read
def LoadFileBlockMap(self, fn, clobbered_blocks): def LoadFileBlockMap(self, fn, clobbered_blocks, allow_shared_blocks):
"""Loads the given block map file.
Args:
fn: The filename of the block map file.
clobbered_blocks: A RangeSet instance for the clobbered blocks.
allow_shared_blocks: Whether having shared blocks is allowed.
"""
remaining = self.care_map remaining = self.care_map
self.file_map = out = {} self.file_map = out = {}
@ -217,6 +225,18 @@ class SparseImage(object):
for line in f: for line in f:
fn, ranges = line.split(None, 1) fn, ranges = line.split(None, 1)
ranges = rangelib.RangeSet.parse(ranges) ranges = rangelib.RangeSet.parse(ranges)
if allow_shared_blocks:
# Find the shared blocks that have been claimed by others.
shared_blocks = ranges.subtract(remaining)
if shared_blocks:
ranges = ranges.subtract(shared_blocks)
if not ranges:
continue
# Tag the entry so that we can skip applying imgdiff on this file.
ranges.extra['uses_shared_blocks'] = True
out[fn] = ranges out[fn] = ranges
assert ranges.size() == ranges.intersect(remaining).size() assert ranges.size() == ranges.intersect(remaining).size()