|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v5 2/2] docs/designs: Add a design document for migration of xenstore data
Hi Paul, On 13/02/2020 10:53, Paul Durrant wrote: This patch details proposes extra migration data and xenstore protocol extensions to support non-cooperative live migration of guests. Signed-off-by: Paul Durrant <pdurrant@xxxxxxxxxx> --- Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> Cc: George Dunlap <George.Dunlap@xxxxxxxxxxxxx> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx> Cc: Jan Beulich <jbeulich@xxxxxxxx> Cc: Julien Grall <julien@xxxxxxx> Cc: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx> Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx> Cc: Wei Liu <wl@xxxxxxx> v5: - Add QUIESCE - Make semantics of <index> in GET_DOMAIN_WATCHES more clear v4: - Drop the restrictions on special paths v3: - New in v3 --- docs/designs/xenstore-migration.md | 136 +++++++++++++++++++++++++++++ 1 file changed, 136 insertions(+) create mode 100644 docs/designs/xenstore-migration.md diff --git a/docs/designs/xenstore-migration.md b/docs/designs/xenstore-migration.md new file mode 100644 index 0000000000..5cfe2d9a7d --- /dev/null +++ b/docs/designs/xenstore-migration.md @@ -0,0 +1,136 @@ +# Xenstore Migration + +## Background + +The design for *Non-Cooperative Migration of Guests*[1] explains that extra +save records are required in the migrations stream to allow a guest running +PV drivers to be migrated without its co-operation. Moreover the save +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. + +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 +specified such that keys are relative to the path +`/local/domain/$dm_domid/device-model/$domid`. Thus it is necessary to +define at least one new save record type. + +## Proposal + +### New Save Record + +A new mandatory record type should be defined within the libxenlight Domain +Image Format: + +`0x00000007: DOMAIN_XENSTORE_DATA` + +The format of each of these new records should be as follows: + + +``` +0 1 2 3 4 5 6 7 octet ++------------------------+------------------------+ +| type | record specific data | ++------------------------+ | +... ++-------------------------------------------------+ +``` + + +| Field | Description | +|---|---| Did you indend to add more - so | is on the same column as the onter lines? +| `type` | 0x00000000: invalid | +| | 0x00000001: node data | +| | 0x00000002: watch data | Should not the last | be some of the columns on all the lines? +| | 0x00000003 - 0xFFFFFFFF: reserved for future use | Looking at the spec, the command TRANSACTION_END *must* be used with an existing transaction. As a guest would be migrate to a new domain, the transaction ID would now be invalid. I understand that xenstored is able to cope with it, but such behavior is not described in the spec. So I am not sure we can expect a guest to cope with an error value other than the ones described for the command. + + +where data is always in the form of a NUL separated and terminated tuple +as follows + + +**node data** + + +`<path>|<value>|<perm-as-string>|` I don't think this would work. From the spec, <value> is a binary data and therefore it can contain zero or nul. So you would not be able to find out where the <perm-as-string> starts. Regarding the <perm-as-string>, it is only describing the permission for one domain. If multiple domains can access the node, then you would have multiple <perm-as-string>. Do we want to transfer all the permissions, if not how do we define which permissions should be transferred? + + +`<path>` is considered relative to the domain path `/local/domain/$domid` +and hence must not begin with `/`. +`<path>` and `<value>` should be suitable to formulate a `WRITE` operation +to the receiving xenstore and `<perm-as-string>` should be similarly suitable +to formulate a subsequent `SET_PERMS` operation. + +**watch data** + + +`<path>|<token>|` + +`<path>` again is considered relative and, together with `<token>`, should +be suitable to formulate an `ADD_DOMAIN_WATCHES` operation (see below). AFAICT, a guest is allowed to watch /. So is it a sensible thing to only transfer relative watch? Also, how about special watch (i.e @...)? + + +### Protocol Extension + +Before xenstore state is migrated it is necessary to wait for any pending +reads, writes, watch registrations etc. to complete, and also to make sure +that xenstored does not start processing any new requests (so that new +requests remain pending on the shared ring for subsequent processing on the +new host). Hence the following operation is needed: + +``` +QUIESCE <domid>| + +Complete processing of any request issued by the specified domain, and +do not process any further requests from the shared ring. +``` + +The `WATCH` operation does not allow specification of a `<domid>`; it is +assumed that the watch pertains to the domain that owns the shared ring +over which the operation is passed. Hence, for the tool-stack to be able +to register a watch on behalf of a domain a new operation is needed: + +``` +ADD_DOMAIN_WATCHES <domid>|<watch>|+ + +Adds watches on behalf of the specified domain. + +<watch> is a NUL separated tuple of <path>|<token>. The semantics of this +operation are identical to the domain issuing WATCH <path>|<token>| for +each <watch>. +``` + +The watch information for a domain also needs to be extracted from the +sending xenstored so the following operation is also needed: + +``` +GET_DOMAIN_WATCHES <domid>|<index> <gencnt>|<watch>|* + +Gets the list of watches that are currently registered for the domain. + +<watch> is a NUL separated tuple of <path>|<token>. The sub-list returned +will start at <index> items into the the overall list of watches and may +be truncated (at a <watch> boundary) such that the returned data fits +within XENSTORE_PAYLOAD_MAX. + +If <index> is beyond the end of the overall list then the returned sub- +list will be empty. If the value of <gencnt> changes then it indicates +that the overall watch list has changed and thus it may be necessary +to re-issue the operation for previous values of <index>. +``` + +It may also be desirable to state in the protocol specification that +the `INTRODUCE` operation should not clear the `<mfn>` specified such that Not directly related to this patch, the '<mfn>' is slightly confusing because, AFAICT, this will actually hold an GFN. To avoid spreading more misuse, it would make sense to update the xenstore accordingly and use the new term here. +a `RELEASE` operation followed by an `INTRODUCE` operation form an +idempotent pair. The current implementation of *C xentored* does this +(in the `domain_conn_reset()` function) but this could be dropped as this +behaviour is not currently specified and the page will always be zeroed +for a newly created domain. + + +* * * + +[1] See https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=docs/designs/non-cooperative-migration.md +[2] See https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=docs/misc/xenstore.txt +[3] See https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=docs/specs/libxl-migration-stream.pandoc Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |