| Message ID | 20220905124746.1975-5-jgross@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tools/xenstore: add some new features to the documentation | expand |
Hi Juergen, On 05/09/2022 13:47, Juergen Gross wrote: > Extend the definition of the Xenstore migration stream to cover new > features: > > - per domain features > - extended watches (watch depth) > - per domain quota > > Signed-off-by: Juergen Gross <jgross@suse.com> > --- > V3: > - new patch > V4: > - add new record types instead of modifying the existing ones > (Julien Grall) > --- > docs/designs/xenstore-migration.md | 160 ++++++++++++++++++++++++++++- > 1 file changed, 155 insertions(+), 5 deletions(-) > > diff --git a/docs/designs/xenstore-migration.md b/docs/designs/xenstore-migration.md > index efa526f420..c70505c43a 100644 > --- a/docs/designs/xenstore-migration.md > +++ b/docs/designs/xenstore-migration.md > @@ -43,7 +43,14 @@ the setting of the endianness bit. > |-----------|---------------------------------------------------| > | `ident` | 0x78656e73746f7265 ('xenstore' in ASCII) | > | | | > -| `version` | 0x00000001 (the version of the specification) | > +| `version` | The version of the specification, defined values: | > +| | 0x00000001: all fields and records without any | > +| | explicitly mentioned version | > +| | dependency are valid. | > +| | 0x00000002: all fields and records valid for | > +| | version 1 plus fields and records | > +| | explicitly stated to be supported in | > +| | version 2 are valid. | 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. > > ``` > 0 1 2 3 octet > @@ -406,6 +417,145 @@ A node permission specifier has the following format: > Note that perm1 defines the domain owning the node. See [4] for more > explanation of node permissions. > > +\pagebreak > + > +### GLOBAL_QUOTA_DATA > + > +This record is only relevant for live update. It contains the global settings > +of xenstored quota. > + > +``` > + 0 1 2 3 octet > ++-------+-------+-------+-------+ > +| n-dom-quota | n-glob-quota | > ++---------------+---------------+ > +| quota-val 1 | > ++-------------------------------+ > +... > ++-------------------------------+ > +| quota-val N | > ++-------------------------------+ > +| quota-names > +... > +``` > + > + > +| Field | Description | > +|----------------|----------------------------------------------| > +| `n-dom-quota` | Number of quota values which apply per | > +| | domain. | 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,
On 06.09.22 19:27, Julien Grall wrote: > Hi Juergen, > > On 05/09/2022 13:47, Juergen Gross wrote: >> Extend the definition of the Xenstore migration stream to cover new >> features: >> >> - per domain features >> - extended watches (watch depth) >> - per domain quota >> >> Signed-off-by: Juergen Gross <jgross@suse.com> >> --- >> V3: >> - new patch >> V4: >> - add new record types instead of modifying the existing ones >> (Julien Grall) >> --- >> docs/designs/xenstore-migration.md | 160 ++++++++++++++++++++++++++++- >> 1 file changed, 155 insertions(+), 5 deletions(-) >> >> diff --git a/docs/designs/xenstore-migration.md >> b/docs/designs/xenstore-migration.md >> index efa526f420..c70505c43a 100644 >> --- a/docs/designs/xenstore-migration.md >> +++ b/docs/designs/xenstore-migration.md >> @@ -43,7 +43,14 @@ the setting of the endianness bit. >> |-----------|---------------------------------------------------| >> | `ident` | 0x78656e73746f7265 ('xenstore' in ASCII) | >> | | | >> -| `version` | 0x00000001 (the version of the specification) | >> +| `version` | The version of the specification, defined values: | >> +| | 0x00000001: all fields and records without any | >> +| | explicitly mentioned version | >> +| | dependency are valid. | >> +| | 0x00000002: all fields and records valid for | >> +| | version 1 plus fields and records | >> +| | explicitly stated to be supported in | >> +| | version 2 are valid. | > > I think it would be useful to outline in the commit message why the version had > to be bumped. Okay. > >> | | | >> | `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? Yes. > >> 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. >> ``` >> 0 1 2 3 octet >> @@ -406,6 +417,145 @@ A node permission specifier has the following format: >> Note that perm1 defines the domain owning the node. See [4] for more >> explanation of node permissions. >> +\pagebreak >> + >> +### GLOBAL_QUOTA_DATA >> + >> +This record is only relevant for live update. It contains the global settings >> +of xenstored quota. >> + >> +``` >> + 0 1 2 3 octet >> ++-------+-------+-------+-------+ >> +| n-dom-quota | n-glob-quota | >> ++---------------+---------------+ >> +| quota-val 1 | >> ++-------------------------------+ >> +... >> ++-------------------------------+ >> +| quota-val N | >> ++-------------------------------+ >> +| quota-names >> +... >> +``` >> + >> + >> +| Field | Description | >> +|----------------|----------------------------------------------| >> +| `n-dom-quota` | Number of quota values which apply per | >> +| | domain. | > > 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'. Okay. > >> +| | | >> +| `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? I think it could help in special cases. Imagine Xenstore A knows about global quota g and domain quota d, while Xenstore B doesn't know both. Initially I'm running Xenstore A on a host, then I'm live-updating to B. B gets the information that g is global, and d is per-domain, but has no other idea what to do with the values of g and d. OTOH it knows that each new domain should get d with the related value, so it can set d for each newly created domain. When B is either downgraded to A again, or a domain is migrated to another host running A, B can add the quota information of d for all domains. While this is nothing I'm planning to do in the near future, it might help e.g. in cases with mixed C-xenstored and O-xenstored setups. It doesn't cost really much, so I wanted to support this possibility in the migration stream from the beginning. > >> +| | | >> +| `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? Oh yes, a survivor of V3. > >> +| | Only valid for version 2 and later. | > > Can you mention explicitly whether the field will unknown or 0 for version 1? We have the general note "padding octets here and in all subsequent format specifications must be written as zero". I think this should suffice. > >> +| | | >> +| `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. I was following the record type numbering, but I can move this record description up if you like that better. > >> + >> +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? I don't understand the question. conn-id is a 4-byte item. Juergen
Hi Juergen, On 07/09/2022 07:28, Juergen Gross wrote: > On 06.09.22 19:27, Julien Grall wrote: >> Hi Juergen, >> >> On 05/09/2022 13:47, Juergen Gross wrote: >>> Extend the definition of the Xenstore migration stream to cover new >>> features: >>> >>> - per domain features >>> - extended watches (watch depth) >>> - per domain quota >>> >>> Signed-off-by: Juergen Gross <jgross@suse.com> >>> --- >>> V3: >>> - new patch >>> V4: >>> - add new record types instead of modifying the existing ones >>> (Julien Grall) >>> --- >>> docs/designs/xenstore-migration.md | 160 ++++++++++++++++++++++++++++- >>> 1 file changed, 155 insertions(+), 5 deletions(-) >>> >>> diff --git a/docs/designs/xenstore-migration.md >>> b/docs/designs/xenstore-migration.md >>> index efa526f420..c70505c43a 100644 >>> --- a/docs/designs/xenstore-migration.md >>> +++ b/docs/designs/xenstore-migration.md >>> @@ -43,7 +43,14 @@ the setting of the endianness bit. >>> |-----------|---------------------------------------------------| >>> | `ident` | 0x78656e73746f7265 ('xenstore' in ASCII) | >>> | | | >>> -| `version` | 0x00000001 (the version of the specification) | >>> +| `version` | The version of the specification, defined values: | >>> +| | 0x00000001: all fields and records without any | >>> +| | explicitly mentioned version | >>> +| | dependency are valid. | >>> +| | 0x00000002: all fields and records valid for | >>> +| | version 1 plus fields and records | >>> +| | explicitly stated to be supported in | >>> +| | version 2 are valid. | >> >> I think it would be useful to outline in the commit message why the >> version had to be bumped. > > Okay. > >> >>> | | | >>> | `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? > > Yes. > >> >>> 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. >>> ``` >>> 0 1 2 3 octet >>> @@ -406,6 +417,145 @@ A node permission specifier has the following >>> format: >>> Note that perm1 defines the domain owning the node. See [4] for more >>> explanation of node permissions. >>> +\pagebreak >>> + >>> +### GLOBAL_QUOTA_DATA >>> + >>> +This record is only relevant for live update. It contains the global >>> settings >>> +of xenstored quota. >>> + >>> +``` >>> + 0 1 2 3 octet >>> ++-------+-------+-------+-------+ >>> +| n-dom-quota | n-glob-quota | >>> ++---------------+---------------+ >>> +| quota-val 1 | >>> ++-------------------------------+ >>> +... >>> ++-------------------------------+ >>> +| quota-val N | >>> ++-------------------------------+ >>> +| quota-names >>> +... >>> +``` >>> + >>> + >>> +| Field | Description | >>> +|----------------|----------------------------------------------| >>> +| `n-dom-quota` | Number of quota values which apply per | >>> +| | domain. | >> >> 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'. > > Okay. > >> >>> +| | | >>> +| `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? > > I think it could help in special cases. > > Imagine Xenstore A knows about global quota g and domain quota d, while > Xenstore B doesn't know both. Initially I'm running Xenstore A on a > host, then I'm live-updating to B. > > B gets the information that g is global, and d is per-domain, but has no > other idea what to do with the values of g and d. OTOH it knows that each > new domain should get d with the related value, so it can set d for each > newly created domain. > > When B is either downgraded to A again, or a domain is migrated to another > host running A, B can add the quota information of d for all domains. > > While this is nothing I'm planning to do in the near future, it might help > e.g. in cases with mixed C-xenstored and O-xenstored setups. > > It doesn't cost really much, so I wanted to support this possibility in the > migration stream from the beginning. I can see the use-case. However... > >> >>> +| | | >>> +| `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. ... this is seem to directly conflict with this sentence as to me "ignore" means drop. What you want is for Xenstore to optionally preserve. Also, I think what you wrote above would be helpful in the commit message. It gives some insights for future reader on how the stream was designed. >>> + >>> +\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? > > Oh yes, a survivor of V3. > >> >>> +| | Only valid for version 2 and later. | >> >> Can you mention explicitly whether the field will unknown or 0 for >> version 1? > > We have the general note "padding octets here and in all subsequent format > specifications must be written as zero". I think this should suffice. I don't view this field as padding because it has a meaning. So I argue it is not cover by the sentence. Therefore I would add "Otherwise, the field is unknown/0" (pick the one you prefer between 0 and unknown). > >> >>> +| | | >>> +| `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. > > I was following the record type numbering, but I can move this record > description up if you like that better. > >> >>> + >>> +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? > > I don't understand the question. conn-id is a 4-byte item. I was referring to the blank after 'depth'. In other record, we use 'pad'. Cheers,
On 07.09.22 10:44, Julien Grall wrote: > Hi Juergen, > > On 07/09/2022 07:28, Juergen Gross wrote: >> On 06.09.22 19:27, Julien Grall wrote: >>> Hi Juergen, >>> >>> On 05/09/2022 13:47, Juergen Gross wrote: >>>> Extend the definition of the Xenstore migration stream to cover new >>>> features: >>>> >>>> - per domain features >>>> - extended watches (watch depth) >>>> - per domain quota >>>> >>>> Signed-off-by: Juergen Gross <jgross@suse.com> >>>> --- >>>> V3: >>>> - new patch >>>> V4: >>>> - add new record types instead of modifying the existing ones >>>> (Julien Grall) >>>> --- >>>> docs/designs/xenstore-migration.md | 160 ++++++++++++++++++++++++++++- >>>> 1 file changed, 155 insertions(+), 5 deletions(-) >>>> >>>> diff --git a/docs/designs/xenstore-migration.md >>>> b/docs/designs/xenstore-migration.md >>>> index efa526f420..c70505c43a 100644 >>>> --- a/docs/designs/xenstore-migration.md >>>> +++ b/docs/designs/xenstore-migration.md >>>> +| | | >>>> +| `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. > > ... this is seem to directly conflict with this sentence as to me "ignore" means > drop. What you want is for Xenstore to optionally preserve. I'll rephrase that. > > Also, I think what you wrote above would be helpful in the commit message. It > gives some insights for future reader on how the stream was designed. Okay. > >>>> + >>>> +\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? >> >> Oh yes, a survivor of V3. >> >>> >>>> +| | Only valid for version 2 and later. | >>> >>> Can you mention explicitly whether the field will unknown or 0 for version 1? >> >> We have the general note "padding octets here and in all subsequent format >> specifications must be written as zero". I think this should suffice. > > I don't view this field as padding because it has a meaning. So I argue it is > not cover by the sentence. I'll rephrase that to: "padding octets or fields not valid in the used version here and in all ..." > > Therefore I would add "Otherwise, the field is unknown/0" (pick the one you > prefer between 0 and unknown). > >> >>> >>>> +| | | >>>> +| `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. >> >> I was following the record type numbering, but I can move this record >> description up if you like that better. >> >>> >>>> + >>>> +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? >> >> I don't understand the question. conn-id is a 4-byte item. > > I was referring to the blank after 'depth'. In other record, we use 'pad'. Okay, will add "pad" to it. Juergen
diff --git a/docs/designs/xenstore-migration.md b/docs/designs/xenstore-migration.md index efa526f420..c70505c43a 100644 --- a/docs/designs/xenstore-migration.md +++ b/docs/designs/xenstore-migration.md @@ -43,7 +43,14 @@ the setting of the endianness bit. |-----------|---------------------------------------------------| | `ident` | 0x78656e73746f7265 ('xenstore' in ASCII) | | | | -| `version` | 0x00000001 (the version of the specification) | +| `version` | The version of the specification, defined values: | +| | 0x00000001: all fields and records without any | +| | explicitly mentioned version | +| | dependency are valid. | +| | 0x00000002: all fields and records valid for | +| | version 1 plus fields and records | +| | explicitly stated to be supported in | +| | version 2 are valid. | | | | | `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 | + 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. ``` 0 1 2 3 octet @@ -406,6 +417,145 @@ A node permission specifier has the following format: Note that perm1 defines the domain owning the node. See [4] for more explanation of node permissions. +\pagebreak + +### GLOBAL_QUOTA_DATA + +This record is only relevant for live update. It contains the global settings +of xenstored quota. + +``` + 0 1 2 3 octet ++-------+-------+-------+-------+ +| n-dom-quota | n-glob-quota | ++---------------+---------------+ +| quota-val 1 | ++-------------------------------+ +... ++-------------------------------+ +| quota-val N | ++-------------------------------+ +| quota-names +... +``` + + +| Field | Description | +|----------------|----------------------------------------------| +| `n-dom-quota` | Number of quota values which apply per | +| | domain. | +| | | +| `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". | +| | | +| `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. | +| | Only valid for version 2 and later. | +| | | +| `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 + +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 | | ++---------------+---------------+ +| 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
Extend the definition of the Xenstore migration stream to cover new features: - per domain features - extended watches (watch depth) - per domain quota Signed-off-by: Juergen Gross <jgross@suse.com> --- V3: - new patch V4: - add new record types instead of modifying the existing ones (Julien Grall) --- docs/designs/xenstore-migration.md | 160 ++++++++++++++++++++++++++++- 1 file changed, 155 insertions(+), 5 deletions(-)