[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 16/17] tools/xenstore: let check_store() check the accounting data



Hi Juergen,

On 18/01/2023 06:23, Juergen Gross wrote:
On 17.01.23 23:36, Julien Grall wrote:
Hi Juergen,

On 17/01/2023 09:11, Juergen Gross wrote:
Today check_store() is only testing the correctness of the node tree.

Add verification of the accounting data (number of nodes)  and correct

NIT: one too many space before 'and'.

the data if it is wrong.

Do the initial check_store() call only after Xenstore entries of a
live update have been read.

Can you clarify whether this is needed for the rest of the patch, or simply a nice thing to have in general?

I'll add: "This is wanted to make sure the accounting data is correct after
a live update."

Fine with me.




Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
---
  tools/xenstore/xenstored_core.c   | 62 ++++++++++++++++------
  tools/xenstore/xenstored_domain.c | 86 +++++++++++++++++++++++++++++++
  tools/xenstore/xenstored_domain.h |  4 ++
  3 files changed, 137 insertions(+), 15 deletions(-)

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 3099077a86..e201f14053 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -2389,8 +2389,6 @@ void setup_structure(bool live_update)
          manual_node("@introduceDomain", NULL);
          domain_nbentry_fix(dom0_domid, 5, true);
      }
-
-    check_store();
  }
  static unsigned int hash_from_key_fn(void *k)
@@ -2433,20 +2431,28 @@ int remember_string(struct hashtable *hash, const char *str)    * As we go, we record each node in the given reachable hashtable. These
   * entries will be used later in clean_store.
   */
+
+struct check_store_data {
+    struct hashtable *reachable;
+    struct hashtable *domains;
+};
+
  static int check_store_step(const void *ctx, struct connection *conn,
                  struct node *node, void *arg)
  {
-    struct hashtable *reachable = arg;
+    struct check_store_data *data = arg;
-    if (hashtable_search(reachable, (void *)node->name)) {
+    if (hashtable_search(data->reachable, (void *)node->name)) {

IIUC the cast is only necessary because hashtable_search() expects a non-const value. I can't think for a reason for the key to be non-const. So I will look to send a follow-up patch.

It is possible, but nasty, due to talloc_free() not taking a const pointer.

I am not sure I understand your reasoning. Looking at hashtable_search(), I don't see a call to talloc_free().

Anyway, this is not directly related to this patch. So I will have a look separately.

Cheers,

--
Julien Grall



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.