Merge pull request #1845 from garlick/attr_cleanup2

libflux: clean up interface for broker attributes

doc/man3/Makefile.am

@@ -76,8 +76,6 @@ MAN3_FILES_SECONDARY = \
 	flux_msg_recvfd.3 \
 	flux_get_size.3 \
 	flux_attr_set.3 \
-	flux_attr_first.3 \
-	flux_attr_next.3 \
 	flux_set_reactor.3 \
 	flux_reactor_destroy.3 \
 	flux_reactor_run.3 \
@@ -225,8 +223,6 @@ flux_msg_decode.3: flux_msg_encode.3
 flux_msg_recvfd.3: flux_msg_sendfd.3
 flux_get_size.3: flux_get_rank.3
 flux_attr_set.3: flux_attr_get.3
-flux_attr_first.3: flux_attr_get.3
-flux_attr_next.3: flux_attr_get.3
 flux_set_reactor.3: flux_get_reactor.3
 flux_reactor_destroy.3: flux_reactor_create.3
 flux_reactor_run.3: flux_reactor_create.3

doc/man3/flux_attr_get.adoc

@@ -5,20 +5,17 @@ flux_attr_get(3)
 
 NAME
 ----
-flux_attr_get, flux_attr_set, flux_attr_first, flux_attr_next - query Flux broker attributes
+flux_attr_get, flux_attr_set - get/set Flux broker attributes
 
 
 SYNOPSIS
 --------
-#include <flux/core.h>
+ #include <flux/core.h>
 
-const char *flux_attr_get (flux_t *h, const char *name, int *flags);
+ const char *flux_attr_get (flux_t *h, const char *name);
 
-int flux_attr_set (flux_t *h, const char *name, const char *val);
+ int flux_attr_set (flux_t *h, const char *name, const char *val);
 
-const char *flux_attr_first (flux_t *h);
-
-const char *flux_attr_next (flux_t *h);
 
 DESCRIPTION
 -----------
@@ -28,34 +25,16 @@ store with scope limited to the local broker rank, and a method for the
 broker to export information needed by Flux comms modules and utilities.
 
 `flux_attr_get()` retrieves the value of the attribute _name_.
-If _flags_ is non-NULL, the flags associated with the attribute are
-stored there.  Flags are a mask of the following bit values:
-
-FLUX_ATTRFLAG_IMMUTABLE::
-The value of this attribute will never change.  It can be cached indefinitely.
-
-FLUX_ATTRFLAG_READONLY::
-The value of this attribute cannot be changed with `flux_attr_set()`,
-however it may change on the broker and should not be cached.
 
-FLUX_ATTRFLAG_ACTIVE::
-Getting or setting this attribute may have side effects in the broker.
-For example, retrieving the "snoop-uri" attribute triggers the creation
-of the snoop socket by the broker on the first call.
-
-Attributes that have the FLUX_ATTRFLAG_IMMUTABLE flag are cached automatically
+Attributes that have the broker tags as _immutable_ are cached automatically
 in the flux_t handle.  For example, the "rank" attribute is a frequently
 accessed attribute whose value never changes.  It will be cached on the first
 access and thereafter does not require an RPC to the broker to access.
 
 `flux_attr_set()` updates the value of attribute _name_ to _val_.
 If _name_ is not currently a valid attribute, it is created.
-Attributes created through this interface will have no flags set.
 If _val_ is NULL, the attribute is unset.
 
-`flux_attr_first()` and `flux_attr_next()` are used to iterate
-through the current set of valid attribute names.
-
 
 RETURN VALUE
 ------------
@@ -66,9 +45,6 @@ is returned and errno is set appropriately.
 `flux_attr_set()` returns zero on success.  On error, -1 is returned
 and errno is set appropriately.
 
-`flux_attr_first()` and `flux_attr_next()` return an attribute name or
-NULL to indicate that iteration is complete.
-
 
 ERRORS
 ------
@@ -80,8 +56,8 @@ EINVAL::
 Some arguments were invalid.
 
 EPERM::
-Set was attempted on an attribute with the FLUX_ATTR_IMMUTABLE or
-FLUX_ATTR_READONLY flags.
+Set was attempted on an attribute that is not writable with the
+user's credentials.
 
 
 AUTHOR

src/bindings/lua/flux-lua.c

@@ -458,7 +458,7 @@ static int l_flux_arity (lua_State *L)
     const char *s;
     int arity;
 
-    if (!(s = flux_attr_get (f, "tbon.arity", NULL)))
+    if (!(s = flux_attr_get (f, "tbon.arity")))
         return lua_pusherror (L, "flux_attr_get tbon.arity error");
     arity = strtoul (s, NULL, 10);
     return (l_pushresult (L, arity));
