Skip to content

Commit 5bde4a7

Browse files
mergify[bot]mergify[bot]
authored
Merge pull request flux-framework#4870 from garlick/broker_attr_cleanup
clean up little used broker attribute functionality
2 parents 6906b6e + 8761ca1 commit 5bde4a7

File tree

21 files changed

+278
-217
lines changed

21 files changed

+278
-217
lines changed

‎doc/man1/flux-getattr.rst

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ SYNOPSIS
1313

1414
**flux** **setattr** *name* *value*
1515

16-
**flux** **setattr** [*--expunge*] *name*
16+
**flux** **setattr** *name*
1717

1818
**flux** **lsattr** [*--values*]
1919

‎doc/man7/flux-broker-attributes.rst

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -211,16 +211,10 @@ tbon.tcp_user_timeout
211211
LOGGING
212212
=======
213213

214-
log-ring-used
215-
The number of log entries currently stored in the ring buffer.
216-
217214
log-ring-size [Updates: C, R]
218215
The maximum number of log entries that can be stored in the ring buffer.
219216
Default: ``1024``.
220217

221-
log-count
222-
The number of log entries ever stored in the ring buffer.
223-
224218
log-forward-level [Updates: C, R]
225219
Log entries at :linux:man3:`syslog` level at or below this value
226220
are forwarded to rank zero for permanent capture. Default ``7``

‎etc/completions/flux.pre

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1173,9 +1173,6 @@ _flux_attr()
11731173
local lsattr_OPTS="\
11741174
-v --values \
11751175
"
1176-
local setattr_OPTS="\
1177-
-e --expunge \
1178-
"
11791176
if [[ $cmd != "flux" ]]; then
11801177
if [[ $cur != -* && $cmd == "getattr" ]]; then
11811178
COMPREPLY=( $(compgen -W "$(flux lsattr)" -- "$cur") )

‎src/broker/attr.c

Lines changed: 7 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -69,12 +69,7 @@ int attr_delete (attr_t *attrs, const char *name, bool force)
6969
int rc = -1;
7070

7171
if ((e = zhash_lookup (attrs->hash, name))) {
72-
if ((e->flags & FLUX_ATTRFLAG_IMMUTABLE)) {
73-
errno = EPERM;
74-
goto done;
75-
}
76-
if (((e->flags & FLUX_ATTRFLAG_READONLY)
77-
|| (e->flags & FLUX_ATTRFLAG_ACTIVE)) && !force) {
72+
if ((e->flags & ATTR_IMMUTABLE)) {
7873
errno = EPERM;
7974
goto done;
8075
}
@@ -89,7 +84,7 @@ int attr_add (attr_t *attrs, const char *name, const char *val, int flags)
8984
{
9085
struct entry *e;
9186

92-
if (attrs == NULL || name == NULL || (flags & FLUX_ATTRFLAG_ACTIVE)) {
87+
if (attrs == NULL || name == NULL) {
9388
errno = EINVAL;
9489
return -1;
9590
}
@@ -127,7 +122,6 @@ int attr_add_active (attr_t *attrs, const char *name, int flags,
127122
e->set = set;
128123
e->get = get;
129124
e->arg = arg;
130-
e->flags |= FLUX_ATTRFLAG_ACTIVE;
131125
zhash_update (attrs->hash, name, e);
132126
zhash_freefn (attrs->hash, name, entry_destroy);
133127
rc = 0;
@@ -149,7 +143,7 @@ int attr_get (attr_t *attrs, const char *name, const char **val, int *flags)
149143
goto done;
150144
}
151145
if (e->get) {
152-
if (!e->val || !(e->flags & FLUX_ATTRFLAG_IMMUTABLE)) {
146+
if (!e->val || !(e->flags & ATTR_IMMUTABLE)) {
153147
const char *tmp;
154148
if (e->get (name, &tmp, e->arg) < 0)
155149
goto done;
@@ -172,7 +166,7 @@ int attr_get (attr_t *attrs, const char *name, const char **val, int *flags)
172166
return rc;
173167
}
174168

175-
int attr_set (attr_t *attrs, const char *name, const char *val, bool force)
169+
int attr_set (attr_t *attrs, const char *name, const char *val)
176170
{
177171
struct entry *e;
178172
int rc = -1;
@@ -181,11 +175,7 @@ int attr_set (attr_t *attrs, const char *name, const char *val, bool force)
181175
errno = ENOENT;
182176
goto done;
183177
}
184-
if ((e->flags & FLUX_ATTRFLAG_IMMUTABLE)) {
185-
errno = EPERM;
186-
goto done;
187-
}
188-
if ((e->flags & FLUX_ATTRFLAG_READONLY) && !force) {
178+
if ((e->flags & ATTR_IMMUTABLE)) {
189179
errno = EPERM;
190180
goto done;
191181
}
@@ -351,7 +341,7 @@ int attr_cache_immutables (attr_t *attrs, flux_t *h)
351341

352342
e = zhash_first (attrs->hash);
353343
while (e) {
354-
if ((e->flags & FLUX_ATTRFLAG_IMMUTABLE)) {
344+
if ((e->flags & ATTR_IMMUTABLE)) {
355345
if (flux_attr_set_cacheonly (h, e->name, e->val) < 0)
356346
return -1;
357347
}
@@ -400,7 +390,7 @@ void setattr_request_cb (flux_t *h, flux_msg_handler_t *mh,
400390
if (flux_request_unpack (msg, NULL, "{s:s s:s}", "name", &name,
401391
"value", &val) < 0)
402392
goto error;
403-
if (attr_set (attrs, name, val, false) < 0) {
393+
if (attr_set (attrs, name, val) < 0) {
404394
if (errno != ENOENT)
405395
goto error;
406396
if (attr_add (attrs, name, val, 0) < 0)
@@ -414,24 +404,6 @@ void setattr_request_cb (flux_t *h, flux_msg_handler_t *mh,
414404
FLUX_LOG_ERROR (h);
415405
}
416406

417-
void rmattr_request_cb (flux_t *h, flux_msg_handler_t *mh,
418-
const flux_msg_t *msg, void *arg)
419-
{
420-
attr_t *attrs = arg;
421-
const char *name;
422-
423-
if (flux_request_unpack (msg, NULL, "{s:s}", "name", &name) < 0)
424-
goto error;
425-
if (attr_delete (attrs, name, false) < 0)
426-
goto error;
427-
if (flux_respond (h, msg, NULL) < 0)
428-
FLUX_LOG_ERROR (h);
429-
return;
430-
error:
431-
if (flux_respond_error (h, msg, errno, NULL) < 0)
432-
FLUX_LOG_ERROR (h);
433-
}
434-
435407
void lsattr_request_cb (flux_t *h, flux_msg_handler_t *mh,
436408
const flux_msg_t *msg, void *arg)
437409
{
@@ -475,7 +447,6 @@ static const struct flux_msg_handler_spec handlers[] = {
475447
{ FLUX_MSGTYPE_REQUEST, "attr.get", getattr_request_cb, FLUX_ROLE_ALL },
476448
{ FLUX_MSGTYPE_REQUEST, "attr.list", lsattr_request_cb, FLUX_ROLE_ALL },
477449
{ FLUX_MSGTYPE_REQUEST, "attr.set", setattr_request_cb, 0 },
478-
{ FLUX_MSGTYPE_REQUEST, "attr.rm", rmattr_request_cb, 0 },
479450
FLUX_MSGHANDLER_TABLE_END,
480451
};
481452

‎src/broker/attr.h

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,7 @@
1515
#include <flux/core.h>
1616

1717
enum {
18-
FLUX_ATTRFLAG_IMMUTABLE = 1, /* attribute is cacheable */
19-
FLUX_ATTRFLAG_READONLY = 2, /* attribute cannot be written */
20-
/* but may change on broker */
21-
FLUX_ATTRFLAG_ACTIVE = 4, /* attribute has get and/or set callbacks */
18+
ATTR_IMMUTABLE = 1, /* attribute is cacheable */
2219
};
2320

2421
/* Callbacks for active values. Return 0 on succes, -1 on eror with
@@ -57,7 +54,7 @@ int attr_add_uint32 (attr_t *attrs, const char *name, uint32_t val,
5754
*/
5855
int attr_get (attr_t *attrs, const char *name, const char **val, int *flags);
5956

60-
int attr_set (attr_t *attrs, const char *name, const char *val, bool force);
57+
int attr_set (attr_t *attrs, const char *name, const char *val);
6158

6259
/* Set an attribute's flags.
6360
*/

‎src/broker/boot_config.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -300,7 +300,7 @@ int boot_config_attr (attr_t *attrs, json_t *hosts)
300300
|| attr_add (attrs,
301301
"hostlist",
302302
hostname,
303-
FLUX_ATTRFLAG_IMMUTABLE) < 0) {
303+
ATTR_IMMUTABLE) < 0) {
304304
log_err ("failed to set hostlist attribute to localhost");
305305
goto error;
306306
}
@@ -327,7 +327,7 @@ int boot_config_attr (attr_t *attrs, json_t *hosts)
327327
if (attr_add (attrs,
328328
"hostlist",
329329
s,
330-
FLUX_ATTRFLAG_IMMUTABLE) < 0) {
330+
ATTR_IMMUTABLE) < 0) {
331331
log_err ("failed to set hostlist attribute to config derived value");
332332
goto error;
333333
}
@@ -351,7 +351,7 @@ int boot_config_attr (attr_t *attrs, json_t *hosts)
351351
}
352352
val = s;
353353
}
354-
if (attr_add (attrs, "broker.mapping", val, FLUX_ATTRFLAG_IMMUTABLE) < 0) {
354+
if (attr_add (attrs, "broker.mapping", val, ATTR_IMMUTABLE) < 0) {
355355
log_err ("setattr broker.mapping");
356356
goto error;
357357
}
@@ -584,7 +584,7 @@ int boot_config (flux_t *h, struct overlay *overlay, attr_t *attrs)
584584
if (attr_add (attrs,
585585
"tbon.endpoint",
586586
my_uri,
587-
FLUX_ATTRFLAG_IMMUTABLE) < 0) {
587+
ATTR_IMMUTABLE) < 0) {
588588
log_err ("setattr tbon.endpoint %s", my_uri);
589589
goto error;
590590
}
@@ -593,7 +593,7 @@ int boot_config (flux_t *h, struct overlay *overlay, attr_t *attrs)
593593
if (attr_add (attrs,
594594
"tbon.endpoint",
595595
NULL,
596-
FLUX_ATTRFLAG_IMMUTABLE) < 0) {
596+
ATTR_IMMUTABLE) < 0) {
597597
log_err ("setattr tbon.endpoint NULL");
598598
goto error;
599599
}
@@ -625,7 +625,7 @@ int boot_config (flux_t *h, struct overlay *overlay, attr_t *attrs)
625625
if (attr_add (attrs,
626626
"instance-level",
627627
"0",
628-
FLUX_ATTRFLAG_IMMUTABLE) < 0) {
628+
ATTR_IMMUTABLE) < 0) {
629629
log_err ("setattr instance-level 0");
630630
goto error;
631631
}

‎src/broker/boot_pmi.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,11 @@ static int set_instance_level_attr (struct upmi *upmi,
5050
rc = upmi_get (upmi, "flux.instance-level", -1, &val, NULL);
5151
if (rc == 0)
5252
level = val;
53-
if (attr_add (attrs, "instance-level", level, FLUX_ATTRFLAG_IMMUTABLE) < 0)
53+
if (attr_add (attrs, "instance-level", level, ATTR_IMMUTABLE) < 0)
5454
return -1;
5555
if (rc == 0)
5656
jobid = name;
57-
if (attr_add (attrs, "jobid", jobid, FLUX_ATTRFLAG_IMMUTABLE) < 0)
57+
if (attr_add (attrs, "jobid", jobid, ATTR_IMMUTABLE) < 0)
5858
return -1;
5959
return 0;
6060
}
@@ -97,7 +97,7 @@ static int set_broker_mapping_attr (struct upmi *upmi,
9797
free (s);
9898
}
9999
}
100-
rc = attr_add (attrs, "broker.mapping", val, FLUX_ATTRFLAG_IMMUTABLE);
100+
rc = attr_add (attrs, "broker.mapping", val, ATTR_IMMUTABLE);
101101
free (val);
102102
return rc;
103103
}
@@ -180,7 +180,7 @@ static int set_hostlist_attr (attr_t *attrs, struct hostlist *hl)
180180
}
181181
else
182182
s = hostlist_encode (hl);
183-
if (s && attr_add (attrs, "hostlist", s, FLUX_ATTRFLAG_IMMUTABLE) == 0)
183+
if (s && attr_add (attrs, "hostlist", s, ATTR_IMMUTABLE) == 0)
184184
rc = 0;
185185
ERRNO_SAFE_WRAP (free, s);
186186
return rc;
@@ -289,7 +289,7 @@ int boot_pmi (struct overlay *overlay, attr_t *attrs)
289289
else {
290290
uri = NULL;
291291
}
292-
if (attr_add (attrs, "tbon.endpoint", uri, FLUX_ATTRFLAG_IMMUTABLE) < 0) {
292+
if (attr_add (attrs, "tbon.endpoint", uri, ATTR_IMMUTABLE) < 0) {
293293
log_err ("setattr tbon.endpoint");
294294
goto error;
295295
}

‎src/broker/broker.c

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ void parse_command_line_arguments (int argc, char *argv[], broker_ctx_t *ctx)
141141
if ((val = strchr (attr, '=')))
142142
*val++ = '\0';
143143
if (attr_add (ctx->attrs, attr, val, 0) < 0)
144-
if (attr_set (ctx->attrs, attr, val, true) < 0)
144+
if (attr_set (ctx->attrs, attr, val) < 0)
145145
log_err_exit ("setattr %s=%s", attr, val);
146146
free (attr);
147147
}
@@ -375,11 +375,11 @@ int main (int argc, char *argv[])
375375
}
376376
int flags;
377377
assert (attr_get (ctx.attrs, "rank", NULL, &flags) == 0
378-
&& (flags & FLUX_ATTRFLAG_IMMUTABLE));
378+
&& (flags & ATTR_IMMUTABLE));
379379
assert (attr_get (ctx.attrs, "size", NULL, &flags) == 0
380-
&& (flags & FLUX_ATTRFLAG_IMMUTABLE));
380+
&& (flags & ATTR_IMMUTABLE));
381381
assert (attr_get (ctx.attrs, "hostlist", NULL, &flags) == 0
382-
&& (flags & FLUX_ATTRFLAG_IMMUTABLE));
382+
&& (flags & ATTR_IMMUTABLE));
383383

