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.

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.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature

Reply via email to