RE: [PATCH] docs: update xenstore migration design document

[Paul Durrant <xadimgnik@xxxxxxxxx>]

> -----Original Message-----
> From: Xen-devel <xen-devel-bounces@xxxxxxxxxxxxxxxxxxxx> On Behalf Of Juergen 
> Gross
> Sent: 14 April 2020 17:00
> To: xen-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Juergen Gross <jgross@xxxxxxxx>; Stefano Stabellini 
> <sstabellini@xxxxxxxxxx>; Julien Grall
> <julien@xxxxxxx>; Wei Liu <wl@xxxxxxx>; Andrew Cooper 
> <andrew.cooper3@xxxxxxxxxx>; Ian Jackson
> <ian.jackson@xxxxxxxxxxxxx>; George Dunlap <george.dunlap@xxxxxxxxxx>; Jan 
> Beulich <jbeulich@xxxxxxxx>
> Subject: [PATCH] docs: update xenstore migration design document
> 
> In the past there have been several attempts to make Xenstore
> restartable. This requires to transfer the internal Xenstore state to
> the new instance. With the Xenstore migration protocol added recently
> to Xen's documentation a first base has been defined to represent the
> state of Xenstore. This can be expanded a little bit in order to have
> a full state representation which is needed as a first step for live
> updating Xenstore.
> 
> Add some definitions to designs/xenstore-migration.md which are needed
> for live update of xenstored.
> 
> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> ---
>  docs/designs/xenstore-migration.md | 90 
> ++++++++++++++++++++++++++++++++++++--
>  1 file changed, 87 insertions(+), 3 deletions(-)
> 
> diff --git a/docs/designs/xenstore-migration.md 
> b/docs/designs/xenstore-migration.md
> index 6ab351e8fe..09bb4700b4 100644
> --- a/docs/designs/xenstore-migration.md
> +++ b/docs/designs/xenstore-migration.md
> @@ -9,6 +9,10 @@ records must include details of registered xenstore watches 
> as well as
>  content; information that cannot currently be recovered from `xenstored`,
>  and hence some extension to the xenstore protocol[2] will also be required.
> 
> +As a similar set of data is needed for transferring xenstore data from
> +one instance to another when live updating xenstored the same definitions
> +are being used.
> +

That makes sense, but using an external entity to extract the state makes less 
sense in the context of live update so, going
forward, I suggest dropping the section on extra commands.
Also, we should define a separate 'xenstore domain image format' which can be 
included as an opaque entity in the migration stream,
in the same way as the qemu save image.

>  The *libxenlight Domain Image Format* specification[3] already defines a
>  record type `EMULATOR_XENSTORE_DATA` but this is not suitable for
>  transferring xenstore data pertaining to the domain directly as it is
> @@ -48,7 +52,10 @@ where type is one of the following values
>  |        | 0x00000001: NODE_DATA                            |
>  |        | 0x00000002: WATCH_DATA                           |
>  |        | 0x00000003: TRANSACTION_DATA                     |
> -|        | 0x00000004 - 0xFFFFFFFF: reserved for future use |
> +|        | 0x00000004: TRANSACTION_NODE_DATA                |
> +|        | 0x00000005: GUEST_RING_DATA                      |
> +|        | 0x00000006: DOMAIN_START (live update only)      |
> +|        | 0x00000007 - 0xFFFFFFFF: reserved for future use |
> 
> 
>  and data is one of the record data formats described in the following
> @@ -79,7 +86,7 @@ as follows:
>  +-------------------------------+
>  | perm count (N)                |
>  +-------------------------------+
> -| perm0                         |
> +| perm1                         |
>  +-------------------------------+
>  ...
>  +-------------------------------+
> @@ -93,7 +100,7 @@ as follows:
>  +-------------------------------+
>  ```
> 
> -where perm0..N are formatted as follows:
> +where perm1..N are formatted as follows:
> 
> 
>  ```
> @@ -164,6 +171,83 @@ as follows:
>  where tx_id is the non-zero identifier values of an open transaction.
> 
> 
> +**TRANSACTION_NODE_DATA**
> +
> +
> +Each TRANSACTION_NODE_DATA record specifies a transaction local xenstore
> +node. Its is similar to the NODE_DATA record with the addition of a
> +transaction id:
> +
> +```
> +    0       1       2       3     octet
> ++-------+-------+-------+-------+
> +| TRANSACTION_NODE_DATA         |
> ++-------------------------------+
> +| tx_id                         |
> ++-------------------------------+
> +| path length                   |
> ++-------------------------------+
> +| path data                     |
> +...
> +| pad (0 to 3 octets)           |
> ++-------------------------------+
> +| perm count (N)                |
> ++-------------------------------+
> +| perm1                         |
> ++-------------------------------+
> +...
> ++-------------------------------+
> +| permN                         |
> ++-------------------------------+
> +| value length                  |
> ++-------------------------------+
> +| value data                    |
> +...
> +| pad (0 to 3 octets)           |
> ++-------------------------------+
> +```
> +
> +where perm1..N are formatted as specified in the NODE_DATA record. A perm
> +count of 0 denotes a node having been deleted in the transaction.
> +

Transferring this state means we can presumably drop the TRANSACTION_DATA, 
since we can infer open transactions from the presence of
these records?

> +
> +**GUEST_RING_DATA**
> +
> +
> +The GUEST_RING_DATA record is used to transfer data which is pending to be
> +written to the guest's xenstore ring buffer. It si formatted as follows:
> +
> +
> +```
> ++-------+-------+-------+-------+
> +| GUEST_RING_DATA               |
> ++-------------------------------+
> +| value length                  |
> ++-------------------------------+
> +| value data                    |
> +...
> +| pad (0 to 3 octets)           |
> ++-------------------------------+
> +```
> +
> +**DOMAIN_START**
> +
> +
> +For live updating xenstored data of multiple domains needs to be transferred.
> +For this purpose the DOMAIN_START record is being used. All records of types
> +other than NODE_DATA always relate to the last DOMAIN_START record in the
> +stream. A DOMAIN_START record just contains a domain-id:

If we define a separate stream format as I suggest above, I'd expect the 
domain-id to be part of the header.

> +
> +
> +```
> ++-------+-------+-------+-------+
> +| DOMAIN_START                  |
> ++-------------------------------+
> +| domid         | pad           |
> ++-------------------------------+
> +```
> +
> +
>  ### Protocol Extension

As mentioned above, this section ought to be dropped for the moment. We should 
define the ways in which xenstored should dump state
in various scenarios: e.g. for LU it will likely be serialize to memory 
(possibly dev/shmem?), for migration/save-restore it will
probably be serialize to fd (socket/file). We should also define how dumping 
state will be triggered.

  Paul