RE: [PATCH v2] docs: update xenstore-migration.md

[Paul Durrant <xadimgnik@gmail.com>]

> -----Original Message-----
> From: Jürgen Groß <jgross@suse.com>
> Sent: 28 May 2020 09:59
> To: paul@xen.org; xen-devel@lists.xenproject.org
> Cc: 'Stefano Stabellini' <sstabellini@kernel.org>; 'Julien Grall' <julien@xen.org>; 'Wei Liu'
> <wl@xen.org>; 'Andrew Cooper' <andrew.cooper3@citrix.com>; 'Ian Jackson' <ian.jackson@eu.citrix.com>;
> 'George Dunlap' <george.dunlap@citrix.com>; 'Jan Beulich' <jbeulich@suse.com>
> Subject: Re: [PATCH v2] docs: update xenstore-migration.md
> 
> On 28.05.20 10:53, Paul Durrant wrote:
> >> -----Original Message-----
> >> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of Juergen Gross
> >> Sent: 28 May 2020 09:22
> >> To: xen-devel@lists.xenproject.org
> >> Cc: Juergen Gross <jgross@suse.com>; Stefano Stabellini <sstabellini@kernel.org>; Julien Grall
> >> <julien@xen.org>; Wei Liu <wl@xen.org>; Andrew Cooper <andrew.cooper3@citrix.com>; Ian Jackson
> >> <ian.jackson@eu.citrix.com>; George Dunlap <george.dunlap@citrix.com>; Jan Beulich
> <jbeulich@suse.com>
> >> Subject: [PATCH v2] docs: update xenstore-migration.md
> >>
> >> Update connection record details: make flags common for sockets and
> >> domains, and add pending incoming data.
> >>
> >> Signed-off-by: Juergen Gross <jgross@suse.com>
> >> ---
> >> V2:
> >> - added out-resp-len to connection record
> >> ---
> >>   docs/designs/xenstore-migration.md | 71 +++++++++++++++++-------------
> >>   1 file changed, 40 insertions(+), 31 deletions(-)
> >>
> >> diff --git a/docs/designs/xenstore-migration.md b/docs/designs/xenstore-migration.md
> >> index 34a2afd17e..5736bbad94 100644
> >> --- a/docs/designs/xenstore-migration.md
> >> +++ b/docs/designs/xenstore-migration.md
> >> @@ -147,43 +147,59 @@ the domain being migrated.
> >>   ```
> >>       0       1       2       3       4       5       6       7    octet
> >>   +-------+-------+-------+-------+-------+-------+-------+-------+
> >> -| conn-id                       | conn-type     | conn-spec
> >> +| conn-id                       | conn-type     | flags         |
> >> ++-------------------------------+---------------+---------------+
> >> +| conn-spec
> >>   ...
> >> -+-------------------------------+-------------------------------+
> >> -| data-len                      | data
> >> -+-------------------------------+
> >> ++---------------+---------------+-------------------------------+
> >> +| in-data-len   | out-resp-len  | out-data-len                  |
> >> ++---------------+---------------+-------------------------------+
> >> +| data
> >>   ...
> >>   ```
> >>
> >>
> >> -| Field       | Description                                     |
> >> -|-------------|-------------------------------------------------|
> >> -| `conn-id`   | A non-zero number used to identify this         |
> >> -|             | connection in subsequent connection-specific    |
> >> -|             | records                                         |
> >> -|             |                                                 |
> >> -| `conn-type` | 0x0000: shared ring                             |
> >> -|             | 0x0001: socket                                  |
> >> -|             | 0x0002 - 0xFFFF: reserved for future use        |
> >> -|             |                                                 |
> >> -| `conn-spec` | See below                                       |
> >> -|             |                                                 |
> >> -| `data-len`  | The length (in octets) of any pending data not  |
> >> -|             | yet written to the connection                   |
> >> -|             |                                                 |
> >> -| `data`      | Pending data (may be empty)                     |
> >> +| Field          | Description                                  |
> >> +|----------------|----------------------------------------------|
> >> +| `conn-id`      | A non-zero number used to identify this      |
> >> +|                | connection in subsequent connection-specific |
> >> +|                | records                                      |
> >> +|                |                                              |
> >> +| `flags`        | A bit-wise OR of:                            |
> >> +|                | 0001: read-only                              |
> >> +|                |                                              |
> >> +| `conn-type`    | 0x0000: shared ring                          |
> >> +|                | 0x0001: socket                               |
> >> +|                | 0x0002 - 0xFFFF: reserved for future use     |
> >> +|                |                                              |
> >
> > Agreed with Julien... the above two would be better swapped to match the order of the fields in the
> record.
> 
> Yes.
> 
> >
> >> +| `conn-spec`    | See below                                    |
> >> +|                |                                              |
> >> +| `in-data-len`  | The length (in octets) of any data read      |
> >> +|                | from the connection not yet processed        |
> >> +|                |                                              |
> >> +| `out-resp-len` | The length (in octets) of a partial response |
> >> +|                | not yet written to the connection (included  |
> >> +|                | in the following `out-data-len`)             |
> >> +|                |                                              |
> >> +| `out-data-len` | The length (in octets) of any pending data   |
> >> +|                | not yet written to the connection            |
> >
> > So, IIUC out-data-len is inclusive of out-resp-len?
> 
> Yes.
> 
> >
> >> +|                |                                              |
> >> +| `data`         | Pending data, first read data, then written  |
> >> +|                | data (any of both may be empty)              |
> >
> > Perhaps be more explicit here:
> >
> > "Pending data: first in-data-len octets of read data, then out-data-len octets of written data"
> 
> Okay.
> 
> >
> >>
> >> -The format of `conn-spec` is dependent upon `conn-type`.
> >> +In case of live update the connection record for the connection via which
> >> +the live update command was issued will contain the response for the live
> >> +update command in the pending write data.
> >
> > s/write/written for consistency I think.
> 
> I'll use "... in the pending not yet written data".
> 

Ok.

  Paul

> 
> Juergen