Re: [PATCH] xen/public: add missing Xenstore commands to xs_wire.h

[Jürgen Groß <jgross@xxxxxxxx>]

On 06.03.25 11:27, Jan Beulich wrote:
> On 06.03.2025 09:21, Juergen Gross wrote:
> > The GET_FEATURE, SET_FEATURE, GET_QUOTA and SET_QUOTA Xenstore commands
> > are defined in docs/misc/xenstore.txt, but they are missing in
> > xs_wire.h.
> >
> > Add the missing commands to xs_wire.h
> >
> > Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> > ---
> >   xen/include/public/io/xs_wire.h | 4 ++++
> >   1 file changed, 4 insertions(+)
> >
> > diff --git a/xen/include/public/io/xs_wire.h b/xen/include/public/io/xs_wire.h
> > index 0d9f49ac89..e92a87a07b 100644
> > --- a/xen/include/public/io/xs_wire.h
> > +++ b/xen/include/public/io/xs_wire.h
> > @@ -35,6 +35,10 @@ enum xsd_sockmsg_type
> >       /* XS_RESTRICT has been removed */
> >       XS_RESET_WATCHES = XS_SET_TARGET + 2,
> >       XS_DIRECTORY_PART,
> > +    XS_GET_FEATURE,
> > +    XS_SET_FEATURE,
> > +    XS_GET_QUOTA,
> > +    XS_SET_QUOTA,
> >   
> >       XS_TYPE_COUNT,      /* Number of valid types. */
> This is effectively extending the ABI with request types that are recognized
> by neither cxenstored nor oxenstored. The assumption therefore appears to be
> that no client would have used those types / numbers either.
Yes, same as e.g. with new hypercall numbers or hypercall sub-options.

A Xenstore implementation not knowing about those new commands would reject
them (this is the reason why we don't need a feature bit for new commands,
again same as for new hypercalls).

> docs/misc/xenstore.txt doesn't spell out what the encoding of the commands is
> in the (binary) representation.
Indeed. I'll spawn another patch fixing that.

> At the same time the effect of this change is a growth of cxenstored's
> wire_funcs[] array, adding four nil entries. Luckily the code accessing the
> array looks to be prepared for such. (Still I wonder whether the array
This isn't pure luck. It was done on purpose. Please note that the array
is sparse already, as XS_RESTRICT has been removed in the past (this is
even visible in the context above).

> wouldn't better be extended right away, by adding just the strings while
> leaving the handler function pointers at NULL.)
No, I don't think so. This should be done when implementing the new
commands.

> Provided all of the above is correct,
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
Thanks,


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature