diff mbox series

docs: retrospectively add XS_DIRECTORY_PART to the xenstore protocol...

Message ID 20200127151907.2877-1-pdurrant@amazon.com (mailing list archive)
State New, archived
Headers show
Series docs: retrospectively add XS_DIRECTORY_PART to the xenstore protocol... | expand

Commit Message

Paul Durrant Jan. 27, 2020, 3:19 p.m. UTC
... specification.

This was added by commit 0ca64ed8 "xenstore: add support for reading
directory with many children" but not added to the specification at that
point. A version of xenstored supporting the command was first released
in Xen 4.9.

Signed-off-by: Paul Durrant <pdurrant@amazon.com>
---
Cc: Juergen Gross <jgross@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Julien Grall <julien@xen.org>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Wei Liu <wl@xen.org>
---
 docs/misc/xenstore.txt | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

Comments

Jürgen Groß Jan. 27, 2020, 3:31 p.m. UTC | #1
On 27.01.20 16:19, Paul Durrant wrote:
> ... specification.
> 
> This was added by commit 0ca64ed8 "xenstore: add support for reading
> directory with many children" but not added to the specification at that
> point. A version of xenstored supporting the command was first released
> in Xen 4.9.
> 
> Signed-off-by: Paul Durrant <pdurrant@amazon.com>
> ---
> Cc: Juergen Gross <jgross@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: George Dunlap <George.Dunlap@eu.citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Julien Grall <julien@xen.org>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Wei Liu <wl@xen.org>
> ---
>   docs/misc/xenstore.txt | 13 +++++++++++--
>   1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/docs/misc/xenstore.txt b/docs/misc/xenstore.txt
> index ae1b6a8c6e..bf42e9ec37 100644
> --- a/docs/misc/xenstore.txt
> +++ b/docs/misc/xenstore.txt
> @@ -125,8 +125,9 @@ Values commonly included in payloads include:
>   
>   
>   
> -The following are the actual type values, including the request and
> -reply payloads as applicable:
> +The following are the actual type values defined in io/xs_wire.h
> +(omitting the XS_ prefix), including the request and reply payloads
> +as applicable:
>   
>   
>   ---------- Database read, write and permissions operations ----------
> @@ -152,6 +153,14 @@ DIRECTORY		<path>|			<child-leaf-name>|*
>   	leafnames.  The resulting children are each named
>   	<path>/<child-leaf-name>.
>   
> +DIRECTORY_PART		<path>|<index|>		<child-leaf-name>|*
> +	Performs the same function as DIRECTORY, but returns a
> +	sub-list of children starting at <index> in the overall
> +	child list and less than or equal to XENSTORE_PAYLOAD_MAX
> +	octets in length. If <index> is beyond the end of the
> +	overall child list then the returned sub-list will be
> +	empty.
> +

Hmm, not quite.

I did send this some years ago:

https://lists.xen.org/archives/html/xen-devel/2017-05/msg00650.html

Ian wanted to suggest something better, but he never did.


Juergen
Ian Jackson Jan. 27, 2020, 3:33 p.m. UTC | #2
Paul Durrant writes ("[PATCH] docs: retrospectively add XS_DIRECTORY_PART to the xenstore protocol..."):
> ... specification.
> 
> This was added by commit 0ca64ed8 "xenstore: add support for reading
> directory with many children" but not added to the specification at that
> point. A version of xenstored supporting the command was first released
> in Xen 4.9.

Thanks for documenting this.  A docs fix like this should be
backported if it applies, IMO.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Backport: 4.9+

I will commit it to staging momentarily.

> +DIRECTORY_PART		<path>|<index|>		<child-leaf-name>|*
> +	Performs the same function as DIRECTORY, but returns a
> +	sub-list of children starting at <index> in the overall
> +	child list and less than or equal to XENSTORE_PAYLOAD_MAX
> +	octets in length. If <index> is beyond the end of the
> +	overall child list then the returned sub-list will be
> +	empty.

I wonder if it should be somehow made more explicit that `index' is a
count of directory entries, not bytes.  Maybe this is obvious.

Ian.
Jürgen Groß Jan. 27, 2020, 3:35 p.m. UTC | #3
On 27.01.20 16:33, Ian Jackson wrote:
> Paul Durrant writes ("[PATCH] docs: retrospectively add XS_DIRECTORY_PART to the xenstore protocol..."):
>> ... specification.
>>
>> This was added by commit 0ca64ed8 "xenstore: add support for reading
>> directory with many children" but not added to the specification at that
>> point. A version of xenstored supporting the command was first released
>> in Xen 4.9.
> 
> Thanks for documenting this.  A docs fix like this should be
> backported if it applies, IMO.
> 
> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
> Backport: 4.9+
> 
> I will commit it to staging momentarily.
> 
>> +DIRECTORY_PART		<path>|<index|>		<child-leaf-name>|*
>> +	Performs the same function as DIRECTORY, but returns a
>> +	sub-list of children starting at <index> in the overall
>> +	child list and less than or equal to XENSTORE_PAYLOAD_MAX
>> +	octets in length. If <index> is beyond the end of the
>> +	overall child list then the returned sub-list will be
>> +	empty.
> 
> I wonder if it should be somehow made more explicit that `index' is a
> count of directory entries, not bytes.  Maybe this is obvious.

But this is wrong. It is bytes, and the generation count returned is
missing (see my original patch back in 2017).


Juergen
Ian Jackson Jan. 27, 2020, 3:48 p.m. UTC | #4
Jürgen Groß writes ("Re: [PATCH] docs: retrospectively add XS_DIRECTORY_PART to the xenstore protocol..."):
> On 27.01.20 16:33, Ian Jackson wrote:
> > Paul Durrant writes ("[PATCH] docs: retrospectively add XS_DIRECTORY_PART to the xenstore protocol..."):
> >> ... specification.
> >>
> >> This was added by commit 0ca64ed8 "xenstore: add support for reading
> >> directory with many children" but not added to the specification at that
> >> point. A version of xenstored supporting the command was first released
> >> in Xen 4.9.
> > 
> > Thanks for documenting this.  A docs fix like this should be
> > backported if it applies, IMO.
> > 
> > Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
> > Backport: 4.9+
> > 
> > I will commit it to staging momentarily.
> > 
> >> +DIRECTORY_PART		<path>|<index|>		<child-leaf-name>|*
> >> +	Performs the same function as DIRECTORY, but returns a
> >> +	sub-list of children starting at <index> in the overall
> >> +	child list and less than or equal to XENSTORE_PAYLOAD_MAX
> >> +	octets in length. If <index> is beyond the end of the
> >> +	overall child list then the returned sub-list will be
> >> +	empty.
> > 
> > I wonder if it should be somehow made more explicit that `index' is a
> > count of directory entries, not bytes.  Maybe this is obvious.
> 
> But this is wrong. It is bytes, and the generation count returned is
> missing (see my original patch back in 2017).

Sorry for being too quick.  I have reverted my commit.

Ian.
Durrant, Paul Jan. 27, 2020, 3:53 p.m. UTC | #5
> -----Original Message-----
> From: Ian Jackson [mailto:ian.jackson@citrix.com]
> Sent: 27 January 2020 15:49
> To: Jürgen Groß <jgross@suse.com>
> Cc: Durrant, Paul <pdurrant@amazon.co.uk>; xen-devel@lists.xenproject.org;
> Andrew Cooper <Andrew.Cooper3@citrix.com>; George Dunlap
> <George.Dunlap@citrix.com>; Jan Beulich <jbeulich@suse.com>; Julien Grall
> <julien@xen.org>; Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>; Stefano
> Stabellini <sstabellini@kernel.org>; Wei Liu <wl@xen.org>
> Subject: Re: [PATCH] docs: retrospectively add XS_DIRECTORY_PART to the
> xenstore protocol...
> 
> Jürgen Groß writes ("Re: [PATCH] docs: retrospectively add
> XS_DIRECTORY_PART to the xenstore protocol..."):
> > On 27.01.20 16:33, Ian Jackson wrote:
> > > Paul Durrant writes ("[PATCH] docs: retrospectively add
> XS_DIRECTORY_PART to the xenstore protocol..."):
> > >> ... specification.
> > >>
> > >> This was added by commit 0ca64ed8 "xenstore: add support for
> > >> reading directory with many children" but not added to the
> > >> specification at that point. A version of xenstored supporting the
> > >> command was first released in Xen 4.9.
> > >
> > > Thanks for documenting this.  A docs fix like this should be
> > > backported if it applies, IMO.
> > >
> > > Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
> > > Backport: 4.9+
> > >
> > > I will commit it to staging momentarily.
> > >
> > >> +DIRECTORY_PART		<path>|<index|>		<child-leaf-name>|*
> > >> +	Performs the same function as DIRECTORY, but returns a
> > >> +	sub-list of children starting at <index> in the overall
> > >> +	child list and less than or equal to XENSTORE_PAYLOAD_MAX
> > >> +	octets in length. If <index> is beyond the end of the
> > >> +	overall child list then the returned sub-list will be
> > >> +	empty.
> > >
> > > I wonder if it should be somehow made more explicit that `index' is
> > > a count of directory entries, not bytes.  Maybe this is obvious.
> >
> > But this is wrong. It is bytes, and the generation count returned is
> > missing (see my original patch back in 2017).
> 
> Sorry for being too quick.  I have reverted my commit.
> 

Since I got it wrong, I suggest just taking Juergen's original text (which I was unaware of before). It seems ok to me.

  Paul

> Ian.
diff mbox series

Patch

diff --git a/docs/misc/xenstore.txt b/docs/misc/xenstore.txt
index ae1b6a8c6e..bf42e9ec37 100644
--- a/docs/misc/xenstore.txt
+++ b/docs/misc/xenstore.txt
@@ -125,8 +125,9 @@  Values commonly included in payloads include:
 
 
 
-The following are the actual type values, including the request and
-reply payloads as applicable:
+The following are the actual type values defined in io/xs_wire.h
+(omitting the XS_ prefix), including the request and reply payloads
+as applicable:
 
 
 ---------- Database read, write and permissions operations ----------
@@ -152,6 +153,14 @@  DIRECTORY		<path>|			<child-leaf-name>|*
 	leafnames.  The resulting children are each named
 	<path>/<child-leaf-name>.
 
+DIRECTORY_PART		<path>|<index|>		<child-leaf-name>|*
+	Performs the same function as DIRECTORY, but returns a
+	sub-list of children starting at <index> in the overall
+	child list and less than or equal to XENSTORE_PAYLOAD_MAX
+	octets in length. If <index> is beyond the end of the
+	overall child list then the returned sub-list will be
+	empty.
+
 GET_PERMS	 	<path>|			<perm-as-string>|+
 SET_PERMS		<path>|<perm-as-string>|+?
 	<perm-as-string> is one of the following