[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 2/4] tools/xenstore: add documentation for new set/get-feature commands



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@xxxxxxxx>
---
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,

--
Julien Grall



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.