AU: delta compress the kernel partition

AU: delta compress the kernel partition

Add kernel partition to proto buf. Change delta generator to compress
the kernel partition and the delta performer to perform operations to
update the kernel partition. Update unittests to test it.

Also, remove checksums from the protobuf while we're at it, since the
partitions themselves will contains checksums.

BUG=None
TEST=Attached unittest

[Daniel Erat, 2010-04-30 01:04:39]

LGTM with some comments

http://codereview.chromium.org/1819002/diff/1/2
File src/platform/update_engine/delta_diff_generator.cc (right):

http://codereview.chromium.org/1819002/diff/1/2#newcode442
src/platform/update_engine/delta_diff_generator.cc:442:
TEST_AND_RETURN_FALSE(old_part_size >= 0);
does it make sense to test this before generating the diff?

http://codereview.chromium.org/1819002/diff/1/2#newcode797
src/platform/update_engine/delta_diff_generator.cc:797: LOG(INFO) << "here";
delete this

http://codereview.chromium.org/1819002/diff/1/4
File src/platform/update_engine/delta_performer.cc (right):

http://codereview.chromium.org/1819002/diff/1/4#newcode118
src/platform/update_engine/delta_performer.cc:118: bool success =
OpenFile(kernel_path, &kernel_fd_, &err);
log errno on failure?

http://codereview.chromium.org/1819002/diff/1/4#newcode140
src/platform/update_engine/delta_performer.cc:140: return -err;
what's the reason to return errno from OpenKernel() and Close() instead of just
including it in the error message on failure and returning true/false?