384384
if (!(ctx.groups = groups_create (&ctx))) {
385385
log_err ("groups_create");
@@ -589,7 +589,7 @@ static void init_attrs_broker_pid (attr_t *attrs, pid_t pid)
589589
if (attr_add (attrs,
590590
attrname,
591591
pidval,
592-
FLUX_ATTRFLAG_IMMUTABLE) < 0)
592+
ATTR_IMMUTABLE) < 0)
593593
log_err_exit ("attr_add %s", attrname);
594594
}
595595

@@ -627,7 +627,7 @@ static void init_attrs_starttime (attr_t *attrs, double starttime)
627627
char buf[32];
628628

629629
snprintf (buf, sizeof (buf), "%.2f", starttime);
630-
if (attr_add (attrs, "broker.starttime", buf, FLUX_ATTRFLAG_IMMUTABLE) < 0)
630+
if (attr_add (attrs, "broker.starttime", buf, ATTR_IMMUTABLE) < 0)
631631
log_err_exit ("error setting broker.starttime attribute");
632632
}
633633

@@ -650,7 +650,7 @@ static void init_attrs (attr_t *attrs, pid_t pid, struct flux_msg_cred *cred)
650650

651651
char tmp[32];
652652
snprintf (tmp, sizeof (tmp), "%ju", (uintmax_t)cred->userid);
653-
if (attr_add (attrs, "security.owner", tmp, FLUX_ATTRFLAG_IMMUTABLE) < 0)
653+
if (attr_add (attrs, "security.owner", tmp, ATTR_IMMUTABLE) < 0)
654654
log_err_exit ("attr_add owner");
655655
}
656656

