Hi Anders Widell ,
*Thank for the review , please find my responses .***
On 4/4/2016 3:51 PM, Anders Widell wrote:
> Hi!
>
> Here are my comments:
>
> 1) I think you need to make this feature configurable, as I see it as
> potentially not backwards compatible.
*[AVM]* The APIs used to fail with TIMEOUT ERR continuously (CPND used
to crash in memcopy) in case of low disk space then requested ,
now this patch only makes the API succeed or returns
correct error SA_AIS_ERR_NO_RESOURCES ,
so in this scenario,in that perspective it is only an
improvement to the behavior.
And since this is a POSIX call, I think it should not
cause any backward compatibility issue from library perspective.
>
> 2) A question: Have you considered alternative solutions? For example,
> installing a signal handler for SIGBUS to catch the problem and then
> propagate the error back to the CKPT API user?
*[AVM]* I did think about that. With this approach, the issue(of lack
of memory) is prevented upfront by returning error at ckptopen()
itself which is beneficial to the user.
Please let me know why it would be backward incompatible,
because this approach improves the application behavior.
-AVM
>
> regards,
> Anders Widell
>
> On 04/04/2016 05:57 AM, A V Mahesh wrote:
>> Hi Anders Widell/Ramesh,
>>
>> Can you please review this ticket by today , this has to go under
>> 5.0.FC tag.
>>
>> -AVM
>>
>>> osaf/libs/core/include/ncs_osprm.h | 1 +
>>> osaf/libs/core/leap/os_defs.c | 13 ++++++++++---
>>> osaf/services/saf/cpsv/cpnd/cpnd_proc.c | 1 +
>>> osaf/services/saf/cpsv/cpnd/cpnd_res.c | 2 ++
>>> osaf/services/saf/glsv/glnd/glnd_shm.c | 1 +
>>> osaf/services/saf/mqsv/mqnd/mqnd_shm.c | 1 +
>>> 6 files changed, 16 insertions(+), 3 deletions(-)
>>>
>>>
>>> Provided ensured disk space is allocated for
>>> NCS_OS_POSIX_SHM_REQ_OPEN request using posix_fallocate()
>>> so that application such as CPSV subsequent writes to bytes in the
>>> specified range are guaranteed not to fail because of lack of disk
>>> space.
>>> Updated the Opensaf services according to new options based on
>>> requirements .
>>>
>>> diff --git a/osaf/libs/core/include/ncs_osprm.h
>>> b/osaf/libs/core/include/ncs_osprm.h
>>> --- a/osaf/libs/core/include/ncs_osprm.h
>>> +++ b/osaf/libs/core/include/ncs_osprm.h
>>> @@ -564,6 +564,7 @@ uint32_t ncs_os_posix_mq(NCS_OS_POSIX_MQ
>>> uint32_t i_flags;
>>> uint32_t i_map_flags;
>>> uint64_t i_size;
>>> + bool ensures_space;
>>> uint64_t i_offset;
>>> void *o_addr;
>>> int32_t o_fd;
>>> diff --git a/osaf/libs/core/leap/os_defs.c
>>> b/osaf/libs/core/leap/os_defs.c
>>> --- a/osaf/libs/core/leap/os_defs.c
>>> +++ b/osaf/libs/core/leap/os_defs.c
>>> @@ -795,9 +795,16 @@ uint32_t ncs_os_posix_shm(NCS_OS_POSIX_S
>>> if (req->info.open.o_fd < 0) {
>>> return NCSCC_RC_FAILURE;
>>> } else {
>>> - if (ftruncate(req->info.open.o_fd, (off_t) shm_size /*
>>> off_t == long */ ) < 0) {
>>> - printf("ftruncate failed with errno value %d \n",
>>> errno);
>>> - return NCSCC_RC_FAILURE;
>>> + if (req->info.open.ensures_space == true) {
>>> + if (posix_fallocate(req->info.open.o_fd, 0,
>>> shm_size) == ENOSPC) {
>>> + LOG_ER("posix_shm:posix_fallocate failed() with
>>> not enough space left ENOSPC: %d \n", errno);
>>> + return NCSCC_RC_FAILURE;
>>> + }
>>> + } else {
>>> + if (ftruncate(req->info.open.o_fd, (off_t) shm_size
>>> /* off_t == long */ ) < 0) {
>>> + printf("ftruncate failed with errno value %d
>>> \n", errno);
>>> + return NCSCC_RC_FAILURE;
>>> + }
>>> }
>>> prot_flag = ncs_shm_prot_flags(req->info.open.i_flags);
>>> diff --git a/osaf/services/saf/cpsv/cpnd/cpnd_proc.c
>>> b/osaf/services/saf/cpsv/cpnd/cpnd_proc.c
>>> --- a/osaf/services/saf/cpsv/cpnd/cpnd_proc.c
>>> +++ b/osaf/services/saf/cpsv/cpnd/cpnd_proc.c
>>> @@ -481,6 +481,7 @@ uint32_t cpnd_ckpt_replica_create(CPND_C
>>> cp_node->replica_info.open.info.open.i_size =
>>> sizeof(CPSV_CKPT_HDR) + cp_node->create_attrib.maxSections
>>> * (sizeof(CPSV_SECT_HDR) +
>>> cp_node->create_attrib.maxSectionSize);
>>> + cp_node->replica_info.open.info.open.ensures_space = true;
>>> cp_node->replica_info.open.info.open.i_offset = 0;
>>> cp_node->replica_info.open.info.open.i_name = buf;
>>> cp_node->replica_info.open.info.open.i_map_flags = MAP_SHARED;
>>> diff --git a/osaf/services/saf/cpsv/cpnd/cpnd_res.c
>>> b/osaf/services/saf/cpsv/cpnd/cpnd_res.c
>>> --- a/osaf/services/saf/cpsv/cpnd/cpnd_res.c
>>> +++ b/osaf/services/saf/cpsv/cpnd/cpnd_res.c
>>> @@ -160,6 +160,7 @@ uint32_t cpnd_ckpt_replica_create_res(NC
>>> open_req->type = NCS_OS_POSIX_SHM_REQ_OPEN;
>>> open_req->info.open.i_size =
>>> sizeof(CPSV_CKPT_HDR) + (cp_info->maxSections *
>>> ((sizeof(CPSV_SECT_HDR) + cp_info->maxSecSize)));
>>> + open_req->info.open.ensures_space = true;
>>> open_req->info.open.i_offset = 0;
>>> open_req->info.open.i_name = buf;
>>> open_req->info.open.i_map_flags = MAP_SHARED;
>>> @@ -344,6 +345,7 @@ void *cpnd_restart_shm_create(NCS_OS_POS
>>> cpnd_open_req->info.open.i_size =
>>> sizeof(CLIENT_HDR) + (MAX_CLIENTS * sizeof(CLIENT_INFO)) +
>>> sizeof(CKPT_HDR) +
>>> (MAX_CKPTS * sizeof(CKPT_INFO));
>>> + cpnd_open_req->info.open.ensures_space = true;
>>> cpnd_open_req->info.open.i_offset = 0;
>>> cpnd_open_req->info.open.i_name = buffer;
>>> cpnd_open_req->info.open.i_map_flags = MAP_SHARED;
>>> diff --git a/osaf/services/saf/glsv/glnd/glnd_shm.c
>>> b/osaf/services/saf/glsv/glnd/glnd_shm.c
>>> --- a/osaf/services/saf/glsv/glnd/glnd_shm.c
>>> +++ b/osaf/services/saf/glsv/glnd/glnd_shm.c
>>> @@ -71,6 +71,7 @@ static uint32_t glnd_shm_open(GLND_CB *c
>>> glnd_open_req.type = NCS_OS_POSIX_SHM_REQ_OPEN;
>>> glnd_open_req.info.open.i_size = shm_size;
>>> + glnd_open_req.info.open.ensures_space = false;
>>> glnd_open_req.info.open.i_offset = 0;
>>> glnd_open_req.info.open.i_name = shm_name;
>>> glnd_open_req.info.open.i_map_flags = MAP_SHARED;
>>> diff --git a/osaf/services/saf/mqsv/mqnd/mqnd_shm.c
>>> b/osaf/services/saf/mqsv/mqnd/mqnd_shm.c
>>> --- a/osaf/services/saf/mqsv/mqnd/mqnd_shm.c
>>> +++ b/osaf/services/saf/mqsv/mqnd/mqnd_shm.c
>>> @@ -66,6 +66,7 @@ uint32_t mqnd_shm_create(MQND_CB *cb)
>>> mqnd_open_req.type = NCS_OS_POSIX_SHM_REQ_OPEN;
>>> mqnd_open_req.info.open.i_size =
>>> sizeof(MQND_SHM_VERSION) + (sizeof(MQND_QUEUE_CKPT_INFO) *
>>> cb->mqnd_shm.max_open_queues);
>>> + mqnd_open_req.info.open.ensures_space = false;
>>> mqnd_open_req.info.open.i_offset = 0;
>>> mqnd_open_req.info.open.i_name = shm_name;
>>> mqnd_open_req.info.open.i_map_flags = MAP_SHARED;
>>>
>>> ------------------------------------------------------------------------------
>>>
>>>
>>> Transform Data into Opportunity.
>>> Accelerate data analysis in your applications with
>>> Intel Data Analytics Acceleration Library.
>>> Click to learn more.
>>> http://pubads.g.doubleclick.net/gampad/clk?id=278785471&iu=/4140
>>> _______________________________________________
>>> Opensaf-devel mailing list
>>> [email protected]
>>> https://lists.sourceforge.net/lists/listinfo/opensaf-devel
>>
>
------------------------------------------------------------------------------
_______________________________________________
Opensaf-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/opensaf-devel
