From: Christos Tsantilas Date: Wed, 3 Aug 2011 08:30:00 +0000 (+0300) Subject: author: Measurement Factory X-Git-Tag: take08~55^2~16 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=7e8c4ee9f88657e5ccd595b344eb81efc4ac10b9;p=thirdparty%2Fsquid.git author: Measurement Factory Bug 3118: ecap_enable on forces icap_enable on We were updating [Icap|Ecap]::TheConfig even when [icap|ecap]_enable was false, which may lead to service activation for Icap or Ecap services that should be disabled. The patch removes such services from service groups before they are activated. The patch also warns the user when an adaptation group loses some but not all of its services due to the new group cleanup code. --- diff --git a/src/adaptation/Config.cc b/src/adaptation/Config.cc index f789686ff7..5db7cb351f 100644 --- a/src/adaptation/Config.cc +++ b/src/adaptation/Config.cc @@ -58,6 +58,63 @@ Adaptation::Config::newServiceConfig() const return new ServiceConfig(); } +void +Adaptation::Config::removeService(const String& service) +{ + removeRule(service); + const Groups& groups = AllGroups(); + for (unsigned int i = 0; i < groups.size(); ) { + const ServiceGroupPointer group = groups[i]; + const ServiceGroup::Store& services = group->services; + typedef ServiceGroup::Store::const_iterator SGSI; + for (SGSI it = services.begin(); it != services.end(); ++it) { + if (*it == service) { + group->removedServices.push_back(service); + group->services.prune(service); + debugs(93, 5, HERE << "adaptation service " << service << + " removed from group " << group->id); + break; + } + } + if (services.empty()) { + removeRule(group->id); + AllGroups().prune(group); + } else { + ++i; + } + } +} + +void +Adaptation::Config::removeRule(const String& id) +{ + typedef AccessRules::const_iterator ARI; + const AccessRules& rules = AllRules(); + for (ARI it = rules.begin(); it != rules.end(); ++it) { + AccessRule* rule = *it; + if (rule->groupId == id) { + debugs(93, 5, HERE << "removing access rules for:" << id); + AllRules().prune(rule); + delete (rule); + break; + } + } +} + +void +Adaptation::Config::clear() +{ + debugs(93, 3, HERE << "rules: " << AllRules().size() << ", groups: " << + AllGroups().size() << ", services: " << serviceConfigs.size()); + typedef ServiceConfigs::const_iterator SCI; + const ServiceConfigs& configs = serviceConfigs; + for (SCI cfg = configs.begin(); cfg != configs.end(); ++cfg) + removeService((*cfg)->key); + serviceConfigs.clean(); + debugs(93, 3, HERE << "rules: " << AllRules().size() << ", groups: " << + AllGroups().size() << ", services: " << serviceConfigs.size()); +} + void Adaptation::Config::parseService() { @@ -94,9 +151,14 @@ Adaptation::Config::dumpService(StoreEntry *entry, const char *name) const } } -void +bool Adaptation::Config::finalize() { + if (!onoff) { + clear(); + return false; + } + // create service reps from service configs int created = 0; @@ -120,6 +182,7 @@ Adaptation::Config::finalize() // services remember their configs; we do not have to serviceConfigs.clean(); + return true; } // poor man for_each diff --git a/src/adaptation/Config.h b/src/adaptation/Config.h index 2740f98081..68558647bf 100644 --- a/src/adaptation/Config.h +++ b/src/adaptation/Config.h @@ -54,12 +54,27 @@ public: void dumpService(StoreEntry *, const char *) const; ServicePointer findService(const String&); - virtual void finalize(); + /** + * Creates and starts the adaptation services. In the case the adaptation + * mechanism is disabled then removes any reference to the services from + * access rules and service groups, and returns false. + * \return true if the services are ready and running, false otherwise + */ + virtual bool finalize(); protected: + /// Removes any reference to the services from configuration + virtual void clear(); + /// creates service configuration object that will parse and keep cfg info virtual ServiceConfig *newServiceConfig() const; + /// Removes the given service from all service groups. + void removeService(const String& service); + + /// Removes access rules of the given service or group + void removeRule(const String& id); + private: Config(const Config &); // unsupported Config &operator =(const Config &); // unsupported diff --git a/src/adaptation/ServiceGroups.cc b/src/adaptation/ServiceGroups.cc index 4716b4c393..def467770f 100644 --- a/src/adaptation/ServiceGroups.cc +++ b/src/adaptation/ServiceGroups.cc @@ -39,6 +39,16 @@ Adaptation::ServiceGroup::finalize() // 2) warn if all-same services have different bypass status // 3) warn if there are seemingly identical services in the group // TODO: optimize by remembering ServicePointers rather than IDs + if (!removedServices.empty()) { + String s; + for (Store::iterator it = removedServices.begin(); it != removedServices.end(); ++it) { + s.append(*it); + s.append(','); + } + s.cut(s.size() - 1); + debugs(93, DBG_IMPORTANT, "Adaptation group '" << id << "' contains disabled member(s) after reconfiguration: " << s); + removedServices.clean(); + } String baselineKey; bool baselineBypass = false; diff --git a/src/adaptation/ServiceGroups.h b/src/adaptation/ServiceGroups.h index 3cf5c99e25..8d46f9c66c 100644 --- a/src/adaptation/ServiceGroups.h +++ b/src/adaptation/ServiceGroups.h @@ -56,6 +56,7 @@ public: String kind; Id id; Store services; + Store removedServices;///< the disabled services in the case ecap or icap is disabled Method method; /// based on the first added service VectPoint point; /// based on the first added service diff --git a/src/adaptation/ecap/Config.cc b/src/adaptation/ecap/Config.cc index c9eedb2093..e6cd269cae 100644 --- a/src/adaptation/ecap/Config.cc +++ b/src/adaptation/ecap/Config.cc @@ -18,12 +18,14 @@ Adaptation::Ecap::Config::~Config() { } -void +bool Adaptation::Ecap::Config::finalize() { - Adaptation::Config::finalize(); + if (!Adaptation::Config::finalize()) + return false; Host::Register(); CheckUnusedAdapterServices(AllServices()); + return true; } Adaptation::ServiceConfig * diff --git a/src/adaptation/ecap/Config.h b/src/adaptation/ecap/Config.h index e541ef4350..f9b5a56fbb 100644 --- a/src/adaptation/ecap/Config.h +++ b/src/adaptation/ecap/Config.h @@ -38,7 +38,7 @@ public: Config(); ~Config(); - virtual void finalize(); + virtual bool finalize(); protected: virtual Adaptation::ServiceConfig *newServiceConfig() const;