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@suse.com>
---
 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. */
 
-- 
2.43.0

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@suse.com>
> ---
>  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.

docs/misc/xenstore.txt doesn't spell out what the encoding of the commands is
in the (binary) representation.

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
wouldn't better be extended right away, by adding just the strings while
leaving the handler function pointers at NULL.)

Provided all of the above is correct,
Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan

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@suse.com>
>> ---
>>   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@suse.com>

Thanks,


Juergen