[PATCH 0/3] docs: add some missing xenstore documentation

[PATCH 0/3] docs: add some missing xenstore documentation

[Juergen Gross]

There were some bits missing in docs/misc/xenstore.txt, so add them.

We might want to include this in 4.9, but I'm not feeling really
strong about this.

Resending with correct email address of Julien.

Juergen Gross (3):
  docs: specify endianess of xenstore protocol header
  docs: add DIRECTORY_PART specification do xenstore protocol doc
  docs: document CONTROL command of xenstore protocol

 docs/misc/xenstore.txt | 41 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 34 insertions(+), 7 deletions(-)

-- 
2.12.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH 1/3] docs: specify endianess of xenstore protocol header

[Juergen Gross]

The endianess of the xenstore protocol header should be specified.

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

diff --git a/docs/misc/xenstore.txt b/docs/misc/xenstore.txt
index ae1b6a8c6e..5051340227 100644
--- a/docs/misc/xenstore.txt
+++ b/docs/misc/xenstore.txt
@@ -46,7 +46,8 @@ them to within 2048 bytes.  (See XENSTORE_*_PATH_MAX in xs_wire.h.)
 Communication with xenstore is via either sockets, or event channel
 and shared memory, as specified in io/xs_wire.h: each message in
 either direction is a header formatted as a struct xsd_sockmsg
-followed by xsd_sockmsg.len bytes of payload.
+followed by xsd_sockmsg.len bytes of payload. The header fields are
+all in little endian byte order.
 
 The payload syntax varies according to the type field.  Generally
 requests each generate a reply with an identical type, req_id and
-- 
2.12.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH 2/3] docs: add DIRECTORY_PART specification do xenstore protocol doc

[Juergen Gross]

DIRECTORY_PART was missing in docs/misc/xenstore.txt. Add it.

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

diff --git a/docs/misc/xenstore.txt b/docs/misc/xenstore.txt
index 5051340227..fd38d781e2 100644
--- a/docs/misc/xenstore.txt
+++ b/docs/misc/xenstore.txt
@@ -153,6 +153,15 @@ DIRECTORY		<path>|			<child-leaf-name>|*
 	leafnames.  The resulting children are each named
 	<path>/<child-leaf-name>.
 
+DIRECTORY_PART		<path>|<offset>		<gencnt>|<child-leaf-name>|*
+	Same as DIRECTORY, but to be used for children lists longer than
+	XENSTORE_PAYLOAD_MAX. Input are <path> and the byte offset into
+	the list of children to return. Return values are the generation
+	count <gencnt> of the node (to be used to ensure the node hasn't
+	changed between two reads: <gencnt> being the same for multiple
+	reads guarantees the node hasn't changed) and the list of children
+	starting at the specified <offset> of the complete list.
+
 GET_PERMS	 	<path>|			<perm-as-string>|+
 SET_PERMS		<path>|<perm-as-string>|+?
 	<perm-as-string> is one of the following
-- 
2.12.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH 3/3] docs: document CONTROL command of xenstore protocol

[Juergen Gross]

The CONTROL command (former DEBUG command) isn't specified in the
xenstore protocol doc. Add it.

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

diff --git a/docs/misc/xenstore.txt b/docs/misc/xenstore.txt
index fd38d781e2..074650f144 100644
--- a/docs/misc/xenstore.txt
+++ b/docs/misc/xenstore.txt
@@ -319,12 +319,29 @@ SET_TARGET		<domid>|<tdomid>|
 
 ---------- Miscellaneous ----------
 
-DEBUG			print|<string>|??	    sends <string> to debug log
-DEBUG			print|<thing-with-no-nul>   EINVAL
-DEBUG			check|??		    checks xenstored innards
-DEBUG			<anything-else|>	    no-op (future extension)
+CONTROL			<command>|[<parameters>|]
+	Send a control command <command> with optional parameters
+	(<parameters>) to Xenstore daemon. <command> support is
+	implementation specific and might change in future.
 
-	These requests should not generally be used and may be
-	withdrawn in the future.
+	Current commands are:
+	check
+		checks xenstored innards
+	log|on
+		turn xenstore logging on
+	log|off
+		turn xenstore logging off
+	logfile|<file-name>
+		log to specified file
+	memreport|[<file-name>]
+		print memory statistics to logfile (no <file-name>
+		specified) or to specific file
+	print|<string>
+		print <string> to syslog (xenstore runs as daemon) or
+		to console (xenstore runs as stubdom)
+	help			<supported-commands>
+		return list of supported commands for CONTROL
 
