In docs/misc/xenstore.txt the description of the Xenstore node access
permissions is missing one important aspect, which can be found only
in the code or in the wiki [1]:

The first permission entry is defining the owner of the node via the
domid, and the access rights for all domains NOT having a dedicated
permission entry.

Make that aspect clear in the official documentation.

[1]: https://wiki.xenproject.org/wiki/XenBus#Permissions

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 docs/misc/xenstore.txt | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/docs/misc/xenstore.txt b/docs/misc/xenstore.txt
index 8887e7df88..d807ef0709 100644
--- a/docs/misc/xenstore.txt
+++ b/docs/misc/xenstore.txt
@@ -45,13 +45,16 @@ them to within 2048 bytes.  (See XENSTORE_*_PATH_MAX in xs_wire.h.)
 
 Each node has one or multiple permission entries.  Permissions are
 granted by domain-id, the first permission entry of each node specifies
-the owner of the node.  Permissions of a node can be changed by the
-owner of the node, the owner can only be modified by the control
-domain (usually domain id 0).  The owner always has the right to read
-and write the node, while other permissions can be setup to allow
-read and/or write access.  When a domain is being removed from Xenstore
-nodes owned by that domain will be removed together with all of those
-nodes' children.
+the owner of the node, who always has full access to the node (read and
+write permission).  The access rights of the first entry specify the
+allowed access for all domains not having a dedicated permission entry
+(the default is "n", removing access for all domains not explicitly
+added via additional permission entries).  Permissions of a node can be
+changed by the owner of the node, the owner can only be modified by the
+control domain (usually domain id 0).  Other permissions can be setup to
+allow read and/or write access for other domains.  When a domain is
+being removed from Xenstore nodes owned by that domain will be removed
+together with all of those nodes' children.
 
 
 Communication with xenstore is via either sockets, or event channel
-- 
2.35.3

On 09/02/2023 2:41 pm, Juergen Gross wrote:
> In docs/misc/xenstore.txt the description of the Xenstore node access
> permissions is missing one important aspect, which can be found only
> in the code or in the wiki [1]:
>
> The first permission entry is defining the owner of the node via the
> domid, and the access rights for all domains NOT having a dedicated
> permission entry.
>
> Make that aspect clear in the official documentation.
>
> [1]: https://wiki.xenproject.org/wiki/XenBus#Permissions
>
> Signed-off-by: Juergen Gross <jgross@suse.com>

I feel as if Edvin deserves some kind of credit here, seeing as it was
his observation...

Also, CC to double check the wording.

~Andrew

> ---
>  docs/misc/xenstore.txt | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/docs/misc/xenstore.txt b/docs/misc/xenstore.txt
> index 8887e7df88..d807ef0709 100644
> --- a/docs/misc/xenstore.txt
> +++ b/docs/misc/xenstore.txt
> @@ -45,13 +45,16 @@ them to within 2048 bytes.  (See XENSTORE_*_PATH_MAX in xs_wire.h.)
>  
>  Each node has one or multiple permission entries.  Permissions are
>  granted by domain-id, the first permission entry of each node specifies
> -the owner of the node.  Permissions of a node can be changed by the
> -owner of the node, the owner can only be modified by the control
> -domain (usually domain id 0).  The owner always has the right to read
> -and write the node, while other permissions can be setup to allow
> -read and/or write access.  When a domain is being removed from Xenstore
> -nodes owned by that domain will be removed together with all of those
> -nodes' children.
> +the owner of the node, who always has full access to the node (read and
> +write permission).  The access rights of the first entry specify the
> +allowed access for all domains not having a dedicated permission entry
> +(the default is "n", removing access for all domains not explicitly
> +added via additional permission entries).  Permissions of a node can be
> +changed by the owner of the node, the owner can only be modified by the
> +control domain (usually domain id 0).  Other permissions can be setup to
> +allow read and/or write access for other domains.  When a domain is
> +being removed from Xenstore nodes owned by that domain will be removed
> +together with all of those nodes' children.
>  
>  
>  Communication with xenstore is via either sockets, or event channel

> On 9 Feb 2023, at 15:15, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> 
> On 09/02/2023 2:41 pm, Juergen Gross wrote:
>> In docs/misc/xenstore.txt the description of the Xenstore node access
>> permissions is missing one important aspect, which can be found only
>> in the code or in the wiki [1]:
>> 
>> The first permission entry is defining the owner of the node via the
>> domid, and the access rights for all domains NOT having a dedicated
>> permission entry.
>> 
>> Make that aspect clear in the official documentation.
>> 
>> [1]: https://wiki.xenproject.org/wiki/XenBus#Permissions
>> 
>> Signed-off-by: Juergen Gross <jgross@suse.com>
> 
> I feel as if Edvin deserves some kind of credit here, seeing as it was
> his observation...
> 
> Also, CC to double check the wording.
> 
> ~Andrew
> 
>> ---
>> docs/misc/xenstore.txt | 17 ++++++++++-------
>> 1 file changed, 10 insertions(+), 7 deletions(-)
>> 
>> diff --git a/docs/misc/xenstore.txt b/docs/misc/xenstore.txt
>> index 8887e7df88..d807ef0709 100644
>> --- a/docs/misc/xenstore.txt
>> +++ b/docs/misc/xenstore.txt
>> @@ -45,13 +45,16 @@ them to within 2048 bytes.  (See XENSTORE_*_PATH_MAX in xs_wire.h.)
>> 
>> Each node has one or multiple permission entries.  Permissions are
>> granted by domain-id, the first permission entry of each node specifies
>> -the owner of the node.  Permissions of a node can be changed by the
>> -owner of the node, the owner can only be modified by the control
>> -domain (usually domain id 0).  The owner always has the right to read
>> -and write the node, while other permissions can be setup to allow
>> -read and/or write access.  When a domain is being removed from Xenstore
>> -nodes owned by that domain will be removed together with all of those
>> -nodes' children.
>> +the owner of the node, who always has full access to the node (read and
>> +write permission).

>>  The access rights of the first entry specify the
>> +allowed access for all domains not having a dedicated permission entry
>> +(the default is "n", removing access for all domains not explicitly
>> +added via additional permission entries).  Permissions of a node can be
>> +changed by the owner of the node, the owner can only be modified by the
>> +control domain (usually domain id 0).

This looks good in general, one small nitpick, maybe we need something like this too:
", or domains equivalent to the owner or control domain (domain equivalence can be established with the privileged SET_TARGET operation)"

Thanks,
--Edwin

>>  Other permissions can be setup to
>> +allow read and/or write access for other domains.  When a domain is
>> +being removed from Xenstore nodes owned by that domain will be removed
>> +together with all of those nodes' children.
>> 
>> 
>> Communication with xenstore is via either sockets, or event channel
> 

On 09.02.23 16:29, Edwin Torok wrote:
> 
> 
>> On 9 Feb 2023, at 15:15, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>>
>> On 09/02/2023 2:41 pm, Juergen Gross wrote:
>>> In docs/misc/xenstore.txt the description of the Xenstore node access
>>> permissions is missing one important aspect, which can be found only
>>> in the code or in the wiki [1]:
>>>
>>> The first permission entry is defining the owner of the node via the
>>> domid, and the access rights for all domains NOT having a dedicated
>>> permission entry.
>>>
>>> Make that aspect clear in the official documentation.
>>>
>>> [1]: https://wiki.xenproject.org/wiki/XenBus#Permissions
>>>
>>> Signed-off-by: Juergen Gross <jgross@suse.com>
>>
>> I feel as if Edvin deserves some kind of credit here, seeing as it was
>> his observation...
>>
>> Also, CC to double check the wording.
>>
>> ~Andrew
>>
>>> ---
>>> docs/misc/xenstore.txt | 17 ++++++++++-------
>>> 1 file changed, 10 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/docs/misc/xenstore.txt b/docs/misc/xenstore.txt
>>> index 8887e7df88..d807ef0709 100644
>>> --- a/docs/misc/xenstore.txt
>>> +++ b/docs/misc/xenstore.txt
>>> @@ -45,13 +45,16 @@ them to within 2048 bytes.  (See XENSTORE_*_PATH_MAX in xs_wire.h.)
>>>
>>> Each node has one or multiple permission entries.  Permissions are
>>> granted by domain-id, the first permission entry of each node specifies
>>> -the owner of the node.  Permissions of a node can be changed by the
>>> -owner of the node, the owner can only be modified by the control
>>> -domain (usually domain id 0).  The owner always has the right to read
>>> -and write the node, while other permissions can be setup to allow
>>> -read and/or write access.  When a domain is being removed from Xenstore
>>> -nodes owned by that domain will be removed together with all of those
>>> -nodes' children.
>>> +the owner of the node, who always has full access to the node (read and
>>> +write permission).
> 
>>>   The access rights of the first entry specify the
>>> +allowed access for all domains not having a dedicated permission entry
>>> +(the default is "n", removing access for all domains not explicitly
>>> +added via additional permission entries).  Permissions of a node can be
>>> +changed by the owner of the node, the owner can only be modified by the
>>> +control domain (usually domain id 0).
> 
> This looks good in general, one small nitpick, maybe we need something like this too:
> ", or domains equivalent to the owner or control domain (domain equivalence can be established with the privileged SET_TARGET operation)"

This is already part of the SET_TARGET description. So I don't think we
need to add it here, too.


Juergen

On 09.02.23 16:15, Andrew Cooper wrote:
> On 09/02/2023 2:41 pm, Juergen Gross wrote:
>> In docs/misc/xenstore.txt the description of the Xenstore node access
>> permissions is missing one important aspect, which can be found only
>> in the code or in the wiki [1]:
>>
>> The first permission entry is defining the owner of the node via the
>> domid, and the access rights for all domains NOT having a dedicated
>> permission entry.
>>
>> Make that aspect clear in the official documentation.
>>
>> [1]: https://wiki.xenproject.org/wiki/XenBus#Permissions
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
> 
> I feel as if Edvin deserves some kind of credit here, seeing as it was
> his observation...

I wouldn't mind. What kind of tag would be appropriate? "Reported-by:"?


Juergen

On 09/02/2023 3:24 pm, Juergen Gross wrote:
> On 09.02.23 16:15, Andrew Cooper wrote:
>> On 09/02/2023 2:41 pm, Juergen Gross wrote:
>>> In docs/misc/xenstore.txt the description of the Xenstore node access
>>> permissions is missing one important aspect, which can be found only
>>> in the code or in the wiki [1]:
>>>
>>> The first permission entry is defining the owner of the node via the
>>> domid, and the access rights for all domains NOT having a dedicated
>>> permission entry.
>>>
>>> Make that aspect clear in the official documentation.
>>>
>>> [1]: https://wiki.xenproject.org/wiki/XenBus#Permissions
>>>
>>> Signed-off-by: Juergen Gross <jgross@suse.com>
>>
>> I feel as if Edvin deserves some kind of credit here, seeing as it was
>> his observation...
>
> I wouldn't mind. What kind of tag would be appropriate? "Reported-by:"?

Probably.  I can't think of anything better.

(and as usual, can be fixed on commit if that's the only issue)

~Andrew

Hi Juergen,

On 09/02/2023 14:41, Juergen Gross wrote:
> In docs/misc/xenstore.txt the description of the Xenstore node access
> permissions is missing one important aspect, which can be found only
> in the code or in the wiki [1]:
> 
> The first permission entry is defining the owner of the node via the
> domid, and the access rights for all domains NOT having a dedicated
> permission entry.
> 
> Make that aspect clear in the official documentation.

Thanks for the commit. I am not very thrilled with the current behavior 
but at least it is now clearly documented in xenstore.txt. So:

Reviewed-by: Julien Grall <jgrall@amazon.com>

Cheers,

-- 
Julien Grall