refactor(controller,xceadmin): drop generic Resource.unit, hardcode bytes for maxmem

Simplification per review feedback on e3b3ffa: a generic schema-driven
unit field touched the capnp wire schema (2 structs), C structs
(xce_resource_t/xce_resval_t), resobj.c's deep-copy/destroy, opget.c's
reply-build lookup, resources.yaml/resources.schema.json, and mapping.py
-- real cross-cutting cost for a mechanism only one resource (maxmem)
actually needs today.

Reverted all of that. The controller CLI (test.c) now just knows
"maxmem" is bytes-valued via a small explicit is_bytes_valued_resource()
name list, and adaptively picks the best-fit human unit at display time
(format_bytes_human, unchanged) -- same observable behavior (resval/
workerlist render maxmem as e.g. "30.90 GB"), zero wire schema surface.
Regenerated the xceadmin golden fixture back to its pre-unit-field size
(5304 bytes, exactly matching the size right after the mxj-only fix,
confirming a clean, complete removal).

Verified: 1095/1095 controller_unit_test (3 runs; one hit the
pre-existing flaky segfault already logged as bug-1553, unrelated),
228/228 xceadmin pytest, clean rebuild of lib/controller/xceadmin/xce_test.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

src/configs/resources.yaml

@@ -58,7 +58,6 @@ resources:
 
   - name: maxmem
     type: numeric
-    unit: bytes
     description: "max memory in bytes (built-in host attribute; CLI display converts to a human-readable unit)"
 
   - name: maxswap

src/configs/schema/resources.schema.json

@@ -13,7 +13,6 @@
           "consumable":  { "type": "boolean" },
           "release":     { "type": "boolean" },
           "description": { "type": "string" },
