Hi Juergen,
On 03/05/2023 06:13, Juergen Gross wrote:
On 02.05.23 20:55, Julien Grall wrote:
Hi Juergen,
On 05/04/2023 08:03, Juergen Gross wrote:
Add the node accounting to the accounting information buffering in
order to avoid having to undo it in case of failure.
Signed-off-by: Juergen Gross <jgr...@suse.com>
---
tools/xenstore/xenstored_core.c | 21 ++-------------------
tools/xenstore/xenstored_domain.h | 4 ++--
2 files changed, 4 insertions(+), 21 deletions(-)
diff --git a/tools/xenstore/xenstored_core.c
b/tools/xenstore/xenstored_core.c
index 84335f5f3d..92a40ccf3f 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -1452,7 +1452,6 @@ static void destroy_node_rm(struct connection
*conn, struct node *node)
static int destroy_node(struct connection *conn, struct node *node)
{
destroy_node_rm(conn, node);
- domain_nbentry_dec(conn, get_node_owner(node));
/*
* It is not possible to easily revert the changes in a
transaction.
@@ -1797,27 +1796,11 @@ static int do_set_perms(const void *ctx,
struct connection *conn,
old_perms = node->perms;
domain_nbentry_dec(conn, get_node_owner(node));
IIRC, we originally said that domain_nbentry_dec() could never fail in
a non-transaction case. But with your current rework, the function can
now fail because of an allocation failure.
How would that be possible to happen?
domain_nbentry_dec() can only be called if a node is being owned by an
already
known domain. So allocation is impossible to happen, as this would be a
major
error in xenstored.
From my understanding, the nodes accounting will be temporary and then
committed at the end of the request.
So we would call acc_add_changed_dom() which may require allocation to
hold the temporary accounting.
Therefore, shouldn't we now check the error? (Possibly in a patch
beforehand).
I don't think so. I can add a comment if you want.
See above.
Cheers,
--
Julien Grall
