Zero free space on rootfs so it's more compressible for auto-update

Zero free space on rootfs so it's more compressible for auto-update

BUG=chromium-os:10228
TEST=Build image, install image to Mario, make sure image still boots

Change-Id: Idc3c2a929515d39cb06926611eff37960074889c

Committed: http://chrome-svn/viewvc/chromeos?view=rev&revision=171321f

[thieule, 2010-12-10 23:39:42]

Separated out zero free space from previous CL
(http://codereview.chromium.org/5682006/).

PTAL

[dgarrett, 2010-12-11 00:24:22]

On 2010/12/10 23:39:42, thieule wrote:
> Separated out zero free space from previous CL
> (http://codereview.chromium.org/5682006/).
> 
> PTAL

LGTM.

[petkov, 2010-12-11 00:25:43]

http://codereview.chromium.org/5704004/diff/1/build_image
File build_image (right):

http://codereview.chromium.org/5704004/diff/1/build_image#newcode506
build_image:506: zero_freespace() {
zero_free_space?

http://codereview.chromium.org/5704004/diff/1/build_image#newcode509
build_image:509: sudo dd if=/dev/zero of="${fs_mount_point}/filler" oflag=sync
|| \
this seems strange, especially with || instead of &&
it seems to me you do want to execute dd || true
then sudo rm -f filler
then sudo sync

do you need oflag=sync?

have you tried non-default bs= values to improve performance?

http://codereview.chromium.org/5704004/diff/1/build_image#newcode694
build_image:694: # packages become smaller
s/packages/payloads/

[thieule, 2010-12-11 00:54:35]

Addressed code review feedbacks.

PTAL

http://codereview.chromium.org/5704004/diff/1/build_image
File build_image (right):

http://codereview.chromium.org/5704004/diff/1/build_image#newcode506
build_image:506: zero_freespace() {
On 2010/12/11 00:25:44, petkov wrote:
> zero_free_space?

Done.

http://codereview.chromium.org/5704004/diff/1/build_image#newcode509
build_image:509: sudo dd if=/dev/zero of="${fs_mount_point}/filler" oflag=sync
|| \
On 2010/12/11 00:25:44, petkov wrote:
> this seems strange, especially with || instead of &&
> it seems to me you do want to execute dd || true
> then sudo rm -f filler
> then sudo sync
> 
> do you need oflag=sync?
> 
> have you tried non-default bs= values to improve performance?

Done.

The oflag is required to force the writes to disk instead of residing in cache. 
Since we delete it immediately, if we don't synchronously write to disk during
dd, the zeroes may not make it all the way to disk.

As for the bs= value, you've brought up an interesting point.  I've changed it
to explicitly use bs=4096 to match the file system block size.  This value is a
good trade off between performance and completeness.  Smaller values will lower
performance while higher values may result in some blocks not getting zero'ed.

http://codereview.chromium.org/5704004/diff/1/build_image#newcode694
build_image:694: # packages become smaller
On 2010/12/11 00:25:44, petkov wrote:
> s/packages/payloads/

Done.

[petkov, 2010-12-13 18:21:31]

LGTM

Just curious -- have you measured the payload size (e.g., full payload) with and
without this patch?

[thieule, 2010-12-13 19:32:52]

The non-deterministic nature of the on-disk layout between runs of
build_image and the fact that delta_generator doesn't separate out just free
space information makes it hard to compare.

On Mon, Dec 13, 2010 at 10:21 AM, <petkov@chromium.org> wrote:

> LGTM
>
> Just curious -- have you measured the payload size (e.g., full payload)
> with and
> without this patch?
>
>
>
> http://codereview.chromium.org/5704004/
>

[adlr, 2010-12-13 20:35:44]

LGTM. thanks!