Re: [PATCH 04/11] tools/xenstored: add GLOBAL_QUOTA_DATA record for live update

["Anthony PERARD" <anthony.perard@vates.tech>]

On Wed, Mar 18, 2026 at 01:16:30PM +0100, Juergen Gross wrote:
> On 16.03.26 09:15, Jürgen Groß wrote:
> > On 13.03.26 18:08, Anthony PERARD wrote:
> > > On Thu, Mar 05, 2026 at 02:52:01PM +0100, Juergen Gross wrote:
> > > > +static void build_quota_data(struct quota *quota, uint32_t *val, char *name)
> > >
> > > I guess we will need a leap of faith to trust that `val` is big enough,
> > > after finding out that it's actually an output of multiple values, and
> > > not an input of a single value.
> >
> > That's what get_quota_size() is calculating.
> >
> > >
> > > And `name` seems to also be an output, and this is actually impossible
> > > to guess from the prototype.
> >
> > True. What about names?
>
> I have chosen names_buf instead, making it more clear that this is an
> output parameter for multiple names.

Sounds good.

> > > > +const char *dump_state_glb_quota(FILE *fp)
> > > > +{
> > > > +    struct xs_state_record_header *head;
> > > > +    struct xs_state_glb_quota *glb;
> > > > +    void *record;
> > > > +    unsigned int n_quota;
> > > > +    unsigned int len = sizeof(*glb);
> > > > +    size_t ret;
> > > > +
> > > > +    n_quota = get_quota_size(quotas, &len);
> > >
> > > So, get_quota_size is actually an "add" the size to this variable, and
> > > not "store" the size in this variable. That's not confusing at all.
> >
> > Would it be better if len is renamed to names_len (both here and the
> > parameter of get_quota_size())?
>
> In the end I have chosen to add a comment above get_quota_size() and
> rename "len" to "rec_len" in dump_state_glb_quota().

It would still be counter-intuitive if get_quota_size() returns different
values depending on the initial value of the second parameter. A comment
won't help. So I would still propose to add "sizeof(*glb)" to `len` or
`rec_len` after calling get_quota_size.

Cheers,


--
Anthony Perard | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech