Hi Lennart,
Thanks for your comments. Below is the SaLogStreamConfig class description -
result of `immlist -c SaLogStreamConfig`.
If we collect attribute names based on `SA_IMM_ATTR_RUNTIME`, we will miss
lot of LOG's interested attribute names.
<< SaLogStreamConfig - CONFIG >>
safLgStrCfg : SA_STRING_T [1] {RDN, CONFIG, INITIALIZED}
saLogStreamSeverityFilter : SA_UINT32_T [0] = Empty {CONFIG, WRITEABLE}
saLogStreamPathName : SA_STRING_T [1] {CONFIG, INITIALIZED}
saLogStreamNumOpeners : SA_UINT32_T [0] {RUNTIME}
saLogStreamMaxLogFileSize : SA_UINT64_T [0] = 5000000 (0x4c4b40) {CONFIG,
WRITEABLE}
saLogStreamMaxFilesRotated : SA_UINT32_T [0] = 4 (0x4) {CONFIG, WRITEABLE}
saLogStreamLogFullHaltThreshold : SA_UINT32_T [0] = 75 (0x4b) {CONFIG,
WRITEABLE}
saLogStreamLogFullAction : SA_UINT32_T [0] = 3 (0x3) {CONFIG, WRITEABLE}
saLogStreamLogFileFormat : SA_STRING_T [0] = Empty {CONFIG, WRITEABLE}
saLogStreamFixedLogRecordSize : SA_UINT32_T [0] = 150 (0x96) {CONFIG,
WRITEABLE}
saLogStreamFileName : SA_STRING_T [1] {CONFIG, WRITEABLE, INITIALIZED}
saLogStreamCreationTimestamp : SA_TIME_T [0] {RUNTIME, CACHED}
saLogRecordDestination : SA_STRING_T [0..*] = Empty {CONFIG, WRITEABLE,
MULTI_VALUE, NO_DUPLICATES}
logStreamDiscardedCounter : SA_UINT64_T [0] {RUNTIME}
I expect to get all attribute names, expect pure runtime attributes. The
reason is, fetching pure runtime attribute
will lead to deadlock. So, below logic is used to ignore pure runtime
attribute names.
+ if ((attr_def->attrFlags & SA_IMM_ATTR_RUNTIME) &&
+ (!(attr_def->attrFlags & SA_IMM_ATTR_CACHED))) continue;
In the final list, `list_attr_names`, will contain other attribute name LOG
don't have interested in, LOG will ignore them.
Regarding your other comments, I edited them even some belongs to legacy
code.
I sent out version 02, please have a look and give your comments. Thanks.
Regards, Vu
> -----Original Message-----
> From: Lennart Lund [mailto:[email protected]]
> Sent: Tuesday, September 12, 2017 10:06 PM
> To: Vu Minh Nguyen <[email protected]>; Canh Van Truong
> <[email protected]>
> Cc: [email protected]; Vu Minh Nguyen
> <[email protected]>
> Subject: RE: [PATCH 1/1] log: fix log server fail to start with old IMM
model
> [#2580]
>
> Hi Vu,
>
> See attached diff for comments.
> Note: I found one possible error
>
> Thanks
> Lennart
>
> > -----Original Message-----
> > From: Vu Minh Nguyen [mailto:[email protected]]
> > Sent: den 12 september 2017 15:57
> > To: Lennart Lund <[email protected]
<mailto:[email protected]> >; Canh Van Truong
> > <[email protected] <mailto:[email protected]> >
> > Cc: [email protected]
<mailto:[email protected]> ; Vu Minh Nguyen
> > <[email protected] <mailto:[email protected]> >
> > Subject: [PATCH 1/1] log: fix log server fail to start with old IMM
model
> > [#2580]
> >
> > LOG introduced the `saLogRecordDestination` attribute since OpenSAF 5.2
> > to handle the alternative destinations of log records, ticket [#2258].
> > During upgrade, if LOG server comes up before IMM model is updated
> > to new one which has saLogRecordDestination in, LOG server will be
> crashed
> >
> > The solution is if not able to fetching new added attribute value from
IMM,
> > LOG will try one more time with the attribute list without new added
ones.
> > ---
> > src/log/logd/lgs_amf.cc | 1 +
> > src/log/logd/lgs_amf.h | 2 +
> > src/log/logd/lgs_imm.cc | 113
++++++++++++++++++++----------------------
> -
> > ---
> > src/log/logd/lgs_mbcsv.cc | 3 +-
> > src/log/logd/lgs_mbcsv.h | 3 ++
> > src/log/logd/lgs_stream.h | 2 +
> > 6 files changed, 59 insertions(+), 65 deletions(-)
> >
> > diff --git a/src/log/logd/lgs_amf.cc b/src/log/logd/lgs_amf.cc
> > index d62be99..30a4d5b 100644
> > --- a/src/log/logd/lgs_amf.cc
> > +++ b/src/log/logd/lgs_amf.cc
> > @@ -19,6 +19,7 @@
> > * other modules.
> > */
> >
> > +#include "log/logd/lgs_amf.h"
> > #include "nid/agent/nid_start_util.h"
> > #include "osaf/immutil/immutil.h"
> > #include "log/logd/lgs.h"
> > diff --git a/src/log/logd/lgs_amf.h b/src/log/logd/lgs_amf.h
> > index 91bd2de..2a7f0ff 100644
> > --- a/src/log/logd/lgs_amf.h
> > +++ b/src/log/logd/lgs_amf.h
> > @@ -18,6 +18,8 @@
> > #ifndef LOG_LOGD_LGS_AMF_H_
> > #define LOG_LOGD_LGS_AMF_H_
> >
> > +#include <saAis.h>
> > +#include "log/logd/lgs_cb.h"
> >
> > SaAisErrorT lgs_amf_init(lgs_cb_t *cb);
> >
> > diff --git a/src/log/logd/lgs_imm.cc b/src/log/logd/lgs_imm.cc
> > index c856ec2..76faf6c 100644
> > --- a/src/log/logd/lgs_imm.cc
> > +++ b/src/log/logd/lgs_imm.cc
> > @@ -34,7 +34,6 @@
> > #include <stdio.h>
> > #include <errno.h>
> > #include <string.h>
> > -
> > #include <saImmOm.h>
> > #include <saImmOi.h>
> >
> > @@ -2733,33 +2732,16 @@ done:
> > * @return SaAisErrorT
> > */
> > static SaAisErrorT stream_create_and_configure(
> > - const std::string &dn, log_stream_t **in_stream, int stream_id,
> > - SaImmAccessorHandleT accessorHandle) {
> > + const std::string &dn, int stream_id,
> > + SaImmAttrValuesT_2** attributes) {
> > SaAisErrorT rc = SA_AIS_OK;
> > - SaNameT objectName;
> > SaImmAttrValuesT_2 *attribute;
> > - SaImmAttrValuesT_2 **attributes;
> > int i = 0;
> > log_stream_t *stream;
> > - char *attribute_names[] = {
> > - const_cast<char *>("saLogStreamFileName"),
> > - const_cast<char *>("saLogStreamPathName"),
> > - const_cast<char *>("saLogStreamMaxLogFileSize"),
> > - const_cast<char *>("saLogStreamFixedLogRecordSize"),
> > - const_cast<char *>("saLogStreamLogFullAction"),
> > - const_cast<char *>("saLogStreamLogFullHaltThreshold"),
> > - const_cast<char *>("saLogStreamMaxFilesRotated"),
> > - const_cast<char *>("saLogStreamLogFileFormat"),
> > - const_cast<char *>("saLogRecordDestination"),
> > - const_cast<char *>("saLogStreamSeverityFilter"),
> > - const_cast<char *>("saLogStreamCreationTimestamp"),
> > - NULL};
> >
> > TRACE_ENTER2("(%s)", dn.c_str());
> >
> > - osaf_extended_name_lend(dn.c_str(), &objectName);
> > -
> > - *in_stream = stream = log_stream_new(dn, stream_id);
> > + stream = log_stream_new(dn, stream_id);
> >
> > if (stream == NULL) {
> > rc = SA_AIS_ERR_NO_MEMORY;
> > @@ -2775,15 +2757,6 @@ static SaAisErrorT
> stream_create_and_configure(
> > else
> > stream->streamType = STREAM_TYPE_APPLICATION;
> >
> > - /* Get all attributes of the object */
> > - if ((rc = immutil_saImmOmAccessorGet_2(accessorHandle,
> &objectName,
> > - attribute_names, &attributes))
!=
> > - SA_AIS_OK) {
> > - LOG_ER("Configuration for %s not found: %s", dn.c_str(),
saf_error(rc));
> > - rc = SA_AIS_ERR_NOT_EXIST;
> > - goto done;
> > - }
> > -
> > while ((attribute = attributes[i++]) != NULL) {
> > void *value;
> >
> > @@ -2910,11 +2883,11 @@ SaAisErrorT
> lgs_imm_init_configStreams(lgs_cb_t
> > *cb) {
> > int int_rc = 0;
> > log_stream_t *stream;
> > SaImmHandleT omHandle;
> > - SaImmAccessorHandleT accessorHandle;
> > SaVersionT immVersion = {'A', 2, 1};
> > SaImmSearchHandleT immSearchHandle;
> > SaImmSearchParametersT_2 objectSearch;
> > SaImmAttrValuesT_2 **attributes;
> > + SaImmAttrDefinitionT_2** attr_definitions;
> > int wellknownStreamId = 0;
> > int appStreamId = 3;
> > uint32_t streamId = 0;
> > @@ -2929,12 +2902,28 @@ SaAisErrorT
> lgs_imm_init_configStreams(lgs_cb_t
> > *cb) {
> > osaf_abort(0);
> > }
> >
> > - om_rc = immutil_saImmOmAccessorInitialize(omHandle,
> > &accessorHandle);
> > - if (om_rc != SA_AIS_OK) {
> > - LOG_ER("immutil_saImmOmAccessorInitialize failed %s",
> > saf_error(om_rc));
> > - osaf_abort(0);
> > + // We don't know the IMM model we are working with is the latest
> model
> > + // or the old ones that could not have our addedd new attribute
names.
> > + // So, we ask helps from `saImmOmClassDescriptionGet_2`.
> > + SaImmClassCategoryT category = SA_IMM_CLASS_CONFIG;
> > + om_rc = immutil_saImmOmClassDescriptionGet_2(
> > + omHandle, const_cast<char*>(className), &category,
> > &attr_definitions);
> > + osafassert(om_rc == SA_AIS_OK);
> > +
> > + // Get list of non-pure of `SaLogStreamConfig` class.
> > + unsigned ii = 0;
> > + SaImmAttrDefinitionT_2* temp;
> > + std::vector<char*> list_temp;
> > + while ((temp = attr_definitions[ii++]) != nullptr) {
> > + if (temp->attrFlags & SA_IMM_ATTR_RUNTIME) continue;
> > + list_temp.push_back(temp->attrName);
> > }
> >
> > + ii = 0;
> > + char* list_attributes[list_temp.size() + 1];
> > + for (auto& item : list_temp) list_attributes[ii++] = item;
> > + list_attributes[ii] = nullptr;
> > +
> > /* Search for all objects of class "SaLogStreamConfig". */
> > /**
> > * Should not base on the attribute name `safLgStrCfg`
> > @@ -2951,29 +2940,32 @@ SaAisErrorT
> lgs_imm_init_configStreams(lgs_cb_t
> > *cb) {
> > * `safApp=safLogService` might miss configurable app stream eg: app
> > stream
> > * with DN `saLgStrCfg=test`
> > */
> > - if ((om_rc = immutil_saImmOmSearchInitialize_2(
> > - omHandle, NULL, SA_IMM_SUBTREE,
> > - SA_IMM_SEARCH_ONE_ATTR | SA_IMM_SEARCH_GET_NO_ATTR,
> > &objectSearch,
> > - NULL, /* Get no attributes */
> > - &immSearchHandle)) == SA_AIS_OK) {
> > - while (immutil_saImmOmSearchNext_2(immSearchHandle,
> > &objectName,
> > - &attributes) == SA_AIS_OK) {
> > - /**
> > - * With headless mode enabled, when lgsv restarts, there could be
> other
> > - * configurable app streams. It differs from legacy mode in which
no
> app
> > - * stream could be exist at startup.
> > - *
> > - * With well-known streams, stream ID is in reserved numbers
[0-2].
> > - */
> > - SaConstStringT name = osaf_extended_name_borrow(&objectName);
> > - streamId =
> > - is_well_know_stream(name) ? wellknownStreamId++ :
> > appStreamId++;
> > - ais_rc =
> > - stream_create_and_configure(name, &stream, streamId,
> > accessorHandle);
> > - if (ais_rc != SA_AIS_OK) {
> > - LOG_WA("stream_create_and_configure failed %d", ais_rc);
> > - goto done;
> > - }
> > +
> > + om_rc = immutil_saImmOmSearchInitialize_2(
> > + omHandle, NULL, SA_IMM_SUBTREE,
> > + SA_IMM_SEARCH_ONE_ATTR | SA_IMM_SEARCH_GET_SOME_ATTR,
> > + &objectSearch,
> > + list_attributes,
> > + &immSearchHandle);
> > + osafassert(om_rc == SA_AIS_OK);
> > +
> > + while (immutil_saImmOmSearchNext_2(immSearchHandle,
> > &objectName,
> > + &attributes) == SA_AIS_OK) {
> > + /**
> > + * With headless mode enabled, when lgsv restarts, there could be
> other
> > + * configurable app streams. It differs from legacy mode in which
no
> app
> > + * stream could be exist at startup.
> > + *
> > + * With well-known streams, stream ID is in reserved numbers [0-2].
> > + */
> > + SaConstStringT name = osaf_extended_name_borrow(&objectName);
> > + streamId =
> > + is_well_know_stream(name) ? wellknownStreamId++ :
> > appStreamId++;
> > + ais_rc =
> > + stream_create_and_configure(name, streamId, attributes);
> > + if (ais_rc != SA_AIS_OK) {
> > + LOG_WA("stream_create_and_configure failed %d", ais_rc);
> > + goto done;
> > }
> > }
> >
> > @@ -3022,11 +3014,6 @@ SaAisErrorT
> lgs_imm_init_configStreams(lgs_cb_t
> > *cb) {
> >
> > done:
> > /* Do not abort if error when finalizing */
> > - om_rc = immutil_saImmOmAccessorFinalize(accessorHandle);
> > - if (om_rc != SA_AIS_OK) {
> > - LOG_NO("%s immutil_saImmOmAccessorFinalize() Fail %d",
> > __FUNCTION__, om_rc);
> > - }
> > -
> > om_rc = immutil_saImmOmSearchFinalize(immSearchHandle);
> > if (om_rc != SA_AIS_OK) {
> > LOG_NO("%s immutil_saImmOmSearchFinalize() Fail %d",
> > __FUNCTION__, om_rc);
> > diff --git a/src/log/logd/lgs_mbcsv.cc b/src/log/logd/lgs_mbcsv.cc
> > index 175858b..33149de 100644
> > --- a/src/log/logd/lgs_mbcsv.cc
> > +++ b/src/log/logd/lgs_mbcsv.cc
> > @@ -16,9 +16,8 @@
> > *
> > */
> >
> > +#include "log/logd/lgs_mbcsv.h"
> > #include "log/logd/lgs.h"
> > -#include "base/ncssysf_def.h"
> > -#include "base/ncssysf_mem.h"
> > #include "base/osaf_time.h"
> >
> > #include "osaf/immutil/immutil.h"
> > diff --git a/src/log/logd/lgs_mbcsv.h b/src/log/logd/lgs_mbcsv.h
> > index 30ee47b..e602378 100644
> > --- a/src/log/logd/lgs_mbcsv.h
> > +++ b/src/log/logd/lgs_mbcsv.h
> > @@ -22,6 +22,9 @@
> >
> > #include <stdint.h>
> > #include <saAmf.h>
> > +#include "log/logd/lgs_cb.h"
> > +#include "base/ncssysf_def.h"
> > +#include "base/ncssysf_mem.h"
> >
> > /* Version 1: Logservice check-point version older versions than
OpenSAF
> > 4.4.
> > * Cannot be configured for split filesystem.
> > diff --git a/src/log/logd/lgs_stream.h b/src/log/logd/lgs_stream.h
> > index 3c52534..efc39d4 100644
> > --- a/src/log/logd/lgs_stream.h
> > +++ b/src/log/logd/lgs_stream.h
> > @@ -89,6 +89,8 @@ typedef struct log_stream {
> > uint32_t stb_curFileSize; /* Bytes written to current log file */
> >
> > // Hold vector of destname string {"name1", "name2", etc.}
> > + // Default value is empty. It MUST be synced with
> > + // `saLogRecordDestination` default value.
> > std::vector<std::string> dest_names;
> > // Hold a list of strings separated by semicolon "name1;name2;etc"
> > // This data is used to checkpoint to standby
> > --
> > 1.9.1
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
