diff mbox

[v2,1/1] KVM: make pid available for uevents without debugfs

Message ID 1500896403-25663-1-git-send-email-imbrenda@linux.vnet.ibm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Claudio Imbrenda July 24, 2017, 11:40 a.m. UTC 
Simplify and improve the code so that the PID is always available in
the uevent even when debugfs is not available.

This adds a userspace_pid field to struct kvm, as per Radim's
suggestion, so that the PID can be retrieved on destruction too.

Acked-by: Janosch Frank <frankja@linux.vnet.ibm.com>
Fixes: 286de8f6ac9202 ("KVM: trigger uevents when creating or destroying a VM")
Signed-off-by: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>
---
 include/linux/kvm_host.h |  1 +
 virt/kvm/kvm_main.c      | 35 ++++++++++++-----------------------
 2 files changed, 13 insertions(+), 23 deletions(-)

Comments

Paolo Bonzini July 24, 2017, 2:30 p.m. UTC | #1 
On 24/07/2017 13:40, Claudio Imbrenda wrote:
> Simplify and improve the code so that the PID is always available in
> the uevent even when debugfs is not available.
> 
> This adds a userspace_pid field to struct kvm, as per Radim's
> suggestion, so that the PID can be retrieved on destruction too.
> 
> Acked-by: Janosch Frank <frankja@linux.vnet.ibm.com>
> Fixes: 286de8f6ac9202 ("KVM: trigger uevents when creating or destroying a VM")
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>
> ---
>  include/linux/kvm_host.h |  1 +
>  virt/kvm/kvm_main.c      | 35 ++++++++++++-----------------------
>  2 files changed, 13 insertions(+), 23 deletions(-)
> 
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 648b34c..890b706 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -445,6 +445,7 @@ struct kvm {
>  	struct kvm_stat_data **debugfs_stat_data;
>  	struct srcu_struct srcu;
>  	struct srcu_struct irq_srcu;
> +	pid_t userspace_pid;
>  };
>  
>  #define kvm_err(fmt, ...) \
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 82987d4..f3f7427 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -3883,7 +3883,6 @@ static const struct file_operations *stat_fops[] = {
>  static void kvm_uevent_notify_change(unsigned int type, struct kvm *kvm)
>  {
>  	struct kobj_uevent_env *env;
> -	char *tmp, *pathbuf = NULL;
>  	unsigned long long created, active;
>  
>  	if (!kvm_dev.this_device || !kvm)
> @@ -3907,38 +3906,28 @@ static void kvm_uevent_notify_change(unsigned int type, struct kvm *kvm)
>  	add_uevent_var(env, "CREATED=%llu", created);
>  	add_uevent_var(env, "COUNT=%llu", active);
>  
> -	if (type == KVM_EVENT_CREATE_VM)
> +	if (type == KVM_EVENT_CREATE_VM) {
>  		add_uevent_var(env, "EVENT=create");
> -	else if (type == KVM_EVENT_DESTROY_VM)
> +		kvm->userspace_pid = task_pid_nr(current);
> +	} else if (type == KVM_EVENT_DESTROY_VM) {
>  		add_uevent_var(env, "EVENT=destroy");
> +	}
> +	add_uevent_var(env, "PID=%d", kvm->userspace_pid);
>  
>  	if (kvm->debugfs_dentry) {
> -		char p[ITOA_MAX_LEN];
> -
> -		snprintf(p, sizeof(p), "%s", kvm->debugfs_dentry->d_name.name);
> -		tmp = strchrnul(p + 1, '-');
> -		*tmp = '\0';
> -		add_uevent_var(env, "PID=%s", p);
> -		pathbuf = kmalloc(PATH_MAX, GFP_KERNEL);
> -		if (pathbuf) {
> -			/* sizeof counts the final '\0' */
> -			int len = sizeof("STATS_PATH=") - 1;
> -			const char *pvar = "STATS_PATH=";
> -
> -			tmp = dentry_path_raw(kvm->debugfs_dentry,
> -					      pathbuf + len,
> -					      PATH_MAX - len);
> -			if (!IS_ERR(tmp)) {
> -				memcpy(tmp - len, pvar, len);
> -				env->envp[env->envp_idx++] = tmp - len;
> -			}
> +		char *tmp, *p = kmalloc(PATH_MAX, GFP_KERNEL);
> +
> +		if (p) {
> +			tmp = dentry_path_raw(kvm->debugfs_dentry, p, PATH_MAX);
> +			if (!IS_ERR(tmp))
> +				add_uevent_var(env, "STATS_PATH=%s", tmp);
> +			kfree(p);
>  		}
>  	}
>  	/* no need for checks, since we are adding at most only 5 keys */
>  	env->envp[env->envp_idx++] = NULL;
>  	kobject_uevent_env(&kvm_dev.this_device->kobj, KOBJ_CHANGE, env->envp);
>  	kfree(env);
> -	kfree(pathbuf);
>  }
>  
>  static int kvm_init_debug(void)
> 

Looks good, thanks.

Paolo
David Hildenbrand July 24, 2017, 6:03 p.m. UTC | #2 
On 24.07.2017 13:40, Claudio Imbrenda wrote:
> Simplify and improve the code so that the PID is always available in
> the uevent even when debugfs is not available.
> 
> This adds a userspace_pid field to struct kvm, as per Radim's
> suggestion, so that the PID can be retrieved on destruction too.
> 
> Acked-by: Janosch Frank <frankja@linux.vnet.ibm.com>
> Fixes: 286de8f6ac9202 ("KVM: trigger uevents when creating or destroying a VM")
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>
> ---
>  include/linux/kvm_host.h |  1 +
>  virt/kvm/kvm_main.c      | 35 ++++++++++++-----------------------
>  2 files changed, 13 insertions(+), 23 deletions(-)
> 
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 648b34c..890b706 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -445,6 +445,7 @@ struct kvm {
>  	struct kvm_stat_data **debugfs_stat_data;
>  	struct srcu_struct srcu;
>  	struct srcu_struct irq_srcu;
> +	pid_t userspace_pid;
>  };
>  
>  #define kvm_err(fmt, ...) \
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 82987d4..f3f7427 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -3883,7 +3883,6 @@ static const struct file_operations *stat_fops[] = {
>  static void kvm_uevent_notify_change(unsigned int type, struct kvm *kvm)
>  {
>  	struct kobj_uevent_env *env;
> -	char *tmp, *pathbuf = NULL;
>  	unsigned long long created, active;
>  
>  	if (!kvm_dev.this_device || !kvm)
> @@ -3907,38 +3906,28 @@ static void kvm_uevent_notify_change(unsigned int type, struct kvm *kvm)
>  	add_uevent_var(env, "CREATED=%llu", created);
>  	add_uevent_var(env, "COUNT=%llu", active);
>  
> -	if (type == KVM_EVENT_CREATE_VM)
> +	if (type == KVM_EVENT_CREATE_VM) {
>  		add_uevent_var(env, "EVENT=create");
> -	else if (type == KVM_EVENT_DESTROY_VM)
> +		kvm->userspace_pid = task_pid_nr(current);
> +	} else if (type == KVM_EVENT_DESTROY_VM) {
>  		add_uevent_var(env, "EVENT=destroy");
> +	}
> +	add_uevent_var(env, "PID=%d", kvm->userspace_pid);
>  
>  	if (kvm->debugfs_dentry) {
> -		char p[ITOA_MAX_LEN];
> -
> -		snprintf(p, sizeof(p), "%s", kvm->debugfs_dentry->d_name.name);
> -		tmp = strchrnul(p + 1, '-');
> -		*tmp = '\0';
> -		add_uevent_var(env, "PID=%s", p);
> -		pathbuf = kmalloc(PATH_MAX, GFP_KERNEL);
> -		if (pathbuf) {
> -			/* sizeof counts the final '\0' */
> -			int len = sizeof("STATS_PATH=") - 1;
> -			const char *pvar = "STATS_PATH=";
> -
> -			tmp = dentry_path_raw(kvm->debugfs_dentry,
> -					      pathbuf + len,
> -					      PATH_MAX - len);
> -			if (!IS_ERR(tmp)) {
> -				memcpy(tmp - len, pvar, len);
> -				env->envp[env->envp_idx++] = tmp - len;
> -			}
> +		char *tmp, *p = kmalloc(PATH_MAX, GFP_KERNEL);
> +
> +		if (p) {
> +			tmp = dentry_path_raw(kvm->debugfs_dentry, p, PATH_MAX);
> +			if (!IS_ERR(tmp))
> +				add_uevent_var(env, "STATS_PATH=%s", tmp);
> +			kfree(p);
>  		}
>  	}
>  	/* no need for checks, since we are adding at most only 5 keys */
>  	env->envp[env->envp_idx++] = NULL;
>  	kobject_uevent_env(&kvm_dev.this_device->kobj, KOBJ_CHANGE, env->envp);
>  	kfree(env);
> -	kfree(pathbuf);
>  }
>  
>  static int kvm_init_debug(void)
> 

Yes, much better!

Reviewed-by: David Hildenbrand <david@redhat.com>
diff mbox

Patch

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 648b34c..890b706 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -445,6 +445,7 @@  struct kvm {
 	struct kvm_stat_data **debugfs_stat_data;
 	struct srcu_struct srcu;
 	struct srcu_struct irq_srcu;
+	pid_t userspace_pid;
 };
 
 #define kvm_err(fmt, ...) \
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 82987d4..f3f7427 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -3883,7 +3883,6 @@  static const struct file_operations *stat_fops[] = {
 static void kvm_uevent_notify_change(unsigned int type, struct kvm *kvm)
 {
 	struct kobj_uevent_env *env;
-	char *tmp, *pathbuf = NULL;
 	unsigned long long created, active;
 
 	if (!kvm_dev.this_device || !kvm)
@@ -3907,38 +3906,28 @@  static void kvm_uevent_notify_change(unsigned int type, struct kvm *kvm)
 	add_uevent_var(env, "CREATED=%llu", created);
 	add_uevent_var(env, "COUNT=%llu", active);
 
-	if (type == KVM_EVENT_CREATE_VM)
+	if (type == KVM_EVENT_CREATE_VM) {
 		add_uevent_var(env, "EVENT=create");
-	else if (type == KVM_EVENT_DESTROY_VM)
+		kvm->userspace_pid = task_pid_nr(current);
+	} else if (type == KVM_EVENT_DESTROY_VM) {
 		add_uevent_var(env, "EVENT=destroy");
+	}
+	add_uevent_var(env, "PID=%d", kvm->userspace_pid);
 
 	if (kvm->debugfs_dentry) {
-		char p[ITOA_MAX_LEN];
-
-		snprintf(p, sizeof(p), "%s", kvm->debugfs_dentry->d_name.name);
-		tmp = strchrnul(p + 1, '-');
-		*tmp = '\0';
-		add_uevent_var(env, "PID=%s", p);
-		pathbuf = kmalloc(PATH_MAX, GFP_KERNEL);
-		if (pathbuf) {
-			/* sizeof counts the final '\0' */
-			int len = sizeof("STATS_PATH=") - 1;
-			const char *pvar = "STATS_PATH=";
-
-			tmp = dentry_path_raw(kvm->debugfs_dentry,
-					      pathbuf + len,
-					      PATH_MAX - len);
-			if (!IS_ERR(tmp)) {
-				memcpy(tmp - len, pvar, len);
-				env->envp[env->envp_idx++] = tmp - len;
-			}
+		char *tmp, *p = kmalloc(PATH_MAX, GFP_KERNEL);
+
+		if (p) {
+			tmp = dentry_path_raw(kvm->debugfs_dentry, p, PATH_MAX);
+			if (!IS_ERR(tmp))
+				add_uevent_var(env, "STATS_PATH=%s", tmp);
+			kfree(p);
 		}
 	}
 	/* no need for checks, since we are adding at most only 5 keys */
 	env->envp[env->envp_idx++] = NULL;
 	kobject_uevent_env(&kvm_dev.this_device->kobj, KOBJ_CHANGE, env->envp);
 	kfree(env);
-	kfree(pathbuf);
 }
 
 static int kvm_init_debug(void)