Re: [PATCH v2 01/18] tools/xenstore: explicitly specify create or modify for tdb_store()

[Julien Grall <julien@xxxxxxx>]

Hi Juergen,

On 10/07/2023 07:59, Juergen Gross wrote:
> Instead of using TDB_REPLACE for either creating or modifying a TDB
> entry, use either TDB_INSERT or TDB_MODIFY when calling tdb_store().
>
> At higher function levels use the abstract flag values NODE_CREATE
Were you meant to replace 'flag' with 'mode'?

[...]

> diff --git a/tools/xenstore/xenstored_transaction.c 
> b/tools/xenstore/xenstored_transaction.c
> index 334f1609f1..0655073de7 100644
> --- a/tools/xenstore/xenstored_transaction.c
> +++ b/tools/xenstore/xenstored_transaction.c
> @@ -290,7 +290,8 @@ int access_node(struct connection *conn, struct node *node,
>                         i->check_gen = true;
>                         if (node->generation != NO_GENERATION) {
>                                 set_tdb_key(i->trans_name, &local_key);
> -                               ret = write_node_raw(conn, &local_key, node, 
> true);
> +                               ret = write_node_raw(conn, &local_key, node,
> +                                                    NODE_CREATE, true);
>                                 if (ret)
>                                         goto err;
>                                 i->ta_node = true;
> @@ -363,6 +364,7 @@ static int finalize_transaction(struct connection *conn,
>         TDB_DATA key, ta_key, data;
>         struct xs_tdb_record_hdr *hdr;
>         uint64_t gen;
> +       int flag;
I think this should be 'enum write_node_mode mode'. Also, can't the 
variable be defined a bit below in the if ( ... )?
>   
>   	list_for_each_entry_safe(i, n, &trans->accessed, list) {
>                 if (i->check_gen) {
> @@ -405,8 +407,10 @@ static int finalize_transaction(struct connection *conn,
>                                           ta_key.dsize + data.dsize);
>                                 hdr = (void *)data.dptr;
>                                 hdr->generation = ++generation;
> +                               flag = (i->generation == NO_GENERATION)
> +                                      ? NODE_CREATE : NODE_MODIFY;
>                                 *is_corrupt |= do_tdb_write(conn, &key, &data,
> -                                                           NULL, true);
> +                                                           NULL, flag, true);
>                                 talloc_free(data.dptr);
>                                 if (do_tdb_delete(conn, &ta_key, NULL))
>                                         *is_corrupt = true;
Cheers,

--
Julien Grall