Hi Lennart, Thank you so much for your comments. Please see my answer inline.
BR, Tai Dinh DEK Technologies Vietnam 121/137 Le Loi Street, Ben Thanh Ward, District 1, HCM City, Vietnam Mobile: +84 9 33 37 82 90 -----Original Message----- From: Lennart Lund [mailto:[email protected]] Sent: Monday, February 16, 2015 7:50 PM To: Tai Dinh C; [email protected] Cc: [email protected]; Lennart Lund Subject: RE: [PATCH 1 of 3] LOG: ownership of directories and files should be configurable [#1181] Hi Tai Dinh, General remarks and comments: 1. Remark The code does not build lgs_imm.c: In function 'config_ccb_completed_modify': lgs_imm.c:927:36: error: 'value' may be used uninitialized in this function [-Werror=uninitialized] [Tai] OK, I'll fix it. 2. Comment In function ckpt_proc_lgs_cfg_v3() the following note can be found: * NOTE! Changing lgs config does not work if standby is V1. Cannot be fixed * since V2 has no applier for config object and V1 does not checkpoint * config data! This is correct. The only attribute affected by this is runtime configuration of the log root directory. When introducing a configuration enabling the log service to run in an "embedded" environment in practice meaning that the log service is able to write log records to local file systems on the SC nodes meaning writing the same log record to two log files. When this was introduced it was no longer possible to handle synchronization of log configuration using an applier on the standby and check-pointing of the log root directory setting was introduced (V2). Next the limits used for mailbox configuration was also made possible to change in runtime however the checkpoint protocol was not changed instead the mailbox is updated (including reading the parameters from the configuration object) every time the standby becomes active eliminating the need for check-pointing these attributes. If we see the need to fix handling of runtime update of the log root directory if standby is running v2 or newer and active v1 a solution as for the mailbox limits can probably be used. I think you should remove the Note from the source code and instead write a ticket. [TAI] OK, then when receiving the ckpt_proc_lgs_cfg() we can re-read the configuration from IMM if peer_v1 is detected instead of reading from check pointed messages. I'll fix this for V3 and raise a ticket for V2. 3. Question Will this feature work if configured for split file system? or are there any restrictions ? If it is not possible to use group name in "split file system" mode then runtime setting of group name should be denied. Also some sort of error handling for the case of a conflicting configuration should exist. [TAI] It works for the "split mode". Although there is a limitation on this one. It's the user must make sure that the same group exist and is a supplementary group of standby LOG process. Otherwise: - LOG will not be able to change the ownership of log files - And error message will be logged into syslog. - The service keep working. The reason of this is because we have no verification about configuration change on standby node. We currently have the same issue with root directory, I will raise another ticket later. To reproduce: - Running OpenSAF on split mode. - Create a root directory and grand permission on active node. - Change the root directory to this new one. - Verification passed on active and modification will be applied. - But on standby the root directory may not exist and LOGSV may lack permission to create it. 4. Test comment The following test case fails if the correct setting is not manually done outside the test. "4 FAILED (expected EXIT_SUCCESS, got EXIT_FAILURE (1)) CCB Object Modify, data group. Group exists. OK" This is not good. Pre-settings should be fixed by the test case itself if possible. E.g. a similar problem exist when testing to change the log root directory. In order to change log root directory the new directory must already exists. The test case starts by creating a new directory then change to this directory. If not possible the test case should discover that the correct pre-setting is not done and maybe print some information and "PASS" without actually being executed. [TAI] - It a bit hard to setup it inside the testcase: + The testing group need to be created and to added into opensaf user => TCs need to be run under root user which is what I could not guarantee. + That modification need to be done before OpenSAF starting otherwise an OpenSAF restart need to be done. Hence I decided to mention that setup in the README file instead. Anyway, I can do a check and let the TC passed with information of the setting is not done. Thanks Lennart -----Original Message----- From: Tai Dinh [mailto:[email protected]] Sent: den 13 februari 2015 08:58 To: [email protected]; Lennart Lund Cc: [email protected] Subject: [PATCH 1 of 3] LOG: ownership of directories and files should be configurable [#1181] osaf/libs/core/common/osaf_secutil.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) Explicitly treats ENOENT as no error for setgrent() since it always failed on UML diff --git a/osaf/libs/core/common/osaf_secutil.c b/osaf/libs/core/common/osaf_secutil.c --- a/osaf/libs/core/common/osaf_secutil.c +++ b/osaf/libs/core/common/osaf_secutil.c @@ -359,7 +359,9 @@ int osaf_get_group_list(const uid_t uid, /* Reset entry to beginning */ errno = 0; setgrent(); - if (errno != 0) { + /* setgrent() sometimes returns ENOENT on UML + * Explicitly treats it as not an error */ + if (errno != 0 && errno != ENOENT) { LOG_NO("setgrent failed: %s", strerror(errno)); return -1; } ------------------------------------------------------------------------------ Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server from Actuate! Instantly Supercharge Your Business Reports and Dashboards with Interactivity, Sharing, Native Excel Exports, App Integration & more Get technology previously reserved for billion-dollar corporations, FREE http://pubads.g.doubleclick.net/gampad/clk?id=190641631&iu=/4140/ostg.clktrk _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
