From 6a428503eb2c1097eead31d762a9cda1a24e1a3d Mon Sep 17 00:00:00 2001 From: Morris Jette <jette@schedmd.com> Date: Thu, 21 May 2015 13:53:00 -0700 Subject: [PATCH] Add record count to the layouts info RPC This replaces the logic that reads until we run out of buffer space and setting the record count from that. Also add an unlock if the layout dump failed to prevent a slurmctld deadlock. --- src/common/layouts_mgr.c | 22 +++++++++++++++++----- src/common/slurm_protocol_pack.c | 28 +++++++++------------------- src/slurmctld/proc_req.c | 9 +++++---- 3 files changed, 31 insertions(+), 28 deletions(-) diff --git a/src/common/layouts_mgr.c b/src/common/layouts_mgr.c index 025d7d1f3da..8fc09e17251 100644 --- a/src/common/layouts_mgr.c +++ b/src/common/layouts_mgr.c @@ -1587,6 +1587,7 @@ typedef struct _pack_args { char *type; uint32_t all; uint32_t no_relation; + uint32_t record_count; } _pack_args_t; /* @@ -1747,6 +1748,7 @@ static uint8_t _pack_layout_tree(xtree_node_t* node, uint8_t which, hostlist_find(pargs->list_entities, e_name) != -1) { str = xstrdup_printf("Root=%s\n", e_name); packstr(str, buffer); + pargs->record_count++; xfree(str); } } @@ -1761,9 +1763,10 @@ static uint8_t _pack_layout_tree(xtree_node_t* node, uint8_t which, /* add entity keys matching the layout to the current str */ pargs->current_line = str; - if (enode && enode->entity) + if (enode && enode->entity) { xhash_walk(enode->entity->data, _pack_entity_layout_data, pargs); + } /* the current line might have been extended/remalloced, so * we need to sync it again in str for further actions */ str = pargs->current_line; @@ -1812,6 +1815,7 @@ static uint8_t _pack_layout_tree(xtree_node_t* node, uint8_t which, } packstr(str, buffer); + pargs->record_count++; xfree(str); return 1; @@ -2527,12 +2531,14 @@ int layouts_pack_layout(char *l_type, char *char_entities, char *type, { _pack_args_t pargs; layout_t* layout; + int orig_offset, fini_offset; char *str; slurm_mutex_lock(&mgr->lock); layout = layouts_get_layout_nolock(l_type); if (layout == NULL) { + slurm_mutex_unlock(&mgr->lock); error("unable to get layout of type '%s'", l_type); return SLURM_ERROR; } @@ -2550,7 +2556,9 @@ int layouts_pack_layout(char *l_type, char *char_entities, char *type, } pargs.type = type; pargs.no_relation = no_relation; - + pargs.record_count = 0; + orig_offset = get_buf_offset(buffer); + pack32(pargs.record_count, buffer); if ( pargs.no_relation == 0 && pargs.list_entities == NULL @@ -2558,21 +2566,25 @@ int layouts_pack_layout(char *l_type, char *char_entities, char *type, /* start by packing the layout priority */ str = xstrdup_printf("Priority=%u\n", layout->priority); packstr(str, buffer); + pargs.record_count++; xfree(str); } /* pack according to the layout struct type */ - switch(layout->struct_type) { + switch (layout->struct_type) { case LAYOUT_STRUCT_TREE: xtree_walk(layout->tree, NULL, 0, XTREE_LEVEL_MAX, _pack_layout_tree, &pargs); break; } + if (pargs.list_entities != NULL) slurm_hostlist_destroy(pargs.list_entities); - /* EOF */ - packstr("", buffer); + fini_offset = get_buf_offset(buffer); + set_buf_offset(buffer, orig_offset); + pack32(pargs.record_count, buffer); + set_buf_offset(buffer, fini_offset); slurm_mutex_unlock(&mgr->lock); diff --git a/src/common/slurm_protocol_pack.c b/src/common/slurm_protocol_pack.c index f8c0166534d..a5aa6192312 100644 --- a/src/common/slurm_protocol_pack.c +++ b/src/common/slurm_protocol_pack.c @@ -4962,43 +4962,33 @@ static int _unpack_layout_info_msg(layout_info_msg_t ** msg, Buf buffer, uint16_t protocol_version) { - static int base_buffer_size = 1024; int i; char **records; uint32_t utmp32; char *tmp_str = NULL; xassert(msg != NULL); - *msg = xmalloc(sizeof(layout_info_msg_t)); if (protocol_version >= SLURM_MIN_PROTOCOL_VERSION) { - records = (*msg)->records = - xmalloc(sizeof(char*) * base_buffer_size); - (*msg)->record_count = base_buffer_size; + *msg = xmalloc(sizeof(layout_info_msg_t)); + safe_unpack32(&(*msg)->record_count, buffer); + (*msg)->records = xmalloc(sizeof(char*) * (*msg)->record_count); + records = (*msg)->records; i = 0; while (remaining_buf(buffer) > 0) { safe_unpackstr_xmalloc(&tmp_str, &utmp32, buffer); if (tmp_str != NULL) { - if (*tmp_str == '\0') { - xfree(tmp_str); - break; + if (tmp_str[0] == '\0') { + xfree(tmp_str); + break; } - if (i == (*msg)->record_count) { - (*msg)->record_count - += base_buffer_size; - records = xrealloc(records, - sizeof(char*) * - (*msg)->record_count); - } - records[i] = tmp_str; - ++i; - //xfree(tmp_str); + records[i++] = tmp_str; + // tmp_str = NULL; /* Nothing left to free */ continue; } } (*msg)->record_count = i; - xrealloc(records, sizeof(char*) * (*msg)->record_count); } else { error("%s: protocol_version %hu not supported", __func__, protocol_version); diff --git a/src/slurmctld/proc_req.c b/src/slurmctld/proc_req.c index 5e888d40123..7c596524cd3 100644 --- a/src/slurmctld/proc_req.c +++ b/src/slurmctld/proc_req.c @@ -3950,7 +3950,8 @@ static void _slurm_rpc_layout_show(slurm_msg_t * msg) debug2("Processing RPC: REQUEST_LAYOUT_INFO"); if (layout_req_msg->layout_type == NULL) { dump = slurm_get_layouts(); - packstr(dump,buffer); + pack32((uint32_t) 1, buffer); /* Record count */ + packstr(dump, buffer); xfree(dump); } else { if ( layouts_pack_layout(layout_req_msg->layout_type, @@ -3958,15 +3959,15 @@ static void _slurm_rpc_layout_show(slurm_msg_t * msg) layout_req_msg->type, layout_req_msg->no_relation, buffer) != SLURM_SUCCESS) { - debug2("_slurm_rpc_layout_show, unable to get layout[%s]", - layout_req_msg->layout_type); - //FIXME: use an adapted response + debug2("%s: unable to get layout[%s]", + __func__, layout_req_msg->layout_type); slurm_send_rc_msg(msg, SLURM_NO_CHANGE_IN_DATA); flag = 0; } } if ( flag == 1 ) { dump_size = get_buf_offset(buffer); + high_buffer_size = MAX(high_buffer_size, dump_size); dump = xfer_buf_data(buffer); END_TIMER2("_slurm_rpc_resv_show"); -- GitLab