Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix redundant file backups

Browse Source 
We'd observed a bug in which an unchanged file was nevertheless
being redundantly transmitted for backup on every backup pass.
The underlying issue turns out to have been the FileBackupHelper
base implementation's logic for diffing the prior-state file
set against the current state, in the case when there had been
deletions of prior files.  In addition, there was also a
parallel bug in which file checksums were not calculated
properly in some cases, leading to at least one additional
redundant backup of the file in question.

Bug 18694053

Change-Id: Ie0dec06486b5fef4624561737019569c85d6b2a0
This commit is contained in:
Christopher Tate
2015-01-08 18:42:33 -08:00
parent a2fa3d219b
commit 43a4a8c777
3 changed files with 42 additions and 40 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
core/java/android/app/backup/BackupHelperDispatcher.java
View File

@@ -50,7 +50,6 @@ public class BackupHelperDispatcher {
Header header = new Header();
TreeMap<String,BackupHelper> helpers = (TreeMap<String,BackupHelper>)mHelpers.clone();
FileDescriptor oldStateFD = null;
FileDescriptor newStateFD = newState.getFileDescriptor();
if (oldState != null) {
oldStateFD = oldState.getFileDescriptor();

2
include/androidfw/BackupHelpers.h
View File

@@ -152,7 +152,7 @@ private:
KeyedVector<String8,FileRec> m_files;
};
#define TEST_BACKUP_HELPERS 1
//#define TEST_BACKUP_HELPERS 1
#if TEST_BACKUP_HELPERS
int backup_helper_test_empty();

79
libs/androidfw/BackupHelpers.cpp
View File

@@ -225,8 +225,6 @@ write_update_file(BackupDataWriter* dataStream, int fd, int mode, const String8&
file_metadata_v1 metadata;
char* buf = (char*)malloc(bufsize);
int crc = crc32(0L, Z_NULL, 0);
fileSize = lseek(fd, 0, SEEK_END);
lseek(fd, 0, SEEK_SET);
@@ -310,8 +308,12 @@ write_update_file(BackupDataWriter* dataStream, const String8& key, char const*
}
static int
compute_crc32(int fd)
{
compute_crc32(const char* file, FileRec* out) {
int fd = open(file, O_RDONLY);
if (fd < 0) {
return -1;
}
const int bufsize = 4*1024;
int amt;
@@ -324,8 +326,11 @@ compute_crc32(int fd)
crc = crc32(crc, (Bytef*)buf, amt);
}
close(fd);
free(buf);
return crc;
out->s.crc32 = crc;
return NO_ERROR;
}
int
@@ -353,7 +358,8 @@ back_up_files(int oldSnapshotFD, BackupDataWriter* dataStream, int newSnapshotFD
err = stat(file, &st);
if (err != 0) {
r.deleted = true;
// not found => treat as deleted
continue;
} else {
r.deleted = false;
r.s.modTime_sec = st.st_mtime;
@@ -361,12 +367,17 @@ back_up_files(int oldSnapshotFD, BackupDataWriter* dataStream, int newSnapshotFD
//r.s.modTime_nsec = st.st_mtime_nsec;
r.s.mode = st.st_mode;
r.s.size = st.st_size;
// we compute the crc32 later down below, when we already have the file open.
if (newSnapshot.indexOfKey(key) >= 0) {
LOGP("back_up_files key already in use '%s'", key.string());
return -1;
}
// compute the CRC
if (compute_crc32(file, &r) != NO_ERROR) {
ALOGW("Unable to open file %s", file);
continue;
}
}
newSnapshot.add(key, r);
}
@@ -374,49 +385,41 @@ back_up_files(int oldSnapshotFD, BackupDataWriter* dataStream, int newSnapshotFD
int n = 0;
int N = oldSnapshot.size();
int m = 0;
int M = newSnapshot.size();
while (n<N && m<fileCount) {
while (n<N && m<M) {
const String8& p = oldSnapshot.keyAt(n);
const String8& q = newSnapshot.keyAt(m);
FileRec& g = newSnapshot.editValueAt(m);
int cmp = p.compare(q);
if (g.deleted || cmp < 0) {
// file removed
if (cmp < 0) {
// file present in oldSnapshot, but not present in newSnapshot
LOGP("file removed: %s", p.string());
g.deleted = true; // They didn't mention the file, but we noticed that it's gone.
dataStream->WriteEntityHeader(p, -1);
write_delete_file(dataStream, p);
n++;
}
else if (cmp > 0) {
} else if (cmp > 0) {
// file added
LOGP("file added: %s", g.file.string());
LOGP("file added: %s crc=0x%08x", g.file.string(), g.s.crc32);
write_update_file(dataStream, q, g.file.string());
m++;
}
else {
// both files exist, check them
} else {
// same file exists in both old and new; check whether to update
const FileState& f = oldSnapshot.valueAt(n);
int fd = open(g.file.string(), O_RDONLY);
if (fd < 0) {
// We can't open the file. Don't report it as a delete either. Let the
// server keep the old version. Maybe they'll be able to deal with it
// on restore.
LOGP("Unable to open file %s - skipping", g.file.string());
} else {
g.s.crc32 = compute_crc32(fd);
LOGP("%s", q.string());
LOGP(" new: modTime=%d,%d mode=%04o size=%-3d crc32=0x%08x",
f.modTime_sec, f.modTime_nsec, f.mode, f.size, f.crc32);
LOGP(" old: modTime=%d,%d mode=%04o size=%-3d crc32=0x%08x",
g.s.modTime_sec, g.s.modTime_nsec, g.s.mode, g.s.size, g.s.crc32);
if (f.modTime_sec != g.s.modTime_sec || f.modTime_nsec != g.s.modTime_nsec
|| f.mode != g.s.mode || f.size != g.s.size || f.crc32 != g.s.crc32) {
LOGP("%s", q.string());
LOGP(" old: modTime=%d,%d mode=%04o size=%-3d crc32=0x%08x",
f.modTime_sec, f.modTime_nsec, f.mode, f.size, f.crc32);
LOGP(" new: modTime=%d,%d mode=%04o size=%-3d crc32=0x%08x",
g.s.modTime_sec, g.s.modTime_nsec, g.s.mode, g.s.size, g.s.crc32);
if (f.modTime_sec != g.s.modTime_sec || f.modTime_nsec != g.s.modTime_nsec
|| f.mode != g.s.mode || f.size != g.s.size || f.crc32 != g.s.crc32) {
int fd = open(g.file.string(), O_RDONLY);
if (fd < 0) {
ALOGE("Unable to read file for backup: %s", g.file.string());
} else {
write_update_file(dataStream, fd, g.s.mode, p, g.file.string());
close(fd);
}
close(fd);
}
n++;
m++;
@@ -425,12 +428,12 @@ back_up_files(int oldSnapshotFD, BackupDataWriter* dataStream, int newSnapshotFD
// these were deleted
while (n<N) {
dataStream->WriteEntityHeader(oldSnapshot.keyAt(n), -1);
write_delete_file(dataStream, oldSnapshot.keyAt(n));
n++;
}
// these were added
while (m<fileCount) {
while (m<M) {
const String8& q = newSnapshot.keyAt(m);
FileRec& g = newSnapshot.editValueAt(m);
write_update_file(dataStream, q, g.file.string());