AU: Start checkpointing update progress.

AU: Start checkpointing update progress.

Checkpoint the manifest metadata size and the update
check response hash in the preference store. Also checkpoint
the next operation and data offset.
Add methods for getting/setting hash context.

BUG=7390, 7394
TEST=unit tests

Change-Id: I25291bf598ac9b0f1033e11cfe59df45b1f6eeab

Committed: http://chrome-svn/viewvc/chromeos?view=rev&revision=73058b4

[petkov, 2010-10-06 18:49:44]

Doing this one step/patch at a time...

[petkov, 2010-10-06 20:43:31]

Added more code and updated the CL description.

[adlr, 2010-10-06 21:16:38]

http://codereview.chromium.org/3521016/diff/4001/5001
File delta_performer.cc (right):

http://codereview.chromium.org/3521016/diff/4001/5001#newcode470
delta_performer.cc:470: //
TEST_AND_RETURN_FALSE(prefs_->SetString(kPrefsUpdateStateSignedSHA256Context,
80 cols

http://codereview.chromium.org/3521016/diff/4001/5007
File generate_delta_main.cc (right):

http://codereview.chromium.org/3521016/diff/4001/5007#newcode73
generate_delta_main.cc:73:
CHECK(file_util::CreateNewTempDirectory("update_engine_prefs", &prefs_dir));
it might be handy for testing to be able to kill this process while it's
applying a delta on the host and then resume, so for that it would be good if
the prefs directory were the same each time. will this do that? (I'm not
familiar w/ file_util::CreateNewTempDirectory). Do you think that is worth
doing?

http://codereview.chromium.org/3521016/diff/4001/5009
File omaha_hash_calculator.h (right):

http://codereview.chromium.org/3521016/diff/4001/5009#newcode44
omaha_hash_calculator.h:44: std::string GetContext() const;
if this string may contain null chars, it's probably worth a comment.

http://codereview.chromium.org/3521016/diff/4001/5011
File omaha_response_handler_action.cc (right):

http://codereview.chromium.org/3521016/diff/4001/5011#newcode32
omaha_response_handler_action.cc:32: LOG_IF(WARNING,
!prefs_->SetString(kPrefsUpdateCheckResponseHash,
should you mark invalid the number of ops performed thus far?

that seems to be the one key piece of state in the prefs for deciding if we
should resume.

[petkov, 2010-10-06 23:08:33]

Thanks for the review. PTAL -- I've added the SHA-256 state to the context --
there was no change in the performance from it.

http://codereview.chromium.org/3521016/diff/4001/5001
File delta_performer.cc (right):

http://codereview.chromium.org/3521016/diff/4001/5001#newcode470
delta_performer.cc:470: //
TEST_AND_RETURN_FALSE(prefs_->SetString(kPrefsUpdateStateSignedSHA256Context,
On 2010/10/06 21:16:38, adlr wrote:
> 80 cols

Done -- and TODO removed after rebasing.

http://codereview.chromium.org/3521016/diff/4001/5007
File generate_delta_main.cc (right):

http://codereview.chromium.org/3521016/diff/4001/5007#newcode73
generate_delta_main.cc:73:
CHECK(file_util::CreateNewTempDirectory("update_engine_prefs", &prefs_dir));
On 2010/10/06 21:16:38, adlr wrote:
> it might be handy for testing to be able to kill this process while it's
> applying a delta on the host and then resume, so for that it would be good if
> the prefs directory were the same each time. will this do that? (I'm not
> familiar w/ file_util::CreateNewTempDirectory). Do you think that is worth
> doing?

I added a flag that defaults to /tmp/update_engine_prefs.

http://codereview.chromium.org/3521016/diff/4001/5009
File omaha_hash_calculator.h (right):

http://codereview.chromium.org/3521016/diff/4001/5009#newcode44
omaha_hash_calculator.h:44: std::string GetContext() const;
On 2010/10/06 21:16:38, adlr wrote:
> if this string may contain null chars, it's probably worth a comment.

Done.

http://codereview.chromium.org/3521016/diff/4001/5011
File omaha_response_handler_action.cc (right):

http://codereview.chromium.org/3521016/diff/4001/5011#newcode32
omaha_response_handler_action.cc:32: LOG_IF(WARNING,
!prefs_->SetString(kPrefsUpdateCheckResponseHash,
On 2010/10/06 21:16:38, adlr wrote:
> should you mark invalid the number of ops performed thus far?
> 
> that seems to be the one key piece of state in the prefs for deciding if we
> should resume.

I've updated the TODO to be explicit about this. But yes, this would be a place
where we need to decide whether to resume or not, and reset the state if not.
This will be a subsequent CL.

[adlr, 2010-10-06 23:22:02]

LGTM

http://codereview.chromium.org/3521016/diff/4001/5007
File generate_delta_main.cc (right):

http://codereview.chromium.org/3521016/diff/4001/5007#newcode92
generate_delta_main.cc:92: file_util::Delete(prefs_dir, true);
deleting is probably fine.. either way. if you killed it, of course the delete
wouldn't run

[petkov, 2010-10-06 23:24:56]

Thanks, I'll push soon.

http://codereview.chromium.org/3521016/diff/4001/5007
File generate_delta_main.cc (right):

http://codereview.chromium.org/3521016/diff/4001/5007#newcode92
generate_delta_main.cc:92: file_util::Delete(prefs_dir, true);
On 2010/10/06 23:22:03, adlr wrote:
> deleting is probably fine.. either way. if you killed it, of course the delete
> wouldn't run

Let's not delete it for now -- this way we can also test if test state from an
update that completes fully messes up the next update.