paycheck: truncate partitions resulting from applying a payl... [chromiumos/platform/dev-util : master]
0 views
Skip to first unread message
Gilad Arnold (Code Review)
May 23, 2013, 8:02:53 PM5/23/13
to Ryan Cui, Chris Sosa
Gilad Arnold has uploaded a new change for review.
https://gerrit.chromium.org/gerrit/56532
Change subject: paycheck: truncate partitions resulting from applying a payload
......................................................................
paycheck: truncate partitions resulting from applying a payload
Since the correctness of the result only encompasses the filesystem (or
otherwise "meaningful data") on the target partition, it is desirable to
actually get rid of whatever is past that point. There are different
reasons for the presence of such extra space in delta updates, including
remnants from a source partition that served as baseline for a delta
update, or scratch space used by MOVE operations for breaking cycles.
We make truncation the default behavior, although it can be suppressed
by passing the right flag (truncate_to_expected_size=False). Note that
this change is necessary for comparing the results of applying a payload
to the partitions as they are extracted from a target image, which is to
be attempted during payload generation.
This also fixes tiny gpylint complaints.
BUG=chromium:241283
TEST=Emitted partition files truncated as expected
Change-Id: Ibb71e4f2305ec41224afdc503168ae02c312f6fe
---
M host/lib/update_payload/applier.py
M host/lib/update_payload/payload.py
2 files changed, 30 insertions(+), 9 deletions(-)
git pull ssh://gerrit.chromium.org:29418/chromiumos/platform/dev-util refs/changes/32/56532/1
diff --git a/host/lib/update_payload/applier.py b/host/lib/update_payload/applier.py
index f5ae6b7..a7527de 100644
--- a/host/lib/update_payload/applier.py
+++ b/host/lib/update_payload/applier.py
@@ -397,7 +397,8 @@
def _ApplyToPartition(self, operations, part_name, base_name,
new_part_file_name, new_part_info,
- old_part_file_name=None, old_part_info=None):
+ old_part_file_name=None, old_part_info=None,
+ truncate_to_expected_size=True):
"""Applies an update to a partition.
Args:
@@ -408,6 +409,8 @@
new_part_info: size and expected hash of dest partition
old_part_file_name: file name of source partition (optional)
old_part_info: size and expected hash of source partition (optional)
+ truncate_to_expected_size: whether to truncate the resulting partition
+ to its final expected sizes (optional)
Raises:
PayloadError if anything goes wrong with the update.
@@ -430,6 +433,12 @@
with open(new_part_file_name, 'r+b') as new_part_file:
self._ApplyOperations(operations, base_name, new_part_file,
new_part_info.size)
+ # Truncate the result, if so instructed.
+ if truncate_to_expected_size:
+ new_part_file.seek(0, 2)
+ if new_part_file.tell() > new_part_info.size:
+ new_part_file.seek(new_part_info.size)
+ new_part_file.truncate()
# Verify the resulting partition.
with open(new_part_file_name, 'rb') as new_part_file:
@@ -437,7 +446,7 @@
length=new_part_info.size)
def Run(self, new_kernel_part, new_rootfs_part, old_kernel_part=None,
- old_rootfs_part=None):
+ old_rootfs_part=None, truncate_to_expected_size=True):
"""Applier entry point, invoking all update operations.
Args:
@@ -445,6 +454,9 @@
new_rootfs_part: name of dest rootfs partition file
old_kernel_part: name of source kernel partition file (optional)
old_rootfs_part: name of source rootfs partition file (optional)
+ truncate_to_expected_size: whether to truncate the resulting partitions
+ to their expected sizes, as specified in the
+ payload (optional)
Raises:
PayloadError if payload application failed.
@@ -471,11 +483,13 @@
self.payload.manifest.install_operations, 'rootfs',
'install_operations', new_rootfs_part,
self.payload.manifest.new_rootfs_info, old_rootfs_part,
- self.payload.manifest.old_rootfs_info)
+ self.payload.manifest.old_rootfs_info,
+ truncate_to_expected_size=truncate_to_expected_size)
# Apply update to kernel update.
self._ApplyToPartition(
self.payload.manifest.kernel_install_operations, 'kernel',
'kernel_install_operations', new_kernel_part,
self.payload.manifest.new_kernel_info, old_kernel_part,
- self.payload.manifest.old_kernel_info)
+ self.payload.manifest.old_kernel_info,
+ truncate_to_expected_size=truncate_to_expected_size)
diff --git a/host/lib/update_payload/payload.py b/host/lib/update_payload/payload.py
index 1796f51..2dbc975 100644
--- a/host/lib/update_payload/payload.py
+++ b/host/lib/update_payload/payload.py
@@ -155,7 +155,7 @@
self.is_init = True
def Describe(self):
-
+ """Emits the payload embedded description data to standard output."""
def _DescribeImageInfo(description, image_info):
def _DisplayIndentedValue(name, value):
print ' {:<14} {}'.format(name+':', value)
@@ -166,16 +166,18 @@
_DisplayIndentedValue('Version', image_info.version)
_DisplayIndentedValue('Key', image_info.key)
- if (image_info.build_channel != image_info.channel):
+ if image_info.build_channel != image_info.channel:
_DisplayIndentedValue('Build channel', image_info.build_channel)
- if (image_info.build_version != image_info.version):
+ if image_info.build_version != image_info.version:
_DisplayIndentedValue('Build version', image_info.build_version)
if self.manifest.HasField('old_image_info'):
+ # pylint: disable=E1101
_DescribeImageInfo('Old Image', self.manifest.old_image_info)
if self.manifest.HasField('new_image_info'):
+ # pylint: disable=E1101
_DescribeImageInfo('New Image', self.manifest.new_image_info)
def _AssertInit(self):
@@ -230,7 +232,8 @@
report_out_file=report_out_file)
def Apply(self, new_kernel_part, new_rootfs_part, old_kernel_part=None,
- old_rootfs_part=None, bsdiff_in_place=True):
+ old_rootfs_part=None, bsdiff_in_place=True,
+ truncate_to_expected_size=True):
"""Applies the update payload.
Args:
@@ -239,6 +242,9 @@
old_kernel_part: name of source kernel partition file (optional)
old_rootfs_part: name of source rootfs partition file (optional)
bsdiff_in_place: whether to perform BSDIFF operations in-place (optional)
+ truncate_to_expected_size: whether to truncate the resulting partitions
+ to their expected sizes, as specified in the
+ payload (optional)
Raises:
PayloadError if payload application failed.
@@ -249,7 +255,8 @@
helper = applier.PayloadApplier(self, bsdiff_in_place=bsdiff_in_place)
helper.Run(new_kernel_part, new_rootfs_part,
old_kernel_part=old_kernel_part,
- old_rootfs_part=old_rootfs_part)
+ old_rootfs_part=old_rootfs_part,
+ truncate_to_expected_size=truncate_to_expected_size)
def TraceBlock(self, block, skip, trace_out_file, is_kernel):
"""Traces the origin(s) of a given dest partition block.
--
To view, visit https://gerrit.chromium.org/gerrit/56532
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibb71e4f2305ec41224afdc503168ae02c312f6fe
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/platform/dev-util
Gerrit-Branch: master
Gerrit-Owner: Gilad Arnold <gar...@chromium.org>
https://gerrit.chromium.org/gerrit/56532
Change subject: paycheck: truncate partitions resulting from applying a payload
......................................................................
paycheck: truncate partitions resulting from applying a payload
Since the correctness of the result only encompasses the filesystem (or
otherwise "meaningful data") on the target partition, it is desirable to
actually get rid of whatever is past that point. There are different
reasons for the presence of such extra space in delta updates, including
remnants from a source partition that served as baseline for a delta
update, or scratch space used by MOVE operations for breaking cycles.
We make truncation the default behavior, although it can be suppressed
by passing the right flag (truncate_to_expected_size=False). Note that
this change is necessary for comparing the results of applying a payload
to the partitions as they are extracted from a target image, which is to
be attempted during payload generation.
This also fixes tiny gpylint complaints.
BUG=chromium:241283
TEST=Emitted partition files truncated as expected
Change-Id: Ibb71e4f2305ec41224afdc503168ae02c312f6fe
---
M host/lib/update_payload/applier.py
M host/lib/update_payload/payload.py
2 files changed, 30 insertions(+), 9 deletions(-)
git pull ssh://gerrit.chromium.org:29418/chromiumos/platform/dev-util refs/changes/32/56532/1
diff --git a/host/lib/update_payload/applier.py b/host/lib/update_payload/applier.py
index f5ae6b7..a7527de 100644
--- a/host/lib/update_payload/applier.py
+++ b/host/lib/update_payload/applier.py
@@ -397,7 +397,8 @@
def _ApplyToPartition(self, operations, part_name, base_name,
new_part_file_name, new_part_info,
- old_part_file_name=None, old_part_info=None):
+ old_part_file_name=None, old_part_info=None,
+ truncate_to_expected_size=True):
"""Applies an update to a partition.
Args:
@@ -408,6 +409,8 @@
new_part_info: size and expected hash of dest partition
old_part_file_name: file name of source partition (optional)
old_part_info: size and expected hash of source partition (optional)
+ truncate_to_expected_size: whether to truncate the resulting partition
+ to its final expected sizes (optional)
Raises:
PayloadError if anything goes wrong with the update.
@@ -430,6 +433,12 @@
with open(new_part_file_name, 'r+b') as new_part_file:
self._ApplyOperations(operations, base_name, new_part_file,
new_part_info.size)
+ # Truncate the result, if so instructed.
+ if truncate_to_expected_size:
+ new_part_file.seek(0, 2)
+ if new_part_file.tell() > new_part_info.size:
+ new_part_file.seek(new_part_info.size)
+ new_part_file.truncate()
# Verify the resulting partition.
with open(new_part_file_name, 'rb') as new_part_file:
@@ -437,7 +446,7 @@
length=new_part_info.size)
def Run(self, new_kernel_part, new_rootfs_part, old_kernel_part=None,
- old_rootfs_part=None):
+ old_rootfs_part=None, truncate_to_expected_size=True):
"""Applier entry point, invoking all update operations.
Args:
@@ -445,6 +454,9 @@
new_rootfs_part: name of dest rootfs partition file
old_kernel_part: name of source kernel partition file (optional)
old_rootfs_part: name of source rootfs partition file (optional)
+ truncate_to_expected_size: whether to truncate the resulting partitions
+ to their expected sizes, as specified in the
+ payload (optional)
Raises:
PayloadError if payload application failed.
@@ -471,11 +483,13 @@
self.payload.manifest.install_operations, 'rootfs',
'install_operations', new_rootfs_part,
self.payload.manifest.new_rootfs_info, old_rootfs_part,
- self.payload.manifest.old_rootfs_info)
+ self.payload.manifest.old_rootfs_info,
+ truncate_to_expected_size=truncate_to_expected_size)
# Apply update to kernel update.
self._ApplyToPartition(
self.payload.manifest.kernel_install_operations, 'kernel',
'kernel_install_operations', new_kernel_part,
self.payload.manifest.new_kernel_info, old_kernel_part,
- self.payload.manifest.old_kernel_info)
+ self.payload.manifest.old_kernel_info,
+ truncate_to_expected_size=truncate_to_expected_size)
diff --git a/host/lib/update_payload/payload.py b/host/lib/update_payload/payload.py
index 1796f51..2dbc975 100644
--- a/host/lib/update_payload/payload.py
+++ b/host/lib/update_payload/payload.py
@@ -155,7 +155,7 @@
self.is_init = True
def Describe(self):
-
+ """Emits the payload embedded description data to standard output."""
def _DescribeImageInfo(description, image_info):
def _DisplayIndentedValue(name, value):
print ' {:<14} {}'.format(name+':', value)
@@ -166,16 +166,18 @@
_DisplayIndentedValue('Version', image_info.version)
_DisplayIndentedValue('Key', image_info.key)
- if (image_info.build_channel != image_info.channel):
+ if image_info.build_channel != image_info.channel:
_DisplayIndentedValue('Build channel', image_info.build_channel)
- if (image_info.build_version != image_info.version):
+ if image_info.build_version != image_info.version:
_DisplayIndentedValue('Build version', image_info.build_version)
if self.manifest.HasField('old_image_info'):
+ # pylint: disable=E1101
_DescribeImageInfo('Old Image', self.manifest.old_image_info)
if self.manifest.HasField('new_image_info'):
+ # pylint: disable=E1101
_DescribeImageInfo('New Image', self.manifest.new_image_info)
def _AssertInit(self):
@@ -230,7 +232,8 @@
report_out_file=report_out_file)
def Apply(self, new_kernel_part, new_rootfs_part, old_kernel_part=None,
- old_rootfs_part=None, bsdiff_in_place=True):
+ old_rootfs_part=None, bsdiff_in_place=True,
+ truncate_to_expected_size=True):
"""Applies the update payload.
Args:
@@ -239,6 +242,9 @@
old_kernel_part: name of source kernel partition file (optional)
old_rootfs_part: name of source rootfs partition file (optional)
bsdiff_in_place: whether to perform BSDIFF operations in-place (optional)
+ truncate_to_expected_size: whether to truncate the resulting partitions
+ to their expected sizes, as specified in the
+ payload (optional)
Raises:
PayloadError if payload application failed.
@@ -249,7 +255,8 @@
helper = applier.PayloadApplier(self, bsdiff_in_place=bsdiff_in_place)
helper.Run(new_kernel_part, new_rootfs_part,
old_kernel_part=old_kernel_part,
- old_rootfs_part=old_rootfs_part)
+ old_rootfs_part=old_rootfs_part,
+ truncate_to_expected_size=truncate_to_expected_size)
def TraceBlock(self, block, skip, trace_out_file, is_kernel):
"""Traces the origin(s) of a given dest partition block.
--
To view, visit https://gerrit.chromium.org/gerrit/56532
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibb71e4f2305ec41224afdc503168ae02c312f6fe
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/platform/dev-util
Gerrit-Branch: master
Gerrit-Owner: Gilad Arnold <gar...@chromium.org>
Gilad Arnold (Code Review)
May 23, 2013, 8:03:10 PM5/23/13
to ChromeBot
Gilad Arnold has posted comments on this change.
Change subject: paycheck: truncate partitions resulting from applying a payload
......................................................................
Patch Set 1: Verified
--
To view, visit https://gerrit.chromium.org/gerrit/56532
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings
Gerrit-MessageType: comment
Gerrit-Reviewer: Gilad Arnold <gar...@chromium.org>
Change subject: paycheck: truncate partitions resulting from applying a payload
......................................................................
--
To view, visit https://gerrit.chromium.org/gerrit/56532
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings
Gerrit-Change-Id: Ibb71e4f2305ec41224afdc503168ae02c312f6fe
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/platform/dev-util
Gerrit-Branch: master
Gerrit-Owner: Gilad Arnold <gar...@chromium.org>
Gerrit-Reviewer: ChromeBot <chrom...@google.com>
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/platform/dev-util
Gerrit-Branch: master
Gerrit-Owner: Gilad Arnold <gar...@chromium.org>
Gerrit-Reviewer: Gilad Arnold <gar...@chromium.org>
Don Garrett (Code Review)
May 23, 2013, 9:01:24 PM5/23/13
to Gilad Arnold, ChromeBot, Chris Sosa
Don Garrett has posted comments on this change.
Change subject: paycheck: truncate partitions resulting from applying a payload
......................................................................
Patch Set 1: Looks good to me, approved
(1 inline comment)
+2 with a suggestion.
....................................................
File host/lib/update_payload/applier.py
Line 449: old_rootfs_part=None, truncate_to_expected_size=True):
I would instead consider making this value a class property so it doesn't get passed around so much.
--
To view, visit https://gerrit.chromium.org/gerrit/56532
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings
Gerrit-MessageType: comment
Gerrit-Reviewer: ChromeBot <chrom...@google.com>
Gerrit-Reviewer: Don Garrett <dgar...@chromium.org>
Gerrit-Reviewer: Gilad Arnold <gar...@chromium.org>
Change subject: paycheck: truncate partitions resulting from applying a payload
......................................................................
(1 inline comment)
+2 with a suggestion.
....................................................
File host/lib/update_payload/applier.py
Line 449: old_rootfs_part=None, truncate_to_expected_size=True):
I would instead consider making this value a class property so it doesn't get passed around so much.
--
To view, visit https://gerrit.chromium.org/gerrit/56532
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings
Gerrit-Change-Id: Ibb71e4f2305ec41224afdc503168ae02c312f6fe
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/platform/dev-util
Gerrit-Branch: master
Gerrit-Owner: Gilad Arnold <gar...@chromium.org>
Gerrit-Reviewer: Chris Sosa <so...@chromium.org>
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/platform/dev-util
Gerrit-Branch: master
Gerrit-Owner: Gilad Arnold <gar...@chromium.org>
Gerrit-Reviewer: ChromeBot <chrom...@google.com>
Gerrit-Reviewer: Don Garrett <dgar...@chromium.org>
Gerrit-Reviewer: Gilad Arnold <gar...@chromium.org>
Gilad Arnold (Code Review)
May 23, 2013, 11:53:29 PM5/23/13
to ChromeBot, Chris Sosa, Don Garrett
Gilad Arnold has posted comments on this change.
Change subject: paycheck: truncate partitions resulting from applying a payload
......................................................................
Patch Set 1: (1 inline comment)
....................................................
File host/lib/update_payload/applier.py
Line 449: old_rootfs_part=None, truncate_to_expected_size=True):
Of course. Done.
--
To view, visit https://gerrit.chromium.org/gerrit/56532
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings
Gerrit-MessageType: comment
Change subject: paycheck: truncate partitions resulting from applying a payload
......................................................................
....................................................
File host/lib/update_payload/applier.py
Line 449: old_rootfs_part=None, truncate_to_expected_size=True):
--
To view, visit https://gerrit.chromium.org/gerrit/56532
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings
Gerrit-Change-Id: Ibb71e4f2305ec41224afdc503168ae02c312f6fe
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/platform/dev-util
Gerrit-Branch: master
Gerrit-Owner: Gilad Arnold <gar...@chromium.org>
Gerrit-PatchSet: 1
Gerrit-Project: chromiumos/platform/dev-util
Gerrit-Branch: master
Gerrit-Owner: Gilad Arnold <gar...@chromium.org>
Gilad Arnold (Code Review)
May 23, 2013, 11:54:50 PM5/23/13
to ChromeBot, Chris Sosa, Don Garrett
Gilad Arnold has posted comments on this change.
Change subject: paycheck: truncate partitions resulting from applying a payload
......................................................................
Patch Set 2: Verified
......................................................................
--
To view, visit https://gerrit.chromium.org/gerrit/56532
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings
Gerrit-Change-Id: Ibb71e4f2305ec41224afdc503168ae02c312f6fe
Gerrit-PatchSet: 2
Gerrit-Project: chromiumos/platform/dev-util
Gerrit-Branch: master
Gerrit-Owner: Gilad Arnold <gar...@chromium.org>
Gerrit-Branch: master
Gerrit-Owner: Gilad Arnold <gar...@chromium.org>
Gilad Arnold (Code Review)
May 24, 2013, 12:05:49 PM5/24/13
to ChromeBot, Chris Sosa, Don Garrett
Gilad Arnold has posted comments on this change.
Change subject: paycheck: truncate partitions resulting from applying a payload
......................................................................
Patch Set 2: Looks good to me, approved; Ready
......................................................................
Inheriting +2 from dgarrett
--
To view, visit https://gerrit.chromium.org/gerrit/56532
To unsubscribe, visit https://gerrit.chromium.org/gerrit/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibb71e4f2305ec41224afdc503168ae02c312f6fe
Gerrit-PatchSet: 2
Gerrit-Change-Id: Ibb71e4f2305ec41224afdc503168ae02c312f6fe
Gerrit-PatchSet: 2
Gerrit-Project: chromiumos/platform/dev-util
Gerrit-Branch: master
Gerrit-Owner: Gilad Arnold <gar...@chromium.org>
Gerrit-Branch: master
Gerrit-Owner: Gilad Arnold <gar...@chromium.org>
Reply all
Reply to author
Forward
0 new messages