|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v4 4/4] tools/xenstore: add migration stream extensions for new features
Hi Juergen, On 05/09/2022 13:47, Juergen Gross wrote: I think it would be useful to outline in the commit message why the version had to be bumped. | | | | `flags` | 0 (LSB): Endianness: 0 = little, 1 = big | | | | @@ -77,7 +84,10 @@ NOTE: padding octets here and in all subsequent format specifications must be | | 0x00000003: WATCH_DATA | | | 0x00000004: TRANSACTION_DATA | | | 0x00000005: NODE_DATA | -| | 0x00000006 - 0xFFFFFFFF: reserved for future use | +| | 0x00000006: GLOBAL_QUOTA_DATA | +| | 0x00000007: DOMAIN_DATA | +| | 0x00000008: WATCH_DATA_EXTENDED (version 2 and up) | +| | 0x00000009 - 0xFFFFFFFF: reserved for future use | | | | | `len` | The length (in octets) of `body` | | | | @@ -129,6 +139,7 @@ xenstored state that needs to be restored. | `evtchn-fd` | The file descriptor used to communicate with | | | the event channel driver |+ Spurious change? xenstored will resume in the original process context. Hence `rw-socket-fd` simply specifies the file descriptor of the socket. Sockets are not always used, however, and so -1 will be used to denote an unused socket. @@ -241,9 +252,9 @@ the file descriptor of the socket connection.### WATCH_DATA -The image format will contain a `WATCH_DATA` record for each watch registered-by a connection for which there is `CONNECTION_DATA` record previously present. - +The image format will contain either a `WATCH_DATA` or a `WATCH_DATA_EXTENDED` +record for each watch registered by a connection for which there is +`CONNECTION_DATA` record previously present.``` I would add "by default" or something similar to make clear that the value in DOMAIN_DATA will override any quota set here. But see below about 'n-dom-quota' and 'n-glob-quota'. +| | | +| `n-glob-quota` | Number of quota values which apply globally | +| | only. | +| | | +| `quota-val` | Quota values, first the ones applying per | +| | domain, then the ones applying globally. A | +| | value of 0 has the semantics of "unlimited". | It is unclear to me why you need to make the distinction between "per domain" and "globally". IOW shouldn't be the name of the quota already indicates that? +| | | +| `quota-names` | 0 delimited strings of the quota names in | +| | the same sequence as the `quota-val` values. | + + +Allowed quota names are those explicitly named in [2] for the `GET_QUOTA` +and `SET_QUOTA` commands, plus implementation specific ones. Quota names not +recognized by the receiving side must be ignored. + +\pagebreak + +### DOMAIN_DATA + +This record is optional and can be present once for each domain. + + +``` + 0 1 2 3 octet ++-------+-------+-------+-------+ +| domain-id | n-quota | ++---------------+---------------+ +| features | ++-------------------------------+ +| quota-val 1 | ++-------------------------------+ +... ++-------------------------------+ +| quota-val N | ++-------------------------------+ +| quota-names +... +``` + + +| Field | Description | +|----------------|----------------------------------------------| +| `domain-id` | The domain-id of the domain this record | +| | belongs to. | +| | | +| `n-quota` | Number of quota values. | +| | | +| `features` | Value of the feature field visible by the | +| | guest at offset 2064 of the ring page. | +| | Aligned to the next 4 octet boundary. | Stale sentence? +| | Only valid for version 2 and later. | Can you mention explicitly whether the field will unknown or 0 for version 1? +| | | +| `quota-val` | Quota values, a value of 0 has the semantics | +| | "unlimited". | +| | | +| `quota-names` | 0 delimited strings of the quota names in | +| | the same sequence as the `quota-val` values. | + +Allowed quota names are those explicitly named in [2] for the `GET_QUOTA` +and `SET_QUOTA` commands, plus implementation specific ones. Quota names not +recognized by the receiving side must be ignored. + +\pagebreak + +### WATCH_DATA_EXTENDED NIT: I think it would be more logical if this is defined right next after WATCH_DATA. + +The image format will contain either a `WATCH_DATA` or a `WATCH_DATA_EXTENDED` +record for each watch registered by a connection for which there is +`CONNECTION_DATA` record previously present. The `WATCH_DATA_EXTENDED` record +type is valid only in version 2 and later. + +``` + 0 1 2 3 octet ++-------+-------+-------+-------+ +| conn-id | ++---------------+---------------+ +| wpath-len | token-len | ++---------------+---------------+ +| depth | | ++---------------+---------------+ It is not clear what would be the value of octet 2-3. Is it RES0 or UNKNOWN? +| wpath +... +| token +... +``` + + +| Field | Description | +|-------------|-------------------------------------------------| +| `conn-id` | The connection that issued the `WATCH` | +| | operation [2] | +| | | +| `wpath-len` | The length (in octets) of `wpath` including the | +| | NUL terminator | +| | | +| `token-len` | The length (in octets) of `token` including the | +| | NUL terminator | +| | | +| `depth` | The number of directory levels below the | +| | watched path to consider for a match. | +| | A value of 0xffff is used for unlimited depth. | +| | | +| `wpath` | The watch path, as specified in the `WATCH` | +| | operation | +| | | +| `token` | The watch identifier token, as specified in the | +| | `WATCH` operation | + +\pagebreak + + * * *[1] See https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=docs/designs/non-cooperative-migration.md Cheers, -- Julien Grall
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |