xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] docs: specify endianess of xenstore protocol header
  2017-05-08  6:58 Juergen Gross
@ 2017-05-08  6:58 ` Juergen Gross
  2017-05-08 10:07   ` Ian Jackson
  0 siblings, 1 reply; 19+ messages in thread
From: Juergen Gross @ 2017-05-08  6:58 UTC (permalink / raw)
  To: xen-devel
  Cc: Juergen Gross, julien.grall, sstabellini, wei.liu2, George.Dunlap,
	andrew.cooper3, ian.jackson, tim, jbeulich

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 0/3] docs: add some missing xenstore documentation
@ 2017-05-08  7:00 Juergen Gross
  2017-05-08  7:00 ` [PATCH 1/3] docs: specify endianess of xenstore protocol header Juergen Gross
                   ` (3 more replies)
  0 siblings, 4 replies; 19+ messages in thread
From: Juergen Gross @ 2017-05-08  7:00 UTC (permalink / raw)
  To: xen-devel
  Cc: Juergen Gross, sstabellini, wei.liu2, George.Dunlap,
	andrew.cooper3, ian.jackson, tim, julien.grall, jbeulich

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 1/3] docs: specify endianess of xenstore protocol header
  2017-05-08  7:00 [PATCH 0/3] docs: add some missing xenstore documentation Juergen Gross
@ 2017-05-08  7:00 ` Juergen Gross
  2017-05-08  7:00 ` [PATCH 2/3] docs: add DIRECTORY_PART specification do xenstore protocol doc Juergen Gross
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 19+ messages in thread
From: Juergen Gross @ 2017-05-08  7:00 UTC (permalink / raw)
  To: xen-devel
  Cc: Juergen Gross, sstabellini, wei.liu2, George.Dunlap,
	andrew.cooper3, ian.jackson, tim, julien.grall, jbeulich

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 2/3] docs: add DIRECTORY_PART specification do xenstore protocol doc
  2017-05-08  7:00 [PATCH 0/3] docs: add some missing xenstore documentation Juergen Gross
  2017-05-08  7:00 ` [PATCH 1/3] docs: specify endianess of xenstore protocol header Juergen Gross
@ 2017-05-08  7:00 ` Juergen Gross
  2017-05-08 10:09   ` Ian Jackson
  2017-05-08  7:00 ` [PATCH 3/3] docs: document CONTROL command of xenstore protocol Juergen Gross
  2017-05-08 10:39 ` [PATCH 0/3] docs: add some missing xenstore documentation Julien Grall
  3 siblings, 1 reply; 19+ messages in thread
From: Juergen Gross @ 2017-05-08  7:00 UTC (permalink / raw)
  To: xen-devel
  Cc: Juergen Gross, sstabellini, wei.liu2, George.Dunlap,
	andrew.cooper3, ian.jackson, tim, julien.grall, jbeulich

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 3/3] docs: document CONTROL command of xenstore protocol
  2017-05-08  7:00 [PATCH 0/3] docs: add some missing xenstore documentation Juergen Gross
  2017-05-08  7:00 ` [PATCH 1/3] docs: specify endianess of xenstore protocol header Juergen Gross
  2017-05-08  7:00 ` [PATCH 2/3] docs: add DIRECTORY_PART specification do xenstore protocol doc Juergen Gross
@ 2017-05-08  7:00 ` Juergen Gross
  2017-05-08 10:13   ` Ian Jackson
  2017-05-08 10:39 ` [PATCH 0/3] docs: add some missing xenstore documentation Julien Grall
  3 siblings, 1 reply; 19+ messages in thread
From: Juergen Gross @ 2017-05-08  7:00 UTC (permalink / raw)
  To: xen-devel
  Cc: Juergen Gross, sstabellini, wei.liu2, George.Dunlap,
	andrew.cooper3, ian.jackson, tim, julien.grall, jbeulich

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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/3] docs: specify endianess of xenstore protocol header
  2017-05-08  6:58 ` [PATCH 1/3] docs: specify endianess of xenstore protocol header Juergen Gross
@ 2017-05-08 10:07   ` Ian Jackson
  2017-05-08 10:15     ` Ian Jackson
                       ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: Ian Jackson @ 2017-05-08 10:07 UTC (permalink / raw)
  To: Juergen Gross
  Cc: julien.grall, sstabellini, wei.liu2, George.Dunlap,
	andrew.cooper3, tim, jbeulich, xen-devel

Juergen Gross writes ("[PATCH 1/3] docs: specify endianess of xenstore protocol header"):
> The endianess of the xenstore protocol header should be specified.
...
> -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.

Yes, but this is not correct.  On a big-endian cpu, they would be in
big-endian.

On a bytesexual cpu, the endianness should be specified but it will be
the same endianness as shared ring fields, etc.  So this doc probably
ought not to contain a list of endiannesses.  Best just to say that
the fields are all in host native byte order.

Ian.

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/3] docs: add DIRECTORY_PART specification do xenstore protocol doc
  2017-05-08  7:00 ` [PATCH 2/3] docs: add DIRECTORY_PART specification do xenstore protocol doc Juergen Gross
@ 2017-05-08 10:09   ` Ian Jackson
  2017-05-08 10:31     ` Juergen Gross
  0 siblings, 1 reply; 19+ messages in thread
From: Ian Jackson @ 2017-05-08 10:09 UTC (permalink / raw)
  To: Juergen Gross
  Cc: sstabellini, wei.liu2, George.Dunlap, andrew.cooper3, tim,
	julien.grall, jbeulich, xen-devel

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/3] docs: document CONTROL command of xenstore protocol
  2017-05-08  7:00 ` [PATCH 3/3] docs: document CONTROL command of xenstore protocol Juergen Gross
@ 2017-05-08 10:13   ` Ian Jackson
  2017-05-08 10:22     ` Juergen Gross
  0 siblings, 1 reply; 19+ messages in thread
From: Ian Jackson @ 2017-05-08 10:13 UTC (permalink / raw)
  To: Juergen Gross
  Cc: sstabellini, wei.liu2, George.Dunlap, andrew.cooper3, tim,
	julien.grall, jbeulich, xen-devel

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/3] docs: specify endianess of xenstore protocol header
  2017-05-08 10:07   ` Ian Jackson
@ 2017-05-08 10:15     ` Ian Jackson
  2017-05-08 10:17     ` Juergen Gross
  2017-05-08 18:23     ` Stefano Stabellini
  2 siblings, 0 replies; 19+ messages in thread
From: Ian Jackson @ 2017-05-08 10:15 UTC (permalink / raw)
  To: Juergen Gross, xen-devel, andrew.cooper3, George.Dunlap, jbeulich,
	konrad.wilk, sstabellini, tim, wei.liu2, julien.grall

(resending this one with Julien's right address.)

Ian Jackson writes ("Re: [PATCH 1/3] docs: specify endianess of xenstore protocol header"):
> Juergen Gross writes ("[PATCH 1/3] docs: specify endianess of xenstore protocol header"):
> > The endianess of the xenstore protocol header should be specified.
> ...
> > -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.
> 
> Yes, but this is not correct.  On a big-endian cpu, they would be in
> big-endian.
> 
> On a bytesexual cpu, the endianness should be specified but it will be
> the same endianness as shared ring fields, etc.  So this doc probably
> ought not to contain a list of endiannesses.  Best just to say that
> the fields are all in host native byte order.

Also: in the subject, `endianess' should be `endianness'.

Ian.

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/3] docs: specify endianess of xenstore protocol header
  2017-05-08 10:07   ` Ian Jackson
  2017-05-08 10:15     ` Ian Jackson
@ 2017-05-08 10:17     ` Juergen Gross
  2017-05-08 10:24       ` Ian Jackson
  2017-05-08 10:59       ` Julien Grall
  2017-05-08 18:23     ` Stefano Stabellini
  2 siblings, 2 replies; 19+ messages in thread
From: Juergen Gross @ 2017-05-08 10:17 UTC (permalink / raw)
  To: Ian Jackson
  Cc: julien.grall, sstabellini, wei.liu2, George.Dunlap,
	andrew.cooper3, tim, jbeulich, xen-devel

On 08/05/17 12:07, Ian Jackson wrote:
> Juergen Gross writes ("[PATCH 1/3] docs: specify endianess of xenstore protocol header"):
>> The endianess of the xenstore protocol header should be specified.
> ...
>> -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.
> 
> Yes, but this is not correct.  On a big-endian cpu, they would be in
> big-endian.

We don't support big-endian cpus, right? Do we want to specify the
protocol for unsupported cpus?

> On a bytesexual cpu, the endianness should be specified but it will be
> the same endianness as shared ring fields, etc.  So this doc probably
> ought not to contain a list of endiannesses.  Best just to say that
> the fields are all in host native byte order.

Hmm, this is problematic. How does a guest started e.g. big-endian on a
cpu capable of both byte orders know which endianess the host has? I
think specifying one endianess in this case is the better approach.

BTW: I'm quite sure we don't support big-endian guests (or host) on ARM
either, do we?

I could reword the paragraph to:

