Hi Juergen,
I haven't yet look at the code itself. I wanted to comment on the
external interfaces.
On 01/11/2022 15:28, Juergen Gross wrote:
Make the xenstored internal trace configurable by adding classes
which can be switched on and off independently from each other.
Define the following classes:
- obj: Creation and deletion of interesting "objects" (watch,
transaction, connection)
- io: incoming requests and outgoing responses
- wrl: write limiting
Per default "obj" and "io" are switched on.
Entries written via trace() will always be printed (if tracing is on
at all).
Rename the misnamed xenstore-control commands "logfile" to "tracefile"
and "log" to "trace".
While I understand this may be misnamed, this also means that there is
an ABI breakage between current Xenstored and the future one.
I am not convinced this is justified. Therefore, I think we should at
minimum keep the compatibility.
Add the capability to control the trace settings via the "trace"
command and via a new "--trace-control" command line option.
Add a missing trace_create() call for creating a transaction.
Signed-off-by: Juergen Gross <jgr...@suse.com>
---
docs/misc/xenstore.txt | 18 +++++++----
tools/xenstore/xenstored_control.c | 41 +++++++++++++++++++-----
tools/xenstore/xenstored_core.c | 44 +++++++++++++++++++++++---
tools/xenstore/xenstored_core.h | 6 ++++
tools/xenstore/xenstored_domain.c | 27 ++++++++--------
tools/xenstore/xenstored_transaction.c | 1 +
6 files changed, 105 insertions(+), 32 deletions(-)
diff --git a/docs/misc/xenstore.txt b/docs/misc/xenstore.txt
index 44428ae3a7..9db0385120 100644
--- a/docs/misc/xenstore.txt
+++ b/docs/misc/xenstore.txt
@@ -409,14 +409,8 @@ CONTROL <command>|[<parameters>|]
error string in case of failure. -s can return "BUSY" in case
of an active transaction, a retry of -s can be done in that
case.
- log|on
- turn xenstore logging on
- log|off
- turn xenstore logging off
- logfile|<file-name>
- log to specified file
Technically xenstore.txt is meant to describe an interface that is
implementation agnostics. Can you outline in the documentation why
removing them is fine?
memreport|[<file-name>]
- print memory statistics to logfile (no <file-name>
+ print memory statistics to tracefile (no <file-name>
specified) or to specific file
print|<string>
print <string> to syslog (xenstore runs as daemon) or
@@ -432,6 +426,16 @@ CONTROL <command>|[<parameters>|]
the domain <domid>
quota-soft|[set <name> <val>]
like the "quota" command, but for soft-quota.
+ trace|[+|-<switch>]
The regex here is a bit ambiguous because it technically means either
"+" or "-<switch>". Furthermore, within this docs, there are case where
| is included in between [] to indicate the this is terminated by a \0.
So it would be better to define it over 3 lines:
trace
trace|+<switch>
trace|-<switch>
I think it would be fine if there is only one paragraph of explanation
for the 3.
+ without parameters: show possible trace switches
+ +<switch> activates trace entries for <switch>,
+ -<switch> deactivates trace entries for <switch>
+ trace|on
+ turn xenstore tracing on
+ trace|off
+ turn xenstore tracing off
+ tracefile|<file-name>
+ trace to specified file
help <supported-commands>
return list of supported commands for CONTROL
Cheers,
--
Julien Grall