+DEBUG
+	Deprecated, now named CONTROL
 
-- 
2.12.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 2/3] docs: add DIRECTORY_PART specification do xenstore protocol doc

[Ian Jackson]

Juergen Gross writes ("[PATCH 2/3] docs: add DIRECTORY_PART specification do xenstore protocol doc"):
> DIRECTORY_PART was missing in docs/misc/xenstore.txt. Add it.
...
> +DIRECTORY_PART		<path>|<offset>		<gencnt>|<child-leaf-name>|*
> +	Same as DIRECTORY, but to be used for children lists longer than
> +	XENSTORE_PAYLOAD_MAX. Input are <path> and the byte offset into
> +	the list of children to return. Return values are the generation
> +	count <gencnt> of the node (to be used to ensure the node hasn't
> +	changed between two reads: <gencnt> being the same for multiple
> +	reads guarantees the node hasn't changed) and the list of children
> +	starting at the specified <offset> of the complete list.

The "generation count" is not defined anywhere else in this protocol
spec, so shouldn't be referred to here without definition.  We should
explicitly state whether using a transaction is sufficient to ensure
that this check will never fail.

Taken together, I think the right approach is to specify a method to
use this and to say that if a different method is used, the results
are not reliable.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 3/3] docs: document CONTROL command of xenstore protocol

[Ian Jackson]

Juergen Gross writes ("[PATCH 3/3] docs: document CONTROL command of xenstore protocol"):
> The CONTROL command (former DEBUG command) isn't specified in the
> xenstore protocol doc. Add it.
...
> -DEBUG			print|<string>|??	    sends <string> to debug log
> -DEBUG			print|<thing-with-no-nul>   EINVAL
> -DEBUG			check|??		    checks xenstored innards
> -DEBUG			<anything-else|>	    no-op (future extension)
> +CONTROL			<command>|[<parameters>|]
> +	Send a control command <command> with optional parameters
> +	(<parameters>) to Xenstore daemon. <command> support is
> +	implementation specific and might change in future.
>  
> -	These requests should not generally be used and may be
> -	withdrawn in the future.

I should retain a stronger imprecation.  How about:

      The set of commands and their semantics is implementation
      specific and is likely to change from one Xen version to the
      next.  Out-of-tree users will encounter compatibility issues.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 3/3] docs: document CONTROL command of xenstore protocol

[Juergen Gross]

On 08/05/17 12:13, Ian Jackson wrote:
> Juergen Gross writes ("[PATCH 3/3] docs: document CONTROL command of xenstore protocol"):
>> The CONTROL command (former DEBUG command) isn't specified in the
>> xenstore protocol doc. Add it.
> ...
>> -DEBUG			print|<string>|??	    sends <string> to debug log
>> -DEBUG			print|<thing-with-no-nul>   EINVAL
>> -DEBUG			check|??		    checks xenstored innards
>> -DEBUG			<anything-else|>	    no-op (future extension)
>> +CONTROL			<command>|[<parameters>|]
>> +	Send a control command <command> with optional parameters
>> +	(<parameters>) to Xenstore daemon. <command> support is
>> +	implementation specific and might change in future.
>>  
>> -	These requests should not generally be used and may be
>> -	withdrawn in the future.
> 
> I should retain a stronger imprecation.  How about:
> 
>       The set of commands and their semantics is implementation
>       specific and is likely to change from one Xen version to the
>       next.  Out-of-tree users will encounter compatibility issues.

I'm fine with this.


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 2/3] docs: add DIRECTORY_PART specification do xenstore protocol doc

[Juergen Gross]

On 08/05/17 12:09, Ian Jackson wrote:
> Juergen Gross writes ("[PATCH 2/3] docs: add DIRECTORY_PART specification do xenstore protocol doc"):
>> DIRECTORY_PART was missing in docs/misc/xenstore.txt. Add it.
> ...
>> +DIRECTORY_PART		<path>|<offset>		<gencnt>|<child-leaf-name>|*
>> +	Same as DIRECTORY, but to be used for children lists longer than
>> +	XENSTORE_PAYLOAD_MAX. Input are <path> and the byte offset into
>> +	the list of children to return. Return values are the generation
>> +	count <gencnt> of the node (to be used to ensure the node hasn't
>> +	changed between two reads: <gencnt> being the same for multiple
>> +	reads guarantees the node hasn't changed) and the list of children
>> +	starting at the specified <offset> of the complete list.
> 
> The "generation count" is not defined anywhere else in this protocol
> spec, so shouldn't be referred to here without definition.  We should
> explicitly state whether using a transaction is sufficient to ensure
> that this check will never fail.

As the generation count is if no interest anywhere else in this protocol
I don't see why the definition given in parentheses isn't enough.

The solution with <gencnt> was explicitly demanded in order to _not_
have to use transactions. So referring to transactions now seems to be
counterproductive.

> Taken together, I think the right approach is to specify a method to
> use this and to say that if a different method is used, the results
> are not reliable.

So libxenstore is using DIRECTORY_PART without transactions and will
still deliver correct results.


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 0/3] docs: add some missing xenstore documentation

[Julien Grall]

Hi Juergen,

On 08/05/17 08:00, Juergen Gross wrote:
> There were some bits missing in docs/misc/xenstore.txt, so add them.
>
> We might want to include this in 4.9, but I'm not feeling really
> strong about this.

I think any documentation patch should go in the release. Better 
documentation will make happier users :).

Release-acked-by: Julien Grall <julien.grall@arm.com>

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 2/3] docs: add DIRECTORY_PART specification do xenstore protocol doc

[Ian Jackson]

Juergen Gross writes ("Re: [PATCH 2/3] docs: add DIRECTORY_PART specification do xenstore protocol doc"):
> On 08/05/17 12:09, Ian Jackson wrote:
> > The "generation count" is not defined anywhere else in this protocol
> > spec, so shouldn't be referred to here without definition.  We should
> > explicitly state whether using a transaction is sufficient to ensure
> > that this check will never fail.
> 
> As the generation count is if no interest anywhere else in this protocol
> I don't see why the definition given in parentheses isn't enough.

I think it's rather inexplicit.  How about if I propose an
alternative ?

> The solution with <gencnt> was explicitly demanded in order to _not_
> have to use transactions. So referring to transactions now seems to be
> counterproductive.

The question is whether a client can use transactions instead.  Your
current wording seems to leave this question open.

Do you have an opinion about the answer this question ?

Thanks,
Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 2/3] docs: add DIRECTORY_PART specification do xenstore protocol doc

[Juergen Gross]

On 08/05/17 13:53, Ian Jackson wrote:
> Juergen Gross writes ("Re: [PATCH 2/3] docs: add DIRECTORY_PART specification do xenstore protocol doc"):
>> On 08/05/17 12:09, Ian Jackson wrote:
>>> The "generation count" is not defined anywhere else in this protocol
>>> spec, so shouldn't be referred to here without definition.  We should
>>> explicitly state whether using a transaction is sufficient to ensure
>>> that this check will never fail.
>>
>> As the generation count is if no interest anywhere else in this protocol
>> I don't see why the definition given in parentheses isn't enough.
> 
> I think it's rather inexplicit.  How about if I propose an
> alternative ?
> 
>> The solution with <gencnt> was explicitly demanded in order to _not_
>> have to use transactions. So referring to transactions now seems to be
>> counterproductive.
> 
> The question is whether a client can use transactions instead.  Your
> current wording seems to leave this question open.
> 
> Do you have an opinion about the answer this question ?

Using transactions instead will work, of course. Otherwise transaction
handling would be broken.


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH 2/3] docs: add DIRECTORY_PART specification do xenstore protocol doc

[Juergen Gross]

DIRECTORY_PART was missing in docs/misc/xenstore.txt. Add it.

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

diff --git a/docs/misc/xenstore.txt b/docs/misc/xenstore.txt
index 5051340227..fd38d781e2 100644
--- a/docs/misc/xenstore.txt
+++ b/docs/misc/xenstore.txt
@@ -153,6 +153,15 @@ DIRECTORY		<path>|			<child-leaf-name>|*
 	leafnames.  The resulting children are each named
 	<path>/<child-leaf-name>.
 
+DIRECTORY_PART		<path>|<offset>		<gencnt>|<child-leaf-name>|*
+	Same as DIRECTORY, but to be used for children lists longer than
+	XENSTORE_PAYLOAD_MAX. Input are <path> and the byte offset into
+	the list of children to return. Return values are the generation
+	count <gencnt> of the node (to be used to ensure the node hasn't
+	changed between two reads: <gencnt> being the same for multiple
+	reads guarantees the node hasn't changed) and the list of children
+	starting at the specified <offset> of the complete list.
+
 GET_PERMS	 	<path>|			<perm-as-string>|+
 SET_PERMS		<path>|<perm-as-string>|+?
 	<perm-as-string> is one of the following
-- 
2.12.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel