| Message ID | 20220527072427.20327-3-jgross@suse.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tools/xenstore: add some new features to the documentation | expand |
Hi Juergen, On 27/05/2022 08:24, Juergen Gross wrote: > Add documentation for two new Xenstore wire commands SET_FEATURE and > GET_FEATURE used to set or query the Xenstore features visible in the > ring page of a given domain. > > Signed-off-by: Juergen Gross <jgross@suse.com> > --- > Do we need support in the migration protocol for the features? I would say yes. You want to make sure that the client can be migrated without loosing features between two xenstored. > V2: > - remove feature bit (Julien Grall) > - GET_FEATURE without domid will return Xenstore supported features > (triggered by Julien Grall) > --- > docs/misc/xenstore.txt | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/docs/misc/xenstore.txt b/docs/misc/xenstore.txt > index a3d3da0a5b..00f6969202 100644 > --- a/docs/misc/xenstore.txt > +++ b/docs/misc/xenstore.txt > @@ -331,6 +331,20 @@ SET_TARGET <domid>|<tdomid>| > > xenstored prevents the use of SET_TARGET other than by dom0. > > +GET_FEATURE [<domid>|] <value>| > +SET_FEATURE <domid>|<value>| > + Returns or sets the contents of the "feature" field located at > + offset 2064 of the Xenstore ring page of the domain specified by > + <domid>. <value> is a decimal number being a logical or of the In the context of migration, I am stilll a bit concerned that the features are stored in the ring because the guest could overwrite it. I would expect the migration code to check that the GET_FEATURE <domid> is a subset of GET_FEATURE on the targeted Xenstored. So it can easily prevent a guest to migrate. So I think this should be a shadow copy that will be returned instead of the contents of the "feature" field. > + feature bits as defined in docs/misc/xenstore-ring.txt. Trying > + to set a bit for a feature not being supported by the running > + Xenstore will be denied. Providing no <domid> with the > + GET_FEATURE command will return the features which are supported > + by Xenstore. Do we want to allow modifying the features when the guest is running? > + > + xenstored prevents the use of GET_FEATURE and SET_FEATURE other > + than by dom0. Cheers,
On 23.06.22 20:27, Julien Grall wrote: > Hi Juergen, > > On 27/05/2022 08:24, Juergen Gross wrote: >> Add documentation for two new Xenstore wire commands SET_FEATURE and >> GET_FEATURE used to set or query the Xenstore features visible in the >> ring page of a given domain. >> >> Signed-off-by: Juergen Gross <jgross@suse.com> >> --- >> Do we need support in the migration protocol for the features? > > I would say yes. You want to make sure that the client can be migrated without > loosing features between two xenstored. Okay, will add that in V3. > >> V2: >> - remove feature bit (Julien Grall) >> - GET_FEATURE without domid will return Xenstore supported features >> (triggered by Julien Grall) >> --- >> docs/misc/xenstore.txt | 14 ++++++++++++++ >> 1 file changed, 14 insertions(+) >> >> diff --git a/docs/misc/xenstore.txt b/docs/misc/xenstore.txt >> index a3d3da0a5b..00f6969202 100644 >> --- a/docs/misc/xenstore.txt >> +++ b/docs/misc/xenstore.txt >> @@ -331,6 +331,20 @@ SET_TARGET <domid>|<tdomid>| >> xenstored prevents the use of SET_TARGET other than by dom0. >> +GET_FEATURE [<domid>|] <value>| >> +SET_FEATURE <domid>|<value>| >> + Returns or sets the contents of the "feature" field located at >> + offset 2064 of the Xenstore ring page of the domain specified by >> + <domid>. <value> is a decimal number being a logical or of the > > In the context of migration, I am stilll a bit concerned that the features are > stored in the ring because the guest could overwrite it. > > I would expect the migration code to check that the GET_FEATURE <domid> is a > subset of GET_FEATURE on the targeted Xenstored. So it can easily prevent a > guest to migrate. > > So I think this should be a shadow copy that will be returned instead of the > contents of the "feature" field. Of course. The value in the ring is meant only for the guest. Xenstore will have the authoritative value in its private memory. > >> + feature bits as defined in docs/misc/xenstore-ring.txt. Trying >> + to set a bit for a feature not being supported by the running >> + Xenstore will be denied. Providing no <domid> with the >> + GET_FEATURE command will return the features which are supported >> + by Xenstore. > > Do we want to allow modifying the features when the guest is running? I think we can't remove features, but adding would be fine. For simplicity it might be better to just deny a modification while the guest is running. Juergen
Hi Juergen, On 24/06/2022 05:13, Juergen Gross wrote: > On 23.06.22 20:27, Julien Grall wrote: >> On 27/05/2022 08:24, Juergen Gross wrote: >>> Add documentation for two new Xenstore wire commands SET_FEATURE and >>> GET_FEATURE used to set or query the Xenstore features visible in the >>> ring page of a given domain. >>> >>> Signed-off-by: Juergen Gross <jgross@suse.com> >>> --- >>> Do we need support in the migration protocol for the features? >> >> I would say yes. You want to make sure that the client can be migrated >> without loosing features between two xenstored. >> >>> V2: >>> - remove feature bit (Julien Grall) >>> - GET_FEATURE without domid will return Xenstore supported features >>> (triggered by Julien Grall) >>> --- >>> docs/misc/xenstore.txt | 14 ++++++++++++++ >>> 1 file changed, 14 insertions(+) >>> >>> diff --git a/docs/misc/xenstore.txt b/docs/misc/xenstore.txt >>> index a3d3da0a5b..00f6969202 100644 >>> --- a/docs/misc/xenstore.txt >>> +++ b/docs/misc/xenstore.txt >>> @@ -331,6 +331,20 @@ SET_TARGET <domid>|<tdomid>| >>> xenstored prevents the use of SET_TARGET other than by dom0. >>> +GET_FEATURE [<domid>|] <value>| >>> +SET_FEATURE <domid>|<value>| >>> + Returns or sets the contents of the "feature" field located at >>> + offset 2064 of the Xenstore ring page of the domain specified by >>> + <domid>. <value> is a decimal number being a logical or of the >> >> In the context of migration, I am stilll a bit concerned that the >> features are stored in the ring because the guest could overwrite it. >> >> I would expect the migration code to check that the GET_FEATURE >> <domid> is a subset of GET_FEATURE on the targeted Xenstored. So it >> can easily prevent a guest to migrate. >> >> So I think this should be a shadow copy that will be returned instead >> of the contents of the "feature" field. > > Of course. The value in the ring is meant only for the guest. Xenstore > will have the authoritative value in its private memory. Good. I would suggest to clarify it in xenstore.txt because it suggests otherwise so far. >>> + feature bits as defined in docs/misc/xenstore-ring.txt. Trying >>> + to set a bit for a feature not being supported by the running >>> + Xenstore will be denied. Providing no <domid> with the >>> + GET_FEATURE command will return the features which are supported >>> + by Xenstore. >> >> Do we want to allow modifying the features when the guest is running? > > I think we can't remove features, but adding would be fine. For Agree with removing features, regarding adding I think we would need a mechanism to tell the client there is a new feature. It would require some work, so... > simplicity it might be better to just deny a modification while the > guest is running. ... I agree this is probably best for a first shot. This can be relaxed afterwards. Cheers,
diff --git a/docs/misc/xenstore.txt b/docs/misc/xenstore.txt index a3d3da0a5b..00f6969202 100644 --- a/docs/misc/xenstore.txt +++ b/docs/misc/xenstore.txt @@ -331,6 +331,20 @@ SET_TARGET <domid>|<tdomid>| xenstored prevents the use of SET_TARGET other than by dom0. +GET_FEATURE [<domid>|] <value>| +SET_FEATURE <domid>|<value>| + Returns or sets the contents of the "feature" field located at + offset 2064 of the Xenstore ring page of the domain specified by + <domid>. <value> is a decimal number being a logical or of the + feature bits as defined in docs/misc/xenstore-ring.txt. Trying + to set a bit for a feature not being supported by the running + Xenstore will be denied. Providing no <domid> with the + GET_FEATURE command will return the features which are supported + by Xenstore. + + xenstored prevents the use of GET_FEATURE and SET_FEATURE other + than by dom0. + ---------- Miscellaneous ---------- CONTROL <command>|[<parameters>|]
Add documentation for two new Xenstore wire commands SET_FEATURE and GET_FEATURE used to set or query the Xenstore features visible in the ring page of a given domain. Signed-off-by: Juergen Gross <jgross@suse.com> --- Do we need support in the migration protocol for the features? V2: - remove feature bit (Julien Grall) - GET_FEATURE without domid will return Xenstore supported features (triggered by Julien Grall) --- docs/misc/xenstore.txt | 14 ++++++++++++++ 1 file changed, 14 insertions(+)