Re: [PATCH 04/11] tools/xenstore: rename do_tdb_delete() and change parameter type

[Julien Grall <julien@xxxxxxx>]

Hi Juergen,

On 30/05/2023 10:13, Juergen Gross wrote:
> diff --git a/tools/xenstore/xenstored_core.h b/tools/xenstore/xenstored_core.h
> index f7cb035f26..7fc6d73e5a 100644
> --- a/tools/xenstore/xenstored_core.h
> +++ b/tools/xenstore/xenstored_core.h
> @@ -358,11 +358,12 @@ extern xengnttab_handle **xgt_handle;
>   
>   int remember_string(struct hashtable *hash, const char *str);
>   
> +/* Data base access functions. */
>   void set_tdb_key(const char *name, TDB_DATA *key);
>   int do_tdb_write(struct connection *conn, TDB_DATA *key, TDB_DATA *data,
>                  struct node_account_data *acc, int flag, bool no_quota_check);
> -int do_tdb_delete(struct connection *conn, TDB_DATA *key,
> -                 struct node_account_data *acc);
> +int db_delete(struct connection *conn, const char *name,
> +             struct node_account_data *acc);
>   
>   void conn_free_buffered_data(struct connection *conn);
>   
> diff --git a/tools/xenstore/xenstored_transaction.c b/tools/xenstore/xenstored_transaction.c
> index 1646c07040..bf173f3d1d 100644
> --- a/tools/xenstore/xenstored_transaction.c
> +++ b/tools/xenstore/xenstored_transaction.c
> @@ -385,8 +385,7 @@ static int finalize_transaction(struct connection *conn,
>                 /* Entries for unmodified nodes can be removed early. */
>                 if (!i->modified) {
>                         if (i->ta_node) {
> -                               set_tdb_key(i->trans_name, &ta_key);
> -                               if (do_tdb_delete(conn, &ta_key, NULL))
> +                               if (db_delete(conn, i->trans_name, NULL))
>                                         return EIO;
>                         }
>                         list_del(&i->list);
> @@ -395,21 +394,21 @@ static int finalize_transaction(struct connection *conn,
>         }
>   
>   	while ((i = list_top(&trans->accessed, struct accessed_node, list))) {
> -               set_tdb_key(i->node, &key);
It is not clear to me why you are moving later the call to set_tdb_key() 
in this patch.
>                 if (i->ta_node) {
>                         set_tdb_key(i->trans_name, &ta_key);
>                         data = tdb_fetch(tdb_ctx, ta_key);
>                         if (data.dptr) {
> -                               trace_tdb("read %s size %zu\n", ta_key.dptr,
> +                               trace_tdb("read %s size %zu\n", i->trans_name,
This change seems unrelated to this patch.

>                                           ta_key.dsize + data.dsize);
>                                 hdr = (void *)data.dptr;
>                                 hdr->generation = ++generation;
>                                 flag = (i->generation == NO_GENERATION)
>                                        ? NODE_CREATE : NODE_MODIFY;
> +                               set_tdb_key(i->node, &key);
>                                 *is_corrupt |= do_tdb_write(conn, &key, &data,
>                                                             NULL, flag, true);
>                                 talloc_free(data.dptr);
> -                               if (do_tdb_delete(conn, &ta_key, NULL))
> +                               if (db_delete(conn, i->trans_name, NULL))
>                                         *is_corrupt = true;
>                         } else {
>                                 *is_corrupt = true;
> @@ -422,7 +421,7 @@ static int finalize_transaction(struct connection *conn,
>                          */
>                         *is_corrupt |= (i->generation == NO_GENERATION)
>                                        ? false
> -                                      : do_tdb_delete(conn, &key, NULL);
> +                                      : db_delete(conn, i->node, NULL);
>                 }
>                 if (i->fire_watch)
>                         fire_watches(conn, trans, i->node, NULL, i->watch_exact,
> @@ -439,15 +438,12 @@ static int destroy_transaction(void *_transaction)
>   {
>         struct transaction *trans = _transaction;
>         struct accessed_node *i;
> -       TDB_DATA key;
>   
>   	wrl_ntransactions--;
>         trace_destroy(trans, "transaction");
>         while ((i = list_top(&trans->accessed, struct accessed_node, list))) {
> -               if (i->ta_node) {
> -                       set_tdb_key(i->trans_name, &key);
> -                       do_tdb_delete(trans->conn, &key, NULL);
> -               }
> +               if (i->ta_node)
> +                       db_delete(trans->conn, i->trans_name, NULL);
>                 list_del(&i->list);
>                 talloc_free(i);
>         }
Cheers,

--
Julien Grall