"The header fields are in the default endianess of the processor, e.g.
little endian on x86 and ARM."


Juergen


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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 3/3] docs: document CONTROL command of xenstore protocol
  2017-05-08 10:13   ` Ian Jackson
@ 2017-05-08 10:22     ` Juergen Gross
  0 siblings, 0 replies; 19+ messages in thread
From: Juergen Gross @ 2017-05-08 10:22 UTC (permalink / raw)
  To: Ian Jackson
  Cc: sstabellini, wei.liu2, George.Dunlap, andrew.cooper3, tim,
	julien.grall, jbeulich, xen-devel

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/3] docs: specify endianess of xenstore protocol header
  2017-05-08 10:17     ` Juergen Gross
@ 2017-05-08 10:24       ` Ian Jackson
  2017-05-08 10:41         ` Juergen Gross
  2017-05-08 10:59       ` Julien Grall
  1 sibling, 1 reply; 19+ messages in thread
From: Ian Jackson @ 2017-05-08 10:24 UTC (permalink / raw)
  To: Juergen Gross
  Cc: julien.grall, sstabellini, wei.liu2, George.Dunlap,
	andrew.cooper3, tim, jbeulich, xen-devel

Juergen Gross writes ("Re: [PATCH 1/3] docs: specify endianess of xenstore protocol header"):
> On 08/05/17 12:07, Ian Jackson wrote:
> > Yes, but this is not correct.  On a big-endian cpu, they would be in
> > big-endian.
> 
> We don't support big-endian cpus, right? Do we want to specify the
> protocol for unsupported cpus?

We have in the past supported big-endian CPUs.  There is no
particular reason to think that a future Xen port will be to only a
little-endian CPU.

> > On a bytesexual cpu, the endianness should be specified but it will be
> > the same endianness as shared ring fields, etc.  So this doc probably
> > ought not to contain a list of endiannesses.  Best just to say that
> > the fields are all in host native byte order.
> 
> Hmm, this is problematic. How does a guest started e.g. big-endian on a
> cpu capable of both byte orders know which endianess the host has? I
> think specifying one endianess in this case is the better approach.

The same way that the guest knows the endianness of the other cpu
structures.

> BTW: I'm quite sure we don't support big-endian guests (or host) on ARM
> either, do we?

I have no idea.  If we do, they will need to byteswap things when
talking PV protocols.

> I could reword the paragraph to:
> 
> "The header fields are in the default endianess of the processor, e.g.
> little endian on x86 and ARM."

What information about endianness is in xen/include/public ?

I don't think the xenstore doc should contain its own indication of
endianness.  That leaves open the possibility that the docs might
specify (and someone might implement!) a mixed-endian system, where
the public headers and PV protocols are in one endianness, but
xenstore in another, because of differences in docs wording.

How about if xenstore.txt says something like `the endianness is the
same as that of the structures in the Xen public headers and the Xen
PV protocols' ?

Ian.

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/3] docs: add DIRECTORY_PART specification do xenstore protocol doc
  2017-05-08 10:09   ` Ian Jackson
@ 2017-05-08 10:31     ` Juergen Gross
  2017-05-08 11:53       ` Ian Jackson
  0 siblings, 1 reply; 19+ messages in thread
From: Juergen Gross @ 2017-05-08 10:31 UTC (permalink / raw)
  To: Ian Jackson
  Cc: sstabellini, wei.liu2, George.Dunlap, andrew.cooper3, tim,
	julien.grall, jbeulich, xen-devel

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/3] docs: add some missing xenstore documentation
  2017-05-08  7:00 [PATCH 0/3] docs: add some missing xenstore documentation Juergen Gross
                   ` (2 preceding siblings ...)
  2017-05-08  7:00 ` [PATCH 3/3] docs: document CONTROL command of xenstore protocol Juergen Gross
@ 2017-05-08 10:39 ` Julien Grall
  3 siblings, 0 replies; 19+ messages in thread
From: Julien Grall @ 2017-05-08 10:39 UTC (permalink / raw)
  To: Juergen Gross, xen-devel
  Cc: sstabellini, wei.liu2, George.Dunlap, andrew.cooper3, ian.jackson,
	tim, jbeulich

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/3] docs: specify endianess of xenstore protocol header
  2017-05-08 10:24       ` Ian Jackson
@ 2017-05-08 10:41         ` Juergen Gross
  0 siblings, 0 replies; 19+ messages in thread
From: Juergen Gross @ 2017-05-08 10:41 UTC (permalink / raw)
  To: Ian Jackson
  Cc: sstabellini, wei.liu2, George.Dunlap, andrew.cooper3, tim,
	julien.grall, jbeulich, xen-devel

On 08/05/17 12:24, Ian Jackson wrote:
> Juergen Gross writes ("Re: [PATCH 1/3] docs: specify endianess of xenstore protocol header"):
>> On 08/05/17 12:07, Ian Jackson wrote:
>>> Yes, but this is not correct.  On a big-endian cpu, they would be in
>>> big-endian.
>>
>> We don't support big-endian cpus, right? Do we want to specify the
>> protocol for unsupported cpus?
> 
> We have in the past supported big-endian CPUs.  There is no
> particular reason to think that a future Xen port will be to only a
> little-endian CPU.
> 
>>> On a bytesexual cpu, the endianness should be specified but it will be
>>> the same endianness as shared ring fields, etc.  So this doc probably
>>> ought not to contain a list of endiannesses.  Best just to say that
>>> the fields are all in host native byte order.
>>
>> Hmm, this is problematic. How does a guest started e.g. big-endian on a
>> cpu capable of both byte orders know which endianess the host has? I
>> think specifying one endianess in this case is the better approach.
> 
> The same way that the guest knows the endianness of the other cpu
> structures.
> 
>> BTW: I'm quite sure we don't support big-endian guests (or host) on ARM
>> either, do we?
> 
> I have no idea.  If we do, they will need to byteswap things when
> talking PV protocols.
> 
>> I could reword the paragraph to:
>>
>> "The header fields are in the default endianess of the processor, e.g.
>> little endian on x86 and ARM."
> 
> What information about endianness is in xen/include/public ?

xen/include/public> grep -iRI endian .
./arch-arm.h: * hypercall arguments are always little endian.
./arch-arm.h:#define PSR_BIG_ENDIAN  (1<<9)        /* arm32: Big Endian
Mode */
./arch-x86/hvm/start_info.h: * The address and sizes are always a 64bit
little endian unsigned integer.
./io/sndif.h: * XENSND_PCM_FORMAT_<format>[_<endian>]
./io/sndif.h: * endian: <LE/BE>, may be absent
./io/sndif.h: *     LE - Little endian, BE - Big endian

So with the exception of sndif.h: always little endian.

> I don't think the xenstore doc should contain its own indication of
> endianness.  That leaves open the possibility that the docs might
> specify (and someone might implement!) a mixed-endian system, where
> the public headers and PV protocols are in one endianness, but
> xenstore in another, because of differences in docs wording.
> 
> How about if xenstore.txt says something like `the endianness is the
> same as that of the structures in the Xen public headers and the Xen
> PV protocols' ?

Same as hypercall argument endianness?


Juergen

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/3] docs: specify endianess of xenstore protocol header
  2017-05-08 10:17     ` Juergen Gross
  2017-05-08 10:24       ` Ian Jackson
@ 2017-05-08 10:59       ` Julien Grall
  1 sibling, 0 replies; 19+ messages in thread
From: Julien Grall @ 2017-05-08 10:59 UTC (permalink / raw)
  To: Juergen Gross, Ian Jackson
  Cc: julien.grall, sstabellini, wei.liu2, George.Dunlap,
	andrew.cooper3, tim, jbeulich, xen-devel

Hi Juergen,

On 08/05/17 11:17, Juergen Gross wrote:
> On 08/05/17 12:07, Ian Jackson wrote:
>> Juergen Gross writes ("[PATCH 1/3] docs: specify endianess of xenstore protocol header"):
>>> The endianess of the xenstore protocol header should be specified.
>> ...
>>> -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.
>>
>> Yes, but this is not correct.  On a big-endian cpu, they would be in
>> big-endian.
>
> We don't support big-endian cpus, right? Do we want to specify the
> protocol for unsupported cpus?
>
>> On a bytesexual cpu, the endianness should be specified but it will be
>> the same endianness as shared ring fields, etc.  So this doc probably
>> ought not to contain a list of endiannesses.  Best just to say that
>> the fields are all in host native byte order.
>
> Hmm, this is problematic. How does a guest started e.g. big-endian on a
> cpu capable of both byte orders know which endianess the host has? I
> think specifying one endianess in this case is the better approach.
>
> BTW: I'm quite sure we don't support big-endian guests (or host) on ARM
> either, do we?

At the moment, Xen is always little endian and all the structure between 
Xen and the guests are little-endian.

We don't yet support big-end guests but there are nothing to prevent 
that. The only change I am aware of is in the MMIO emulation (see [1]). 
All the Xen hypercall argument will stay little-endian and the guest 
would have to take care of passing the arguments with the correct 
endianness.

>
> I could reword the paragraph to:
>
> "The header fields are in the default endianess of the processor, e.g.
> little endian on x86 and ARM."

Whilst instruction fetches are always little-endian, the memory 
endianness of data access does not have a particular default in the ARM 
ARM. This is left up to the implementor.

Cheers,

[1] 
https://xenbits.xen.org/gitweb/?p=people/julieng/xen-unstable.git;a=commit;h=e49cecef96d57622e9dcbc6199be1f018d405fc0

-- 
Julien Grall

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/3] docs: add DIRECTORY_PART specification do xenstore protocol doc
  2017-05-08 10:31     ` Juergen Gross
