ctdb-protocol: Fix marshalling for ctdb_db_statistics
authorAmitay Isaacs <amitay@gmail.com>
Wed, 26 Jul 2017 15:00:51 +0000 (01:00 +1000)
committerMartin Schwenke <martins@samba.org>
Wed, 30 Aug 2017 12:59:25 +0000 (14:59 +0200)
Signed-off-by: Amitay Isaacs <amitay@gmail.com>
Reviewed-by: Martin Schwenke <martin@meltin.net>
ctdb/protocol/protocol_control.c
ctdb/protocol/protocol_private.h
ctdb/protocol/protocol_types.c
ctdb/tests/src/protocol_common.c
ctdb/tests/src/protocol_types_compat_test.c
ctdb/tests/src/protocol_types_test.c

index 9476c3564f4f3c8aa9cbbd5bd58fb22166dd3ca5..c271852d984a8220d3da9ccc6461c2193f938d6f 100644 (file)
@@ -1542,7 +1542,7 @@ static void ctdb_reply_control_data_push(struct ctdb_reply_control_data *cd,
                break;
 
        case CTDB_CONTROL_GET_DB_STATISTICS:
-               ctdb_db_statistics_push(cd->data.dbstats, buf);
+               ctdb_db_statistics_push(cd->data.dbstats, buf, &np);
                break;
 
        case CTDB_CONTROL_RECEIVE_RECORDS:
@@ -1727,7 +1727,7 @@ static int ctdb_reply_control_data_pull(uint8_t *buf, size_t buflen,
 
        case CTDB_CONTROL_GET_DB_STATISTICS:
                ret = ctdb_db_statistics_pull(buf, buflen, mem_ctx,
-                                             &cd->data.dbstats);
+                                             &cd->data.dbstats, &np);
                break;
 
        case CTDB_CONTROL_RECEIVE_RECORDS:
index 61453c49b5f6b99e4f16f849f06ff0d2acf44af7..280fc9904d8ae095c638800689d84eb7b278444c 100644 (file)
@@ -300,10 +300,11 @@ void ctdb_key_data_push(struct ctdb_key_data *in, uint8_t *buf, size_t *npush);
 int ctdb_key_data_pull(uint8_t *buf, size_t buflen, TALLOC_CTX *mem_ctx,
                       struct ctdb_key_data **out, size_t *npull);
 
-size_t ctdb_db_statistics_len(struct ctdb_db_statistics *dbstats);
-void ctdb_db_statistics_push(struct ctdb_db_statistics *dbstats, void *buf);
+size_t ctdb_db_statistics_len(struct ctdb_db_statistics *in);
+void ctdb_db_statistics_push(struct ctdb_db_statistics *in, uint8_t *buf,
+                            size_t *npush);
 int ctdb_db_statistics_pull(uint8_t *buf, size_t buflen, TALLOC_CTX *mem_ctx,
-                           struct ctdb_db_statistics **out);
+                           struct ctdb_db_statistics **out, size_t *npull);
 
 size_t ctdb_election_message_len(struct ctdb_election_message *election);
 void ctdb_election_message_push(struct ctdb_election_message *election,
index f384c2ff7ad69ccbd953153cd80ce320bd2a983b..f3d5d60b3025e1c9a1f69eddcddfe60edff36926 100644 (file)
@@ -4324,100 +4324,305 @@ fail:
        return ret;
 }
 
-struct ctdb_db_statistics_wire {
-       struct ctdb_db_statistics dbstats;
-       char hot_keys_wire[1];
-};
+/* In the tdb_data structure marshalling, we are only interested in dsize.
+ * The dptr value is ignored.  The actual tdb_data blob is stored separately.
+ *
+ * This is only required for ctdb_db_statistics and will be dropped in future.
+ */
 
-size_t ctdb_db_statistics_len(struct ctdb_db_statistics *dbstats)
+static size_t tdb_data_struct_len(TDB_DATA *data)
 {
+       return sizeof(void *) + sizeof(size_t);
+}
+
+static void tdb_data_struct_push(TDB_DATA *data, uint8_t *buf, size_t *npush)
+{
+       size_t offset = 0;
+
+       memcpy(buf+offset, &data->dptr, sizeof(void *));
+       offset += sizeof(void *);
+
+       memcpy(buf+offset, &data->dsize, sizeof(size_t));
+       offset += sizeof(size_t);
+
+       *npush = offset;
+}
+
+static int tdb_data_struct_pull(uint8_t *buf, size_t buflen, TDB_DATA *data,
+                               size_t *npull)
+{
+       size_t offset = 0;
+       void *ptr;
+
+       if (buflen-offset < sizeof(void *)) {
+               return EMSGSIZE;
+       }
+
+       memcpy(&ptr, buf+offset, sizeof(void *));
+       offset += sizeof(void *);
+       data->dptr = NULL;
+
+       if (buflen-offset < sizeof(size_t)) {
+               return EMSGSIZE;
+       }
+
+       memcpy(&data->dsize, buf+offset, sizeof(size_t));
+       offset += sizeof(size_t);
+
+       *npull = offset;
+       return 0;
+}
+
+size_t ctdb_db_statistics_len(struct ctdb_db_statistics *in)
+{
+       TDB_DATA data = { 0 };
        size_t len;
+       uint32_t u32 = 0;
        int i;
 
-       len = sizeof(struct ctdb_db_statistics);
+       len = ctdb_uint32_len(&in->locks.num_calls) +
+               ctdb_uint32_len(&in->locks.num_current) +
+               ctdb_uint32_len(&in->locks.num_pending) +
+               ctdb_uint32_len(&in->locks.num_failed) +
+               ctdb_latency_counter_len(&in->locks.latency) +
+               MAX_COUNT_BUCKETS *
+                       ctdb_uint32_len(&in->locks.buckets[0]) +
+               ctdb_latency_counter_len(&in->vacuum.latency) +
+               ctdb_uint32_len(&in->db_ro_delegations) +
+               ctdb_uint32_len(&in->db_ro_revokes) +
+               MAX_COUNT_BUCKETS *
+                       ctdb_uint32_len(&in->hop_count_bucket[0]) +
+               ctdb_uint32_len(&in->num_hot_keys) +
+               ctdb_padding_len(4) +
+               MAX_HOT_KEYS *
+                       (ctdb_uint32_len(&u32) + ctdb_padding_len(4) +
+                        tdb_data_struct_len(&data));
+
        for (i=0; i<MAX_HOT_KEYS; i++) {
-               len += dbstats->hot_keys[i].key.dsize;
+               len += ctdb_tdb_data_len(&in->hot_keys[i].key);
        }
+
        return len;
 }
 
-void ctdb_db_statistics_push(struct ctdb_db_statistics *dbstats, void *buf)
+void ctdb_db_statistics_push(struct ctdb_db_statistics *in, uint8_t *buf,
+                            size_t *npush)
 {
-       struct ctdb_db_statistics_wire *wire =
-               (struct ctdb_db_statistics_wire *)buf;
-       size_t offset;
+       size_t offset = 0, np;
+       uint32_t num_hot_keys;
        int i;
 
-       dbstats->num_hot_keys = MAX_HOT_KEYS;
-       memcpy(wire, dbstats, sizeof(struct ctdb_db_statistics));
+       ctdb_uint32_push(&in->locks.num_calls, buf+offset, &np);
+       offset += np;
+
+       ctdb_uint32_push(&in->locks.num_current, buf+offset, &np);
+       offset += np;
+
+       ctdb_uint32_push(&in->locks.num_pending, buf+offset, &np);
+       offset += np;
+
+       ctdb_uint32_push(&in->locks.num_failed, buf+offset, &np);
+       offset += np;
+
+       ctdb_latency_counter_push(&in->locks.latency, buf+offset, &np);
+       offset += np;
+
+       for (i=0; i<MAX_COUNT_BUCKETS; i++) {
+               ctdb_uint32_push(&in->locks.buckets[i], buf+offset, &np);
+               offset += np;
+       }
+
+       ctdb_latency_counter_push(&in->vacuum.latency, buf+offset, &np);
+       offset += np;
+
+       ctdb_uint32_push(&in->db_ro_delegations, buf+offset, &np);
+       offset += np;
+
+       ctdb_uint32_push(&in->db_ro_revokes, buf+offset, &np);
+       offset += np;
+
+       for (i=0; i<MAX_COUNT_BUCKETS; i++) {
+               ctdb_uint32_push(&in->hop_count_bucket[i], buf+offset, &np);
+               offset += np;
+       }
+
+       num_hot_keys = MAX_HOT_KEYS;
+       ctdb_uint32_push(&num_hot_keys, buf+offset, &np);
+       offset += np;
+
+       ctdb_padding_push(4, buf+offset, &np);
+       offset += np;
+
+       for (i=0; i<MAX_HOT_KEYS; i++) {
+               ctdb_uint32_push(&in->hot_keys[i].count, buf+offset, &np);
+               offset += np;
+
+               ctdb_padding_push(4, buf+offset, &np);
+               offset += np;
+
+               tdb_data_struct_push(&in->hot_keys[i].key, buf+offset, &np);
+               offset += np;
+       }
 
-       offset = 0;
        for (i=0; i<MAX_HOT_KEYS; i++) {
-               memcpy(&wire->hot_keys_wire[offset],
-                      dbstats->hot_keys[i].key.dptr,
-                      dbstats->hot_keys[i].key.dsize);
-               offset += dbstats->hot_keys[i].key.dsize;
+               ctdb_tdb_data_push(&in->hot_keys[i].key, buf+offset, &np);
+               offset += np;
        }
+
+       *npush = offset;
 }
 
-int ctdb_db_statistics_pull(uint8_t *buf, size_t buflen, TALLOC_CTX *mem_ctx,
-                           struct ctdb_db_statistics **out)
+static int ctdb_db_statistics_pull_elems(uint8_t *buf, size_t buflen,
+                                        TALLOC_CTX *mem_ctx,
+                                        struct ctdb_db_statistics *out,
+                                        size_t *npull)
 {
-       struct ctdb_db_statistics *dbstats;
-       struct ctdb_db_statistics_wire *wire =
-               (struct ctdb_db_statistics_wire *)buf;
-       size_t offset;
-       int i;
+       size_t offset = 0, np;
+       int ret, i;
 
-       if (buflen < sizeof(struct ctdb_db_statistics)) {
-               return EMSGSIZE;
+       ret = ctdb_uint32_pull(buf+offset, buflen-offset,
+                              &out->locks.num_calls, &np);
+       if (ret != 0) {
+               return ret;
        }
+       offset += np;
 
-       offset = 0;
-       for (i=0; i<wire->dbstats.num_hot_keys; i++) {
-               if (wire->dbstats.hot_keys[i].key.dsize > buflen) {
-                       return EMSGSIZE;
-               }
-               if (offset + wire->dbstats.hot_keys[i].key.dsize < offset) {
-                       return EMSGSIZE;
-               }
-               offset += wire->dbstats.hot_keys[i].key.dsize;
-               if (offset > buflen) {
-                       return EMSGSIZE;
+       ret = ctdb_uint32_pull(buf+offset, buflen-offset,
+                              &out->locks.num_current, &np);
+       if (ret != 0) {
+               return ret;
+       }
+       offset += np;
+
+       ret = ctdb_uint32_pull(buf+offset, buflen-offset,
+                              &out->locks.num_pending, &np);
+       if (ret != 0) {
+               return ret;
+       }
+       offset += np;
+
+       ret = ctdb_uint32_pull(buf+offset, buflen-offset,
+                              &out->locks.num_failed, &np);
+       if (ret != 0) {
+               return ret;
+       }
+       offset += np;
+
+       ret = ctdb_latency_counter_pull(buf+offset, buflen-offset,
+                                       &out->locks.latency, &np);
+       if (ret != 0) {
+               return ret;
+       }
+       offset += np;
+
+       for (i=0; i<MAX_COUNT_BUCKETS; i++) {
+               ret = ctdb_uint32_pull(buf+offset, buflen-offset,
+                                      &out->locks.buckets[i], &np);
+               if (ret != 0) {
+                       return ret;
                }
+               offset += np;
        }
-       if (sizeof(struct ctdb_db_statistics) + offset <
-           sizeof(struct ctdb_db_statistics)) {
-               return EMSGSIZE;
+
+       ret = ctdb_latency_counter_pull(buf+offset, buflen-offset,
+                                       &out->vacuum.latency, &np);
+       if (ret != 0) {
+               return ret;
        }
-       if (buflen < sizeof(struct ctdb_db_statistics) + offset) {
-               return EMSGSIZE;
+       offset += np;
+
+       ret = ctdb_uint32_pull(buf+offset, buflen-offset,
+                              &out->db_ro_delegations, &np);
+       if (ret != 0) {
+               return ret;
        }
+       offset += np;
 
-       dbstats = talloc(mem_ctx, struct ctdb_db_statistics);
-       if (dbstats == NULL) {
-               return ENOMEM;
+       ret = ctdb_uint32_pull(buf+offset, buflen-offset,
+                              &out->db_ro_revokes, &np);
+       if (ret != 0) {
+               return ret;
        }
+       offset += np;
 
-       memcpy(dbstats, wire, sizeof(struct ctdb_db_statistics));
+       for (i=0; i<MAX_COUNT_BUCKETS; i++) {
+               ret = ctdb_uint32_pull(buf+offset, buflen-offset,
+                                      &out->hop_count_bucket[i], &np);
+               if (ret != 0) {
+                       return ret;
+               }
+               offset += np;
+       }
 
-       offset = 0;
-       for (i=0; i<wire->dbstats.num_hot_keys; i++) {
-               uint8_t *ptr;
-               size_t key_size;
-
-               key_size = dbstats->hot_keys[i].key.dsize;
-               ptr = talloc_memdup(mem_ctx, &wire->hot_keys_wire[offset],
-                                   key_size);
-               if (ptr == NULL) {
-                       talloc_free(dbstats);
-                       return ENOMEM;
+       ret = ctdb_uint32_pull(buf+offset, buflen-offset,
+                              &out->num_hot_keys, &np);
+       if (ret != 0) {
+               return ret;
+       }
+       offset += np;
+
+       ret = ctdb_padding_pull(buf+offset, buflen-offset, 4, &np);
+       if (ret != 0) {
+               return ret;
+       }
+       offset += np;
+
+       for (i=0; i<MAX_HOT_KEYS; i++) {
+               ret = ctdb_uint32_pull(buf+offset, buflen-offset,
+                                      &out->hot_keys[i].count, &np);
+               if (ret != 0) {
+                       return ret;
+               }
+               offset += np;
+
+               ret = ctdb_padding_pull(buf+offset, buflen-offset, 4, &np);
+               if (ret != 0) {
+                       return ret;
+               }
+               offset += np;
+
+               ret = tdb_data_struct_pull(buf+offset, buflen-offset,
+                                          &out->hot_keys[i].key, &np);
+               if (ret != 0) {
+                       return ret;
+               }
+               offset += np;
+       }
+
+       for (i=0; i<MAX_HOT_KEYS; i++) {
+               ret = ctdb_tdb_data_pull(buf+offset,
+                                        out->hot_keys[i].key.dsize,
+                                        out, &out->hot_keys[i].key, &np);
+               if (ret != 0) {
+                       return ret;
                }
-               dbstats->hot_keys[i].key.dptr = ptr;
-               offset += key_size;
+               offset += np;
        }
 
-       *out = dbstats;
+       *npull = offset;
+       return 0;
+}
+
+int ctdb_db_statistics_pull(uint8_t *buf, size_t buflen, TALLOC_CTX *mem_ctx,
+                           struct ctdb_db_statistics **out, size_t *npull)
+{
+       struct ctdb_db_statistics *val;
+       size_t np;
+       int ret;
+
+       val = talloc(mem_ctx, struct ctdb_db_statistics);
+       if (val == NULL) {
+               return ENOMEM;
+       }
+
+       ret = ctdb_db_statistics_pull_elems(buf, buflen, val, val, &np);
+       if (ret != 0) {
+               talloc_free(val);
+               return ret;
+       }
+
+       *out = val;
+       *npull = np;
        return 0;
 }
 
index 494e0e091c37703bc961525ff751e063fd6a08fb..5b0d67d2b42e4e5c34d5a4898e7d2b4ef35cde65 100644 (file)
@@ -1231,8 +1231,24 @@ void fill_ctdb_db_statistics(TALLOC_CTX *mem_ctx,
 {
        int i;
 
-       fill_buffer(p, offsetof(struct ctdb_db_statistics, num_hot_keys));
-       p->num_hot_keys = 10;
+       p->locks.num_calls = rand32();
+       p->locks.num_current = rand32();
+       p->locks.num_pending = rand32();
+       p->locks.num_failed = rand32();
+       fill_ctdb_latency_counter(&p->locks.latency);
+       for (i=0; i<MAX_COUNT_BUCKETS; i++) {
+               p->locks.buckets[i] = rand32();
+       }
+
+       fill_ctdb_latency_counter(&p->vacuum.latency);
+
+       p->db_ro_delegations = rand32();
+       p->db_ro_revokes = rand32();
+       for (i=0; i<MAX_COUNT_BUCKETS; i++) {
+               p->hop_count_bucket[i] = rand32();
+       }
+
+       p->num_hot_keys = MAX_HOT_KEYS;
        for (i=0; i<p->num_hot_keys; i++) {
                p->hot_keys[i].count = rand32();
                fill_tdb_data(mem_ctx, &p->hot_keys[i].key);
@@ -1244,8 +1260,23 @@ void verify_ctdb_db_statistics(struct ctdb_db_statistics *p1,
 {
        int i;
 
-       verify_buffer(p1, p2, offsetof(struct ctdb_db_statistics,
-                                           num_hot_keys));
+       assert(p1->locks.num_calls == p2->locks.num_calls);
+       assert(p1->locks.num_current == p2->locks.num_current);
+       assert(p1->locks.num_pending == p2->locks.num_pending);
+       assert(p1->locks.num_failed == p2->locks.num_failed);
+       verify_ctdb_latency_counter(&p1->locks.latency, &p2->locks.latency);
+       for (i=0; i<MAX_COUNT_BUCKETS; i++) {
+               assert(p1->locks.buckets[i] == p2->locks.buckets[i]);
+       }
+
+       verify_ctdb_latency_counter(&p1->vacuum.latency, &p2->vacuum.latency);
+
+       assert(p1->db_ro_delegations == p2->db_ro_delegations);
+       assert(p1->db_ro_revokes == p2->db_ro_revokes);
+       for (i=0; i<MAX_COUNT_BUCKETS; i++) {
+               assert(p1->hop_count_bucket[i] == p2->hop_count_bucket[i]);
+       }
+
        assert(p1->num_hot_keys == p2->num_hot_keys);
        for (i=0; i<p1->num_hot_keys; i++) {
                assert(p1->hot_keys[i].count == p2->hot_keys[i].count);
index cf9abf3b2814ced15388503b07a9613888b04d61..8599344d803dd5b4e3e12f8d00eb2614c889497a 100644 (file)
@@ -1977,6 +1977,105 @@ static int ctdb_key_data_pull_old(uint8_t *buf, size_t buflen,
        return 0;
 }
 
+struct ctdb_db_statistics_wire {
+       struct ctdb_db_statistics dbstats;
+       char hot_keys_wire[1];
+};
+
+static size_t ctdb_db_statistics_len_old(struct ctdb_db_statistics *in)
+{
+       size_t len;
+       int i;
+
+       len = sizeof(struct ctdb_db_statistics);
+       for (i=0; i<MAX_HOT_KEYS; i++) {
+               len += in->hot_keys[i].key.dsize;
+       }
+       return len;
+}
+
+static void ctdb_db_statistics_push_old(struct ctdb_db_statistics *in,
+                                       void *buf)
+{
+       struct ctdb_db_statistics_wire *wire =
+               (struct ctdb_db_statistics_wire *)buf;
+       size_t offset;
+       int i;
+
+       in->num_hot_keys = MAX_HOT_KEYS;
+       memcpy(wire, in, sizeof(struct ctdb_db_statistics));
+
+       offset = 0;
+       for (i=0; i<MAX_HOT_KEYS; i++) {
+               memcpy(&wire->hot_keys_wire[offset],
+                      in->hot_keys[i].key.dptr,
+                      in->hot_keys[i].key.dsize);
+               offset += in->hot_keys[i].key.dsize;
+       }
+}
+
+static int ctdb_db_statistics_pull_old(uint8_t *buf, size_t buflen,
+                                      TALLOC_CTX *mem_ctx,
+                                      struct ctdb_db_statistics **out)
+{
+       struct ctdb_db_statistics *val;
+       struct ctdb_db_statistics_wire *wire =
+               (struct ctdb_db_statistics_wire *)buf;
+       size_t offset;
+       int i;
+
+       if (buflen < sizeof(struct ctdb_db_statistics)) {
+               return EMSGSIZE;
+       }
+
+       offset = 0;
+       for (i=0; i<wire->dbstats.num_hot_keys; i++) {
+               if (wire->dbstats.hot_keys[i].key.dsize > buflen) {
+                       return EMSGSIZE;
+               }
+               if (offset + wire->dbstats.hot_keys[i].key.dsize < offset) {
+                       return EMSGSIZE;
+               }
+               offset += wire->dbstats.hot_keys[i].key.dsize;
+               if (offset > buflen) {
+                       return EMSGSIZE;
+               }
+       }
+       if (sizeof(struct ctdb_db_statistics) + offset <
+           sizeof(struct ctdb_db_statistics)) {
+               return EMSGSIZE;
+       }
+       if (buflen < sizeof(struct ctdb_db_statistics) + offset) {
+               return EMSGSIZE;
+       }
+
+       val = talloc(mem_ctx, struct ctdb_db_statistics);
+       if (val == NULL) {
+               return ENOMEM;
+       }
+
+       memcpy(val, wire, sizeof(struct ctdb_db_statistics));
+
+       offset = 0;
+       for (i=0; i<wire->dbstats.num_hot_keys; i++) {
+               uint8_t *ptr;
+               size_t key_size;
+
+               key_size = val->hot_keys[i].key.dsize;
+               ptr = talloc_memdup(mem_ctx, &wire->hot_keys_wire[offset],
+                                   key_size);
+               if (ptr == NULL) {
+                       talloc_free(val);
+                       return ENOMEM;
+               }
+               val->hot_keys[i].key.dptr = ptr;
+               offset += key_size;
+       }
+
+       *out = val;
+       return 0;
+}
+
 
 COMPAT_TYPE3_TEST(struct ctdb_statistics, ctdb_statistics);
 COMPAT_TYPE3_TEST(struct ctdb_vnn_map, ctdb_vnn_map);
@@ -2015,6 +2114,7 @@ COMPAT_TYPE3_TEST(struct ctdb_iface_list, ctdb_iface_list);
 COMPAT_TYPE3_TEST(struct ctdb_public_ip_info, ctdb_public_ip_info);
 COMPAT_TYPE3_TEST(struct ctdb_statistics_list, ctdb_statistics_list);
 COMPAT_TYPE3_TEST(struct ctdb_key_data, ctdb_key_data);
+COMPAT_TYPE3_TEST(struct ctdb_db_statistics, ctdb_db_statistics);
 
 int main(int argc, char *argv[])
 {
@@ -2058,6 +2158,7 @@ int main(int argc, char *argv[])
        COMPAT_TEST_FUNC(ctdb_public_ip_info)();
        COMPAT_TEST_FUNC(ctdb_statistics_list)();
        COMPAT_TEST_FUNC(ctdb_key_data)();
+       COMPAT_TEST_FUNC(ctdb_db_statistics)();
 
        return 0;
 }
index d12276153862ae9a1f25253f37627d15170852f7..4e1e6e2afc6cd0be9c68d15e4527a120c447f6f0 100644 (file)
@@ -83,7 +83,7 @@ PROTOCOL_TYPE3_TEST(struct ctdb_iface_list, ctdb_iface_list);
 PROTOCOL_TYPE3_TEST(struct ctdb_public_ip_info, ctdb_public_ip_info);
 PROTOCOL_TYPE3_TEST(struct ctdb_statistics_list, ctdb_statistics_list);
 PROTOCOL_TYPE3_TEST(struct ctdb_key_data, ctdb_key_data);
-DEFINE_TEST(struct ctdb_db_statistics, ctdb_db_statistics);
+PROTOCOL_TYPE3_TEST(struct ctdb_db_statistics, ctdb_db_statistics);
 DEFINE_TEST(struct ctdb_election_message, ctdb_election_message);
 DEFINE_TEST(struct ctdb_srvid_message, ctdb_srvid_message);
 DEFINE_TEST(struct ctdb_disable_message, ctdb_disable_message);