Hi Hung,
Reviewed with two minor comments, but not tested.
You don't need to send the code to another review.
Ack from me.
PBE configured functions and PBE non-configured functions in imm_pbe_load.cc
should be synchronized, otherwise it won't be possible to compile the code
without "--enable-imm-pbe" flag
int loadImmFromPbe(void* pbeHandle, bool preload, bool* pbeCorrupted) <== PBE
enabled
int loadImmFromPbe(void* pbeHandle, bool preload) <== PBE disabled
In the code below, should be "goto bailout;" in the place of removed "exit(1);".
@@ -784,14 +790,15 @@ int loadImmFromPbe(void* pbeHandle, bool
The "other user" is likely to be a lingering PBE (on the
other SC) or a lingering POSIX
file lock in an NFS client cache.
*/
+ *pbeCorrupted = false;
sqlite3_close(dbHandle);
- exit(1);
}
...... the code could be something like:
*pbeCorrupted = false;
goto bailout; // Here we have sqlite3_close(dbHandle); , so '
sqlite3_close(dbHandle);' can also be removed
}
Best regards,
Zoran
-----Original Message-----
From: Hung Nguyen [mailto:[email protected]]
Sent: Friday, November 27, 2015 12:05 PM
To: Zoran Milinkovic; [email protected]
Cc: [email protected]
Subject: [PATCH 1 of 1] imm: Rename pbe file only when it's corrupted [#1611]
osaf/services/saf/immsv/immloadd/imm_loader.cc | 34 +++++++++--
osaf/services/saf/immsv/immloadd/imm_loader.hh | 10 ++-
osaf/services/saf/immsv/immloadd/imm_pbe_load.cc | 70 ++++++++++++++++++-----
3 files changed, 89 insertions(+), 25 deletions(-)
Failure in sqlite3 api will be considered as pbe file being corrupted.
For failure in IMM API, the error code will be checked to detect if pbe is
corrupted or not.
diff --git a/osaf/services/saf/immsv/immloadd/imm_loader.cc
b/osaf/services/saf/immsv/immloadd/imm_loader.cc
--- a/osaf/services/saf/immsv/immloadd/imm_loader.cc
+++ b/osaf/services/saf/immsv/immloadd/imm_loader.cc
@@ -482,7 +482,8 @@ bool createImmObject(SaImmClassNameT cla
char * objectName,
std::list<SaImmAttrValuesT_2> *attrValuesList,
SaImmCcbHandleT ccbHandle,
- std::map<std::string, SaImmAttrValuesT_2> *classRDNMap)
+ std::map<std::string, SaImmAttrValuesT_2> *classRDNMap,
+ bool* pbeCorrupted)
{
SaNameT parentName;
SaImmAttrValuesT_2** attrValues;
@@ -493,6 +494,7 @@ bool createImmObject(SaImmClassNameT cla
TRACE_ENTER2("CREATE IMM OBJECT %s, %s", className, objectName);
+ if (pbeCorrupted) *pbeCorrupted = true;
TRACE("attrValuesList size:%zu clasRDNMap size:%zu",
attrValuesList->size(),
classRDNMap?classRDNMap->size():0);
@@ -589,9 +591,19 @@ bool createImmObject(SaImmClassNameT cla
if (SA_AIS_OK != errorCode)
{
- LOG_ER("Failed to create object err: %d, class: %s, dn: '%s'. "
- "Check for duplicate attributes, or trace osafimmloadd",
- errorCode, className, objectName);
+ if (pbeCorrupted &&
+ ((errorCode != SA_AIS_ERR_INVALID_PARAM &&
+ errorCode != SA_AIS_ERR_BAD_OPERATION &&
+ errorCode != SA_AIS_ERR_NOT_EXIST &&
+ errorCode != SA_AIS_ERR_EXIST &&
+ errorCode != SA_AIS_ERR_FAILED_OPERATION &&
+ errorCode != SA_AIS_ERR_NAME_TOO_LONG) ||
+ (errorCode == SA_AIS_ERR_FAILED_OPERATION &&
!isValidationAborted(ccbHandle)))) {
+ *pbeCorrupted = false;
+ }
+ LOG_ER("Failed to create object err: %d, class: %s, dn: '%s'. "
+ "Check for duplicate attributes, or trace osafimmloadd",
+ errorCode, className, objectName);
rc = false;
goto freemem;
}
@@ -631,7 +643,8 @@ freemem:
bool createImmClass(SaImmHandleT immHandle,
const SaImmClassNameT className,
SaImmClassCategoryT classCategory,
- std::list<SaImmAttrDefinitionT_2>* attrDefinitions)
+ std::list<SaImmAttrDefinitionT_2>* attrDefinitions,
+ bool* pbeCorrupted)
{
SaImmAttrDefinitionT_2** attrDefinition;
SaAisErrorT errorCode = SA_AIS_OK;
@@ -640,6 +653,7 @@ bool createImmClass(SaImmHandleT immHand
TRACE_ENTER2("CREATING IMM CLASS %s", className);
+ if (pbeCorrupted) *pbeCorrupted = true;
/* Set the attrDefinition array */
attrDefinition = (SaImmAttrDefinitionT_2**)
calloc((attrDefinitions->size() + 1),
@@ -680,6 +694,11 @@ bool createImmClass(SaImmHandleT immHand
if (SA_AIS_OK != errorCode)
{
+ if (pbeCorrupted &&
+ errorCode != SA_AIS_ERR_INVALID_PARAM &&
+ errorCode != SA_AIS_ERR_EXIST) {
+ *pbeCorrupted = false;
+ }
LOG_ER("FAILED to create IMM class %s, err:%d", className, errorCode);
rc = false;
goto freemem;
@@ -2727,6 +2746,7 @@ int main(int argc, char* argv[])
const char* pbe_file = getenv("IMMSV_PBE_FILE");
const char* pbe_file_suffix = getenv("IMMSV_PBE_FILE_SUFFIX");
std::string pbeFile;
+ bool pbeCorrupted = false;
if(pbe_file) {pbeFile.append(pbe_file);}
if(pbe_file_suffix) {
pbeFile.append(pbe_file_suffix);
@@ -2821,13 +2841,13 @@ int main(int argc, char* argv[])
LOG_NO("***** Loading from PBE file %s at %s *****",
pbeFile.c_str(), argv[1]);
}
- if(!loadImmFromPbe(pbeHandle, preload)) {
+ if(!loadImmFromPbe(pbeHandle, preload, &pbeCorrupted)) {
LOG_ER("Load from PBE ending ABNORMALLY dir:%s file:%s",
argv[1], pbeFile.c_str());
/* Try to prevent cyclic restart. Escalation will only work if
xmldir is writable.
*/
- if(!preload) {/* Should perhaps escalate also if preload or return
<0, 0, 0>*/
+ if(!preload && pbeCorrupted) {/* Should perhaps escalate also if
preload or return <0, 0, 0>*/
escalatePbe(xmldir, pbeFile.c_str());
}
goto err;
diff --git a/osaf/services/saf/immsv/immloadd/imm_loader.hh
b/osaf/services/saf/immsv/immloadd/imm_loader.hh
--- a/osaf/services/saf/immsv/immloadd/imm_loader.hh
+++ b/osaf/services/saf/immsv/immloadd/imm_loader.hh
@@ -58,20 +58,24 @@ void addObjectAttributeDefinition(SaImmC
bool createImmClass(SaImmHandleT immHandle,
const SaImmClassNameT className,
SaImmClassCategoryT classCategory,
- std::list<SaImmAttrDefinitionT_2>* attrDefinitions);
+ std::list<SaImmAttrDefinitionT_2>* attrDefinitions,
+ bool* pbeCorrupted = NULL);
bool createImmObject(SaImmClassNameT className,
char * objectName,
std::list<SaImmAttrValuesT_2> *attrValuesList,
SaImmCcbHandleT ccbHandle,
- std::map<std::string, SaImmAttrValuesT_2> *classRDNMap);
+ std::map<std::string, SaImmAttrValuesT_2> *classRDNMap,
+ bool* pbeCorrupted = NULL);
void escalatePbe(std::string dir, std::string file);
+bool isValidationAborted(SaImmCcbHandleT ccbHandle);
+
void* checkPbeRepositoryInit(std::string dir, std::string file);
-int loadImmFromPbe(void* pbeHandle, bool preload);
+int loadImmFromPbe(void* pbeHandle, bool preload, bool* pbeCorrupted);
void sendPreloadParams(SaImmHandleT immHandle, SaImmAdminOwnerHandleT
ownerHandle,
SaUint32T epoch, SaUint32T maxCcbId, SaUint32T maxCommitTime,
diff --git a/osaf/services/saf/immsv/immloadd/imm_pbe_load.cc
b/osaf/services/saf/immsv/immloadd/imm_pbe_load.cc
--- a/osaf/services/saf/immsv/immloadd/imm_pbe_load.cc
+++ b/osaf/services/saf/immsv/immloadd/imm_pbe_load.cc
@@ -211,7 +211,8 @@ bool loadClassFromPbe(void* pbeHandle,
const char* className,
const char* class_id,
SaImmClassCategoryT classCategory,
- ClassInfo* class_info)
+ ClassInfo* class_info,
+ bool* pbeCorrupted)
{
sqlite3* dbHandle = (sqlite3 *) pbeHandle;
sqlite3_stmt *stmt;
@@ -224,6 +225,7 @@ bool loadClassFromPbe(void* pbeHandle,
std::list<SaImmAttrDefinitionT_2>::iterator it;
TRACE_ENTER2("Loading class %s from PBE", className);
+ *pbeCorrupted = true;
stmt = preparedStmt[SQL_SEL_ATTR_DEF];
rc = sqlite3_bind_int(stmt, 1, atoi(class_id));
if(rc != SQLITE_OK) {
@@ -347,7 +349,7 @@ bool loadClassFromPbe(void* pbeHandle,
rc = sqlite3_step(stmt);
}
- if (!createImmClass(immHandle, (char *) className, classCategory,
&attrDefs)) {
+ if (!createImmClass(immHandle, (char *) className, classCategory,
&attrDefs, pbeCorrupted)) {
LOG_ER("Failed to create IMM class");
goto bailout;
}
@@ -365,7 +367,7 @@ bailout:
return false;
}
-bool loadClassesFromPbe(void* pbeHandle, SaImmHandleT immHandle, ClassInfoMap*
classInfoMap)
+bool loadClassesFromPbe(void* pbeHandle, SaImmHandleT immHandle, ClassInfoMap*
classInfoMap, bool* pbeCorrupted)
{
sqlite3* dbHandle = (sqlite3 *) pbeHandle;
const char * sql = "select * from classes";
@@ -377,6 +379,7 @@ bool loadClassesFromPbe(void* pbeHandle,
int r,c;
TRACE_ENTER();
assert(dbHandle);
+ *pbeCorrupted = true;
rc = sqlite3_get_table(dbHandle, sql, &result, &nrows, &ncols, &zErr);
if(rc) {
@@ -411,7 +414,7 @@ bool loadClassesFromPbe(void* pbeHandle,
class_info->className = std::string(class_name);
class_info->class_category = class_category;
if(!loadClassFromPbe(pbeHandle, immHandle, class_name,
- class_id, class_category, class_info))
+ class_id, class_category, class_info, pbeCorrupted))
{
sqlite3_free_table(result);
goto bailout;
@@ -431,7 +434,7 @@ bool loadClassesFromPbe(void* pbeHandle,
}
bool loadObjectFromPbe(void* pbeHandle, SaImmHandleT immHandle,
SaImmCcbHandleT ccbHandle,
- const char* object_id, ClassInfo* class_info, const char* dn)
+ const char* object_id, ClassInfo* class_info, const char* dn, bool*
pbeCorrupted)
{
sqlite3* dbHandle = (sqlite3 *) pbeHandle;
sqlite3_stmt *stmt = NULL;
@@ -449,6 +452,7 @@ bool loadObjectFromPbe(void* pbeHandle,
object_id, dn, class_info->className.c_str(),
class_info->attrInfoVector.size());
+ *pbeCorrupted = true;
/* First take care of the base tuple (single valued attributes). */
it = class_info->attrInfoVector.begin();
while(it != class_info->attrInfoVector.end()) {
@@ -665,7 +669,7 @@ bool loadObjectFromPbe(void* pbeHandle,
if(!createImmObject((char *) class_info->className.c_str(), (char *)
dn,
- &attrValuesList, ccbHandle, NULL))
+ &attrValuesList, ccbHandle, NULL, pbeCorrupted))
{
LOG_NO("Failed to create object - exiting");
goto bailout;
@@ -681,7 +685,7 @@ bailout:
}
bool loadObjectsFromPbe(void* pbeHandle, SaImmHandleT immHandle,
- SaImmCcbHandleT ccbHandle, ClassInfoMap *classInfoMap)
+ SaImmCcbHandleT ccbHandle, ClassInfoMap *classInfoMap, bool*
pbeCorrupted)
{
sqlite3* dbHandle = (sqlite3 *) pbeHandle;
const char * sql = "select * from objects";
@@ -693,6 +697,7 @@ bool loadObjectsFromPbe(void* pbeHandle,
int r,c;
TRACE_ENTER();
assert(dbHandle);
+ *pbeCorrupted = true;
rc = sqlite3_get_table(dbHandle, sql, &result, &nrows, &ncols, &zErr);
if(rc) {
@@ -727,7 +732,7 @@ bool loadObjectsFromPbe(void* pbeHandle,
class_info = (*classInfoMap)[std::string(class_id)];
if(!loadObjectFromPbe(pbeHandle, immHandle, ccbHandle,
object_id,
- class_info, dn)) {
+ class_info, dn, pbeCorrupted)) {
sqlite3_free_table(result);
goto bailout;
}
@@ -743,7 +748,7 @@ bool loadObjectsFromPbe(void* pbeHandle,
}
-int loadImmFromPbe(void* pbeHandle, bool preload)
+int loadImmFromPbe(void* pbeHandle, bool preload, bool* pbeCorrupted)
{
SaVersionT version = {'A', 2, 16};
SaImmHandleT immHandle=0LL;
@@ -759,6 +764,7 @@ int loadImmFromPbe(void* pbeHandle, bool
unsigned int retries=0;
TRACE_ENTER();
assert(dbHandle);
+ *pbeCorrupted = true;
do {
rc = sqlite3_exec(dbHandle, beginT, NULL, NULL, &execErr);
@@ -784,14 +790,15 @@ int loadImmFromPbe(void* pbeHandle, bool
The "other user" is likely to be a lingering PBE (on the
other SC) or a lingering POSIX
file lock in an NFS client cache.
*/
+ *pbeCorrupted = false;
sqlite3_close(dbHandle);
- exit(1);
}
errorCode = saImmOmInitialize(&immHandle, NULL, &version);
if (SA_AIS_OK != errorCode) {
LOG_ER("Failed to initialize the IMM OM interface (%d)",
errorCode);
+ *pbeCorrupted = false;
goto bailout;
}
@@ -799,7 +806,8 @@ int loadImmFromPbe(void* pbeHandle, bool
SA_FALSE, &ownerHandle);
if (errorCode != SA_AIS_OK) {
LOG_ER("Failed on saImmOmAdminOwnerInitialize %d", errorCode);
- exit(1);
+ *pbeCorrupted = false;
+ goto bailout;
}
if(preload) { /* Break out this into a separate function. */
@@ -959,7 +967,7 @@ int loadImmFromPbe(void* pbeHandle, bool
}
/*Fetch classes from PBE and create in IMM */
- if(!loadClassesFromPbe(pbeHandle, immHandle, &classInfoMap)) {
+ if(!loadClassesFromPbe(pbeHandle, immHandle, &classInfoMap,
pbeCorrupted)) {
goto bailout;
}
@@ -967,10 +975,11 @@ int loadImmFromPbe(void* pbeHandle, bool
errorCode = saImmOmCcbInitialize(ownerHandle, 0, &ccbHandle);
if (errorCode != SA_AIS_OK) {
LOG_ER("Failed to initialize ImmOmCcb %d", errorCode);
- exit(1);
+ *pbeCorrupted = false;
+ goto bailout;
}
- if(!loadObjectsFromPbe(pbeHandle, immHandle, ccbHandle, &classInfoMap))
+ if(!loadObjectsFromPbe(pbeHandle, immHandle, ccbHandle, &classInfoMap,
pbeCorrupted))
{
goto bailout;
}
@@ -987,7 +996,13 @@ int loadImmFromPbe(void* pbeHandle, bool
errorCode = saImmOmCcbApply(ccbHandle);
if (errorCode != SA_AIS_OK) {
LOG_ER("Failed to APPLY ImmOmCcb %d", errorCode);
- exit(1);
+ if ((errorCode != SA_AIS_ERR_INVALID_PARAM &&
+ errorCode != SA_AIS_ERR_BAD_OPERATION &&
+ errorCode != SA_AIS_ERR_FAILED_OPERATION) ||
+ (errorCode == SA_AIS_ERR_FAILED_OPERATION &&
!isValidationAborted(ccbHandle))) {
+ *pbeCorrupted = false;
+ }
+ goto bailout;
}
TRACE_2("Successfully Applied the CCB ");
@@ -1011,6 +1026,9 @@ int loadImmFromPbe(void* pbeHandle, bool
} while((errorCode == SA_AIS_ERR_TRY_AGAIN) && (retryCount <
32));
if((errorCode != SA_AIS_OK) &&
(errorCode!=SA_AIS_ERR_NOT_EXIST)) {
+ if (errorCode != SA_AIS_ERR_BUSY) {
+ *pbeCorrupted = false;
+ }
LOG_ER("Delete of class %s FAILED with errorCode:%u",
(char *) OPENSAF_IMM_PBE_RT_CLASS_NAME,
errorCode);
goto bailout;
@@ -1025,6 +1043,7 @@ int loadImmFromPbe(void* pbeHandle, bool
if(!opensafPbeRtClassCreate(immHandle)) {
/* Error already logged. */
+ *pbeCorrupted = false; /* definition of this class is not from
pbe */
goto bailout;
}
@@ -1090,3 +1109,24 @@ void escalatePbe(std::string dir, std::s
itself generate any new journal file.
*/
}
+
+bool isValidationAborted(SaImmCcbHandleT ccbHandle) {
+ SaStringT *errorStrings = NULL;
+ SaAisErrorT errorCode = SA_AIS_OK;
+
+ errorCode = saImmOmCcbGetErrorStrings(ccbHandle, (const SaStringT **)
&errorStrings);
+
+ if (errorCode != SA_AIS_OK) {
+ return false; /* Assuming that this is a resource abort */
+ }
+
+ while (*errorStrings) {
+ TRACE("ErrorStrings: %s", *errorStrings);
+ if (!strncmp((const char *) *errorStrings, "IMM: Validation
abort: ", strlen("IMM: Validation abort: "))) {
+ return true;
+ }
+ errorStrings++;
+ }
+
+ return false;
+}
------------------------------------------------------------------------------
_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
