You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2014/06/30 19:08:19 UTC
svn commit: r1606840 - in /subversion/trunk/subversion:
libsvn_fs_fs/cached_data.c tests/libsvn_fs_fs/fs-fs-pack-test.c
Author: stefan2
Date: Mon Jun 30 17:08:18 2014
New Revision: 1606840
URL: http://svn.apache.org/r1606840
Log:
Verify low-level checksums on FSFS parts that aren't checksummed otherwise.
Since representations (dir, file, prop) do get checksumed when being
reconstructed from deltas or read plainly, we only need to check noderevs
and changed paths lists. Also, we restrict the check to block-read mode
because only then do we get the P2L data containing the checksums for free.
Low memory, "just like f6" setups neither get the overhead nor the benefits
of this.
* subversion/libsvn_fs_fs/cached_data.c
(auto_select_stream): Drop and introduce ...
(read_item): ... instead. It will read and verify the complete item.
(block_read_changes,
block_read_noderev): Update callers.
* subversion/tests/libsvn_fs_fs/fs-fs-pack-test.c
(metadata_checksumming): New test case showing how low-level checksums
will catch otherwise undetected corruption.
(test_funcs): Register the new test.
Suggested by: zhakov
Modified:
subversion/trunk/subversion/libsvn_fs_fs/cached_data.c
subversion/trunk/subversion/tests/libsvn_fs_fs/fs-fs-pack-test.c
Modified: subversion/trunk/subversion/libsvn_fs_fs/cached_data.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/cached_data.c?rev=1606840&r1=1606839&r2=1606840&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/cached_data.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/cached_data.c Mon Jun 30 17:08:18 2014
@@ -3060,41 +3060,54 @@ block_read_contents(svn_fs_t *fs,
return SVN_NO_ERROR;
}
-/* For the given FILE in FS and wrapping FILE_STREAM return the stream
- * object in *STREAM that has the lowest performance overhead for reading
- * and parsing the item addressed by ENTRY. Use POOL for allocations.
+/* For the given REV_FILE in FS, in *STREAM return a stream covering the
+ * item specified by ENTRY. Also, verify the item's content by low-level
+ * checksum. Allocate the result in POOL.
*/
static svn_error_t *
-auto_select_stream(svn_stream_t **stream,
- svn_fs_t *fs,
- svn_fs_fs__revision_file_t *rev_file,
- svn_fs_fs__p2l_entry_t* entry,
- apr_pool_t *pool)
+read_item(svn_stream_t **stream,
+ svn_fs_t *fs,
+ svn_fs_fs__revision_file_t *rev_file,
+ svn_fs_fs__p2l_entry_t* entry,
+ apr_pool_t *pool)
{
- fs_fs_data_t *ffd = fs->fsap_data;
+ apr_uint32_t digest;
+ svn_checksum_t *expected, *actual;
+ apr_uint32_t plain_digest;
+
+ /* Read item into string buffer. */
+ svn_stringbuf_t *text = svn_stringbuf_create_ensure(entry->size, pool);
+ text->len = entry->size;
+ text->data[text->len] = 0;
+ SVN_ERR(svn_io_file_read_full2(rev_file->file, text->data, text->len,
+ NULL, NULL, pool));
+
+ /* Return (construct, calculate) stream and checksum. */
+ *stream = svn_stream_from_stringbuf(text, pool);
+ digest = svn__fnv1a_32x4(text->data, text->len);
- /* Item parser might be crossing block boundaries? */
- if (((entry->offset + entry->size + SVN__LINE_CHUNK_SIZE) ^ entry->offset)
- >= ffd->block_size)
- {
- /* Parsing items that cross block boundaries will cause the file
- buffer to be re-read and misaligned. So, read the whole block
- into memory - it must fit anyways since the parsed object will
- be even larger.
- */
- svn_stringbuf_t *text = svn_stringbuf_create_ensure(entry->size, pool);
- text->len = entry->size;
- text->data[text->len] = 0;
- SVN_ERR(svn_io_file_read_full2(rev_file->file, text->data, text->len,
- NULL, NULL, pool));
- *stream = svn_stream_from_stringbuf(text, pool);
- }
- else
- {
- *stream = rev_file->stream;
- }
+ /* Checksums will match most of the time. */
+ if (entry->fnv1_checksum == digest)
+ return SVN_NO_ERROR;
- return SVN_NO_ERROR;
+ /* Construct proper checksum objects from their digests to allow for
+ * nice error messages. */
+ plain_digest = htonl(entry->fnv1_checksum);
+ expected = svn_checksum__from_digest_fnv1a_32x4(
+ (const unsigned char *)&plain_digest, pool);
+ plain_digest = htonl(digest);
+ actual = svn_checksum__from_digest_fnv1a_32x4(
+ (const unsigned char *)&plain_digest, pool);
+
+ /* Construct the full error message with all the info we have. */
+ return svn_checksum_mismatch_err(expected, actual, pool,
+ _("Low-level checksum mismatch while reading\n"
+ "%s bytes of meta data at offset %s "
+ "for item %s in revision %ld"),
+ apr_psprintf(pool, "%" APR_OFF_T_FMT, entry->size),
+ apr_psprintf(pool, "%" APR_OFF_T_FMT, entry->offset),
+ apr_psprintf(pool, "%" APR_UINT64_T_FMT, entry->item.number),
+ entry->item.revision);
}
/* If not already cached or if MUST_READ is set, read the changed paths
@@ -3127,16 +3140,13 @@ block_read_changes(apr_array_header_t **
return SVN_NO_ERROR;
}
- SVN_ERR(auto_select_stream(&stream, fs, rev_file, entry,
- scratch_pool));
+ SVN_ERR(read_item(&stream, fs, rev_file, entry, scratch_pool));
/* read changes from revision file */
-
SVN_ERR(svn_fs_fs__read_changes(changes, stream, result_pool,
scratch_pool));
/* cache for future reference */
-
if (ffd->changes_cache)
SVN_ERR(svn_cache__set(ffd->changes_cache, &entry->item.revision,
*changes, scratch_pool));
@@ -3178,11 +3188,9 @@ block_read_noderev(node_revision_t **nod
return SVN_NO_ERROR;
}
- SVN_ERR(auto_select_stream(&stream, fs, rev_file, entry,
- scratch_pool));
+ SVN_ERR(read_item(&stream, fs, rev_file, entry, scratch_pool));
/* read node rev from revision file */
-
SVN_ERR(svn_fs_fs__read_noderev(noderev_p, stream,
result_pool, scratch_pool));
Modified: subversion/trunk/subversion/tests/libsvn_fs_fs/fs-fs-pack-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_fs_fs/fs-fs-pack-test.c?rev=1606840&r1=1606839&r2=1606840&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_fs_fs/fs-fs-pack-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_fs_fs/fs-fs-pack-test.c Mon Jun 30 17:08:18 2014
@@ -28,10 +28,12 @@
#include "../../libsvn_fs_fs/fs.h"
#include "../../libsvn_fs_fs/fs_fs.h"
+#include "svn_hash.h"
#include "svn_pools.h"
#include "svn_props.h"
#include "svn_fs.h"
#include "private/svn_string_private.h"
+#include "private/svn_string_private.h"
#include "../svn_test_fs.h"
@@ -1150,6 +1152,62 @@ recursive_locking(const svn_test_opts_t
#undef REPO_NAME
/* ------------------------------------------------------------------------ */
+
+#define REPO_NAME "metadata_checksumming"
+static svn_error_t *
+metadata_checksumming(const svn_test_opts_t *opts,
+ apr_pool_t *pool)
+{
+ svn_fs_t *fs;
+ const char *repo_path, *r0_path;
+ apr_hash_t *fs_config = apr_hash_make(pool);
+ svn_stringbuf_t *r0;
+ svn_fs_root_t *root;
+ apr_hash_t *dir;
+
+ /* Skip this test unless we are FSFS f7+ */
+ if ((strcmp(opts->fs_type, "fsfs") != 0)
+ || (opts->server_minor_version && (opts->server_minor_version < 9)))
+ return svn_error_create(SVN_ERR_TEST_SKIPPED, NULL,
+ "pre-1.9 SVN doesn't checksum metadata");
+
+ /* Create the file system to fiddle with. */
+ SVN_ERR(svn_test__create_fs(&fs, REPO_NAME, opts, pool));
+ repo_path = svn_fs_path(fs, pool);
+
+ /* Manipulate the data on disk.
+ * (change id from '0.0.*' to '1.0.*') */
+ r0_path = svn_dirent_join_many(pool, repo_path, "revs", "0", "0",
+ SVN_VA_NULL);
+ SVN_ERR(svn_stringbuf_from_file2(&r0, r0_path, pool));
+ r0->data[21] = '1';
+ SVN_ERR(svn_io_remove_file2(r0_path, FALSE, pool));
+ SVN_ERR(svn_io_file_create_binary(r0_path, r0->data, r0->len, pool));
+
+ /* Reading the corrupted data on the normal code path triggers no error.
+ * Use a separate namespace to avoid simply reading data from cache. */
+ svn_hash_sets(fs_config, SVN_FS_CONFIG_FSFS_CACHE_NS,
+ svn_uuid_generate(pool));
+ SVN_ERR(svn_fs_open2(&fs, repo_path, fs_config, pool, pool));
+ SVN_ERR(svn_fs_revision_root(&root, fs, 0, pool));
+ SVN_ERR(svn_fs_dir_entries(&dir, root, "/", pool));
+
+ /* The block-read code path uses the P2L index information and compares
+ * low-level checksums. Again, separate cache namespace. */
+ svn_hash_sets(fs_config, SVN_FS_CONFIG_FSFS_CACHE_NS,
+ svn_uuid_generate(pool));
+ svn_hash_sets(fs_config, SVN_FS_CONFIG_FSFS_BLOCK_READ, "1");
+ SVN_ERR(svn_fs_open2(&fs, repo_path, fs_config, pool, pool));
+ SVN_ERR(svn_fs_revision_root(&root, fs, 0, pool));
+ SVN_TEST_ASSERT_ERROR(svn_fs_dir_entries(&dir, root, "/", pool),
+ SVN_ERR_CHECKSUM_MISMATCH);
+
+ return SVN_NO_ERROR;
+}
+
+#undef REPO_NAME
+
+/* ------------------------------------------------------------------------ */
/* The test table. */
@@ -1188,6 +1246,8 @@ static struct svn_test_descriptor_t test
"upgrade txns started before svnadmin upgrade"),
SVN_TEST_OPTS_PASS(recursive_locking,
"prevent recursive locking"),
+ SVN_TEST_OPTS_PASS(metadata_checksumming,
+ "metadata checksums being checked"),
SVN_TEST_NULL
};
Re: svn commit: r1606840 - in /subversion/trunk/subversion:
libsvn_fs_fs/cached_data.c tests/libsvn_fs_fs/fs-fs-pack-test.c
Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Mon, Jun 30, 2014 at 7:44 PM, Bert Huijben <be...@qqmail.nl> wrote:
>
>
> > -----Original Message-----
> > From: stefan2@apache.org [mailto:stefan2@apache.org]
> > Sent: maandag 30 juni 2014 19:08
> > To: commits@subversion.apache.org
> > Subject: svn commit: r1606840 - in /subversion/trunk/subversion:
> > libsvn_fs_fs/cached_data.c tests/libsvn_fs_fs/fs-fs-pack-test.c
> >
> > Author: stefan2
> > Date: Mon Jun 30 17:08:18 2014
> > New Revision: 1606840
> >
> > URL: http://svn.apache.org/r1606840
> > Log:
> > Verify low-level checksums on FSFS parts that aren't checksummed
> > otherwise.
>
> This triggers
>
> [[[
> W: Unexpected error while running 'svnadmin verify'.
> W: EXPECTED STDOUT (regexp):
> W: | .*Verifying.*metadata.*
> W: | .*Verified revision 0.
> W: | .*Verified revision 1.
> W: | .*Error verifying revision 2.
> W: | .*Error verifying revision 3.
> W: | .*
> W: | .*Summary.*
> W: | .*r2: E160004:.*
> W: | .*r2: E160004:.*
> W: | .*r3: E160004:.*
> W: | .*r3: E160004:.*
> W: ACTUAL STDOUT:
> W: | * Verifying metadata at revision 0 ...
> W: | * Verified revision 0.
> W: | * Verified revision 1.
> W: | * Error verifying revision 2.
> W: | * Error verifying revision 3.
> W: |
> W: | -----Summary of corrupt revisions-----
> W: | r2: E200014: Low-level checksum mismatch while reading
> W: | 59 bytes of meta data at offset 0 for item 3 in revision 2:
> W: | expected: f5add73b
> W: | actual: d4d3683c
> W: |
> W: | r3: E200014: Low-level checksum mismatch while reading
> W: | 59 bytes of meta data at offset 0 for item 3 in revision 2:
> W: | expected: f5add73b
> W: | actual: d4d3683c
> W: |
> W: CWD: E:\svn-local\tests\subversion\tests\cmdline
> W: EXCEPTION: SVNLineUnequal
> Traceback (most recent call last):
> File
> "D:\local\svn-local\build\subversion\tests\cmdline\svntest\main.py", line
> 1634, in run
> rc = self.pred.run(sandbox)
> File
> "D:\local\svn-local\build\subversion\tests\cmdline\svntest\testcase.py",
> line 254, in run
> return self._delegate.run(sandbox)
> File
> "D:\local\svn-local\build\subversion\tests\cmdline\svntest\testcase.py",
> line 176, in run
> return self.func(sandbox)
> File
> "D:\local\svn-local\build\subversion\tests\cmdline\svnadmin_tests.py", line
> 2104, in verify_keep_going
> output, errput, exp_out, exp_err):
> File
> "D:\local\svn-local\build\subversion\tests\cmdline\svntest\verify.py", line
> 452, in verify_outputs
> compare_and_display_lines(message, label, expected, actual, raisable)
> File
> "D:\local\svn-local\build\subversion\tests\cmdline\svntest\verify.py", line
> 425, in compare_and_display_lines
> raise raisable
> SVNLineUnequal
> FAIL: svnadmin_tests.py 32: svnadmin verify --keep-going test
> ]]]
>
> On all buildbots.
>
Sorry, oversight from my side.
Should be fixed with r1606902.
-- Stefan^2.
RE: svn commit: r1606840 - in /subversion/trunk/subversion: libsvn_fs_fs/cached_data.c tests/libsvn_fs_fs/fs-fs-pack-test.c
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: stefan2@apache.org [mailto:stefan2@apache.org]
> Sent: maandag 30 juni 2014 19:08
> To: commits@subversion.apache.org
> Subject: svn commit: r1606840 - in /subversion/trunk/subversion:
> libsvn_fs_fs/cached_data.c tests/libsvn_fs_fs/fs-fs-pack-test.c
>
> Author: stefan2
> Date: Mon Jun 30 17:08:18 2014
> New Revision: 1606840
>
> URL: http://svn.apache.org/r1606840
> Log:
> Verify low-level checksums on FSFS parts that aren't checksummed
> otherwise.
This triggers
[[[
W: Unexpected error while running 'svnadmin verify'.
W: EXPECTED STDOUT (regexp):
W: | .*Verifying.*metadata.*
W: | .*Verified revision 0.
W: | .*Verified revision 1.
W: | .*Error verifying revision 2.
W: | .*Error verifying revision 3.
W: | .*
W: | .*Summary.*
W: | .*r2: E160004:.*
W: | .*r2: E160004:.*
W: | .*r3: E160004:.*
W: | .*r3: E160004:.*
W: ACTUAL STDOUT:
W: | * Verifying metadata at revision 0 ...
W: | * Verified revision 0.
W: | * Verified revision 1.
W: | * Error verifying revision 2.
W: | * Error verifying revision 3.
W: |
W: | -----Summary of corrupt revisions-----
W: | r2: E200014: Low-level checksum mismatch while reading
W: | 59 bytes of meta data at offset 0 for item 3 in revision 2:
W: | expected: f5add73b
W: | actual: d4d3683c
W: |
W: | r3: E200014: Low-level checksum mismatch while reading
W: | 59 bytes of meta data at offset 0 for item 3 in revision 2:
W: | expected: f5add73b
W: | actual: d4d3683c
W: |
W: CWD: E:\svn-local\tests\subversion\tests\cmdline
W: EXCEPTION: SVNLineUnequal
Traceback (most recent call last):
File "D:\local\svn-local\build\subversion\tests\cmdline\svntest\main.py", line 1634, in run
rc = self.pred.run(sandbox)
File "D:\local\svn-local\build\subversion\tests\cmdline\svntest\testcase.py", line 254, in run
return self._delegate.run(sandbox)
File "D:\local\svn-local\build\subversion\tests\cmdline\svntest\testcase.py", line 176, in run
return self.func(sandbox)
File "D:\local\svn-local\build\subversion\tests\cmdline\svnadmin_tests.py", line 2104, in verify_keep_going
output, errput, exp_out, exp_err):
File "D:\local\svn-local\build\subversion\tests\cmdline\svntest\verify.py", line 452, in verify_outputs
compare_and_display_lines(message, label, expected, actual, raisable)
File "D:\local\svn-local\build\subversion\tests\cmdline\svntest\verify.py", line 425, in compare_and_display_lines
raise raisable
SVNLineUnequal
FAIL: svnadmin_tests.py 32: svnadmin verify --keep-going test
]]]
On all buildbots.
Bert