From: robertc <> Date: Thu, 14 Sep 2006 06:51:09 +0000 (+0000) Subject: Fix bug 1218 by adding tests for the io engine of coss and ufs swapdirs after parsing... X-Git-Tag: SQUID_3_0_PRE5~86 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b7717b615563b0c01dadf1e91860257fa9c240e2;p=thirdparty%2Fsquid.git Fix bug 1218 by adding tests for the io engine of coss and ufs swapdirs after parsing completes, and providing a heuristic lookup for finding a DiskIOModule when none is specified. --- diff --git a/src/DiskIO/DiskIOModule.cc b/src/DiskIO/DiskIOModule.cc index fbabb84217..866ebbcfd0 100644 --- a/src/DiskIO/DiskIOModule.cc +++ b/src/DiskIO/DiskIOModule.cc @@ -1,6 +1,6 @@ /* - * $Id: DiskIOModule.cc,v 1.2 2006/05/29 00:15:03 robertc Exp $ + * $Id: DiskIOModule.cc,v 1.3 2006/09/14 00:51:10 robertc Exp $ * * DEBUG: section 92 Storage File System * AUTHOR: Robert Collins @@ -115,6 +115,21 @@ DiskIOModule::Find(char const *type) return NULL; } +DiskIOModule * +DiskIOModule::FindDefault() +{ + /* Best IO options are in order: */ + DiskIOModule * result; + result = Find("DiskThreads"); + if (NULL == result) + result = Find("DiskDaemon"); + if (NULL == result) + result = Find("AIO"); + if (NULL == result) + result = Find("Blocking"); + return result; +} + /* disk modules dont export anything by default */ void DiskIOModule::registerWithCacheManager(CacheManager & manager) diff --git a/src/DiskIO/DiskIOModule.h b/src/DiskIO/DiskIOModule.h index 995b92607b..fe3837315f 100644 --- a/src/DiskIO/DiskIOModule.h +++ b/src/DiskIO/DiskIOModule.h @@ -1,6 +1,6 @@ /* - * $Id: DiskIOModule.h,v 1.2 2006/05/29 00:15:03 robertc Exp $ + * $Id: DiskIOModule.h,v 1.3 2006/09/14 00:51:10 robertc Exp $ * * SQUID Web Proxy Cache http://www.squid-cache.org/ * ---------------------------------------------------------- @@ -52,6 +52,10 @@ public: static void ModuleAdd(DiskIOModule &); static void FreeAllModules(); static DiskIOModule *Find(char const *type); + /* find *any* usable disk module. This will look for the 'best' + * available module for this system. + */ + static DiskIOModule *FindDefault(); static Vector const &Modules(); typedef Vector::iterator iterator; typedef Vector::const_iterator const_iterator; diff --git a/src/Makefile.am b/src/Makefile.am index e2621664dc..7d3ffa9563 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1,7 +1,7 @@ # # Makefile for the Squid Object Cache server # -# $Id: Makefile.am,v 1.168 2006/09/13 20:09:50 serassio Exp $ +# $Id: Makefile.am,v 1.169 2006/09/14 00:51:09 robertc Exp $ # # Uncomment and customize the following to suit your needs: # @@ -1097,6 +1097,7 @@ check_PROGRAMS+= \ tests/testACLMaxUserIP \ tests/testBoilerplate \ tests/testCacheManager \ + tests/testDiskIO \ tests/testEvent \ tests/testEventLoop \ tests/testHeaders \ @@ -1378,6 +1379,18 @@ tests_testCacheManager_DEPENDENCIES = $(top_builddir)/lib/libmiscutil.a \ @SQUID_CPPUNIT_LA@ \ @ICAP_LIBS@ +tests_testDiskIO_SOURCES= \ + $(SWAP_TEST_SOURCES) \ + tests/testDiskIO.cc \ + tests/testDiskIO.h \ + tests/testMain.cc +tests_testDiskIO_LDADD= \ + $(SWAP_TEST_LDADD) \ + @SSLLIB@ +tests_testDiskIO_LDFLAGS = $(LIBADD_DL) +tests_testDiskIO_DEPENDENCIES = $(top_builddir)/lib/libmiscutil.a \ + @SQUID_CPPUNIT_LA@ + ## Tests of the Even module. tests_testEvent_SOURCES = \ debug.cc \ diff --git a/src/fs/coss/store_dir_coss.cc b/src/fs/coss/store_dir_coss.cc index c4fdc6b7e4..8bec228407 100644 --- a/src/fs/coss/store_dir_coss.cc +++ b/src/fs/coss/store_dir_coss.cc @@ -1,6 +1,6 @@ /* - * $Id: store_dir_coss.cc,v 1.67 2006/09/03 04:12:03 hno Exp $ + * $Id: store_dir_coss.cc,v 1.68 2006/09/14 00:51:10 robertc Exp $ * vim: set et : * * DEBUG: section 47 Store COSS Directory Routines @@ -1056,6 +1056,9 @@ CossSwapDir::parse(int anIndex, char *aPath) parseOptions(0); + if (NULL == io) + changeIO(DiskIOModule::FindDefault()); + /* Enforce maxobjsize being set to something */ if (max_objsize == -1) fatal("COSS requires max-size to be set to something other than -1!\n"); diff --git a/src/fs/ufs/StoreFSufs.h b/src/fs/ufs/StoreFSufs.h index f622a83cb2..74c3d19100 100644 --- a/src/fs/ufs/StoreFSufs.h +++ b/src/fs/ufs/StoreFSufs.h @@ -1,6 +1,6 @@ /* - * $Id: StoreFSufs.h,v 1.4 2004/12/21 17:28:29 robertc Exp $ + * $Id: StoreFSufs.h,v 1.5 2006/09/14 00:51:12 robertc Exp $ * * SQUID Web Proxy Cache http://www.squid-cache.org/ * ---------------------------------------------------------- @@ -39,8 +39,12 @@ #include "DiskIO/DiskIOModule.h" +/* core UFS class. This template provides compile time aliases for + * ufs/aufs/diskd to ease configuration conversion - each becomes a + * StoreFS module whose createSwapDir method parameterises the common + * UFSSwapDir with an IO module instance. + */ template - class StoreFSufs : public StoreFileSystem { @@ -61,9 +65,6 @@ protected: DiskIOModule *IO; char const *moduleName; char const *label; - -private: - void checkIO(); }; template @@ -84,8 +85,6 @@ SwapDir * StoreFSufs::createSwapDir() { C *result = new C(type(), moduleName); - checkIO(); - result->IO = new UFSStrategy(IO->createStrategy()); return result; } @@ -104,14 +103,4 @@ StoreFSufs::setup() initialised = true; } -template -void -StoreFSufs::checkIO() -{ - if (IO) - return; - - IO = DiskIOModule::Find(moduleName); -} - #endif /* SQUID_STOREFSUFS_H */ diff --git a/src/fs/ufs/store_dir_ufs.cc b/src/fs/ufs/store_dir_ufs.cc index d04d0044ee..bd0df6f8e8 100644 --- a/src/fs/ufs/store_dir_ufs.cc +++ b/src/fs/ufs/store_dir_ufs.cc @@ -1,6 +1,6 @@ /* - * $Id: store_dir_ufs.cc,v 1.75 2006/08/19 12:31:24 robertc Exp $ + * $Id: store_dir_ufs.cc,v 1.76 2006/09/14 00:51:12 robertc Exp $ * * DEBUG: section 47 Store Directory Routines * AUTHOR: Duane Wessels @@ -196,7 +196,9 @@ UFSSwapDir::getOptionTree() const currentIOOptions->options.push_back(new ConfigOptionAdapter(*const_cast(this), &UFSSwapDir::optionIOParse, &UFSSwapDir::optionIODump)); - ConfigOption *ioOptions = IO->io->getOptionTree(); + ConfigOption *ioOptions = NULL; + + IO->io->getOptionTree(); if (ioOptions) currentIOOptions->options.push_back(ioOptions); @@ -223,7 +225,6 @@ UFSSwapDir::init() "\tfor details. Run 'squid -z' to create swap directories\n" "\tif needed, or if running Squid for the first time."; IO->init(); - initBitmap(); if (verifyCacheDirs()) fatal(errmsg); @@ -248,7 +249,13 @@ UFSSwapDir::create() createSwapSubDirs(); } -UFSSwapDir::UFSSwapDir(char const *aType, const char *anIOType) : SwapDir(aType), IO(NULL), map(NULL), suggest(0), swaplog_fd (-1), currentIOOptions(new ConfigOptionVector()), ioType(xstrdup(anIOType)) {} +UFSSwapDir::UFSSwapDir(char const *aType, const char *anIOType) : SwapDir(aType), IO(NULL), map(file_map_create()), suggest(0), swaplog_fd (-1), currentIOOptions(new ConfigOptionVector()), ioType(xstrdup(anIOType)) +{ + /* modulename is only set to disk modules that are built, by configure, + * so the Find call should never return NULL here. + */ + IO = new UFSStrategy(DiskIOModule::Find(anIOType)->createStrategy()); +} UFSSwapDir::~UFSSwapDir() { @@ -482,26 +489,6 @@ UFSSwapDir::mapBitAllocate() return fn; } -/* - * Initialise the ufs bitmap - * - * If there already is a bitmap, and the numobjects is larger than currently - * configured, we allocate a new bitmap and 'grow' the old one into it. - */ -void -UFSSwapDir::initBitmap() -{ - if (map == NULL) { - /* First time */ - map = file_map_create(); - } else if (map->max_n_files) { - /* it grew, need to expand */ - /* XXX We don't need it anymore .. */ - } - - /* else it shrunk, and we leave the old one in place */ -} - char * UFSSwapDir::swapSubDir(int subdirn)const { diff --git a/src/fs/ufs/ufscommon.h b/src/fs/ufs/ufscommon.h index c537f61f58..2d84685f04 100644 --- a/src/fs/ufs/ufscommon.h +++ b/src/fs/ufs/ufscommon.h @@ -1,6 +1,6 @@ /* - * $Id: ufscommon.h,v 1.8 2006/08/07 02:28:26 robertc Exp $ + * $Id: ufscommon.h,v 1.9 2006/09/14 00:51:12 robertc Exp $ * * SQUID Web Proxy Cache http://www.squid-cache.org/ * ---------------------------------------------------------- @@ -130,7 +130,6 @@ private: bool pathIsDirectory(const char *path)const; int swaplog_fd; static EVH CleanEvent; - void initBitmap(); bool verifyCacheDirs(); void rebuild(); int createDirectory(const char *path, int); @@ -148,7 +147,11 @@ private: #include "RefCount.h" #include "DiskIO/IORequestor.h" -/* UFS dir specific IO calls */ +/* UFS dir specific IO calls + * + * This should be whittled away - DiskIOModule should be providing the + * entire needed api. + */ class DiskIOStrategy; @@ -187,10 +190,11 @@ public: /* cachemgr output on the IO instance stats */ virtual void statfs(StoreEntry & sentry)const; + /* The io strategy in use */ + DiskIOStrategy *io; protected: friend class UFSSwapDir; - DiskIOStrategy *io; }; /* Common ufs-store-dir logic */ diff --git a/src/tests/testCoss.cc b/src/tests/testCoss.cc index 38e68b0125..df98c302e4 100644 --- a/src/tests/testCoss.cc +++ b/src/tests/testCoss.cc @@ -267,3 +267,33 @@ testCoss::testCossSearch() if (0 > system ("rm -rf " TESTDIR)) throw std::runtime_error("Failed to clean test work directory"); } + +/* The COSS store should always configure an IO engine even if none is + * supplied on the configuration line. + */ +void +testCoss::testDefaultEngine() +{ + /* boring common test boilerplate */ + if (0 > system ("rm -rf " TESTDIR)) + throw std::runtime_error("Failed to clean test work directory"); + + StorePointer aRoot (new StoreController); + Store::Root(aRoot); + SwapDirPointer aStore (new CossSwapDir()); + addSwapDir(aStore); + commonInit(); + + char *path=xstrdup(TESTDIR); + char *config_line=xstrdup("foo 100 max-size=102400 block-size=512"); + strtok(config_line, w_space); + aStore->parse(0, path); + safe_free(path); + safe_free(config_line); + CPPUNIT_ASSERT(aStore->io != NULL); + + free_cachedir(&Config.cacheSwap); + Store::Root(NULL); + if (0 > system ("rm -rf " TESTDIR)) + throw std::runtime_error("Failed to clean test work directory"); +} diff --git a/src/tests/testCoss.h b/src/tests/testCoss.h index 5eaadf1cc6..70a159a35b 100644 --- a/src/tests/testCoss.h +++ b/src/tests/testCoss.h @@ -13,6 +13,7 @@ class testCoss : public CPPUNIT_NS::TestFixture CPPUNIT_TEST_SUITE( testCoss ); CPPUNIT_TEST( testCossCreate ); CPPUNIT_TEST( testCossSearch ); + CPPUNIT_TEST( testDefaultEngine ); CPPUNIT_TEST_SUITE_END(); public: @@ -21,6 +22,7 @@ protected: void commonInit(); void testCossCreate(); void testCossSearch(); + void testDefaultEngine(); }; #endif diff --git a/src/tests/testUfs.cc b/src/tests/testUfs.cc index c3e724dbb0..59fa357ed4 100644 --- a/src/tests/testUfs.cc +++ b/src/tests/testUfs.cc @@ -38,27 +38,12 @@ searchCallback(void *cbdata) } void -testUfs::testUfsSearch() +testUfs::commonInit() { - /* test sequence - * make a valid working ufs swapdir - * put two entries in it and sync logs - * search the ufs dir - * check the entries we find are what we want - */ - - if (0 > system ("rm -rf " TESTDIR)) - throw std::runtime_error("Failed to clean test work directory"); + static bool inited = false; - StorePointer aRoot (new StoreController); - - Store::Root(aRoot); - - SwapDirPointer aStore (new UFSSwapDir("ufs", "Blocking")); - - aStore->IO = new UFSStrategy(DiskIOModule::Find("Blocking")->createStrategy()); - - addSwapDir(aStore); + if (inited) + return; Config.Store.avgObjectSize = 1024; @@ -83,8 +68,36 @@ testUfs::testUfsSearch() httpReplyInitModule(); /* must go before accepting replies */ + inited = true; +} + +void +testUfs::testUfsSearch() +{ + /* test sequence + * make a valid working ufs swapdir + * put two entries in it and sync logs + * search the ufs dir + * check the entries we find are what we want + */ + + if (0 > system ("rm -rf " TESTDIR)) + throw std::runtime_error("Failed to clean test work directory"); + + StorePointer aRoot (new StoreController); + + Store::Root(aRoot); + + SwapDirPointer aStore (new UFSSwapDir("ufs", "Blocking")); + + aStore->IO = new UFSStrategy(DiskIOModule::Find("Blocking")->createStrategy()); + + addSwapDir(aStore); + + commonInit(); mem_policy = createRemovalPolicy(Config.replPolicy); + char *path=xstrdup(TESTDIR); char *config_line=xstrdup("foo 100 1 1"); @@ -194,6 +207,43 @@ testUfs::testUfsSearch() /* todo: here we should test a dirty rebuild */ + Store::Root(NULL); + safe_free(Config.replPolicy->type); + delete Config.replPolicy; + + if (0 > system ("rm -rf " TESTDIR)) + throw std::runtime_error("Failed to clean test work directory"); +} + +/* The UFS store should always configure an IO engine even if none is + * supplied on the configuration line. + */ +void +testUfs::testUfsDefaultEngine() +{ + /* boring common test boilerplate */ + if (0 > system ("rm -rf " TESTDIR)) + throw std::runtime_error("Failed to clean test work directory"); + + StorePointer aRoot (new StoreController); + Store::Root(aRoot); + SwapDirPointer aStore (new UFSSwapDir("ufs", "Blocking")); + addSwapDir(aStore); + commonInit(); + Config.replPolicy = new RemovalPolicySettings; + Config.replPolicy->type = xstrdup ("lru"); + mem_policy = createRemovalPolicy(Config.replPolicy); + + char *path=xstrdup(TESTDIR); + char *config_line=xstrdup("foo 100 1 1"); + strtok(config_line, w_space); + aStore->parse(0, path); + safe_free(path); + safe_free(config_line); + CPPUNIT_ASSERT(aStore->IO->io != NULL); + + free_cachedir(&Config.cacheSwap); + Store::Root(NULL); safe_free(Config.replPolicy->type); delete Config.replPolicy; diff --git a/src/tests/testUfs.h b/src/tests/testUfs.h index 3cd86f10c8..1d2a0e2922 100644 --- a/src/tests/testUfs.h +++ b/src/tests/testUfs.h @@ -12,12 +12,15 @@ class testUfs : public CPPUNIT_NS::TestFixture { CPPUNIT_TEST_SUITE( testUfs ); CPPUNIT_TEST( testUfsSearch ); + CPPUNIT_TEST( testUfsDefaultEngine ); CPPUNIT_TEST_SUITE_END(); public: protected: + void commonInit(); void testUfsSearch(); + void testUfsDefaultEngine(); }; #endif