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

[Juergen Gross <jgross@xxxxxxxx>]

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 <jgross@xxxxxxxx>
> > ---
> >   tools/xenstore/xenstored_domain.c | 17 +++++++++--------
> >   tools/xenstore/xenstored_domain.h |  5 +++++
> >   2 files changed, 14 insertions(+), 8 deletions(-)
> >
> > diff --git a/tools/xenstore/xenstored_domain.c 
> > b/tools/xenstore/xenstored_domain.c
> > 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;
> > @@ -603,8 +603,8 @@ static struct changed_domain *acc_get_changed_domain(const 
> > void *ctx,
> >       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)
> 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;
> > @@ -613,9 +613,9 @@ static int acc_add_dom_nbentry(const void *ctx, struct 
> > list_head *head, int val,
> >           return 0;
> >       errno = 0;
> > -    cd->nbentry += val;
> I would consider to adding an assert() (or possibly runtime check) to confirm 
> the 'what' is smaller than the array size.
Okay.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature