Daniel Shahaf wrote on Wed, Jul 13, 2011 at 17:11:34 +0300: > Stefan Sperling wrote on Wed, Jul 13, 2011 at 15:59:36 +0200: > > while repairing some bad repository breakage we noticed that > > svnadmin verify does not check whether offsets mentioned in > > rep-cache.db point to valid reps. > > Does implementing this require API changes? (extending the fs-loader.h > vtables) > > There is currently no svn_fs_verify() API.
Well, here it is. I modeled it after svn_fs_upgrade() --- i.e., put it in the library vtable and have the public API take an FS path rather than an FS object --- if that's not right someone please let me know. [[[ Add an svn_fs_verify() function to the public API, and implement it as a no-op in both backends. * subversion/include/svn_fs.h (svn_fs_verify): New. * subversion/libsvn_fs/fs-loader.h (fs_library_vtable_t): Add 'fs_verify' member. * subversion/libsvn_fs/fs-loader.c (svn_fs_verify): Implement * subversion/libsvn_fs_base/fs.c (base_verify): New no-op. * subversion/libsvn_fs_fs/fs.c (fs_verify): New, prepares svn_fs_t and forwards to... * subversion/libsvn_fs_fs/fs_fs.c (svn_fs_fs__verify): .. this new function. Currently no-op. * subversion/libsvn_fs_fs/fs_fs.h (svn_fs_fs__verify): New function. ]]] [[[ Index: subversion/include/svn_fs.h =================================================================== --- subversion/include/svn_fs.h (revision 1145819) +++ subversion/include/svn_fs.h (working copy) @@ -246,6 +246,21 @@ svn_fs_upgrade(const char *path, apr_pool_t *pool); /** + * Perform backend-specific data consistency and correctness validations + * to the Subversion filesystem located in the directory @a path. + * Use @a pool for necessary allocations. + * + * @note You probably don't want to use this directly. Take a look at + * svn_repos_verify_fs2() instead, which does non-backend-specific + * verificatiosn as well. + * + * @since New in 1.7. + */ +svn_error_t * +svn_fs_verify(const char *path, + apr_pool_t *pool); + +/** * Return, in @a *fs_type, a string identifying the back-end type of * the Subversion filesystem located in @a path. Allocate @a *fs_type * in @a pool. Index: subversion/libsvn_fs_fs/fs.c =================================================================== --- subversion/libsvn_fs_fs/fs.c (revision 1145819) +++ subversion/libsvn_fs_fs/fs.c (working copy) @@ -256,6 +256,18 @@ fs_upgrade(svn_fs_t *fs, const char *path, apr_poo } static svn_error_t * +fs_verify(svn_fs_t *fs, const char *path, apr_pool_t *pool, + apr_pool_t *common_pool) +{ + SVN_ERR(svn_fs__check_fs(fs, FALSE)); + SVN_ERR(initialize_fs_struct(fs)); + SVN_ERR(svn_fs_fs__open(fs, path, pool)); + SVN_ERR(svn_fs_fs__initialize_caches(fs, pool)); + SVN_ERR(fs_serialized_init(fs, common_pool, pool)); + return svn_fs_fs__verify(fs, pool); +} + +static svn_error_t * fs_pack(svn_fs_t *fs, const char *path, svn_fs_pack_notify_t notify_func, @@ -342,6 +354,7 @@ static fs_library_vtable_t library_vtable = { fs_open, fs_open_for_recovery, fs_upgrade, + fs_verify, fs_delete_fs, fs_hotcopy, fs_get_description, Index: subversion/libsvn_fs_fs/fs_fs.c =================================================================== --- subversion/libsvn_fs_fs/fs_fs.c (revision 1145819) +++ subversion/libsvn_fs_fs/fs_fs.c (working copy) @@ -1323,6 +1323,14 @@ svn_fs_fs__upgrade(svn_fs_t *fs, apr_pool_t *pool) } +/** Verifying. **/ +svn_error_t * +svn_fs_fs__verify(svn_fs_t *fs, apr_pool_t *pool) +{ + return SVN_NO_ERROR; /* ### Not implemented. Should dereference rep-cache */ +} + + /* SVN_ERR-like macros for dealing with recoverable errors on mutable files * * Revprops, current, and txn-current files are mutable; that is, they Index: subversion/libsvn_fs_fs/fs_fs.h =================================================================== --- subversion/libsvn_fs_fs/fs_fs.h (revision 1145819) +++ subversion/libsvn_fs_fs/fs_fs.h (working copy) @@ -38,6 +38,10 @@ svn_error_t *svn_fs_fs__open(svn_fs_t *fs, svn_error_t *svn_fs_fs__upgrade(svn_fs_t *fs, apr_pool_t *pool); +/* Verify the fsfs filesystem FS. Use POOL for temporary allocations. */ +svn_error_t *svn_fs_fs__verify(svn_fs_t *fs, + apr_pool_t *pool); + /* Copy the fsfs filesystem at SRC_PATH into a new copy at DST_PATH. Use POOL for temporary allocations. */ svn_error_t *svn_fs_fs__hotcopy(const char *src_path, Index: subversion/libsvn_fs/fs-loader.h =================================================================== --- subversion/libsvn_fs/fs-loader.h (revision 1145819) +++ subversion/libsvn_fs/fs-loader.h (working copy) @@ -86,6 +86,10 @@ typedef struct fs_library_vtable_t apr_pool_t *common_pool); svn_error_t *(*upgrade_fs)(svn_fs_t *fs, const char *path, apr_pool_t *pool, apr_pool_t *common_pool); + svn_error_t *(*verify_fs)(svn_fs_t *fs, const char *path, + /* ### notification? */ + svn_cancel_func_t cancel_func, void *cancel_baton, + apr_pool_t *pool); svn_error_t *(*delete_fs)(const char *path, apr_pool_t *pool); svn_error_t *(*hotcopy)(const char *src_path, const char *dest_path, svn_boolean_t clean, apr_pool_t *pool); Index: subversion/libsvn_fs/fs-loader.c =================================================================== --- subversion/libsvn_fs/fs-loader.c (revision 1145819) +++ subversion/libsvn_fs/fs-loader.c (working copy) @@ -459,6 +459,27 @@ svn_fs_upgrade(const char *path, apr_pool_t *pool) return svn_error_trace(err2); } +svn_error_t * +svn_fs_verify(const char *path, apr_pool_t *pool) +{ + svn_error_t *err; + svn_error_t *err2; + fs_library_vtable_t *vtable; + svn_fs_t *fs; + + SVN_ERR(fs_library_vtable(&vtable, path, pool)); + fs = fs_new(NULL, pool); + SVN_ERR(acquire_fs_mutex()); + err = vtable->verify_fs(fs, path, pool, common_pool); + err2 = release_fs_mutex(); + if (err) + { + svn_error_clear(err2); + return svn_error_trace(err); + } + return svn_error_trace(err2); +} + const char * svn_fs_path(svn_fs_t *fs, apr_pool_t *pool) { Index: subversion/libsvn_fs_base/fs.c =================================================================== --- subversion/libsvn_fs_base/fs.c (revision 1145819) +++ subversion/libsvn_fs_base/fs.c (working copy) @@ -874,6 +874,15 @@ base_upgrade(svn_fs_t *fs, const char *path, apr_p } static svn_error_t * +base_verify(svn_fs_t *fs, const char *path, apr_pool_t *pool, + apr_pool_t *common_pool) +{ + /* ### Any boilerplate needed here? */ + /* Verifying is currently a no op for BDB. */ + return SVN_NO_ERROR; +} + +static svn_error_t * base_bdb_recover(svn_fs_t *fs, svn_cancel_func_t cancel_func, void *cancel_baton, apr_pool_t *pool) @@ -1342,6 +1351,7 @@ static fs_library_vtable_t library_vtable = { base_open, base_open_for_recovery, base_upgrade, + base_verify, base_delete_fs, base_hotcopy, base_get_description, ]]]