| Message ID | 1499678444-21879-2-git-send-email-imbrenda@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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().
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().
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(). > >
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;
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(+)