@@ -643,38 +643,15 @@ static int l_flux_rpc (lua_State *L)
     return (rc);
 }
 
-static void push_attr_flags (lua_State *L, int flags)
-{
-    int t;
-    lua_newtable (L);
-    if (flags == 0)
-        return;
-    t = lua_gettop (L);
-    if (flags & FLUX_ATTRFLAG_IMMUTABLE) {
-        lua_pushboolean (L, true);
-        lua_setfield (L, t, "FLUX_ATTRFLAG_IMMUTABLE");
-    }
-    if (flags & FLUX_ATTRFLAG_READONLY) {
-        lua_pushboolean (L, true);
-        lua_setfield (L, t, "FLUX_ATTRFLAG_READONLY");
-    }
-    if (flags & FLUX_ATTRFLAG_ACTIVE) {
-        lua_pushboolean (L, true);
-        lua_setfield (L, t, "FLUX_ATTRFLAG_ACTIVE");
-    }
-}
-
 static int l_flux_getattr (lua_State *L)
 {
     flux_t *f = lua_get_flux (L, 1);
-    int flags;
     const char *name = luaL_checkstring (L, 2);
-    const char *val = flux_attr_get (f, name, &flags);
+    const char *val = flux_attr_get (f, name);
     if (val == NULL)
         return lua_pusherror (L, (char *)flux_strerror (errno));
     lua_pushstring (L, val);
-    push_attr_flags (L, flags);
-    return (2);
+    return (1);
 }
 
 static int l_flux_subscribe (lua_State *L)

src/broker/attr.c

@@ -350,29 +350,38 @@ void setattr_request_cb (flux_t *h, flux_msg_handler_t *mh,
     const char *name;
     const char *val;
 
-    if (flux_request_unpack (msg, NULL, "{s:s s:s}",       /* SET */
-                             "name", &name,
-                             "value", &val) == 0) {
-        if (attr_set (attrs, name, val, false) < 0) {
-            if (errno != ENOENT)
-                goto error;
-            if (attr_add (attrs, name, val, 0) < 0)
-                goto error;
-        }
-    }
-    else if (flux_request_unpack (msg, NULL, "{s:s s:n}",  /* DELETE */
-                                  "name", &name,
-                                  "value") == 0) {
-        if (attr_delete (attrs, name, false) < 0)
+    if (flux_request_unpack (msg, NULL, "{s:s s:s}", "name", &name,
+                                                     "value", &val) < 0)
+        goto error;
+    if (attr_set (attrs, name, val, false) < 0) {
+        if (errno != ENOENT)
+            goto error;
+        if (attr_add (attrs, name, val, 0) < 0)
             goto error;
     }
-    else
+    if (flux_respond (h, msg, 0, NULL) < 0)
+        FLUX_LOG_ERROR (h);
+    return;
+error:
+    if (flux_respond_error (h, msg, errno, NULL) < 0)
+        FLUX_LOG_ERROR (h);
+}
+
+void rmattr_request_cb (flux_t *h, flux_msg_handler_t *mh,
+                        const flux_msg_t *msg, void *arg)
+{
+    attr_t *attrs = arg;
+    const char *name;
+
+    if (flux_request_unpack (msg, NULL, "{s:s}", "name", &name) < 0)
+        goto error;
+    if (attr_delete (attrs, name, false) < 0)
         goto error;
     if (flux_respond (h, msg, 0, NULL) < 0)
         FLUX_LOG_ERROR (h);
     return;
 error:
-    if (flux_respond (h, msg, errno, NULL) < 0)
+    if (flux_respond_error (h, msg, errno, NULL) < 0)
         FLUX_LOG_ERROR (h);
 }
 
@@ -420,6 +429,7 @@ static const struct flux_msg_handler_spec handlers[] = {
     { FLUX_MSGTYPE_REQUEST, "attr.get",    getattr_request_cb, FLUX_ROLE_ALL },
     { FLUX_MSGTYPE_REQUEST, "attr.list",   lsattr_request_cb, FLUX_ROLE_ALL },
     { FLUX_MSGTYPE_REQUEST, "attr.set",    setattr_request_cb, 0 },
+    { FLUX_MSGTYPE_REQUEST, "attr.rm",     rmattr_request_cb, 0 },
     FLUX_MSGHANDLER_TABLE_END,
 };
 

src/broker/attr.h

@@ -4,6 +4,13 @@
 #include <stdint.h>
 #include <flux/core.h>
 
+enum {
+    FLUX_ATTRFLAG_IMMUTABLE = 1,    /* attribute is cacheable */
+    FLUX_ATTRFLAG_READONLY = 2,     /* attribute cannot be written */
+                                    /*   but may change on broker */
+    FLUX_ATTRFLAG_ACTIVE = 4,       /* attribute has get and/or set callbacks */
+};
+
 /* Callbacks for active values.  Return 0 on succes, -1 on eror with
  * errno set.  Errors are propagated to the return of attr_set() and attr_get().
  */

src/broker/broker.c

@@ -841,12 +841,12 @@ static int create_dummyattrs (flux_t *h, uint32_t rank, uint32_t size)
 {
     char *s;
     s = xasprintf ("%"PRIu32, rank);
-    if (flux_attr_fake (h, "rank", s, FLUX_ATTRFLAG_IMMUTABLE) < 0)
+    if (flux_attr_set_cacheonly (h, "rank", s) < 0)
         return -1;
     free (s);
 
     s = xasprintf ("%"PRIu32, size);
-    if (flux_attr_fake (h, "size", s, FLUX_ATTRFLAG_IMMUTABLE) < 0)
+    if (flux_attr_set_cacheonly (h, "size", s) < 0)
         return -1;
     free (s);
 

src/broker/log.c

@@ -663,7 +663,7 @@ static int fake_rank (flux_t *h, uint32_t rank)
 {
     char buf[16];
     snprintf (buf, sizeof (buf), "%u", rank);
-    return flux_attr_fake (h, "rank", buf, FLUX_ATTRFLAG_IMMUTABLE);
+    return flux_attr_set_cacheonly (h, "rank", buf);
 }
 
 int logbuf_initialize (flux_t *h, uint32_t rank, attr_t *attrs)

src/broker/module.c

@@ -135,7 +135,7 @@ static void *module_thread (void *arg)
     if (!(p->h = flux_open (uri, 0)))
         log_err_exit ("flux_open %s", uri);
     rankstr = xasprintf ("%"PRIu32, p->rank);
-    if (flux_attr_fake (p->h, "rank", rankstr, FLUX_ATTRFLAG_IMMUTABLE) < 0) {
+    if (flux_attr_set_cacheonly (p->h, "rank", rankstr) < 0) {
         log_err ("%s: error faking rank attribute", p->name);
         goto done;
     }

src/broker/test/hello.c

@@ -39,8 +39,8 @@ int main (int argc, char **argv)
      * Since broker attrs are not set, hwm defaults to 1 (self).
      * It will immediately complete so no need to start reactor.
      */
-    flux_attr_fake (h, "size", "1", FLUX_ATTRFLAG_IMMUTABLE);
-    flux_attr_fake (h, "rank", "0", FLUX_ATTRFLAG_IMMUTABLE);
+    flux_attr_set_cacheonly (h, "size", "1");
+    flux_attr_set_cacheonly (h, "rank", "0");
     ok (flux_get_size (h, &size) == 0 && size == 1,
         "size == 1");
     ok (flux_get_rank (h, &rank) == 0 && rank == 0,
@@ -67,8 +67,8 @@ int main (int argc, char **argv)
     /* Simulate a 2 node session.
      * Same procedure as above except the session will not complete.
      */
-    flux_attr_fake (h, "size", "3", FLUX_ATTRFLAG_IMMUTABLE);
-    flux_attr_fake (h, "rank", "0", FLUX_ATTRFLAG_IMMUTABLE);
+    flux_attr_set_cacheonly (h, "size", "3");
+    flux_attr_set_cacheonly (h, "rank", "0");
     ok (flux_get_size (h, &size) == 0 && size == 3,
         "size == 1");
     ok (flux_get_rank (h, &rank) == 0 && rank == 0,

src/broker/test/reduce.c

@@ -354,9 +354,9 @@ int main (int argc, char *argv[])
     if (!h)
         BAIL_OUT ("can't continue without loop handle");
 
-    flux_attr_fake (h, "rank", "0", FLUX_ATTRFLAG_IMMUTABLE);
-    flux_attr_fake (h, "tbon.level", "0", FLUX_ATTRFLAG_IMMUTABLE);
-    flux_attr_fake (h, "tbon.maxlevel", "0", FLUX_ATTRFLAG_IMMUTABLE);
+    flux_attr_set_cacheonly (h, "rank", "0");
+    flux_attr_set_cacheonly (h, "tbon.level", "0");
+    flux_attr_set_cacheonly (h, "tbon.maxlevel", "0");
 
     test_nopolicy (h); // 6
     test_hwm (h); // 37