diff mbox series

[v2,09/13] tools/xenstore: add TDB access trace support

Message ID 20230120100028.11142-10-jgross@suse.com (mailing list archive)
State Superseded
Headers show
Series tools/xenstore: rework internal accounting | expand

Commit Message

Jürgen Groß Jan. 20, 2023, 10 a.m. UTC 
Add a new trace switch "tdb" and the related trace calls.

The "tdb" switch is off per default.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/xenstore/xenstored_core.c        | 8 +++++++-
 tools/xenstore/xenstored_core.h        | 6 ++++++
 tools/xenstore/xenstored_transaction.c | 7 ++++++-
 3 files changed, 19 insertions(+), 2 deletions(-)

Comments

Julien Grall Feb. 20, 2023, 10:59 p.m. UTC | #1 
Hi,

On 20/01/2023 10:00, Juergen Gross wrote:
> Add a new trace switch "tdb" and the related trace calls.
> 
> The "tdb" switch is off per default.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

With one remark (see below):

Reviewed-by: Julien Grall <jgrall@amazon.com>

> ---
>   tools/xenstore/xenstored_core.c        | 8 +++++++-
>   tools/xenstore/xenstored_core.h        | 6 ++++++
>   tools/xenstore/xenstored_transaction.c | 7 ++++++-
>   3 files changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
> index 558ef491b1..49e196e7ae 100644
> --- a/tools/xenstore/xenstored_core.c
> +++ b/tools/xenstore/xenstored_core.c
> @@ -589,6 +589,8 @@ static void get_acc_data(TDB_DATA *key, struct node_account_data *acc)
>   		if (old_data.dptr == NULL) {
>   			acc->memory = 0;
>   		} else {
> +			trace_tdb("read %s size %zu\n", key->dptr,
> +				  old_data.dsize + key->dsize);
>   			hdr = (void *)old_data.dptr;
>   			acc->memory = old_data.dsize;
>   			acc->domid = hdr->perms[0].id;
> @@ -655,6 +657,7 @@ int do_tdb_write(struct connection *conn, TDB_DATA *key, TDB_DATA *data,
>   		errno = EIO;
>   		return errno;
>   	}
> +	trace_tdb("store %s size %zu\n", key->dptr, data->dsize + key->dsize);
>   
>   	if (acc) {
>   		/* Don't use new_domid, as it might be a transaction node. */
> @@ -682,6 +685,7 @@ int do_tdb_delete(struct connection *conn, TDB_DATA *key,
>   		errno = EIO;
>   		return errno;
>   	}
> +	trace_tdb("delete %s\n", key->dptr);
>   
>   	if (acc->memory) {
>   		domid = get_acc_domid(conn, key, acc->domid);
> @@ -731,6 +735,8 @@ struct node *read_node(struct connection *conn, const void *ctx,
>   		goto error;
>   	}
>   
> +	trace_tdb("read %s size %zu\n", key.dptr, data.dsize + key.dsize);
> +
>   	node->parent = NULL;
>   	talloc_steal(node, data.dptr);
>   
> @@ -2746,7 +2752,7 @@ static void set_quota(const char *arg, bool soft)
>   
>   /* Sorted by bit values of TRACE_* flags. Flag is (1u << index). */
>   const char *const trace_switches[] = {
> -	"obj", "io", "wrl", "acc",
> +	"obj", "io", "wrl", "acc", "tdb",
>   	NULL
>   };
>   
> diff --git a/tools/xenstore/xenstored_core.h b/tools/xenstore/xenstored_core.h
> index 3e0734a6c6..419a144396 100644
> --- a/tools/xenstore/xenstored_core.h
> +++ b/tools/xenstore/xenstored_core.h
> @@ -303,8 +303,14 @@ extern unsigned int trace_flags;
>   #define TRACE_IO	0x00000002
>   #define TRACE_WRL	0x00000004
>   #define TRACE_ACC	0x00000008
> +#define TRACE_TDB	0x00000010
>   extern const char *const trace_switches[];
>   int set_trace_switch(const char *arg);

Add a newline here.

> +#define trace_tdb(...)				\
> +do {						\
> +	if (trace_flags & TRACE_TDB)		\
> +		trace("tdb: " __VA_ARGS__);	\
> +} while (0)
>   
>   extern TDB_CONTEXT *tdb_ctx;
>   extern int dom0_domid;
> diff --git a/tools/xenstore/xenstored_transaction.c b/tools/xenstore/xenstored_transaction.c
> index 1aa9d3cb3d..19a1175d1b 100644
> --- a/tools/xenstore/xenstored_transaction.c
> +++ b/tools/xenstore/xenstored_transaction.c
> @@ -366,8 +366,11 @@ static int finalize_transaction(struct connection *conn,
>   				if (tdb_error(tdb_ctx) != TDB_ERR_NOEXIST)
>   					return EIO;
>   				gen = NO_GENERATION;
> -			} else
> +			} else {
> +				trace_tdb("read %s size %zu\n", key.dptr,
> +					  key.dsize + data.dsize);
>   				gen = hdr->generation;
> +			}
>   			talloc_free(data.dptr);
>   			if (i->generation != gen)
>   				return EAGAIN;
> @@ -391,6 +394,8 @@ static int finalize_transaction(struct connection *conn,
>   			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,
> +					  ta_key.dsize + data.dsize);
>   				hdr = (void *)data.dptr;
>   				hdr->generation = ++generation;
>   				*is_corrupt |= do_tdb_write(conn, &key, &data,

Cheers,
Jürgen Groß Feb. 21, 2023, 8:41 a.m. UTC | #2 
On 20.02.23 23:59, Julien Grall wrote:
> Hi,
> 
> On 20/01/2023 10:00, Juergen Gross wrote:
>> Add a new trace switch "tdb" and the related trace calls.
>>
>> The "tdb" switch is off per default.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
> 
> With one remark (see below):
> 
> Reviewed-by: Julien Grall <jgrall@amazon.com>
> 
>> ---
>>   tools/xenstore/xenstored_core.c        | 8 +++++++-
>>   tools/xenstore/xenstored_core.h        | 6 ++++++
>>   tools/xenstore/xenstored_transaction.c | 7 ++++++-
>>   3 files changed, 19 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
>> index 558ef491b1..49e196e7ae 100644
>> --- a/tools/xenstore/xenstored_core.c
>> +++ b/tools/xenstore/xenstored_core.c
>> @@ -589,6 +589,8 @@ static void get_acc_data(TDB_DATA *key, struct 
>> node_account_data *acc)
>>           if (old_data.dptr == NULL) {
>>               acc->memory = 0;
>>           } else {
>> +            trace_tdb("read %s size %zu\n", key->dptr,
>> +                  old_data.dsize + key->dsize);
>>               hdr = (void *)old_data.dptr;
>>               acc->memory = old_data.dsize;
>>               acc->domid = hdr->perms[0].id;
>> @@ -655,6 +657,7 @@ int do_tdb_write(struct connection *conn, TDB_DATA *key, 
>> TDB_DATA *data,
>>           errno = EIO;
>>           return errno;
>>       }
>> +    trace_tdb("store %s size %zu\n", key->dptr, data->dsize + key->dsize);
>>       if (acc) {
>>           /* Don't use new_domid, as it might be a transaction node. */
>> @@ -682,6 +685,7 @@ int do_tdb_delete(struct connection *conn, TDB_DATA *key,
>>           errno = EIO;
>>           return errno;
>>       }
>> +    trace_tdb("delete %s\n", key->dptr);
>>       if (acc->memory) {
>>           domid = get_acc_domid(conn, key, acc->domid);
>> @@ -731,6 +735,8 @@ struct node *read_node(struct connection *conn, const void 
>> *ctx,
>>           goto error;
>>       }
>> +    trace_tdb("read %s size %zu\n", key.dptr, data.dsize + key.dsize);
>> +
>>       node->parent = NULL;
>>       talloc_steal(node, data.dptr);
>> @@ -2746,7 +2752,7 @@ static void set_quota(const char *arg, bool soft)
>>   /* Sorted by bit values of TRACE_* flags. Flag is (1u << index). */
>>   const char *const trace_switches[] = {
>> -    "obj", "io", "wrl", "acc",
>> +    "obj", "io", "wrl", "acc", "tdb",
>>       NULL
>>   };
>> diff --git a/tools/xenstore/xenstored_core.h b/tools/xenstore/xenstored_core.h
>> index 3e0734a6c6..419a144396 100644
>> --- a/tools/xenstore/xenstored_core.h
>> +++ b/tools/xenstore/xenstored_core.h
>> @@ -303,8 +303,14 @@ extern unsigned int trace_flags;
>>   #define TRACE_IO    0x00000002
>>   #define TRACE_WRL    0x00000004
>>   #define TRACE_ACC    0x00000008
>> +#define TRACE_TDB    0x00000010
>>   extern const char *const trace_switches[];
>>   int set_trace_switch(const char *arg);
> 
> Add a newline here.

Okay.


Juergen
diff mbox series

Patch

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 558ef491b1..49e196e7ae 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -589,6 +589,8 @@  static void get_acc_data(TDB_DATA *key, struct node_account_data *acc)
 		if (old_data.dptr == NULL) {
 			acc->memory = 0;
 		} else {
+			trace_tdb("read %s size %zu\n", key->dptr,
+				  old_data.dsize + key->dsize);
 			hdr = (void *)old_data.dptr;
 			acc->memory = old_data.dsize;
 			acc->domid = hdr->perms[0].id;
@@ -655,6 +657,7 @@  int do_tdb_write(struct connection *conn, TDB_DATA *key, TDB_DATA *data,
 		errno = EIO;
 		return errno;
 	}
+	trace_tdb("store %s size %zu\n", key->dptr, data->dsize + key->dsize);
 
 	if (acc) {
 		/* Don't use new_domid, as it might be a transaction node. */
@@ -682,6 +685,7 @@  int do_tdb_delete(struct connection *conn, TDB_DATA *key,
 		errno = EIO;
 		return errno;
 	}
+	trace_tdb("delete %s\n", key->dptr);
 
 	if (acc->memory) {
 		domid = get_acc_domid(conn, key, acc->domid);
@@ -731,6 +735,8 @@  struct node *read_node(struct connection *conn, const void *ctx,
 		goto error;
 	}
 
+	trace_tdb("read %s size %zu\n", key.dptr, data.dsize + key.dsize);
+
 	node->parent = NULL;
 	talloc_steal(node, data.dptr);
 
@@ -2746,7 +2752,7 @@  static void set_quota(const char *arg, bool soft)
 
 /* Sorted by bit values of TRACE_* flags. Flag is (1u << index). */
 const char *const trace_switches[] = {
-	"obj", "io", "wrl", "acc",
+	"obj", "io", "wrl", "acc", "tdb",
 	NULL
 };
 
diff --git a/tools/xenstore/xenstored_core.h b/tools/xenstore/xenstored_core.h
index 3e0734a6c6..419a144396 100644
--- a/tools/xenstore/xenstored_core.h
+++ b/tools/xenstore/xenstored_core.h
@@ -303,8 +303,14 @@  extern unsigned int trace_flags;
 #define TRACE_IO	0x00000002
 #define TRACE_WRL	0x00000004
 #define TRACE_ACC	0x00000008
+#define TRACE_TDB	0x00000010
 extern const char *const trace_switches[];
 int set_trace_switch(const char *arg);
+#define trace_tdb(...)				\
+do {						\
+	if (trace_flags & TRACE_TDB)		\
+		trace("tdb: " __VA_ARGS__);	\
+} while (0)
 
 extern TDB_CONTEXT *tdb_ctx;
 extern int dom0_domid;
diff --git a/tools/xenstore/xenstored_transaction.c b/tools/xenstore/xenstored_transaction.c
index 1aa9d3cb3d..19a1175d1b 100644
--- a/tools/xenstore/xenstored_transaction.c
+++ b/tools/xenstore/xenstored_transaction.c
@@ -366,8 +366,11 @@  static int finalize_transaction(struct connection *conn,
 				if (tdb_error(tdb_ctx) != TDB_ERR_NOEXIST)
 					return EIO;
 				gen = NO_GENERATION;
-			} else
+			} else {
+				trace_tdb("read %s size %zu\n", key.dptr,
+					  key.dsize + data.dsize);
 				gen = hdr->generation;
+			}
 			talloc_free(data.dptr);
 			if (i->generation != gen)
 				return EAGAIN;
@@ -391,6 +394,8 @@  static int finalize_transaction(struct connection *conn,
 			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,
+					  ta_key.dsize + data.dsize);
 				hdr = (void *)data.dptr;
 				hdr->generation = ++generation;
 				*is_corrupt |= do_tdb_write(conn, &key, &data,