| Message ID | 20230530091333.7678-4-jgross@suse.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tools/xenstore: drop TDB | expand |
Hi Juergen, On 30/05/2023 10:13, Juergen Gross wrote: > Instead of setting the TDB key for accessing the node in the data base, > let transaction_prepend() return the associated name instead. > > This is in preparation to replace TDB with a more simple data storage. > > Signed-off-by: Juergen Gross <jgross@suse.com> > --- > tools/xenstore/xenstored_core.c | 4 +++- > tools/xenstore/xenstored_transaction.c | 11 ++++------- > tools/xenstore/xenstored_transaction.h | 3 +-- > 3 files changed, 8 insertions(+), 10 deletions(-) > > diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c > index a1d5d4a419..239f8c6bc4 100644 > --- a/tools/xenstore/xenstored_core.c > +++ b/tools/xenstore/xenstored_core.c > @@ -694,6 +694,7 @@ struct node *read_node(struct connection *conn, const void *ctx, > TDB_DATA key, data; > struct xs_tdb_record_hdr *hdr; > struct node *node; > + const char *db_name; NIT: At the moment the local variable seems pointless given... > int err; > > node = talloc(ctx, struct node); > @@ -708,7 +709,8 @@ struct node *read_node(struct connection *conn, const void *ctx, > return NULL; > } > > - transaction_prepend(conn, name, &key); > + db_name = transaction_prepend(conn, name); > + set_tdb_key(db_name, &key); ... you only use in one place. Will this change in follow-up patch? Anyway: Reviewed-by: Julien Grall <jgrall@amazon.com> Cheers,
On 20.06.23 13:19, Julien Grall wrote: > Hi Juergen, > > On 30/05/2023 10:13, Juergen Gross wrote: >> Instead of setting the TDB key for accessing the node in the data base, >> let transaction_prepend() return the associated name instead. >> >> This is in preparation to replace TDB with a more simple data storage. >> >> Signed-off-by: Juergen Gross <jgross@suse.com> >> --- >> tools/xenstore/xenstored_core.c | 4 +++- >> tools/xenstore/xenstored_transaction.c | 11 ++++------- >> tools/xenstore/xenstored_transaction.h | 3 +-- >> 3 files changed, 8 insertions(+), 10 deletions(-) >> >> diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c >> index a1d5d4a419..239f8c6bc4 100644 >> --- a/tools/xenstore/xenstored_core.c >> +++ b/tools/xenstore/xenstored_core.c >> @@ -694,6 +694,7 @@ struct node *read_node(struct connection *conn, const void >> *ctx, >> TDB_DATA key, data; >> struct xs_tdb_record_hdr *hdr; >> struct node *node; >> + const char *db_name; > > NIT: At the moment the local variable seems pointless given... > >> int err; >> node = talloc(ctx, struct node); >> @@ -708,7 +709,8 @@ struct node *read_node(struct connection *conn, const void >> *ctx, >> return NULL; >> } >> - transaction_prepend(conn, name, &key); >> + db_name = transaction_prepend(conn, name); >> + set_tdb_key(db_name, &key); > > ... you only use in one place. Will this change in follow-up patch? Yes. The additional use case will be added in patch 7. > > Anyway: > > Reviewed-by: Julien Grall <jgrall@amazon.com> Thanks, Juergen
diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c index a1d5d4a419..239f8c6bc4 100644 --- a/tools/xenstore/xenstored_core.c +++ b/tools/xenstore/xenstored_core.c @@ -694,6 +694,7 @@ struct node *read_node(struct connection *conn, const void *ctx, TDB_DATA key, data; struct xs_tdb_record_hdr *hdr; struct node *node; + const char *db_name; int err; node = talloc(ctx, struct node); @@ -708,7 +709,8 @@ struct node *read_node(struct connection *conn, const void *ctx, return NULL; } - transaction_prepend(conn, name, &key); + db_name = transaction_prepend(conn, name); + set_tdb_key(db_name, &key); data = tdb_fetch(tdb_ctx, key); diff --git a/tools/xenstore/xenstored_transaction.c b/tools/xenstore/xenstored_transaction.c index 9dab0cd165..1646c07040 100644 --- a/tools/xenstore/xenstored_transaction.c +++ b/tools/xenstore/xenstored_transaction.c @@ -196,20 +196,17 @@ static char *transaction_get_node_name(void *ctx, struct transaction *trans, * Prepend the transaction to name if node has been modified in the current * transaction. */ -void transaction_prepend(struct connection *conn, const char *name, - TDB_DATA *key) +const char *transaction_prepend(struct connection *conn, const char *name) { struct accessed_node *i; if (conn && conn->transaction) { i = find_accessed_node(conn->transaction, name); - if (i) { - set_tdb_key(i->trans_name, key); - return; - } + if (i) + return i->trans_name; } - set_tdb_key(name, key); + return name; } /* diff --git a/tools/xenstore/xenstored_transaction.h b/tools/xenstore/xenstored_transaction.h index f6a2e2f7f5..b196b1ab07 100644 --- a/tools/xenstore/xenstored_transaction.h +++ b/tools/xenstore/xenstored_transaction.h @@ -47,8 +47,7 @@ int __must_check access_node(struct connection *conn, struct node *node, void queue_watches(struct connection *conn, const char *name, bool watch_exact); /* Prepend the transaction to name if appropriate. */ -void transaction_prepend(struct connection *conn, const char *name, - TDB_DATA *key); +const char *transaction_prepend(struct connection *conn, const char *name); /* Mark the transaction as failed. This will prevent it to be committed. */ void fail_transaction(struct transaction *trans);
Instead of setting the TDB key for accessing the node in the data base, let transaction_prepend() return the associated name instead. This is in preparation to replace TDB with a more simple data storage. Signed-off-by: Juergen Gross <jgross@suse.com> --- tools/xenstore/xenstored_core.c | 4 +++- tools/xenstore/xenstored_transaction.c | 11 ++++------- tools/xenstore/xenstored_transaction.h | 3 +-- 3 files changed, 8 insertions(+), 10 deletions(-)