Re: [devel] [PATCH 1/1] log: fix to remove the cfg file when log file rotation [#3045]
Hi Canh
Ack with some comments. I think Note 3 is the most important
Note not tested and new algorithm is not verified.
Comments:
Note 1:
In log_filehdl.cc
Sub titles are missing in function get_number_of_cfg_files_hdl()
This function is quite long and contains many code blocks (mostly if blocks)
and it's not obvious what each block is doing. Each block (where it is not
obvious) should have a comment describing what it is doing (not how it's done).
Also the updated if statement is very complicated, not easy to read. 'if'
statements like this should be avoided if possible. Can often be done by
splitting the if statement or creating a help function to handle the logic.
This function shall return true or false, have a descriptive name and if needed
a header documenting what is is doing and if the algorithm is complicated it
has to be explained.
Note 2:
In log_stream.cc
I can seet that you have added some protection if there is a huge number of cfg
files that not has been deleted. I assume this may have happened because of the
problem that is fixed with this ticket. However a cfg file can never be huge so
I assume that you don't mean huge cfg file in the following comment. You should
change to "...huge number of..."
// If there is too much cfg files that the rotation hasn't deleted them
// in previous, lgs should limit the deleting to avoid main thread is hung
// due to the deleting huge cfg files will take long times.
// The workaround here is that hard-code to delete max 100 cfg files
// in one time. Next rotation will continue to delete them
// It is useful when upgrading system and there are huge cfg files in disk
Note 3:
There should be test cases to verify the rotation mechanism including number of
cfg files, number of log files and the new option for requesting a rotation.
Thanks
Lennart
Från: Canh Van Truong
Skickat: den 30 maj 2019 10:10
Till: Lennart Lund; Vu Minh Nguyen
Kopia: opensaf-devel@lists.sourceforge.net; Canh Van Truong
Ämne: [PATCH 1/1] log: fix to remove the cfg file when log file rotation [#3045]
If the date(include year, month and day) in oldest cfg file name and the
date of created oldest log file is different, logd cannot find out the
oldest cfg file. The oldest cfg file is never removed when log file rotation.
So number of cfg file is huge after time while the log file has already been
rotated.
Update to remove the cfg in this case.
Also limit the number of cfg files should be removed if there are still huge
cfg files in disk to avoid hanging main thread
---
src/log/logd/lgs_filehdl.cc | 8 ++--
src/log/logd/lgs_stream.cc | 96 -
2 files changed, 47 insertions(+), 57 deletions(-)
diff --git a/src/log/logd/lgs_filehdl.cc b/src/log/logd/lgs_filehdl.cc
index be2b95e1f..f31e45883 100644
--- a/src/log/logd/lgs_filehdl.cc
+++ b/src/log/logd/lgs_filehdl.cc
@@ -794,10 +794,10 @@ int get_number_of_cfg_files_hdl(void *indata, void
*outdata,
}
}
-if ((old_ind != -1) && (cfg_old_date == log_old_date) &&
-(cfg_old_time <= log_old_time)) {
- TRACE_1(" (cfg_old_date:%d == log_old_date:%d) &&"
- " (cfg_old_time:%d <= log_old_time:%d )",
+if ((old_ind != -1) && (((cfg_old_date == log_old_date) &&
+(cfg_old_time <= log_old_time)) || (cfg_old_date < log_old_date))) {
+ TRACE_1(" (cfg_old_date:%d - log_old_date:%d) -"
+ " (cfg_old_time:%d - log_old_time:%d )",
cfg_old_date, log_old_date, cfg_old_time, log_old_time);
TRACE_1("oldest: %s", cfg_namelist[old_ind]->d_name);
n = snprintf(oldest_file, max_outsize, "%s/%s", path.c_str(),
diff --git a/src/log/logd/lgs_stream.cc b/src/log/logd/lgs_stream.cc
index d31ae170b..cb9a935e7 100644
--- a/src/log/logd/lgs_stream.cc
+++ b/src/log/logd/lgs_stream.cc
@@ -270,69 +270,59 @@ static int delete_config_file(log_stream_t *stream) {
/**
* Remove oldest log file until there are 'maxFilesRotated' - 1 files left
+ * The oldest cfg files are also removed
*
* @param stream
- * @return -1 on error
+ * @return true/false
*/
-static int rotate_if_needed(log_stream_t *stream) {
+static bool rotate_if_needed(log_stream_t *stream) {
char oldest_log_file[PATH_MAX];
char oldest_cfg_file[PATH_MAX];
- int rc = 0;
- int log_file_cnt, cfg_file_cnt;
- bool oldest_cfg = false;
+ const int max_files_rotated = static_cast(stream->maxFilesRotated);
TRACE_ENTER();
- /* Rotate out log files from previous lifes */
- if ((log_file_cnt = get_number_of_log_files_h(stream, oldest_log_file)) ==
- -1) {
-rc = -1;
-goto done;
- }
-
- /* Rotate out cfg files from previous lifes */
- if (!((cfg_file_cnt = get_number_of_cfg_files_h(stream, oldest_cfg_file)) ==
--1)) {
-oldest_cfg = true;
- }
-
- TRACE("delete oldest_cfg_file: %s oldest_log_file %s", oldest_cfg_file,
-oldest_log_file);
-
- /*
- ** Remove unti
[devel] [PATCH 1/1] amfnd: fix error reading from deallocated memory [#2568]
Invalid read is from the following
- avnd_evt_mds_ava_dn_evh() (amf/amfnd/comp.cc)
- avsv_create_association_class_dn() (amf/common/util.c)
Other changes are to fix cppcheck error report
---
src/amf/amfnd/comp.cc | 17 +
src/amf/common/util.c | 6 +++---
2 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/src/amf/amfnd/comp.cc b/src/amf/amfnd/comp.cc
index 38b9224..857c1dc 100644
--- a/src/amf/amfnd/comp.cc
+++ b/src/amf/amfnd/comp.cc
@@ -428,8 +428,10 @@ uint32_t avnd_evt_mds_ava_dn_evh(AVND_CB *cb, AVND_EVT
*evt) {
entry from the cbk list and delete the cbq */
m_AVND_COMP_CBQ_INV_GET(comp, comp->term_cbq_inv_value, cbk_rec);
comp->term_cbq_inv_value = 0;
+ uint32_t opq_hdl = 0;
+ if (cbk_rec) opq_hdl = cbk_rec->opq_hdl;
rc = avnd_comp_clc_fsm_run(cb, comp,
AVND_COMP_CLC_PRES_FSM_EV_TERM_SUCC);
- if (cbk_rec) avnd_comp_cbq_rec_pop_and_del(cb, comp, cbk_rec->opq_hdl,
false);
+ if (cbk_rec) avnd_comp_cbq_rec_pop_and_del(cb, comp, opq_hdl, false);
goto done;
}
/* found the matching comp; trigger error processing */
@@ -2228,9 +2230,7 @@ uint32_t avnd_amf_resp_send(AVND_CB *cb,
AVSV_AMF_API_TYPE type,
AVND_MSG msg;
AVSV_ND2ND_AVND_MSG *avnd_msg;
uint32_t rc = NCSCC_RC_SUCCESS;
- MDS_DEST i_to_dest;
AVSV_NDA_AVA_MSG *temp_ptr = nullptr;
- NODE_ID node_id = 0;
MDS_SYNC_SND_CTXT temp_ctxt;
TRACE_ENTER();
@@ -2267,8 +2267,8 @@ uint32_t avnd_amf_resp_send(AVND_CB *cb,
AVSV_AMF_API_TYPE type,
msg.info.avnd->type = AVND_AVND_AVA_MSG;
msg.type = AVND_MSG_AVND;
/* Send it to AvND */
-node_id = m_NCS_NODE_ID_FROM_MDS_DEST(*dest);
-i_to_dest = avnd_get_mds_dest_from_nodeid(cb, node_id);
+NODE_ID node_id = m_NCS_NODE_ID_FROM_MDS_DEST(*dest);
+MDS_DEST i_to_dest = avnd_get_mds_dest_from_nodeid(cb, node_id);
rc = avnd_avnd_mds_send(cb, i_to_dest, &msg);
} else {
/* now send the response */
@@ -2646,7 +2646,8 @@ void avnd_comp_cmplete_all_assignment(AVND_CB *cb,
AVND_COMP *comp) {
*/
temp_csi = m_AVND_COMPDB_REC_CSI_GET_FIRST(*comp);
-if (cbk->cbk_info->param.csi_set.ha != temp_csi->si->curr_state) {
+if (temp_csi &&
+ (cbk->cbk_info->param.csi_set.ha != temp_csi->si->curr_state)) {
avnd_comp_cbq_rec_pop_and_del(cb, comp, cbk->opq_hdl, true);
continue;
}
@@ -2788,7 +2789,7 @@ uint32_t comp_restart_initiate(AVND_COMP *comp) {
rc = avnd_comp_curr_info_del(cb, it.second);
if (NCSCC_RC_SUCCESS != rc) goto done;
- // unregister the contained comp
+// unregister the contained comp
rc = avnd_comp_unregister_contained(cb, it.second);
if (NCSCC_RC_SUCCESS != rc) goto done;
@@ -2956,7 +2957,7 @@ void avnd_comp_pres_state_set(const AVND_CB *cb,
AVND_COMP *comp,
(SA_AMF_PRESENCE_ORPHANED == prv_st {
if (cb->is_avd_down == false) {
avnd_di_uns32_upd_send(AVSV_SA_AMF_COMP, saAmfCompPresenceState_ID,
- comp->name.c_str(), comp->pres);
+ comp->name, comp->pres);
}
}
diff --git a/src/amf/common/util.c b/src/amf/common/util.c
index ec76c32..d17b766 100644
--- a/src/amf/common/util.c
+++ b/src/amf/common/util.c
@@ -240,12 +240,12 @@ void avsv_create_association_class_dn(const SaNameT
*child_dn,
}
if (dn) {
+ TRACE("dn: %s", buf);
osaf_extended_name_steal(buf, dn);
}
- TRACE_LEAVE2("child_dn: %s parent_dn: %s dn: %s",
+ TRACE_LEAVE2("child_dn: %s parent_dn: %s",
child_dn_ptr ? child_dn_ptr : "no child dn",
- parent_dn_ptr ? parent_dn_ptr : "no parent dn",
- buf);
+ parent_dn_ptr ? parent_dn_ptr : "no parent dn");
}
void avsv_sanamet_init_from_association_dn(const SaNameT *haystack, SaNameT
*dn,
--
2.7.4
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
[devel] [PATCH 0/1] Review Request for ticket [#2568]
Summary: amfnd: fix error reading from deallocated memory [#2568] Review request for Ticket(s): 2568 Peer Reviewer(s): *** LIST THE TECH REVIEWER(S) / MAINTAINER(S) HERE *** Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE *** Affected branch(es): develop Development branch: ticket-2568 Base revision: 135b0b8862da9a036553c5db02062edb278089aa Personal repository: git://git.code.sf.net/u/xdtthng/review Impacted area Impact y/n Docsn Build systemn RPM/packaging n Configuration files n Startup scripts n SAF servicesn OpenSAF servicesn Core libraries n Samples n Tests n Other n Comments (indicate scope for each "y" above): - *** EXPLAIN/COMMENT THE PATCH SERIES HERE *** revision 9be484f49579861f2c3325403b7680b700d47c0b Author: Thanh Nguyen Date: Wed, 5 Jun 2019 12:11:31 +1000 amfnd: fix error reading from deallocated memory [#2568] Invalid read is from the following - avnd_evt_mds_ava_dn_evh() (amf/amfnd/comp.cc) - avsv_create_association_class_dn() (amf/common/util.c) Other changes are to fix cppcheck error report Complete diffstat: -- src/amf/amfnd/comp.cc | 17 + src/amf/common/util.c | 6 +++--- 2 files changed, 12 insertions(+), 11 deletions(-) Testing Commands: - *** LIST THE COMMAND LINE TOOLS/STEPS TO TEST YOUR CHANGES *** Testing, Expected Results: -- *** PASTE COMMAND OUTPUTS / TEST RESULTS *** Conditions of Submission: - *** HOW MANY DAYS BEFORE PUSHING, CONSENSUS ETC *** Arch Built StartedLinux distro --- mipsn n mips64 n n x86 n n x86_64 n n powerpc n n powerpc64 n n Reviewer Checklist: --- [Submitters: make sure that your review doesn't trigger any checkmarks!] Your checkin has not passed review because (see checked entries): ___ Your RR template is generally incomplete; it has too many blank entries that need proper data filled in. ___ You have failed to nominate the proper persons for review and push. ___ Your patches do not have proper short+long header ___ You have grammar/spelling in your header that is unacceptable. ___ You have exceeded a sensible line length in your headers/comments/text. ___ You have failed to put in a proper Trac Ticket # into your commits. ___ You have incorrectly put/left internal data in your comments/files (i.e. internal bug tracking tool IDs, product names etc) ___ You have not given any evidence of testing beyond basic build tests. Demonstrate some level of runtime or other sanity testing. ___ You have ^M present in some of your files. These have to be removed. ___ You have needlessly changed whitespace or added whitespace crimes like trailing spaces, or spaces before tabs. ___ You have mixed real technical changes with whitespace and other cosmetic code cleanup changes. These have to be separate commits. ___ You need to refactor your submission into logical chunks; there is too much content into a single commit. ___ You have extraneous garbage in your review (merge commits etc) ___ You have giant attachments which should never have been sent; Instead you should place your content in a public tree to be pulled. ___ You have too many commits attached to an e-mail; resend as threaded commits, or place in a public tree for a pull. ___ You have resent this content multiple times without a clear indication of what has changed between each re-send. ___ You have failed to adequately and individually address all of the comments and change requests that were proposed in the initial review. ___ You have a misconfigured ~/.gitconfig file (i.e. user.name, user.email etc) ___ Your computer have a badly configured date and time; confusing the the threaded patch review. ___ Your changes affect IPC mechanism, and you don't present any results for in-service upgradability test. ___ Your changes affect user manual and documentation, your patch series do not contain the patch that updates the Doxygen manual. ___ Opensaf-devel mailing list Opensaf-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/opensaf-devel
[devel] [PATCH 1/1] amfd: do not queue sync messages from 'lost' nodes [#3050]
The 'lost' nodes will be rebooted, thus there is no need
to queue sync messages from these nodes.
In addition, node_sync_window_closed is not reliable as it's not
check pointed. We should remove all uses of it in another ticket?
Instead, check if the timer is running.
---
src/amf/amfd/cb.h | 2 ++
src/amf/amfd/ndproc.cc | 30 ++
src/amf/amfd/timer.h | 12 ++--
3 files changed, 30 insertions(+), 14 deletions(-)
diff --git a/src/amf/amfd/cb.h b/src/amf/amfd/cb.h
index 89cf15d..8902d78 100644
--- a/src/amf/amfd/cb.h
+++ b/src/amf/amfd/cb.h
@@ -237,6 +237,8 @@ typedef struct cl_cb_tag {
*/
bool active_services_exist;
bool all_nodes_synced;
+ // @todo this should be checkpointed to standby? otherwise
+ // after a controller failover, it will still be false?
bool node_sync_window_closed;
/*
diff --git a/src/amf/amfd/ndproc.cc b/src/amf/amfd/ndproc.cc
index 5f5cbcd..20008d9 100644
--- a/src/amf/amfd/ndproc.cc
+++ b/src/amf/amfd/ndproc.cc
@@ -345,19 +345,26 @@ void avd_nd_sisu_state_info_evh(AVD_CL_CB *cb, AVD_EVT
*evt) {
evt->info.avnd_msg->msg_info.n2d_nd_sisu_state_info.node_id,
evt->info.avnd_msg->msg_info.n2d_nd_sisu_state_info.msg_id);
- if (cb->node_sync_window_closed == false) {
+ const SaClmNodeIdT node_id =
+evt->info.avnd_msg->msg_info.n2d_nd_sisu_state_info.node_id;
+
+ if (cb->failover_list.find(node_id) != cb->failover_list.end()) {
+// ignore msg
+LOG_WA("sisu_state_info messages received from lost node (%x)",
+ node_id);
+ } else if (cb->node_sync_tmr.is_active == true) {
AVD_EVT_QUEUE *state_info_evt = new AVD_EVT_QUEUE();
state_info_evt->evt = new AVD_EVT{};
state_info_evt->evt->rcv_evt = evt->rcv_evt;
state_info_evt->evt->info.avnd_msg = n2d_msg;
cb->evt_queue.push(state_info_evt);
+return;
} else {
LOG_WA(
-"Ignore this sisu_state_info message since node sync window has
closed");
-avsv_dnd_msg_free(n2d_msg);
+ "Ignore this sisu_state_info message since node sync window has closed");
}
- TRACE_LEAVE();
+ avsv_dnd_msg_free(n2d_msg);
}
/*
@@ -387,19 +394,26 @@ void avd_nd_compcsi_state_info_evh(AVD_CL_CB *cb, AVD_EVT
*evt) {
evt->info.avnd_msg->msg_info.n2d_nd_csicomp_state_info.node_id,
evt->info.avnd_msg->msg_info.n2d_nd_csicomp_state_info.msg_id);
- if (cb->node_sync_window_closed == false) {
+ const SaClmNodeIdT node_id =
+evt->info.avnd_msg->msg_info.n2d_nd_csicomp_state_info.node_id;
+
+ if (cb->failover_list.find(node_id) != cb->failover_list.end()) {
+// ignore msg
+LOG_WA("compcsi_state_info messages received from lost node (%x)",
+ node_id);
+ } else if (cb->node_sync_tmr.is_active == true) {
AVD_EVT_QUEUE *state_info_evt = new AVD_EVT_QUEUE();
state_info_evt->evt = new AVD_EVT{};
state_info_evt->evt->rcv_evt = evt->rcv_evt;
state_info_evt->evt->info.avnd_msg = n2d_msg;
cb->evt_queue.push(state_info_evt);
+return;
} else {
LOG_WA(
-"Ignore this compcsi_state_info message since node sync window has
closed");
-avsv_dnd_msg_free(n2d_msg);
+ "Ignore this compcsi_state_info message since node sync window has
closed");
}
- TRACE_LEAVE();
+ avsv_dnd_msg_free(n2d_msg);
}
/**
diff --git a/src/amf/amfd/timer.h b/src/amf/amfd/timer.h
index 5316879..6db04c7 100644
--- a/src/amf/amfd/timer.h
+++ b/src/amf/amfd/timer.h
@@ -52,12 +52,12 @@ typedef enum avd_tmr_type {
/* AVD Timer definition */
typedef struct avd_tmr_tag {
- tmr_t tmr_id;
- AVD_TMR_TYPE type;
- SaClmNodeIdT node_id;
- std::string spons_si_name;
- std::string dep_si_name;
- bool is_active;
+ tmr_t tmr_id{};
+ AVD_TMR_TYPE type{AVD_TMR_MAX};
+ SaClmNodeIdT node_id{};
+ std::string spons_si_name{};
+ std::string dep_si_name{};
+ bool is_active{};
} AVD_TMR;
/* macro to start the cluster init timer. The cb structure
--
2.7.4
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
[devel] [PATCH 0/1] Review Request for amfd: do not queue sync messages from 'lost' nodes [#3050]
Summary: amfd: do not queue sync messages from 'lost' nodes [#3050] Review request for Ticket(s): 3050 Peer Reviewer(s): Hans, Minh Pull request to: *** LIST THE PERSON WITH PUSH ACCESS HERE *** Affected branch(es): develop Development branch: ticket-3050 Base revision: 135b0b8862da9a036553c5db02062edb278089aa Personal repository: git://git.code.sf.net/u/userid-2226215/review Impacted area Impact y/n Docsn Build systemn RPM/packaging n Configuration files n Startup scripts n SAF servicesy OpenSAF servicesn Core libraries n Samples n Tests n Other n Comments (indicate scope for each "y" above): - revision 9d64d3c1d386f1019103d12588ab46fa830ee793 Author: Gary Lee Date: Wed, 5 Jun 2019 13:49:45 +1000 amfd: do not queue sync messages from 'lost' nodes [#3050] The 'lost' nodes will be rebooted, thus there is no need to queue sync messages from these nodes. In addition, node_sync_window_closed is not reliable as it's not check pointed. We should remove all uses of it in another ticket? Instead, check if the timer is running. Complete diffstat: -- src/amf/amfd/cb.h | 2 ++ src/amf/amfd/ndproc.cc | 30 ++ src/amf/amfd/timer.h | 12 ++-- 3 files changed, 30 insertions(+), 14 deletions(-) Testing Commands: - See ticket for reproduction steps. Testing, Expected Results: -- Sync messages should be discarded and not put back into the queue. 2019-06-05 12:52:31.833 SC-2 osafamfd[254]: NO Receive message with event type:12, msg_type:31, from node:2030f, msg_id:0 2019-06-05 12:52:31.834 SC-2 osafamfd[254]: WA sisu_state_info messages received from lost node (2030f) 2019-06-05 12:52:31.834 SC-2 osafamfd[254]: NO Receive message with event type:13, msg_type:32, from node:2030f, msg_id:0 2019-06-05 12:52:31.834 SC-2 osafamfd[254]: WA compcsi_state_info messages received from lost node (2030f) Conditions of Submission: - Ack from anyone Arch Built StartedLinux distro --- mipsn n mips64 n n x86 n n x86_64 y y powerpc n n powerpc64 n n Reviewer Checklist: --- [Submitters: make sure that your review doesn't trigger any checkmarks!] Your checkin has not passed review because (see checked entries): ___ Your RR template is generally incomplete; it has too many blank entries that need proper data filled in. ___ You have failed to nominate the proper persons for review and push. ___ Your patches do not have proper short+long header ___ You have grammar/spelling in your header that is unacceptable. ___ You have exceeded a sensible line length in your headers/comments/text. ___ You have failed to put in a proper Trac Ticket # into your commits. ___ You have incorrectly put/left internal data in your comments/files (i.e. internal bug tracking tool IDs, product names etc) ___ You have not given any evidence of testing beyond basic build tests. Demonstrate some level of runtime or other sanity testing. ___ You have ^M present in some of your files. These have to be removed. ___ You have needlessly changed whitespace or added whitespace crimes like trailing spaces, or spaces before tabs. ___ You have mixed real technical changes with whitespace and other cosmetic code cleanup changes. These have to be separate commits. ___ You need to refactor your submission into logical chunks; there is too much content into a single commit. ___ You have extraneous garbage in your review (merge commits etc) ___ You have giant attachments which should never have been sent; Instead you should place your content in a public tree to be pulled. ___ You have too many commits attached to an e-mail; resend as threaded commits, or place in a public tree for a pull. ___ You have resent this content multiple times without a clear indication of what has changed between each re-send. ___ You have failed to adequately and individually address all of the comments and change requests that were proposed in the initial review. ___ You have a misconfigured ~/.gitconfig file (i.e. user.name, user.email etc) ___ Your computer have a badly configured date and time; confusing the the threaded patch review. ___ Your changes affect IPC mechanism, and you don't present any results for in-service upgradability test. ___ Your changes affect user manual and documentation, your patch series do not contain the patch that updates the Doxygen manual. ___ Opensaf-devel mailing list Opensaf-devel@lists.source
Re: [devel] [PATCH 1/1] amfnd: fix error reading from deallocated memory [#2568]
Hi Thanh
I will push on your behalf.
Thanks
Gary
On 5/6/19 12:29 pm, Thanh Nguyen wrote:
Invalid read is from the following
- avnd_evt_mds_ava_dn_evh() (amf/amfnd/comp.cc)
- avsv_create_association_class_dn() (amf/common/util.c)
Other changes are to fix cppcheck error report
---
src/amf/amfnd/comp.cc | 17 +
src/amf/common/util.c | 6 +++---
2 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/src/amf/amfnd/comp.cc b/src/amf/amfnd/comp.cc
index 38b9224..857c1dc 100644
--- a/src/amf/amfnd/comp.cc
+++ b/src/amf/amfnd/comp.cc
@@ -428,8 +428,10 @@ uint32_t avnd_evt_mds_ava_dn_evh(AVND_CB *cb, AVND_EVT
*evt) {
entry from the cbk list and delete the cbq */
m_AVND_COMP_CBQ_INV_GET(comp, comp->term_cbq_inv_value, cbk_rec);
comp->term_cbq_inv_value = 0;
+ uint32_t opq_hdl = 0;
+ if (cbk_rec) opq_hdl = cbk_rec->opq_hdl;
rc = avnd_comp_clc_fsm_run(cb, comp,
AVND_COMP_CLC_PRES_FSM_EV_TERM_SUCC);
- if (cbk_rec) avnd_comp_cbq_rec_pop_and_del(cb, comp, cbk_rec->opq_hdl,
false);
+ if (cbk_rec) avnd_comp_cbq_rec_pop_and_del(cb, comp, opq_hdl, false);
goto done;
}
/* found the matching comp; trigger error processing */
@@ -2228,9 +2230,7 @@ uint32_t avnd_amf_resp_send(AVND_CB *cb,
AVSV_AMF_API_TYPE type,
AVND_MSG msg;
AVSV_ND2ND_AVND_MSG *avnd_msg;
uint32_t rc = NCSCC_RC_SUCCESS;
- MDS_DEST i_to_dest;
AVSV_NDA_AVA_MSG *temp_ptr = nullptr;
- NODE_ID node_id = 0;
MDS_SYNC_SND_CTXT temp_ctxt;
TRACE_ENTER();
@@ -2267,8 +2267,8 @@ uint32_t avnd_amf_resp_send(AVND_CB *cb, AVSV_AMF_API_TYPE type,
msg.info.avnd->type = AVND_AVND_AVA_MSG;
msg.type = AVND_MSG_AVND;
/* Send it to AvND */
-node_id = m_NCS_NODE_ID_FROM_MDS_DEST(*dest);
-i_to_dest = avnd_get_mds_dest_from_nodeid(cb, node_id);
+NODE_ID node_id = m_NCS_NODE_ID_FROM_MDS_DEST(*dest);
+MDS_DEST i_to_dest = avnd_get_mds_dest_from_nodeid(cb, node_id);
rc = avnd_avnd_mds_send(cb, i_to_dest, &msg);
} else {
/* now send the response */
@@ -2646,7 +2646,8 @@ void avnd_comp_cmplete_all_assignment(AVND_CB *cb,
AVND_COMP *comp) {
*/
temp_csi = m_AVND_COMPDB_REC_CSI_GET_FIRST(*comp);
-if (cbk->cbk_info->param.csi_set.ha != temp_csi->si->curr_state) {
+if (temp_csi &&
+ (cbk->cbk_info->param.csi_set.ha != temp_csi->si->curr_state)) {
avnd_comp_cbq_rec_pop_and_del(cb, comp, cbk->opq_hdl, true);
continue;
}
@@ -2788,7 +2789,7 @@ uint32_t comp_restart_initiate(AVND_COMP *comp) {
rc = avnd_comp_curr_info_del(cb, it.second);
if (NCSCC_RC_SUCCESS != rc) goto done;
- // unregister the contained comp
+// unregister the contained comp
rc = avnd_comp_unregister_contained(cb, it.second);
if (NCSCC_RC_SUCCESS != rc) goto done;
@@ -2956,7 +2957,7 @@ void avnd_comp_pres_state_set(const AVND_CB *cb, AVND_COMP *comp,
(SA_AMF_PRESENCE_ORPHANED == prv_st {
if (cb->is_avd_down == false) {
avnd_di_uns32_upd_send(AVSV_SA_AMF_COMP, saAmfCompPresenceState_ID,
- comp->name.c_str(), comp->pres);
+ comp->name, comp->pres);
}
}
diff --git a/src/amf/common/util.c b/src/amf/common/util.c
index ec76c32..d17b766 100644
--- a/src/amf/common/util.c
+++ b/src/amf/common/util.c
@@ -240,12 +240,12 @@ void avsv_create_association_class_dn(const SaNameT
*child_dn,
}
if (dn) {
+ TRACE("dn: %s", buf);
osaf_extended_name_steal(buf, dn);
}
- TRACE_LEAVE2("child_dn: %s parent_dn: %s dn: %s",
+ TRACE_LEAVE2("child_dn: %s parent_dn: %s",
child_dn_ptr ? child_dn_ptr : "no child dn",
- parent_dn_ptr ? parent_dn_ptr : "no parent dn",
- buf);
+ parent_dn_ptr ? parent_dn_ptr : "no parent dn");
}
void avsv_sanamet_init_from_association_dn(const SaNameT *haystack, SaNameT *dn,
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
Re: [devel] [PATCH 1/1] amfd: disallow delete of CtCs object if Ct maps to comp [#3028]
Hi Phuc
Some comments below.
Thanks
Gary
On 23/5/19 4:48 pm, phuc.h.chau wrote:
Amfd crashes when su is unlocked, The reason for the crash is in the
function avd_snd_susi_msg(),get_comp_capability() is called
with csi and comp as input parameter.
In the function, get_comp_capability(), there is no CtCs object available
so ctcstype_db->find returns NULL to ctcs_type.
While accessing ctcs_type->saAmfCtCompCapability,
AMfd crashes because ctcs_type is NULL.
---
src/amf/amfd/ctcstype.cc | 65 +++-
1 file changed, 64 insertions(+), 1 deletion(-)
diff --git a/src/amf/amfd/ctcstype.cc b/src/amf/amfd/ctcstype.cc
index 5dffdae..3f30ebc 100644
--- a/src/amf/amfd/ctcstype.cc
+++ b/src/amf/amfd/ctcstype.cc
@@ -28,6 +28,10 @@
AmfDb *ctcstype_db = nullptr;
+static void find_ct_name_from_association(const std::string& haystack,
+ std::string *dn,
+ const char *needle);
+
static void ctcstype_db_add(AVD_CTCS_TYPE *ctcstype) {
unsigned int rc = ctcstype_db->insert(ctcstype->name, ctcstype);
osafassert(rc == NCSCC_RC_SUCCESS);
@@ -187,16 +191,75 @@ static SaAisErrorT
ctcstype_ccb_completed_cb(CcbUtilOperationData_t *opdata) {
opdata, "Modification of SaAmfCtCsType not supported");
break;
case CCBUTIL_DELETE:
+ AVD_CTCS_TYPE *ctcstype;
+ AVD_COMP_TYPE *comp_type;
+ AVD_COMP *comp;
+ CcbUtilOperationData_t *t_opData;
+
+ ctcstype = ctcstype_db->find(Amf::to_string(&opdata->objectName));
+ if (ctcstype != nullptr) {
+std::string ct_name;
+find_ct_name_from_association(Amf::to_string(&opdata->objectName),
+ &ct_name, ",safVersion");
+TRACE("'%s'", ct_name.c_str());
+comp_type = comptype_db->find(ct_name);
+if ((comp_type) && (nullptr != comp_type->list_of_comp)) {
+ /* check whether there exists a delete operation for
+ * each of the Comp in the comp_type list in the current CCB
+ */
+ bool comp_exist = false;
+ TRACE("SaAmfCompType '%s' has components", comp_type->name.c_str());
+ comp = comp_type->list_of_comp;
+ while (comp != nullptr) {
+TRACE("%s", osaf_extended_name_borrow(&comp->comp_info.name));
+t_opData = ccbutil_getCcbOpDataByDN(opdata->ccbId,
+&comp->comp_info.name);
+TRACE("%p", t_opData);
+if ((t_opData == nullptr) ||
+(t_opData->operationType != CCBUTIL_DELETE)) {
+ TRACE("Here %p", t_opData);
[Gary] Maybe replace "Here" with a more useful description.
+ comp_exist = true;
+ break;
+}
+comp = comp->comp_type_list_comp_next;
+ }
+ if (comp_exist == true) {
+rc = SA_AIS_ERR_BAD_OPERATION;
+report_ccb_validation_error(opdata, "SaAmfCompType '%s' is in use",
+comp_type->name.c_str());
+goto done;
+ }
+} else {
+TRACE("SaAmfCompType '%p'. SaAmfCompType '%s' has no components",
+ comp_type, ct_name.c_str());
+ }
+ }
rc = SA_AIS_OK;
break;
default:
osafassert(0);
break;
}
-
+done:
TRACE_LEAVE2("%u", rc);
return rc;
}
[Gary] avsv_sanamet_init() should already do what you need below.
+/**
+* Initialize a DN by searching for needle in haystack
+* where two times safVersion comes.
+* @param haystack
+* @param dn
+* @param needle
+* @note: "safSupportedCsType=safVersion=1\,
+* safCSType=AmfDemo1,safVersion=1,safCompType=AmfDemo1"
+*/
+static void find_ct_name_from_association(const std::string& haystack,
+ std::string *dn,
+ const char *needle) {
+ std::string::size_type pos = haystack.find(needle);
+ *dn = haystack.substr(pos + 1);
+ TRACE("dn %s", (*dn).c_str());
+}
static void ctcstype_ccb_apply_cb(CcbUtilOperationData_t *opdata) {
AVD_CTCS_TYPE *ctcstype;
___
Opensaf-devel mailing list
Opensaf-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
