[PATCH v3 0/1]

[PATCH v3 0/1]

[Claudio Imbrenda]

This patch adds a few lines to the KVM common code to fire a
KOBJ_CHANGE uevent whenever a KVM VM is created or destroyed. The event
carries five environment variables:

CREATED indicates how many times a new VM has been created. It is
	useful for example to trigger specific actions when the first
	VM is started
COUNT indicates how many VMs are currently active. This can be used for
	logging or monitoring purposes
PID has the pid of the KVM process that has been started or stopped.
	This can be used to perform process-specific tuning.
STATS_PATH contains the path in debugfs to the directory with all the
	runtime statistics for this VM. This is useful for performance
	monitoring and profiling.
EVENT described the type of event, its value can be either "create" or
	"destroy"

Specific udev rules can be then set up in userspace to deal with the
creation or destruction of VMs as needed.

v2 -> v3:
* added EVENT
* shortened the names of the other variables

v1 -> v2:
* added KVM_VM_PID and KVM_VM_STATS_PATH
* some cleanup, the patch should look nicer now
* rebased on 4.12

*** BLURB HERE ***

Claudio Imbrenda (1):
  KVM: trigger uevents when starting or stopping a VM

 virt/kvm/kvm_main.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

-- 
2.7.4

[PATCH v3 1/1] KVM: trigger uevents when starting or stopping a VM

[Claudio Imbrenda]

This patch adds a few lines to the KVM common code to fire a
KOBJ_CHANGE uevent whenever a KVM VM is created or destroyed. The event
carries five environment variables:

CREATED indicates how many times a new VM has been created. It is
	useful for example to trigger specific actions when the first
	VM is started
COUNT indicates how many VMs are currently active. This can be used for
	logging or monitoring purposes
PID has the pid of the KVM process that has been started or stopped.
	This can be used to perform process-specific tuning.
STATS_PATH contains the path in debugfs to the directory with all the
	runtime statistics for this VM. This is useful for performance
	monitoring and profiling.
EVENT described the type of event, its value can be either "create" or
	"destroy"

Specific udev rules can be then set up in userspace to deal with the
creation or destruction of VMs as needed.

Signed-off-by: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>
---
 virt/kvm/kvm_main.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index f0fe9d0..7c39783 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -130,6 +130,12 @@ EXPORT_SYMBOL_GPL(kvm_rebooting);
 
 static bool largepages_enabled = true;
 
+#define KVM_EVENT_CREATE_VM 0
+#define KVM_EVENT_DESTROY_VM 1
+static void kvm_uevent_notify_change(unsigned int type, struct kvm *kvm);
+static unsigned long long kvm_createvm_count;
+static unsigned long long kvm_active_vms;
+
 bool kvm_is_reserved_pfn(kvm_pfn_t pfn)
 {
 	if (pfn_valid(pfn))
@@ -728,6 +734,7 @@ static void kvm_destroy_vm(struct kvm *kvm)
 	int i;
 	struct mm_struct *mm = kvm->mm;
 
+	kvm_uevent_notify_change(KVM_EVENT_DESTROY_VM, kvm);
 	kvm_destroy_vm_debugfs(kvm);
 	kvm_arch_sync_events(kvm);
 	spin_lock(&kvm_lock);
@@ -3196,6 +3203,7 @@ static int kvm_dev_ioctl_create_vm(unsigned long type)
 		fput(file);
 		return -ENOMEM;
 	}
+	kvm_uevent_notify_change(KVM_EVENT_CREATE_VM, kvm);
 
 	fd_install(r, file);
 	return r;
@@ -3848,6 +3856,57 @@ static const struct file_operations *stat_fops[] = {
 	[KVM_STAT_VM]   = &vm_stat_fops,
 };
 
+static void kvm_uevent_notify_change(unsigned int type, struct kvm *kvm)
+{
+	char cbuf[32], abuf[32], pidbuf[32], evbuf[16];
+	const char pathvar[11] = "STATS_PATH=";
+	char *ptr[6] = {cbuf, abuf, pidbuf, NULL, NULL, NULL};
+	char *tmp, *pathbuf;
+	unsigned long long created, active;
+	int idx = 3;
+
+	if (!kvm_dev.this_device || !kvm || !kvm->debugfs_dentry)
+		return;
+
+	spin_lock(&kvm_lock);
+	if (type == KVM_EVENT_CREATE_VM) {
+		kvm_createvm_count++;
+		kvm_active_vms++;
+	} else if (type == KVM_EVENT_DESTROY_VM) {
+		kvm_active_vms--;
+	}
+	created = kvm_createvm_count;
+	active = kvm_active_vms;
+	spin_unlock(&kvm_lock);
+
+	pathbuf = kmalloc(PATH_MAX, GFP_KERNEL);
+	if (pathbuf) {
+		tmp = dentry_path_raw(kvm->debugfs_dentry,
+				      pathbuf + sizeof(pathvar),
+				      PATH_MAX - sizeof(pathvar));
+		if (!IS_ERR(tmp)) {
+			memcpy(tmp - sizeof(pathvar), pathvar, sizeof(pathvar));
+			ptr[idx++] = tmp - sizeof(pathvar);
+		}
+	}
+	snprintf(cbuf, sizeof(cbuf), "CREATED=%llu", created);
+	snprintf(abuf, sizeof(abuf), "COUNT=%llu", active);
+	snprintf(pidbuf, sizeof(pidbuf), "PID=%s",
+		 kvm->debugfs_dentry->d_name.name);
+	tmp = strchr(pidbuf, '-');
+	if (tmp)
+		*tmp = '\0';
+	if (type == KVM_EVENT_CREATE_VM) {
+		snprintf(evbuf, sizeof(evbuf), "EVENT=create");
+		ptr[idx++] = evbuf;
+	} else if (type == KVM_EVENT_DESTROY_VM) {
+		snprintf(evbuf, sizeof(evbuf), "EVENT=destroy");
+		ptr[idx++] = evbuf;
+	}
+	kobject_uevent_env(&kvm_dev.this_device->kobj, KOBJ_CHANGE, ptr);
+	kfree(pathbuf);
+}
+
 static int kvm_init_debug(void)
 {
 	int r = -EEXIST;
-- 
2.7.4

Re: [PATCH v3 1/1] KVM: trigger uevents when starting or stopping a VM

[David Hildenbrand]

On 10.07.2017 11:20, Claudio Imbrenda wrote:

Minor minor nit:

The subject should state "creating or destroying a VM"

> This patch adds a few lines to the KVM common code to fire a
> KOBJ_CHANGE uevent whenever a KVM VM is created or destroyed. The event
> carries five environment variables:
> 
> CREATED indicates how many times a new VM has been created. It is
> 	useful for example to trigger specific actions when the first
> 	VM is started
> COUNT indicates how many VMs are currently active. This can be used for
> 	logging or monitoring purposes
> PID has the pid of the KVM process that has been started or stopped.
> 	This can be used to perform process-specific tuning.
> STATS_PATH contains the path in debugfs to the directory with all the
> 	runtime statistics for this VM. This is useful for performance
> 	monitoring and profiling.
> EVENT described the type of event, its value can be either "create" or
> 	"destroy"
> 
> Specific udev rules can be then set up in userspace to deal with the
> creation or destruction of VMs as needed.
> 
> Signed-off-by: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>
> ---
>  virt/kvm/kvm_main.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 59 insertions(+)
> 
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index f0fe9d0..7c39783 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -130,6 +130,12 @@ EXPORT_SYMBOL_GPL(kvm_rebooting);
>  
>  static bool largepages_enabled = true;
>  
> +#define KVM_EVENT_CREATE_VM 0
> +#define KVM_EVENT_DESTROY_VM 1
> +static void kvm_uevent_notify_change(unsigned int type, struct kvm *kvm);
> +static unsigned long long kvm_createvm_count;
> +static unsigned long long kvm_active_vms;
> +
>  bool kvm_is_reserved_pfn(kvm_pfn_t pfn)
>  {
>  	if (pfn_valid(pfn))
> @@ -728,6 +734,7 @@ static void kvm_destroy_vm(struct kvm *kvm)
>  	int i;
>  	struct mm_struct *mm = kvm->mm;
>  
> +	kvm_uevent_notify_change(KVM_EVENT_DESTROY_VM, kvm);
>  	kvm_destroy_vm_debugfs(kvm);
>  	kvm_arch_sync_events(kvm);
>  	spin_lock(&kvm_lock);
> @@ -3196,6 +3203,7 @@ static int kvm_dev_ioctl_create_vm(unsigned long type)
>  		fput(file);
>  		return -ENOMEM;
>  	}
> +	kvm_uevent_notify_change(KVM_EVENT_CREATE_VM, kvm);
>  
>  	fd_install(r, file);
>  	return r;
> @@ -3848,6 +3856,57 @@ static const struct file_operations *stat_fops[] = {
>  	[KVM_STAT_VM]   = &vm_stat_fops,
>  };
>  
> +static void kvm_uevent_notify_change(unsigned int type, struct kvm *kvm)
> +{
> +	char cbuf[32], abuf[32], pidbuf[32], evbuf[16];

do we really need that much space for a pid?

> +	const char pathvar[11] = "STATS_PATH=";
> +	char *ptr[6] = {cbuf, abuf, pidbuf, NULL, NULL, NULL};
> +	char *tmp, *pathbuf;
> +	unsigned long long created, active;
> +	int idx = 3;
> +
> +	if (!kvm_dev.this_device || !kvm || !kvm->debugfs_dentry)
> +		return;
> +
> +	spin_lock(&kvm_lock);
> +	if (type == KVM_EVENT_CREATE_VM) {
> +		kvm_createvm_count++;
> +		kvm_active_vms++;
> +	} else if (type == KVM_EVENT_DESTROY_VM) {
> +		kvm_active_vms--;
> +	}
> +	created = kvm_createvm_count;
> +	active = kvm_active_vms;
> +	spin_unlock(&kvm_lock);
> +
> +	pathbuf = kmalloc(PATH_MAX, GFP_KERNEL);
> +	if (pathbuf) {
> +		tmp = dentry_path_raw(kvm->debugfs_dentry,
> +				      pathbuf + sizeof(pathvar),
> +				      PATH_MAX - sizeof(pathvar));
> +		if (!IS_ERR(tmp)) {
> +			memcpy(tmp - sizeof(pathvar), pathvar, sizeof(pathvar));
> +			ptr[idx++] = tmp - sizeof(pathvar);
> +		}
> +	}
> +	snprintf(cbuf, sizeof(cbuf), "CREATED=%llu", created);

Did you think about using struct kobj_uevent_env / add_uevent_var()?

But most probably the problem here is special handling for
dentry_path_raw().


-- 

Thanks,

David

Re: [PATCH v3 1/1] KVM: trigger uevents when starting or stopping a VM

[Claudio Imbrenda]

On Mon, 10 Jul 2017 11:40:55 +0200
David Hildenbrand <david@redhat.com> wrote:

> On 10.07.2017 11:20, Claudio Imbrenda wrote:
> 
> Minor minor nit:
> 
> The subject should state "creating or destroying a VM"

I'll fix it

[...]

> > +static void kvm_uevent_notify_change(unsigned int type, struct kvm
> > *kvm) +{
> > +	char cbuf[32], abuf[32], pidbuf[32], evbuf[16];  
> 
> do we really need that much space for a pid?

unfortunately yes. we don't have access to the pid when destroying the
VM, so I take it from the debugfs entry, and that has the format
"%d-%d", with pid and file descriptor, both can be 10 bytes long.

> > +	const char pathvar[11] = "STATS_PATH=";
> > +	char *ptr[6] = {cbuf, abuf, pidbuf, NULL, NULL, NULL};
> > +	char *tmp, *pathbuf;
> > +	unsigned long long created, active;
> > +	int idx = 3;
> > +
> > +	if (!kvm_dev.this_device || !kvm || !kvm->debugfs_dentry)
> > +		return;
> > +
> > +	spin_lock(&kvm_lock);
> > +	if (type == KVM_EVENT_CREATE_VM) {
> > +		kvm_createvm_count++;
> > +		kvm_active_vms++;
> > +	} else if (type == KVM_EVENT_DESTROY_VM) {
> > +		kvm_active_vms--;
> > +	}
> > +	created = kvm_createvm_count;
> > +	active = kvm_active_vms;
> > +	spin_unlock(&kvm_lock);
> > +
> > +	pathbuf = kmalloc(PATH_MAX, GFP_KERNEL);
> > +	if (pathbuf) {
> > +		tmp = dentry_path_raw(kvm->debugfs_dentry,
> > +				      pathbuf + sizeof(pathvar),
> > +				      PATH_MAX - sizeof(pathvar));
> > +		if (!IS_ERR(tmp)) {
> > +			memcpy(tmp - sizeof(pathvar), pathvar,
> > sizeof(pathvar));
> > +			ptr[idx++] = tmp - sizeof(pathvar);
> > +		}
> > +	}
> > +	snprintf(cbuf, sizeof(cbuf), "CREATED=%llu", created);  
> 
> Did you think about using struct kobj_uevent_env / add_uevent_var()?

for what I could see, that only works with a more invasive patch. I'll
see what I can do.
 
> But most probably the problem here is special handling for
> dentry_path_raw().
 

Re: [PATCH v3 1/1] KVM: trigger uevents when starting or stopping a VM

[Paolo Bonzini]

On 10/07/2017 13:53, Claudio Imbrenda wrote:
> On Mon, 10 Jul 2017 11:40:55 +0200
> David Hildenbrand <david@redhat.com> wrote:
> 
>> On 10.07.2017 11:20, Claudio Imbrenda wrote:
>>
>> Minor minor nit:
>>
>> The subject should state "creating or destroying a VM"
> 
> I'll fix it

I can fix it when applying, no problem.

Paolo

> [...]
> 
>>> +static void kvm_uevent_notify_change(unsigned int type, struct kvm
>>> *kvm) +{
>>> +	char cbuf[32], abuf[32], pidbuf[32], evbuf[16];  
>>
>> do we really need that much space for a pid?
> 
> unfortunately yes. we don't have access to the pid when destroying the
> VM, so I take it from the debugfs entry, and that has the format
> "%d-%d", with pid and file descriptor, both can be 10 bytes long.
> 
>>> +	const char pathvar[11] = "STATS_PATH=";
>>> +	char *ptr[6] = {cbuf, abuf, pidbuf, NULL, NULL, NULL};
>>> +	char *tmp, *pathbuf;
>>> +	unsigned long long created, active;
>>> +	int idx = 3;
>>> +
>>> +	if (!kvm_dev.this_device || !kvm || !kvm->debugfs_dentry)
>>> +		return;
>>> +
>>> +	spin_lock(&kvm_lock);
>>> +	if (type == KVM_EVENT_CREATE_VM) {
>>> +		kvm_createvm_count++;
>>> +		kvm_active_vms++;
>>> +	} else if (type == KVM_EVENT_DESTROY_VM) {
>>> +		kvm_active_vms--;
>>> +	}
>>> +	created = kvm_createvm_count;
>>> +	active = kvm_active_vms;
>>> +	spin_unlock(&kvm_lock);
>>> +
>>> +	pathbuf = kmalloc(PATH_MAX, GFP_KERNEL);
>>> +	if (pathbuf) {
>>> +		tmp = dentry_path_raw(kvm->debugfs_dentry,
>>> +				      pathbuf + sizeof(pathvar),
>>> +				      PATH_MAX - sizeof(pathvar));
>>> +		if (!IS_ERR(tmp)) {
>>> +			memcpy(tmp - sizeof(pathvar), pathvar,
>>> sizeof(pathvar));
>>> +			ptr[idx++] = tmp - sizeof(pathvar);
>>> +		}
>>> +	}
>>> +	snprintf(cbuf, sizeof(cbuf), "CREATED=%llu", created);  
>>
>> Did you think about using struct kobj_uevent_env / add_uevent_var()?
> 
> for what I could see, that only works with a more invasive patch. I'll
> see what I can do.
>  
>> But most probably the problem here is special handling for
>> dentry_path_raw().
>  
>