[PATCH v3] docs: specify numerical values of Xenstore commands

Juergen Gross posted 1 patch 9 months, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20250312084143.14045-1-jgross@suse.com
There is a newer version of this series
docs/misc/xenstore.txt | 61 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 61 insertions(+)
[PATCH v3] docs: specify numerical values of Xenstore commands
Posted by Juergen Gross 9 months, 1 week ago
In docs/misc/xenstore.txt all Xenstore commands are specified, but
the specifications lack the numerical values of the commands.

Add a table with all commands, their values, and a potential remark
(e.g. whether the command is optional).

Reported-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
---
V2:
- replace "ŕ" with plain "r" (Jan Beulich)
- replace hard tabs with blanks (Jan Beulich)
- allow GET_FEATURES and GET_QUOTA support without SET_* (Jan Beulich)
V3:
- specify error code returned for unsupported commands (Julien Grall)
- reword XS_DIRECTORY_PART related text (Julien Grall)
---
 docs/misc/xenstore.txt | 61 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/docs/misc/xenstore.txt b/docs/misc/xenstore.txt
index 7e1f031520..72db73deef 100644
--- a/docs/misc/xenstore.txt
+++ b/docs/misc/xenstore.txt
@@ -86,6 +86,67 @@ parts of xenstore inaccessible to some clients.  In any case passing
 bulk data through xenstore is not recommended as the performance
 properties are poor.
 
+---------- Defined Xenstore message types ----------
+
+Below is a table with all defined Xenstore message types (type name
+and its associated numerical value).
+
+Some types are optional to be supported by a specific Xenstore
+implementation.  If an optional type is not supported by a Xenstore
+implementation, Xen tools will continue to work, maybe with slightly
+reduced functionality.  A mandatory type not being supported will
+result in severely reduced functionality, like inability to create
+domains.  In case a type is optional, this is stated in the table with
+the lost functionality in case Xenstore doesn't support that type.
+Any not supported type sent to Xenstore will result in an error response
+with the "ENOSYS" error.
+
+XS_CONTROL               0    optional
+    If not supported, xenstore-control command will not work.
+    XS_DEBUG is a deprecated alias of XS_CONTROL.
+XS_DIRECTORY             1
+XS_READ                  2
+XS_GET_PERMS             3
+XS_WATCH                 4
+XS_UNWATCH               5
+XS_TRANSACTION_START     6
+XS_TRANSACTION_END       7
+XS_INTRODUCE             8
+XS_RELEASE               9
+XS_GET_DOMAIN_PATH      10
+XS_WRITE                11
+XS_MKDIR                12
+XS_RM                   13
+XS_SET_PERMS            14
+XS_WATCH_EVENT          15
+    Not valid in client sent messages.
+    Only valid in Xenstore replies.
+XS_ERROR                16
+    Not valid in client sent messages.
+    Only valid in Xenstore replies.
+XS_IS_DOMAIN_INTRODUCED 17
+XS_RESUME               18
+XS_SET_TARGET           19
+XS_RESTRICT             20    no longer supported
+    XS_RESTRICT has been removed, the type value 20 is invalid.
+XS_RESET_WATCHES        21
+XS_DIRECTORY_PART       22    optional
+    If not supported, the output of xenstore-ls might be incomplete
+    with a node's sub-node list exceeding the maximum payload size
+    (e.g. the "/local/domain" node with more than ca. 1000 domains
+    active).
+XS_GET_FEATURE          23    optional
+XS_SET_FEATURE          24    optional
+    XS_SET_FEATURE requires XS_GET_FEATURE to be supported.
+    If unsupported, setting availability of Xenstore features per
+    domain is not possible.
+XS_GET_QUOTA            25    optional
+XS_SET_QUOTA            26    optional
+    XS_SET_QUOTA requires XS_GET_QUOTA to be supported.
+    If unsupported, setting of Xenstore quota per domain is not
+    possible.
+XS_INVALID           65535
+    Guaranteed invalid type (never supported).
 
 ---------- Xenstore protocol details - introduction ----------
 
-- 
2.43.0


Re: [PATCH v3] docs: specify numerical values of Xenstore commands
Posted by Anthony PERARD 9 months, 1 week ago
On Wed, Mar 12, 2025 at 09:41:43AM +0100, Juergen Gross wrote:
> diff --git a/docs/misc/xenstore.txt b/docs/misc/xenstore.txt
> index 7e1f031520..72db73deef 100644
> --- a/docs/misc/xenstore.txt
> +++ b/docs/misc/xenstore.txt
> @@ -86,6 +86,67 @@ parts of xenstore inaccessible to some clients.  In any case passing
> +XS_CONTROL               0    optional
> +    If not supported, xenstore-control command will not work.
> +    XS_DEBUG is a deprecated alias of XS_CONTROL.
> +XS_DIRECTORY             1
> +XS_READ                  2
> +XS_GET_PERMS             3

This new table prefix message type names with "XS_", but the rest of the
document describe each type without the prefix. Isn't it going to be
confusing, and make it slightly harder to link this table to rest of the
document? (I often search by full word, like '\<GET_PERMS\>', because
that one key stroke in vim '*', so having different prefix makes it
harder to search)

Thanks,

-- 

Anthony Perard | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech
Re: [PATCH v3] docs: specify numerical values of Xenstore commands
Posted by Jürgen Groß 9 months, 1 week ago
On 12.03.25 17:46, Anthony PERARD wrote:
> On Wed, Mar 12, 2025 at 09:41:43AM +0100, Juergen Gross wrote:
>> diff --git a/docs/misc/xenstore.txt b/docs/misc/xenstore.txt
>> index 7e1f031520..72db73deef 100644
>> --- a/docs/misc/xenstore.txt
>> +++ b/docs/misc/xenstore.txt
>> @@ -86,6 +86,67 @@ parts of xenstore inaccessible to some clients.  In any case passing
>> +XS_CONTROL               0    optional
>> +    If not supported, xenstore-control command will not work.
>> +    XS_DEBUG is a deprecated alias of XS_CONTROL.
>> +XS_DIRECTORY             1
>> +XS_READ                  2
>> +XS_GET_PERMS             3
> 
> This new table prefix message type names with "XS_", but the rest of the
> document describe each type without the prefix. Isn't it going to be
> confusing, and make it slightly harder to link this table to rest of the
> document? (I often search by full word, like '\<GET_PERMS\>', because
> that one key stroke in vim '*', so having different prefix makes it
> harder to search)

Question is, should I change the table to drop "XS_", or the rest document
to add "XS_" instead? After all xs_wire.h is defining the names with "XS_".

I'm slightly leaning towards a preparatory patch adding "XS_".


Juergen
Re: [PATCH v3] docs: specify numerical values of Xenstore commands
Posted by Anthony PERARD 9 months, 1 week ago
On Thu, Mar 13, 2025 at 10:51:06AM +0100, Jürgen Groß wrote:
> On 12.03.25 17:46, Anthony PERARD wrote:
> > On Wed, Mar 12, 2025 at 09:41:43AM +0100, Juergen Gross wrote:
> > > diff --git a/docs/misc/xenstore.txt b/docs/misc/xenstore.txt
> > > index 7e1f031520..72db73deef 100644
> > > --- a/docs/misc/xenstore.txt
> > > +++ b/docs/misc/xenstore.txt
> > > @@ -86,6 +86,67 @@ parts of xenstore inaccessible to some clients.  In any case passing
> > > +XS_CONTROL               0    optional
> > > +    If not supported, xenstore-control command will not work.
> > > +    XS_DEBUG is a deprecated alias of XS_CONTROL.
> > > +XS_DIRECTORY             1
> > > +XS_READ                  2
> > > +XS_GET_PERMS             3
> > 
> > This new table prefix message type names with "XS_", but the rest of the
> > document describe each type without the prefix. Isn't it going to be
> > confusing, and make it slightly harder to link this table to rest of the
> > document? (I often search by full word, like '\<GET_PERMS\>', because
> > that one key stroke in vim '*', so having different prefix makes it
> > harder to search)
> 
> Question is, should I change the table to drop "XS_", or the rest document
> to add "XS_" instead? After all xs_wire.h is defining the names with "XS_".
> 
> I'm slightly leaning towards a preparatory patch adding "XS_".

