[PATCH v5 08/11] tools/xenstored: Use priv_domid for manual nodes and permission

Jason Andryuk posted 11 patches 3 months ago
There is a newer version of this series
[PATCH v5 08/11] tools/xenstored: Use priv_domid for manual nodes and permission
Posted by Jason Andryuk 3 months ago
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.

If priv_domid is unset, set to dom0_domid to have a functional
xenstored.

Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
---
Held off R-b Juergen because of priv_domid setting

v5:
Add unset priv_domid setting
Additional change for continue check inside the loop
---
 tools/xenstored/core.c   |  4 ++--
 tools/xenstored/domain.c | 16 ++++++++--------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/tools/xenstored/core.c b/tools/xenstored/core.c
index dbf3548a8e..098958f611 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 5e53fe8736..94cbe81ca5 100644
--- a/tools/xenstored/domain.c
+++ b/tools/xenstored/domain.c
@@ -1014,7 +1014,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)
@@ -1037,13 +1037,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. */
@@ -1311,22 +1311,22 @@ void init_domains(void)
 		}
 	}
 
-	if (dom0_domid == DOMID_INVALID)
-		dom0_domid = priv_domid;
+	if (priv_domid == DOMID_INVALID)
+		priv_domid = dom0_domid;
 
-	if (dom0_domid == DOMID_INVALID)
+	if (priv_domid == DOMID_INVALID)
 		barf("Could not determine xenstore domid\n");
 
 	/*
 	 * Local domid must be first to setup structures for firing the special
 	 * watches.
 	 */
-	if (init_domain(dom0_domid))
+	if (init_domain(priv_domid))
 		introduce_count++;
 
 	for (unsigned int i = 0; i < nr_domids; i++) {
 		domid = domids[i];
-		if (domid == dom0_domid)
+		if (domid == priv_domid)
 			continue;
 
 		if (init_domain(domid))
-- 
2.50.1
Re: [PATCH v5 08/11] tools/xenstored: Use priv_domid for manual nodes and permission
Posted by Jürgen Groß 3 months ago
On 26.07.25 01:58, 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.
> 
> That leads to follow on changes to ensure that the priv_domid is created
> first.
> 
> If priv_domid is unset, set to dom0_domid to have a functional
> xenstored.
> 
> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
> ---
> Held off R-b Juergen because of priv_domid setting
> 
> v5:
> Add unset priv_domid setting
> Additional change for continue check inside the loop
> ---
>   tools/xenstored/core.c   |  4 ++--
>   tools/xenstored/domain.c | 16 ++++++++--------
>   2 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/tools/xenstored/core.c b/tools/xenstored/core.c
> index dbf3548a8e..098958f611 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 5e53fe8736..94cbe81ca5 100644
> --- a/tools/xenstored/domain.c
> +++ b/tools/xenstored/domain.c
> @@ -1014,7 +1014,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)
> @@ -1037,13 +1037,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. */
> @@ -1311,22 +1311,22 @@ void init_domains(void)
>   		}
>   	}
>   
> -	if (dom0_domid == DOMID_INVALID)
> -		dom0_domid = priv_domid;
> +	if (priv_domid == DOMID_INVALID)
> +		priv_domid = dom0_domid;
>   
> -	if (dom0_domid == DOMID_INVALID)
> +	if (priv_domid == DOMID_INVALID)
>   		barf("Could not determine xenstore domid\n");
>   
>   	/*
>   	 * Local domid must be first to setup structures for firing the special

Oh, just saw it now:

s/Local/Privileged/

With that My R-b: stands.


Juergen
Re: [PATCH v5 08/11] tools/xenstored: Use priv_domid for manual nodes and permission
Posted by Jürgen Groß 3 months ago
On 26.07.25 01:58, 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.
> 
> That leads to follow on changes to ensure that the priv_domid is created
> first.
> 
> If priv_domid is unset, set to dom0_domid to have a functional
> xenstored.
> 
> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>

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


Juergen