Re: [PATCH 01/12] tools/xenstore: manage per-transaction domain accounting data in an array

[Juergen Gross]

On 01.12.22 23:13, Julien Grall wrote:

Hi Juergen,

On 01/11/2022 16:04, Juergen Gross wrote:

In order to prepare keeping accounting data in an array instead of
using independent fields, switch the struct changed_domain accounting
data to that scheme, for now only using an array with one element.

In order to be able to extend this scheme add the needed indexing enum
to xenstored_domain.h.

Signed-off-by: Juergen Gross <jgr...@suse.com>
---
  tools/xenstore/xenstored_domain.c | 17 +++++++++--------
  tools/xenstore/xenstored_domain.h |  5 +++++
  2 files changed, 14 insertions(+), 8 deletions(-)


index 582b78b0ee..399fc920bb 100644
--- a/tools/xenstore/xenstored_domain.c
+++ b/tools/xenstore/xenstored_domain.c
@@ -99,8 +99,8 @@ struct changed_domain
      /* Identifier of the changed domain. */
      unsigned int domid;
-    /* Amount by which this domain's nbentry field has changed. */
-    int nbentry;
+    /* Accounting data. */
+    int acc[ACC_TR_N];
  };
  static struct hashtable *domhash;
@@ -558,7 +558,7 @@ int acc_fix_domains(struct list_head *head, bool update)
      int cnt;
      list_for_each_entry(cd, head, list) {
-        cnt = domain_nbentry_fix(cd->domid, cd->nbentry, update);
+        cnt = domain_nbentry_fix(cd->domid, cd->acc[ACC_NODES], update);
          if (!update) {
              if (cnt >= quota_nb_entry_per_domain)
                  return ENOSPC;

      return cd;
  }
-static int acc_add_dom_nbentry(const void *ctx, struct list_head *head, int 
val,
-                   unsigned int domid)
+static int acc_add_changed_dom(const void *ctx, struct list_head *head,
+                   unsigned int what, int val, unsigned int domid)

diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c @@ -603,8 +603,8 @@ static struct changed_domain *acc_get_changed_domain(const void *ctx,

AFAIU "what" will be a value from the enum. So let's do some hardening and use the type "enum ...".

Yes, will do that (and in the following patches, too).

  {
      struct changed_domain *cd;

          return 0;
      errno = 0;
-    cd->nbentry += val;

@@ -613,9 +613,9 @@ static int acc_add_dom_nbentry(const void *ctx, struct list_head *head, int val,

I would consider to adding an assert() (or possibly runtime check) to confirm the 'what' is smaller than the array size.

Okay.


Juergen

OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

OpenPGP_signature
Description: OpenPGP digital signature