@ 2017-05-08 11:53       ` Ian Jackson
  2017-05-08 12:33         ` Juergen Gross
  0 siblings, 1 reply; 19+ messages in thread
From: Ian Jackson @ 2017-05-08 11:53 UTC (permalink / raw)
  To: Juergen Gross
  Cc: sstabellini, wei.liu2, George.Dunlap, andrew.cooper3, tim,
	julien.grall, jbeulich, xen-devel

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/3] docs: add DIRECTORY_PART specification do xenstore protocol doc
  2017-05-08 11:53       ` Ian Jackson
@ 2017-05-08 12:33         ` Juergen Gross
  0 siblings, 0 replies; 19+ messages in thread
From: Juergen Gross @ 2017-05-08 12:33 UTC (permalink / raw)
  To: Ian Jackson
  Cc: sstabellini, wei.liu2, George.Dunlap, andrew.cooper3, tim,
	julien.grall, jbeulich, xen-devel

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/3] docs: specify endianess of xenstore protocol header
  2017-05-08 10:07   ` Ian Jackson
  2017-05-08 10:15     ` Ian Jackson
  2017-05-08 10:17     ` Juergen Gross
@ 2017-05-08 18:23     ` Stefano Stabellini
  2 siblings, 0 replies; 19+ messages in thread
From: Stefano Stabellini @ 2017-05-08 18:23 UTC (permalink / raw)
  To: Ian Jackson
  Cc: Juergen Gross, julien.grall, sstabellini, wei.liu2, George.Dunlap,
	andrew.cooper3, tim, jbeulich, xen-devel

On Mon, 8 May 2017, Ian Jackson wrote:
> Juergen Gross writes ("[PATCH 1/3] docs: specify endianess of xenstore protocol header"):
> > The endianess of the xenstore protocol header should be specified.
> ...
> > -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.
> 
> Yes, but this is not correct.  On a big-endian cpu, they would be in
> big-endian.
> 
> On a bytesexual cpu, the endianness should be specified but it will be
> the same endianness as shared ring fields, etc.  So this doc probably
> ought not to contain a list of endiannesses.  Best just to say that
> the fields are all in host native byte order.

We only have two supported architectures today: x86 and ARM. Speaking
for ARM, we need to say clearly that it's little endian, because ARM
actually support both and it is possible to have a mix of big and little
endian guests on a little endian hypervisor.

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2017-05-08 18:23 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-05-08  7:00 [PATCH 0/3] docs: add some missing xenstore documentation Juergen Gross
2017-05-08  7:00 ` [PATCH 1/3] docs: specify endianess of xenstore protocol header Juergen Gross
2017-05-08  7:00 ` [PATCH 2/3] docs: add DIRECTORY_PART specification do xenstore protocol doc Juergen Gross
2017-05-08 10:09   ` Ian Jackson
2017-05-08 10:31     ` Juergen Gross
2017-05-08 11:53       ` Ian Jackson
2017-05-08 12:33         ` Juergen Gross
2017-05-08  7:00 ` [PATCH 3/3] docs: document CONTROL command of xenstore protocol Juergen Gross
2017-05-08 10:13   ` Ian Jackson
2017-05-08 10:22     ` Juergen Gross
2017-05-08 10:39 ` [PATCH 0/3] docs: add some missing xenstore documentation Julien Grall
  -- strict thread matches above, loose matches on Subject: below --
2017-05-08  6:58 Juergen Gross
2017-05-08  6:58 ` [PATCH 1/3] docs: specify endianess of xenstore protocol header Juergen Gross
2017-05-08 10:07   ` Ian Jackson
2017-05-08 10:15     ` Ian Jackson
2017-05-08 10:17     ` Juergen Gross
2017-05-08 10:24       ` Ian Jackson
2017-05-08 10:41         ` Juergen Gross
2017-05-08 10:59       ` Julien Grall
2017-05-08 18:23     ` Stefano Stabellini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).