[PATCH 0/4] trace-cmd: Fix up kvm time synchronization

[PATCH 0/4] trace-cmd: Fix up kvm time synchronization

[Steven Rostedt]

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

Fix the kvm time synchronization so that it works with the agent proxy. That
includes using "stat()" instead of just reading the content of the file to
see if they exist. Have the kvm time sync functions test for a valid VM
instead of just seeing if there are no failures, as no VMs or VCPUs would
pass when it should not. Set the proper role for the agent proxy.

Finally, mount the debugfs file system if it is not already mounted,
otherwise it will say that KVM time synchronization is not supported when it
actually is.

Steven Rostedt (Google) (4):
  trace-cmd kvm timesync: Use stat() in kvm_scaling_check_vm_cpu()
  trace-cmd kvm timesync: Check for one valid VM
  trace-cmd record: Set the proper role when connected to a proxy
  trace-cmd: Mount debugfs if needed for KVM data

 Makefile                           |  2 +-
 lib/trace-cmd/trace-timesync-kvm.c | 99 ++++++++++++++++++++++--------
 tracecmd/trace-record.c            |  6 +-
 3 files changed, 80 insertions(+), 27 deletions(-)

-- 
2.35.1

[PATCH 1/4] trace-cmd kvm timesync: Use stat() in kvm_scaling_check_vm_cpu()

[Steven Rostedt]

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

kvm_scaling_check_vm_cpu() uses read_ll_from_file() to determine if the
kvm scaling and fraction files exist. The read_ll_from_file() does not
return if the contents of the files are legit or not. No need to read
them in this file. Just use stat().

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 lib/trace-cmd/trace-timesync-kvm.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/trace-cmd/trace-timesync-kvm.c b/lib/trace-cmd/trace-timesync-kvm.c
index 12a22d4c4d6a..a645fa2cc70b 100644
--- a/lib/trace-cmd/trace-timesync-kvm.c
+++ b/lib/trace-cmd/trace-timesync-kvm.c
@@ -75,22 +75,22 @@ static int read_ll_from_file(char *file, long long *res)
 
 static bool kvm_scaling_check_vm_cpu(char *vname, char *cpu)
 {
-	long long scaling, frac;
 	bool has_scaling = false;
 	bool has_frac = false;
+	struct stat st;
 	char *path;
 	int ret;
 
 	if (asprintf(&path, "%s/%s/%s", vname, cpu, KVM_DEBUG_SCALING_FILE) < 0)
 		return false;
-	ret = read_ll_from_file(path, &scaling);
+	ret = stat(path, &st);
 	free(path);
 	if (!ret)
 		has_scaling = true;
 
 	if (asprintf(&path, "%s/%s/%s", vname, cpu, KVM_DEBUG_FRACTION_FILE) < 0)
 		return false;
-	ret = read_ll_from_file(path, &frac);
+	ret = stat(path, &st);
 	free(path);
 	if (!ret)
 		has_frac = true;
-- 
2.35.1

[PATCH 2/4] trace-cmd kvm timesync: Check for one valid VM

[Steven Rostedt]

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

Currently if there are no valid VMs the kvm supported check can return
true. Check for at least one valid VM, and that echa VM has one valid
VCPU.

Also comment the code as it is very confusing to why the scaling checks
return true or false.

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 lib/trace-cmd/trace-timesync-kvm.c | 46 +++++++++++++++++++++++-------
 1 file changed, 36 insertions(+), 10 deletions(-)

diff --git a/lib/trace-cmd/trace-timesync-kvm.c b/lib/trace-cmd/trace-timesync-kvm.c
index a645fa2cc70b..671eafaf62b8 100644
--- a/lib/trace-cmd/trace-timesync-kvm.c
+++ b/lib/trace-cmd/trace-timesync-kvm.c
@@ -73,6 +73,11 @@ static int read_ll_from_file(char *file, long long *res)
 	return 0;
 }
 
+/*
+ * Returns true if both scaling and fraction exist or both do
+ * not exist. false if one exists without the other or if there
+ * is a memory error.
+ */
 static bool kvm_scaling_check_vm_cpu(char *vname, char *cpu)
 {
 	bool has_scaling = false;
@@ -101,46 +106,66 @@ static bool kvm_scaling_check_vm_cpu(char *vname, char *cpu)
 	return true;
 }
 
+/*
+ * Returns true if a VCPU exists with a tsc-offset file and that
+ * the scaling files for ratio and fraction both exist or both
+ * do not exist. False if there is no VM with a tsc-offset or
+ * there is only one of the two scaling files, or there's a
+ * memory issue.
+ */
 static bool kvm_scaling_check_vm(char *name)
 {
 	struct dirent *entry;
 	char *vdir;
 	DIR *dir;
+	bool valid = false;
 
 	if (asprintf(&vdir, "%s/%s", KVM_DEBUG_FS, name) < 0)
-		return true;
+		return false;
 
 	dir = opendir(vdir);
 	if (!dir) {
 		free(vdir);
-		return true;
+		return false;
 	}
 	while ((entry = readdir(dir))) {
-		if (entry->d_type == DT_DIR && !strncmp(entry->d_name, "vcpu", 4) &&
-		    !kvm_scaling_check_vm_cpu(vdir, entry->d_name))
-			break;
+		if (entry->d_type == DT_DIR && !strncmp(entry->d_name, "vcpu", 4)) {
+			if (!kvm_scaling_check_vm_cpu(vdir, entry->d_name))
+				break;
+			valid = true;
+		}
 	}
 
 	closedir(dir);
 	free(vdir);
-	return entry == NULL;
+	return valid && entry == NULL;
 }
+
+/*
+ * Returns true if all VMs have a tsc-offset file and that
+ * the scaling files for ratio and fraction both exist or both
+ * do not exist. False if a VM with a tsc-offset or there is only
+ * one of the two scaling files, or no VM exists or there's a memory issue.
+ */
 static bool kvm_scaling_check(void)
 {
 	struct dirent *entry;
 	DIR *dir;
+	bool valid = false;
 
 	dir = opendir(KVM_DEBUG_FS);
 	if (!dir)
 		return true;
 
 	while ((entry = readdir(dir))) {
-		if (entry->d_type == DT_DIR && isdigit(entry->d_name[0]) &&
-		    !kvm_scaling_check_vm(entry->d_name))
-			break;
+		if (entry->d_type == DT_DIR && isdigit(entry->d_name[0])) {
+			if (!kvm_scaling_check_vm(entry->d_name))
+				break;
+			valid = true;
+		}
 	}
 	closedir(dir);
-	return entry == NULL;
+	return valid && entry == NULL;
 }
 
 static bool kvm_support_check(bool guest)
@@ -148,6 +173,7 @@ static bool kvm_support_check(bool guest)
 	struct stat st;
 	int ret;
 
+	/* The kvm files are only in the host so we can ignore guests */
 	if (guest)
 		return true;
 
-- 
2.35.1

[PATCH 3/4] trace-cmd record: Set the proper role when connected to a proxy

[Steven Rostedt]

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

When a guest uses trace-cmd record to connect to the agent proxy on the
host, it needs to set its role to "GUEST" when initializing the time
synchronization protocols.

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 tracecmd/trace-record.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index b43f3272dfc6..ce20402af7c2 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -3905,7 +3905,11 @@ static void connect_to_agent(struct common_record_context *ctx,
 			die("Failed to connect to host %s:%u",
 			    instance->name, instance->port);
 	} else {
-		role = TRACECMD_TIME_SYNC_ROLE_HOST;
+		/* If connecting to a proxy, then this is the guest */
+		if (is_proxy(ctx->instance))
+			role = TRACECMD_TIME_SYNC_ROLE_GUEST;
+		else
+			role = TRACECMD_TIME_SYNC_ROLE_HOST;
 		sd = trace_vsock_open(instance->cid, instance->port);
 		if (sd < 0)
 			die("Failed to connect to vsocket @%u:%u",
-- 
2.35.1

[PATCH 4/4] trace-cmd: Mount debugfs if needed for KVM data

[Steven Rostedt]

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

The KVM offset and multiplier are in the debugfs file system. If it is
not currently mounted, then try to mount it with the new tracefs API
tracefs_debug_dir() which will return the debugfs file system path, and
even mount it if possible (or NULL if it could not).

This also removes the hard coded path for /sys/kernel/debug/kvm and
removes the stat() of the directory as tracefs_debug_dir() will only
return mount locations which are directories.

Set libtracefs minimum version to 1.4 as that is what will have the
tarcefs_debug_dir() in it.

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 Makefile                           |  2 +-
 lib/trace-cmd/trace-timesync-kvm.c | 47 ++++++++++++++++++++++--------
 2 files changed, 36 insertions(+), 13 deletions(-)

diff --git a/Makefile b/Makefile
index abc4ac723db4..3452649229df 100644
--- a/Makefile
+++ b/Makefile
@@ -23,7 +23,7 @@ export LIBTRACECMD_VERSION
 VERSION_FILE = ltc_version.h
 
 LIBTRACEEVENT_MIN_VERSION = 1.5
-LIBTRACEFS_MIN_VERSION = 1.3
+LIBTRACEFS_MIN_VERSION = 1.4
 
 MAKEFLAGS += --no-print-directory
 
diff --git a/lib/trace-cmd/trace-timesync-kvm.c b/lib/trace-cmd/trace-timesync-kvm.c
index 671eafaf62b8..1db63d94f545 100644
--- a/lib/trace-cmd/trace-timesync-kvm.c
+++ b/lib/trace-cmd/trace-timesync-kvm.c
@@ -16,7 +16,6 @@
 #include "tracefs.h"
 #include "trace-tsync-local.h"
 
-#define KVM_DEBUG_FS "/sys/kernel/debug/kvm"
 #define KVM_DEBUG_OFFSET_FILE	"tsc-offset"
 #define KVM_DEBUG_SCALING_FILE	"tsc-scaling-ratio"
 #define KVM_DEBUG_FRACTION_FILE	"tsc-scaling-ratio-frac-bits"
@@ -106,6 +105,24 @@ static bool kvm_scaling_check_vm_cpu(char *vname, char *cpu)
 	return true;
 }
 
+static const char *kvm_debug_dir(void)
+{
+	const char *debugfs;
+	static char *kvm_dir;
+
+	if (kvm_dir)
+		return kvm_dir;
+
+	debugfs = tracefs_debug_dir();
+	if (!debugfs)
+		return NULL;
+
+	if (asprintf(&kvm_dir, "%s/kvm", debugfs) < 0)
+		return NULL;
+
+	return kvm_dir;
+}
+
 /*
  * Returns true if a VCPU exists with a tsc-offset file and that
  * the scaling files for ratio and fraction both exist or both
@@ -116,11 +133,16 @@ static bool kvm_scaling_check_vm_cpu(char *vname, char *cpu)
 static bool kvm_scaling_check_vm(char *name)
 {
 	struct dirent *entry;
+	const char *kvm;
 	char *vdir;
 	DIR *dir;
 	bool valid = false;
 
-	if (asprintf(&vdir, "%s/%s", KVM_DEBUG_FS, name) < 0)
+	kvm = kvm_debug_dir();
+	if (!kvm)
+		return false;
+
+	if (asprintf(&vdir, "%s/%s", kvm, name) < 0)
 		return false;
 
 	dir = opendir(vdir);
@@ -150,10 +172,15 @@ static bool kvm_scaling_check_vm(char *name)
 static bool kvm_scaling_check(void)
 {
 	struct dirent *entry;
+	const char *kvm;
 	DIR *dir;
 	bool valid = false;
 
-	dir = opendir(KVM_DEBUG_FS);
+	kvm = kvm_debug_dir();
+	if (!kvm)
+		return false;
+
+	dir = opendir(kvm);
 	if (!dir)
 		return true;
 
@@ -170,18 +197,14 @@ static bool kvm_scaling_check(void)
 
 static bool kvm_support_check(bool guest)
 {
-	struct stat st;
-	int ret;
+	const char *kvm;
 
 	/* The kvm files are only in the host so we can ignore guests */
 	if (guest)
 		return true;
 
-	ret = stat(KVM_DEBUG_FS, &st);
-	if (ret < 0)
-		return false;
-
-	if (!S_ISDIR(st.st_mode))
+	kvm = kvm_debug_dir();
+	if (!kvm)
 		return false;
 
 	return kvm_scaling_check();
@@ -242,7 +265,7 @@ static int kvm_open_debug_files(struct kvm_clock_sync *kvm, int pid)
 	DIR *dir;
 	int i;
 
-	dir = opendir(KVM_DEBUG_FS);
+	dir = opendir(kvm_debug_dir());
 	if (!dir)
 		goto error;
 	if (asprintf(&pid_str, "%d-", pid) <= 0)
@@ -251,7 +274,7 @@ static int kvm_open_debug_files(struct kvm_clock_sync *kvm, int pid)
 		if (!(entry->d_type == DT_DIR &&
 		    !strncmp(entry->d_name, pid_str, strlen(pid_str))))
 			continue;
-		asprintf(&vm_dir_str, "%s/%s", KVM_DEBUG_FS, entry->d_name);
+		asprintf(&vm_dir_str, "%s/%s", kvm_debug_dir(), entry->d_name);
 		break;
 	}
 	closedir(dir);
-- 
2.35.1