Well, I'm actually for dropping the prefix from the table. The prefix is
more of a C specific namespace than anything else. The ocaml
implementation in tree doesn't use this prefix, but a different one (if
we ignore the different case:
> Xenbus.Xb.Op.Watch
https://elixir.bootlin.com/xen/v4.20.0/source/tools/ocaml/xenstored/process.ml#L632
And have a link to a string without the prefix:
> | Watch			-> "WATCH"
https://elixir.bootlin.com/xen/v4.20.0/source/tools/ocaml/libs/xb/op.ml#L49

There's also a version in Rust which also use a different prefix,
"XsMessageType::".
https://github.com/Wenzel/xenstore/blob/f82bd45cbcd1aa98306c57d35847e3d77f7cc8ee/src/wire.rs#L55

So the prefix is really programming language specific and I don't think
introducing it to this document would be useful.

Thanks,

-- 

Anthony Perard | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech
Re: [PATCH v3] docs: specify numerical values of Xenstore commands
Posted by Jürgen Groß 9 months, 1 week ago
On 13.03.25 11:16, Anthony PERARD wrote:
> On Thu, Mar 13, 2025 at 10:51:06AM +0100, Jürgen Groß wrote:
>> On 12.03.25 17:46, Anthony PERARD wrote:
>>> On Wed, Mar 12, 2025 at 09:41:43AM +0100, Juergen Gross wrote:
>>>> diff --git a/docs/misc/xenstore.txt b/docs/misc/xenstore.txt
>>>> index 7e1f031520..72db73deef 100644
>>>> --- a/docs/misc/xenstore.txt
>>>> +++ b/docs/misc/xenstore.txt
>>>> @@ -86,6 +86,67 @@ parts of xenstore inaccessible to some clients.  In any case passing
>>>> +XS_CONTROL               0    optional
>>>> +    If not supported, xenstore-control command will not work.
>>>> +    XS_DEBUG is a deprecated alias of XS_CONTROL.
>>>> +XS_DIRECTORY             1
>>>> +XS_READ                  2
>>>> +XS_GET_PERMS             3
>>>
>>> This new table prefix message type names with "XS_", but the rest of the
>>> document describe each type without the prefix. Isn't it going to be
>>> confusing, and make it slightly harder to link this table to rest of the
>>> document? (I often search by full word, like '\<GET_PERMS\>', because
>>> that one key stroke in vim '*', so having different prefix makes it
>>> harder to search)
>>
>> Question is, should I change the table to drop "XS_", or the rest document
>> to add "XS_" instead? After all xs_wire.h is defining the names with "XS_".
>>
>> I'm slightly leaning towards a preparatory patch adding "XS_".
> 
> Well, I'm actually for dropping the prefix from the table. The prefix is
> more of a C specific namespace than anything else. The ocaml
> implementation in tree doesn't use this prefix, but a different one (if
> we ignore the different case:
>> Xenbus.Xb.Op.Watch
> https://elixir.bootlin.com/xen/v4.20.0/source/tools/ocaml/xenstored/process.ml#L632
> And have a link to a string without the prefix:
>> | Watch			-> "WATCH"
> https://elixir.bootlin.com/xen/v4.20.0/source/tools/ocaml/libs/xb/op.ml#L49
> 
> There's also a version in Rust which also use a different prefix,
> "XsMessageType::".
> https://github.com/Wenzel/xenstore/blob/f82bd45cbcd1aa98306c57d35847e3d77f7cc8ee/src/wire.rs#L55
> 
> So the prefix is really programming language specific and I don't think
> introducing it to this document would be useful.

Fair enough. Stay tuned for V4. :-)


Juergen