Usually, priv_domid == dom0_domid == 0, and that is what is expected.
If we rename s/dom0_domid/store_domid/, it seems more likely we want to
actually have the priv_domid as the owner.

That leads to follow on changes to ensure that the priv_domid is created
first.

Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
---
Will this blow up if priv_domid doesn't exist?

Maybe it would be better to just create these as store_domid.
---
 tools/xenstored/core.c   |  4 ++--
 tools/xenstored/domain.c | 15 ++++++++-------
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/tools/xenstored/core.c b/tools/xenstored/core.c
index c2f8d20211..19edd75bc3 100644
--- a/tools/xenstored/core.c
+++ b/tools/xenstored/core.c
@@ -2266,7 +2266,7 @@ struct connection *get_connection_by_id(unsigned int conn_id)
 static void manual_node(const char *name, const char *child)
 {
 	struct node *node;
-	struct xs_permissions perms = { .id = dom0_domid,
+	struct xs_permissions perms = { .id = priv_domid,
 					.perms = XS_PERM_NONE };
 
 	node = talloc_zero(NULL, struct node);
@@ -2317,7 +2317,7 @@ void setup_structure(bool live_update)
 		manual_node("/tool/xenstored", NULL);
 		manual_node("@releaseDomain", NULL);
 		manual_node("@introduceDomain", NULL);
-		domain_nbentry_fix(dom0_domid, 5, true);
+		domain_nbentry_fix(priv_domid, 5, true);
 	}
 }
 
diff --git a/tools/xenstored/domain.c b/tools/xenstored/domain.c
index ac4ac72f99..dbeacaa93e 100644
--- a/tools/xenstored/domain.c
+++ b/tools/xenstored/domain.c
@@ -1009,7 +1009,7 @@ static struct domain *introduce_domain(const void *ctx,
 	struct domain *domain;
 	int rc;
 	struct xenstore_domain_interface *interface;
-	bool is_master_domain = (domid == xenbus_master_domid());
+	bool is_priv_domain = (domid == priv_domid);
 
 	domain = find_or_alloc_domain(ctx, domid);
 	if (!domain)
@@ -1032,13 +1032,13 @@ static struct domain *introduce_domain(const void *ctx,
 		}
 		domain->interface = interface;
 
-		if (is_master_domain)
+		if (is_priv_domain)
 			setup_structure(restore);
 
 		/* Now domain belongs to its connection. */
 		talloc_steal(domain->conn, domain);
 
-		if (!is_master_domain && !restore)
+		if (!is_priv_domain && !restore)
 			fire_special_watches("@introduceDomain");
 	} else {
 		/* Use XS_INTRODUCE for recreating the xenbus event-channel. */
@@ -1295,17 +1295,18 @@ void init_domains(void)
 		domids[nr_domids - 1] = domid;
 
 		if (caps & XENMANAGE_GETDOMSTATE_CAP_XENSTORE) {
+			dom0_domid = domid;
+		}
+
+		if (caps & XENMANAGE_GETDOMSTATE_CAP_CONTROL) {
 			memmove(&domids[1], domids, (nr_domids - 1) * sizeof(*domids));
 			/*
 			 * Local domid must be first to setup structures for
 			 * firing the special watches.
 			 */
 			domids[0] = domid;
-			dom0_domid = domid;
-		}
-
-		if (caps & XENMANAGE_GETDOMSTATE_CAP_CONTROL)
 			priv_domid = domid;
+		}
 	}
 
 	for (unsigned int i = 0; i < nr_domids; i++) {
-- 
2.50.1

On 25.07.25 04:28, Jason Andryuk wrote:
> Usually, priv_domid == dom0_domid == 0, and that is what is expected.
> If we rename s/dom0_domid/store_domid/, it seems more likely we want to
> actually have the priv_domid as the owner.

Yes, I agree.

> 
> That leads to follow on changes to ensure that the priv_domid is created
> first.
> 
> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
> ---
> Will this blow up if priv_domid doesn't exist?

That won't be a problem. The problematic case will be when priv_domid is
never changed due to no doamin having the CONTROL cap, and some random
other domU happens to have domid 0.

So maybe priv_domid should be initialized statically to e.g. DOMID_IDLE,
as with your init_domains() loop a "normal" dom0 will always have the
CONTROL cap and thus will result in priv_domid being set.

Same applies probably to store_domid, but that should be set to priv_domid
after init_domains() in case no domain had the XENSTORE cap.

If both aren't detected it should be fine to bail out early.

> 
> Maybe it would be better to just create these as store_domid.

No, reasoning see above. And xenstore-stubdom accesses to Xenstore nodes
via the "normal" interfaces shouldn't need any special privileges.

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen

On 2025-07-25 03:24, Jürgen Groß wrote:
> On 25.07.25 04:28, Jason Andryuk wrote:
>> Usually, priv_domid == dom0_domid == 0, and that is what is expected.
>> If we rename s/dom0_domid/store_domid/, it seems more likely we want to
>> actually have the priv_domid as the owner.
> 
> Yes, I agree.
> 
>>
>> That leads to follow on changes to ensure that the priv_domid is created
>> first.
>>
>> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
>> ---
>> Will this blow up if priv_domid doesn't exist?
> 
> That won't be a problem. The problematic case will be when priv_domid is
> never changed due to no doamin having the CONTROL cap, and some random
> other domU happens to have domid 0.
> 
> So maybe priv_domid should be initialized statically to e.g. DOMID_IDLE,
> as with your init_domains() loop a "normal" dom0 will always have the
> CONTROL cap and thus will result in priv_domid being set.

There is existing use of DOMID_INVALID, so I'll use that

> Same applies probably to store_domid, but that should be set to priv_domid
> after init_domains() in case no domain had the XENSTORE cap.
> 
> If both aren't detected it should be fine to bail out early.

For our use cases, we want the possibility to run xenstored without a 
control domain.  In that case, I think it would make sense to set 
priv_domid = store_domid to get xenstored to run.

>>
>> Maybe it would be better to just create these as store_domid.
> 
> No, reasoning see above. And xenstore-stubdom accesses to Xenstore nodes
> via the "normal" interfaces shouldn't need any special privileges.
> 
> Reviewed-by: Juergen Gross <jgross@suse.com>

Thanks, but I held off applying with the priv_domid = store_domid 
assignment.  (Well dom0_domid since it's before the rename).

Regards,
Jason