Date: Tue, 4 Jul 2017 12:28:02 +0200
From: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1 1/1] KVM: trigger uevents when starting or stopping a
 VM
In-Reply-To: <a8ecc547-1f8b-48dc-c45d-6aa769f9a6fa@redhat.com>
References: <1499159018-19782-1-git-send-email-imbrenda@linux.vnet.ibm.com>
        <8d239a6c-3625-6964-7fd2-b753f2e15bf5@redhat.com>
        <3de04174-46d3-8741-2088-218b48ad20be@de.ibm.com>
        <a8ecc547-1f8b-48dc-c45d-6aa769f9a6fa@redhat.com>
Organization: IBM
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Message-Id: <20170704122802.16794f5c@p-imbrenda.boeblingen.de.ibm.com>
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 4258
Lines: 128

On Tue, 4 Jul 2017 11:27:44 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 04/07/2017 11:25, Christian Borntraeger wrote:
> > On 07/04/2017 11:23 AM, Paolo Bonzini wrote:  
> >>
> >>
> >> On 04/07/2017 11:03, Claudio Imbrenda wrote:  
> >>> 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 two environment variables:
> >>> KVM_VM_CREATED which indicates how many times a new VM has been
> >>> created, KVM_VM_COUNT which indicates how many VMs are currently
> >>> active.  
> >>
> >> I'm not sure why KVM_VM_CREATED is useful, KVM_VM_COUNT can be a
> >> bit more interesting though not much.  
> > 
> > I am certainly interested in an trigger from "kvm was never used"
> > to "kvm was used", so having something like KVM_VM_CREATED has a
> > value for me.  
> 
> Fair enough, that's a good use.  Please add it to the commit message,
> however.

I'll add that too

> Paolo
> 
> >> But since we are at it, let's also add a PID.  That one is the
> >> really useful one, because it gives you something to look at in
> >> debugfs. Another possibility is to add the debugfs directory name
> >> directly (KVM_VM_STATS_PATH or something like that).
> >>
> >> Paolo
> >>  
> >>> 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 | 25 +++++++++++++++++++++++++
> >>>  1 file changed, 25 insertions(+)
> >>>
> >>> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> >>> index 6e3b12c..f67aa59 100644
> >>> --- a/virt/kvm/kvm_main.c
> >>> +++ b/virt/kvm/kvm_main.c
> >>> @@ -130,6 +130,10 @@ EXPORT_SYMBOL_GPL(kvm_rebooting);
> >>>  
> >>>  static bool largepages_enabled = true;
> >>>  
> >>> +static void kvm_uevent_notify_change(u64 created, u64 active);
> >>> +static u64 kvm_createvm_count;
> >>> +static u64 kvm_active_vms;
> >>> +
> >>>  bool kvm_is_reserved_pfn(kvm_pfn_t pfn)
> >>>  {
> >>>  	if (pfn_valid(pfn))
> >>> @@ -627,6 +631,7 @@ static struct kvm *kvm_create_vm(unsigned
> >>> long type) {
> >>>  	int r, i;
> >>>  	struct kvm *kvm = kvm_arch_alloc_vm();
> >>> +	u64 activevms, createdvms;
> >>>  
> >>>  	if (!kvm)
> >>>  		return ERR_PTR(-ENOMEM);
> >>> @@ -686,9 +691,12 @@ static struct kvm *kvm_create_vm(unsigned
> >>> long type) 
> >>>  	spin_lock(&kvm_lock);
> >>>  	list_add(&kvm->vm_list, &vm_list);
> >>> +	createdvms = ++kvm_createvm_count;
> >>> +	activevms = ++kvm_active_vms;
> >>>  	spin_unlock(&kvm_lock);
> >>>  
> >>>  	preempt_notifier_inc();
> >>> +	kvm_uevent_notify_change(createdvms, activevms);
> >>>  
> >>>  	return kvm;
> >>>  
> >>> @@ -739,11 +747,14 @@ static void kvm_destroy_vm(struct kvm *kvm)
> >>>  {
> >>>  	int i;
> >>>  	struct mm_struct *mm = kvm->mm;
> >>> +	u64 activevms, createdvms;
> >>>  
> >>>  	kvm_destroy_vm_debugfs(kvm);
> >>>  	kvm_arch_sync_events(kvm);
> >>>  	spin_lock(&kvm_lock);
> >>>  	list_del(&kvm->vm_list);
> >>> +	activevms = --kvm_active_vms;
> >>> +	createdvms = kvm_createvm_count;
> >>>  	spin_unlock(&kvm_lock);
> >>>  	kvm_free_irq_routing(kvm);
> >>>  	for (i = 0; i < KVM_NR_BUSES; i++) {
> >>> @@ -767,6 +778,7 @@ static void kvm_destroy_vm(struct kvm *kvm)
> >>>  	preempt_notifier_dec();
> >>>  	hardware_disable_all();
> >>>  	mmdrop(mm);
> >>> +	kvm_uevent_notify_change(createdvms, activevms);
> >>>  }
> >>>  
> >>>  void kvm_get_kvm(struct kvm *kvm)
> >>> @@ -3864,6 +3876,19 @@ static const struct file_operations
> >>> *stat_fops[] = { [KVM_STAT_VM]   = &vm_stat_fops,
> >>>  };
> >>>  
> >>> +static void kvm_uevent_notify_change(u64 created, u64 active)
> >>> +{
> >>> +	char createvm_buf[40];
> >>> +	char activevm_buf[40];
> >>> +	char *ptr[3] = {createvm_buf, activevm_buf, NULL};
> >>> +
> >>> +	if (!kvm_dev.this_device)
> >>> +		return;
> >>> +	snprintf(createvm_buf, 40, "KVM_VM_CREATED=%llu",
> >>> created);
> >>> +	snprintf(activevm_buf, 40, "KVM_VM_ACTIVE=%llu", active);
> >>> +	kobject_uevent_env(&kvm_dev.this_device->kobj,
> >>> KOBJ_CHANGE, ptr); +}
> >>> +
> >>>  static int kvm_init_debug(void)
> >>>  {
> >>>  	int r = -EEXIST;
> >>>  
> >>  
> >   
>