-          "unit":        { "type": "string", "enum": ["bytes"], "description": "Display-unit hint for CLI/UI formatting; the stored value itself is always in this unit, never converted at rest. Omit for a plain unitless number (the default for every existing resource)." },
           "resourcemap": {
             "type": "array",
             "items": {

src/controller/opget.c

@@ -62,20 +62,9 @@ static int do_reply_resvals(cluster_t* c, endpoint_t* ep,
             rc            = get_resval(c, condition->condition.reskey->resname,
                                        condition->condition.reskey->hostname, &val);
             if (rc >= 0) {
-                xce_resource_t* r = NULL;
-
                 resval.val         = &val;
                 resval.name        = condition->condition.reskey->resname;
                 resval.host        = condition->condition.reskey->hostname;
-                resval.unit        = NULL;
-
-                lock(c->resmgr, RTTI_LOCK_READ);
-                key_find(c->resmgr, (void*)resval.name, (void**)&r);
-                unlock(c->resmgr);
-                if (r != NULL) {
-                    resval.unit = r->unit;
-                }
-
                 resvaldata.n_total = 1;
                 resvaldata.vals    = vals;
             }

src/controller/testsuite/test.c

@@ -2036,11 +2036,40 @@ cleanup:
 }
 
 /*
- * Render a raw byte count as a human-readable string (e.g. "30.90 GB").
+ * Resource names whose canonical stored/wire value is raw bytes (not a
- * Only used for display -- the stored/wire value stays raw bytes always,
+ * generic schema-driven unit system -- deliberately kept as a small,
- * this never feeds back into anything that's compared or persisted. A
+ * explicit, client-side list rather than a wire field, since exactly one
- * pure function so it's directly unit-testable without a live client
+ * resource needs this today and a wire-level unit hint would have grown
- * connection.
+ * every Resource/ResourceVal message regardless of whether it's used).
+ * CLI display picks the best-fit human unit adaptively (format_bytes_human);
+ * the stored value is never converted.
+ */
+static int is_bytes_valued_resource(const char* name)
+{
+    static const char* const BYTES_VALUED_RESOURCES[] = { "maxmem" };
+    const int nres = (int)(sizeof(BYTES_VALUED_RESOURCES)
+                           / sizeof(BYTES_VALUED_RESOURCES[0]));
+    int       i;
+
+    if (name == NULL) {
+        return 0;
+    }
+
+    for (i = 0; i < nres; i++) {
+        if (strcmp(name, BYTES_VALUED_RESOURCES[i]) == 0) {
+            return 1;
+        }
+    }
+
+    return 0;
+}
+
+/*
+ * Render a raw byte count as a human-readable string (e.g. "30.90 GB"),
+ * adaptively picking the smallest unit the value fits in. Only used for
+ * display -- the stored/wire value stays raw bytes always, this never
+ * feeds back into anything that's compared or persisted. A pure function
+ * so it's directly unit-testable without a live client connection.
  */
 static void format_bytes_human(double bytes, char* buf, size_t bufsize)
 {
@@ -2098,7 +2127,7 @@ int cmd_resval(int argc, char* argv[])
                 printf("%s\n", val->val->u.svalue);
                 break;
             case Value_u_fvalue:
-                if ((val->unit != NULL) && (strcmp(val->unit, "bytes") == 0)) {
+                if (is_bytes_valued_resource(val->name)) {
                     char hbuf[64];
 
                     format_bytes_human(val->val->u.fvalue, hbuf,

src/include/xce/types.h

@@ -49,7 +49,6 @@ typedef struct {
     char*      name;
     char*      host;
     xce_val_t* val;
-    char*      unit;
 } xce_resval_t;
 
 typedef struct {
@@ -143,7 +142,6 @@ typedef struct {
     int                  ndistributions;
     res_distribution_t** distributions;
     xce_val_t*           hbased_defined;
-    char*                unit;
 } xce_resource_t;
 
 typedef struct {

src/lib/resobj.c

@@ -35,7 +35,6 @@ static int resobj_init(meta_obj_t* obj, va_list ap)
 
     dst->name = makeString(src->name);
     dst->desc = makeString(src->desc);
-    dst->unit = makeString(src->unit);
 
 #if 0
     if (src->distributions != NULL) {
@@ -65,7 +64,6 @@ static void resobj_destroy(meta_obj_t* obj)
 
     dereference(r->name);
     dereference(r->desc);
-    dereference(r->unit);
 
     memset(priv, 0x0, sizeof(*priv));
 

src/schema/controller.capnp

@@ -209,17 +209,6 @@ struct Resource $C.mapname("xce_resource_t"){
   lastReport     @7: UInt64 $C.mapname("last_report");
 #  distributions  @8: List(ResourceDistribution) $C.maplistcount("ndistributions");
 #  hostBased      @9: Value $C.mapname("hbased_defined");
-  # capnp requires ordinals to be sequential with no holes, so @8/@9 above
-  # (commented out, not yet implemented) cannot be skipped -- unit takes @8
-  # here; whoever implements distributions/hostBased will need to renumber
-  # them to @9/@10 rather than reclaiming @8.
-  # Optional display-unit hint (e.g. "bytes") for resources whose canonical
-  # internal value isn't already human-scale -- empty/unset means "raw
-  # number, no conversion" (every pre-existing resource, unaffected). CLI
-  # display (resval/workerlist) converts to a human-readable form for
-  # known units instead of the raw value; the stored/wire value itself
-  # stays canonical (e.g. maxmem stays bytes, never converted at rest).
-  unit           @8: Text;
 }
 
 struct Threshold $C.mapname("xce_res_threshold_t"){
@@ -357,9 +346,6 @@ struct ResourceVal $C.mapname("xce_resval_t") {
   name       @0: Text $C.mapname("name");
   host       @1: Text $C.mapname("host");
   val        @2: Value $C.mapname("val");
-  # Display-unit hint copied from the resource's own definition (Resource.unit)
-  # at reply-build time -- empty/unset means "raw number, no conversion".
-  unit       @3: Text $C.mapname("unit");
 }
 
 struct ResourceVals $C.mapname("xce_resval_data_t") {

src/xceadmin/src/xcecommon/mapping.py

@@ -71,11 +71,7 @@ reason -- never a silent drop.
     runtime (e.g. ``scheduler.c`` copies these three fields straight out
     of the parsed resource). See :data:`_RESOURCE_VALTYPE`,
     :data:`_RESOURCE_ORDTYPE`, and the flag constants below for the exact
-    values. ``unit`` is mapped straight through from YAML when present --
+    values. ``idx`` and ``lastReport`` remain unset: ``idx`` has no YAML source at all
-    a display-only hint (e.g. ``"bytes"``) the controller CLI (``resval``/
-    ``workerlist``) uses to render a human-readable value; the stored
-    value itself is never converted, only its display. ``idx`` and
-    ``lastReport`` remain unset: ``idx`` has no YAML source at all
     (controller/runtime-assigned), and ``lastReport`` is a runtime
     timestamp field (epoch seconds of last report) -- excluded by
     Requirement 9.2's "no timestamp" rule. ``resourcemap`` has no
@@ -328,8 +324,6 @@ def _build_resource(item: dict, target: "capnp._DynamicStructBuilder") -> None:
     if item.get("release"):
         flags |= _RESOURCE_FLAG_RELEASE
     target.flags = flags
-    if "unit" in item:
-        target.unit = item["unit"]
 
 
 def _build_queue(item: dict, target: "capnp._DynamicStructBuilder") -> None:

src/xceadmin/tests/test_mapping.py

@@ -173,20 +173,6 @@ def test_map_list_section_noop_on_empty_or_missing_items(cluster_config):
     assert len(cluster_config.resources) == 0
 
 
-def test_map_list_section_resources_maps_unit_when_present(cluster_config):
-    items = [
-        {"name": "maxmem", "type": "numeric", "unit": "bytes"},
-        {"name": "maxcpus", "type": "numeric"},
-    ]
-    mapping.map_list_section(_spec_for("resources"), items, cluster_config)
-
-    r0, r1 = cluster_config.resources
-    assert r0.unit == "bytes"
-    # Every existing resource has no "unit" key -- must stay unset (empty
-    # Text default), not silently inherit or default to any value.
-    assert r1.unit == ""
-
-
 # ---------------------------------------------------------------------------
 # map_list_section: users.yaml/hosts.yaml -> ugroups/hgroups (named groups)
 # ---------------------------------------------------------------------------