@@ -804,15 +804,15 @@ static int check_statedir (attr_t *attrs)
804804
const char *statedir;
805805

806806
if (attr_get (attrs, "statedir", &statedir, NULL) < 0) {
807-
if (attr_add (attrs, "statedir", NULL, FLUX_ATTRFLAG_IMMUTABLE) < 0) {
807+
if (attr_add (attrs, "statedir", NULL, ATTR_IMMUTABLE) < 0) {
808808
log_err ("error creating statedir broker attribute");
809809
return -1;
810810
}
811811
}
812812
else {
813813
if (checkdir ("statedir", statedir) < 0)
814814
return -1;
815-
if (attr_set_flags (attrs, "statedir", FLUX_ATTRFLAG_IMMUTABLE) < 0) {
815+
if (attr_set_flags (attrs, "statedir", ATTR_IMMUTABLE) < 0) {
816816
log_err ("error setting statedir broker attribute flags");
817817
return -1;
818818
}
@@ -884,7 +884,7 @@ static int create_rundir (attr_t *attrs)
884884
/* rundir is now fixed, so make the attribute immutable, and
885885
* schedule the dir for cleanup at exit if we created it here.
886886
*/
887-
if (attr_set_flags (attrs, "rundir", FLUX_ATTRFLAG_IMMUTABLE) < 0) {
887+
if (attr_set_flags (attrs, "rundir", ATTR_IMMUTABLE) < 0) {
888888
log_err ("error setting rundir broker attribute flags");
889889
goto done;
890890
}
@@ -913,7 +913,7 @@ static int init_local_uri_attr (struct overlay *ov, attr_t *attrs)
913913
log_msg ("buffer overflow while building local-uri");
914914
return -1;
915915
}
916-
if (attr_add (attrs, "local-uri", buf, FLUX_ATTRFLAG_IMMUTABLE) < 0) {
916+
if (attr_add (attrs, "local-uri", buf, ATTR_IMMUTABLE) < 0) {
917917
log_err ("setattr local-uri");
918918
return -1;
919919
}
@@ -963,7 +963,7 @@ static int init_critical_ranks_attr (struct overlay *ov, attr_t *attrs)
963963
if (attr_add (attrs,
964964
"broker.critical-ranks",
965965
ranks,
966-
FLUX_ATTRFLAG_IMMUTABLE) < 0) {
966+
ATTR_IMMUTABLE) < 0) {
967967
log_err ("attr_add critical_ranks");
968968
goto out;
969969
}
@@ -978,7 +978,7 @@ static int init_critical_ranks_attr (struct overlay *ov, attr_t *attrs)
978978
*/
979979
if (attr_set_flags (attrs,
980980
"broker.critical-ranks",
981-
FLUX_ATTRFLAG_IMMUTABLE) < 0) {
981+
ATTR_IMMUTABLE) < 0) {
982982
log_err ("failed to make broker.criitcal-ranks attr immutable");
983983
goto out;
984984
}

‎src/broker/brokercfg.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -646,7 +646,7 @@ struct brokercfg *brokercfg_create (flux_t *h,
646646
}
647647
if (flux_msg_handler_addvec (h, htab, cfg, &cfg->handlers) < 0)
648648
goto error;
649-
if (attr_add (attrs, "config.path", path, FLUX_ATTRFLAG_IMMUTABLE) < 0)
649+
if (attr_add (attrs, "config.path", path, ATTR_IMMUTABLE) < 0)
650650
goto error;
651651
return cfg;
652652
error:

0 commit comments

Comments
 (0)