http://codereview.chromium.org/1819002/diff/1/4#newcode187
src/platform/update_engine/delta_performer.cc:187: while (next_operation_ <
total_operations) {
next_operation_num_ would be a better name

http://codereview.chromium.org/1819002/diff/1/5
File src/platform/update_engine/delta_performer.h (right):

http://codereview.chromium.org/1819002/diff/1/5#newcode29
src/platform/update_engine/delta_performer.h:29: // Opens the Kernel. Should be
called before or after Open(), but before
s/Kernel/kernel/

http://codereview.chromium.org/1819002/diff/1/9
File src/platform/update_engine/generate_delta_main.cc (right):

http://codereview.chromium.org/1819002/diff/1/9#newcode87
src/platform/update_engine/generate_delta_main.cc:87: LOG(FATAL) << "Missing
required argument(s)";
something like this would probably be more helpful:

CHECK(!FLAGS_old_dir.empty());
CHECK(!FLAGS_new_dir.empty());
etc.

[Daniel Erat, 2010-04-30 01:04:42]

LGTM with some comments

[adlr, 2010-04-30 04:27:11]

I've made the changes, and if I don't hear anything I'll submit in the morning.
I'd submit now, given your LGTM, but I'm about to head out and don't want to
break the build.

http://codereview.chromium.org/1819002/diff/1/2
File src/platform/update_engine/delta_diff_generator.cc (right):

http://codereview.chromium.org/1819002/diff/1/2#newcode442
src/platform/update_engine/delta_diff_generator.cc:442:
TEST_AND_RETURN_FALSE(old_part_size >= 0);
On 2010/04/30 01:04:40, Daniel Erat wrote:
> does it make sense to test this before generating the diff?

Yeah, I'd like to keep these here b/c it's right near where it's used, but i
added a similar check that occurs earlier so that it can fail faster.

http://codereview.chromium.org/1819002/diff/1/2#newcode797
src/platform/update_engine/delta_diff_generator.cc:797: LOG(INFO) << "here";
On 2010/04/30 01:04:40, Daniel Erat wrote:
> delete this

Done.

http://codereview.chromium.org/1819002/diff/1/4
File src/platform/update_engine/delta_performer.cc (right):

http://codereview.chromium.org/1819002/diff/1/4#newcode118
src/platform/update_engine/delta_performer.cc:118: bool success =
OpenFile(kernel_path, &kernel_fd_, &err);
On 2010/04/30 01:04:40, Daniel Erat wrote:
> log errno on failure?

Done. (within OpenFile())

http://codereview.chromium.org/1819002/diff/1/4#newcode140
src/platform/update_engine/delta_performer.cc:140: return -err;
On 2010/04/30 01:04:40, Daniel Erat wrote:
> what's the reason to return errno from OpenKernel() and Close() instead of
just
> including it in the error message on failure and returning true/false?

It's part of the FileWriter API. I'll log the specific error here, it's another
question if we should change the API to return a bool for Close().

http://codereview.chromium.org/1819002/diff/1/4#newcode187
src/platform/update_engine/delta_performer.cc:187: while (next_operation_ <
total_operations) {
On 2010/04/30 01:04:40, Daniel Erat wrote:
> next_operation_num_ would be a better name

Done.

http://codereview.chromium.org/1819002/diff/1/5
File src/platform/update_engine/delta_performer.h (right):

http://codereview.chromium.org/1819002/diff/1/5#newcode29
src/platform/update_engine/delta_performer.h:29: // Opens the Kernel. Should be
called before or after Open(), but before
On 2010/04/30 01:04:40, Daniel Erat wrote:
> s/Kernel/kernel/

Done.

http://codereview.chromium.org/1819002/diff/1/9
File src/platform/update_engine/generate_delta_main.cc (right):

http://codereview.chromium.org/1819002/diff/1/9#newcode87
src/platform/update_engine/generate_delta_main.cc:87: LOG(FATAL) << "Missing
required argument(s)";
On 2010/04/30 01:04:40, Daniel Erat wrote:
> something like this would probably be more helpful:
> 
> CHECK(!FLAGS_old_dir.empty());
> CHECK(!FLAGS_new_dir.empty());
> etc.

Done.

[Daniel Erat, 2010-04-30 05:29:14]

Thanks, sounds good.

On Thu, Apr 29, 2010 at 9:27 PM,  <adlr@chromium.org> wrote:
> I've made the changes, and if I don't hear anything I'll submit in the
> morning.
> I'd submit now, given your LGTM, but I'm about to head out and don't want to
> break the build.
>
>
> http://codereview.chromium.org/1819002/diff/1/2
> File src/platform/update_engine/delta_diff_generator.cc (right):
>
> http://codereview.chromium.org/1819002/diff/1/2#newcode442
> src/platform/update_engine/delta_diff_generator.cc:442:
> TEST_AND_RETURN_FALSE(old_part_size >= 0);
> On 2010/04/30 01:04:40, Daniel Erat wrote:
>>
>> does it make sense to test this before generating the diff?
>
> Yeah, I'd like to keep these here b/c it's right near where it's used,
> but i added a similar check that occurs earlier so that it can fail
> faster.
>
> http://codereview.chromium.org/1819002/diff/1/2#newcode797
> src/platform/update_engine/delta_diff_generator.cc:797: LOG(INFO) <<
> "here";
> On 2010/04/30 01:04:40, Daniel Erat wrote:
>>
>> delete this
>
> Done.
>
> http://codereview.chromium.org/1819002/diff/1/4
> File src/platform/update_engine/delta_performer.cc (right):
>
> http://codereview.chromium.org/1819002/diff/1/4#newcode118
> src/platform/update_engine/delta_performer.cc:118: bool success =
> OpenFile(kernel_path, &kernel_fd_, &err);
> On 2010/04/30 01:04:40, Daniel Erat wrote:
>>
>> log errno on failure?
>
> Done. (within OpenFile())
>
> http://codereview.chromium.org/1819002/diff/1/4#newcode140
> src/platform/update_engine/delta_performer.cc:140: return -err;
> On 2010/04/30 01:04:40, Daniel Erat wrote:
>>
>> what's the reason to return errno from OpenKernel() and Close()
>
> instead of just
>>
>> including it in the error message on failure and returning true/false?
>
> It's part of the FileWriter API. I'll log the specific error here, it's
> another question if we should change the API to return a bool for
> Close().
>
> http://codereview.chromium.org/1819002/diff/1/4#newcode187
> src/platform/update_engine/delta_performer.cc:187: while
> (next_operation_ < total_operations) {
> On 2010/04/30 01:04:40, Daniel Erat wrote:
>>
>> next_operation_num_ would be a better name
>
> Done.
>
> http://codereview.chromium.org/1819002/diff/1/5
> File src/platform/update_engine/delta_performer.h (right):
>
> http://codereview.chromium.org/1819002/diff/1/5#newcode29
> src/platform/update_engine/delta_performer.h:29: // Opens the Kernel.
> Should be called before or after Open(), but before
> On 2010/04/30 01:04:40, Daniel Erat wrote:
>>
>> s/Kernel/kernel/
>
> Done.
>
> http://codereview.chromium.org/1819002/diff/1/9
> File src/platform/update_engine/generate_delta_main.cc (right):
>
> http://codereview.chromium.org/1819002/diff/1/9#newcode87
> src/platform/update_engine/generate_delta_main.cc:87: LOG(FATAL) <<
> "Missing required argument(s)";
> On 2010/04/30 01:04:40, Daniel Erat wrote:
>>
>> something like this would probably be more helpful:
>
>> CHECK(!FLAGS_old_dir.empty());
>> CHECK(!FLAGS_new_dir.empty());
>> etc.
>
> Done.
>
> http://codereview.chromium.org/1819002/show
>