[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v2] docs/designs: re-work the xenstore migration document...
On 27.04.20 13:25, Paul Durrant wrote: -----Original Message----- From: Jürgen Groß <jgross@xxxxxxxx> Sent: 27 April 2020 12:13 To: paul@xxxxxxx; xen-devel@xxxxxxxxxxxxxxxxxxxx Cc: 'Paul Durrant' <pdurrant@xxxxxxxxxx> Subject: Re: [PATCH v2] docs/designs: re-work the xenstore migration document... On 27.04.20 12:45, Paul Durrant wrote:-----Original Message----- From: Jürgen Groß <jgross@xxxxxxxx> Sent: 27 April 2020 11:37 To: Paul Durrant <paul@xxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx Cc: Paul Durrant <pdurrant@xxxxxxxxxx> Subject: Re: [PATCH v2] docs/designs: re-work the xenstore migration document... On 27.04.20 09:53, Paul Durrant wrote:From: Paul Durrant <pdurrant@xxxxxxxxxx> ... to specify a separate migration stream that will also be suitable for live update. The original scope of the document was to support non-cooperative migration of guests [1] but, since then, live update of xenstored has been brought into scope. Thus it makes more sense to define a separate image format for serializing xenstore state that is suitable for both purposes. The document has been limited to specifying a new image format. The mechanism for acquiring the image for live update or migration is not covered as that is more appropriately dealt with by a patch to docs/misc/xenstore.txt. It is also expected that, when the first implementation of live update or migration making use of this specification is committed, that the document is moved from docs/designs into docs/specs. [1] See https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=docs/designs/non-cooperative-migration.mdSigned-off-by: Paul Durrant <pdurrant@xxxxxxxxxx> --- Juergen Gross <jgross@xxxxxxxx> Andrew Cooper <andrew.cooper3@xxxxxxxxxx> George Dunlap <george.dunlap@xxxxxxxxxx> Ian Jackson <ian.jackson@xxxxxxxxxxxxx> Jan Beulich <jbeulich@xxxxxxxx> Julien Grall <julien@xxxxxxx> Stefano Stabellini <sstabellini@xxxxxxxxxx> Wei Liu <wl@xxxxxxx>Mind adding CC: before those mail addresses in order to let git add those to the recipients list?D'oh... good spot.v2: - Address comments from JuergenNot all unfortunately. :-(OK.+### CONNECTION_DATA -Each WATCH_DATA record specifies a registered watch and is formatted as -follows: +For live update the image format will contain a `CONNECTION_DATA` record for +each connection to xenstore. For migration it will only contain a record for +the domain being migrated. ``` - 0 1 2 3 octet -+-------+-------+-------+-------+ -| WATCH_DATA | -+-------------------------------+ -| wpath length | -+-------------------------------+ -| wpath data | -... -| pad (0 to 3 octets) | -+-------------------------------+ + 0 1 2 3 4 5 6 7 octet ++-------+-------+-------+-------+-------+-------+-------+-------+ +| conn-id | pad | ++---------------+-----------------------------------------------+ +| conn-type | conn-spec ...I asked whether it wouldn't be better to drop the pad and move conn-type and a 2-byte (unified) flag field at its position. This together ...++-------------------------------+-------------------------------+ +| data-len | data +-------------------------------+ -| token length | -+-------------------------------+ -| token data | ... -| pad (0 to 3 octets) | -+-------------------------------+ ``` -wpath length and token length are specified in octets (excluding the NUL -terminator). The wpath should be as described for the `WATCH` operation in -[2]. The token is an arbitrary string of octets not containing any NUL -values. +| 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 | +| | | +| `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) | -**TRANSACTION_DATA** +The format of `conn-spec` is dependent upon `conn-type`. +\pagebreak -Each TRANSACTION_DATA record specifies an open transaction and is formatted -as follows: +For `shared ring` connections it is as follows: ``` - 0 1 2 3 octet -+-------+-------+-------+-------+ -| TRANSACTION_DATA | -+-------------------------------+ -| tx_id | -+-------------------------------+ + 0 1 2 3 4 5 6 7 octet + +-------+-------+-------+-------+-------+-------+ + | domid | tdomid | flags | ++---------------+---------------+---------------+---------------+ +| revtchn | levtchn | ++-------------------------------+-------------------------------+ +| mfn | ++---------------------------------------------------------------+... with dropping levtchn (which isn't needed IMO) will make it much easier to have a union in C (which needs to be aligned to 8 bytes and have a length of a multiple of 8 bytes due to mfn). So something like: struct xs_state_connection { uint32_t conn_id; uint16_t conn_type; #define XS_STATE_CONN_TYPE_RING 0 #define XS_STATE_CONN_TYPE_SOCKET 1 uint16_t flags; #define XS_STATE_CONN_INTRODUCED 0x0001 #define XS_STATE_CONN_RELEASED 0x0002 #define XS_STATE_CONN_READONLY 0x0004 union { struct { uint16_t domid; uint16_t tdomid; #define XS_STATE_DOMID_INVALID 0xffffU uint32_t evtchn; uint64_t mfn; #define XS_STATE_MFN_INVALID 0xffffffffffffffffUL } ring; int32_t socket_fd; } spec; uint32_t data_out_len; uint8_t data[]; };The issue is making sure that the mfn is properly aligned. If I can drop the levtchn then this getseasier.``` -where tx_id is the non-zero identifier values of an open transaction. - -### Protocol Extension +| Field | Description | +|------------|--------------------------------------------------| +| `domid` | The domain-id that owns the shared page | +| | | +| `tdomid` | The domain-id that `domid` acts on behalf of if | +| | it has been subject to an SET_TARGET | +| | operation [2] or DOMID_INVALID otherwise |DOMID_INVALID needs to be defined (or we need a reference where it is coming from).OK. It's in a public header... I'll reference it.+| | | +| `flags` | A bit-wise OR of: | +| | 0x0001: INTRODUCE has been issued |Just realized, I think we can drop those flags. Reasoning: if INTRODUCE hasn't been issued, there can't be an active connection to Xenstore for that domain, as Xenstore doesn't know about the parameters to connect (especially the event channel is missing).+| | 0x0002: RELEASE has been issued |And the same applies here: RELEASE will drop the connection to the domain, so it can't appear in a connection record.I think the presence of the RESUME command in xenstore.txt makes it non-obvious that we can forget about a domain once RELEASE has been called. The text there does say: "It is not clear whether this is possible since one would normally expect a domain not to be restarted after being shut down without being destroyed in the meantime. There are currently no users of this request in xen-unstable." So, perhaps this would be the time to remove RESUME from the spec? +1 Juergen
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |