| Message ID | 20190507162047.17152-1-clg@kaod.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | KVM: fix 'release' method of KVM device | expand |
On 07.05.19 18:20, Cédric Le Goater wrote: > There is no need to test for the device pointer validity when releasing > a KVM device. The file descriptor should identify it safely. "Fix" implies it is broken. Is it broken? > > Signed-off-by: Cédric Le Goater <clg@kaod.org> > --- > > Fixes http://patchwork.ozlabs.org/patch/1087506/ > https://git.kernel.org/pub/scm/linux/kernel/git/paulus/powerpc.git/commit/?h=kvm-ppc-next&id=2bde9b3ec8bdf60788e9e2ce8c07a2f8d6003dbd > > virt/kvm/kvm_main.c | 6 ------ > 1 file changed, 6 deletions(-) > > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > index 161830ec0aa5..ac15b8fd8399 100644 > --- a/virt/kvm/kvm_main.c > +++ b/virt/kvm/kvm_main.c > @@ -2939,12 +2939,6 @@ static int kvm_device_release(struct inode *inode, struct file *filp) > struct kvm_device *dev = filp->private_data; > struct kvm *kvm = dev->kvm; > > - if (!dev) > - return -ENODEV; > - > - if (dev->kvm != kvm) > - return -EPERM; > - > if (dev->ops->release) { > mutex_lock(&kvm->lock); > list_del(&dev->vm_node); >
On 08/05/2019 02:20, Cédric Le Goater wrote: > There is no need to test for the device pointer validity when releasing > a KVM device. The file descriptor should identify it safely. > > Signed-off-by: Cédric Le Goater <clg@kaod.org> > --- > > Fixes http://patchwork.ozlabs.org/patch/1087506/ > https://git.kernel.org/pub/scm/linux/kernel/git/paulus/powerpc.git/commit/?h=kvm-ppc-next&id=2bde9b3ec8bdf60788e9e2ce8c07a2f8d6003dbd Usually suggested way is: [fstn1-p1 kernel]$ git log -1 --format='Fixes: %h ("%s")' 2bde9b3ec8bd Fixes: 2bde9b3ec8bd ("KVM: Introduce a 'release' method for KVM devices") Anyway, Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru> > > virt/kvm/kvm_main.c | 6 ------ > 1 file changed, 6 deletions(-) > > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > index 161830ec0aa5..ac15b8fd8399 100644 > --- a/virt/kvm/kvm_main.c > +++ b/virt/kvm/kvm_main.c > @@ -2939,12 +2939,6 @@ static int kvm_device_release(struct inode *inode, struct file *filp) > struct kvm_device *dev = filp->private_data; > struct kvm *kvm = dev->kvm; > > - if (!dev) > - return -ENODEV; > - > - if (dev->kvm != kvm) > - return -EPERM; > - > if (dev->ops->release) { > mutex_lock(&kvm->lock); > list_del(&dev->vm_node); >
On 5/7/19 9:32 PM, David Hildenbrand wrote: > On 07.05.19 18:20, Cédric Le Goater wrote: >> There is no need to test for the device pointer validity when releasing >> a KVM device. The file descriptor should identify it safely. > > "Fix" implies it is broken. Is it broken? no, it's not broken indeed. The changes are removing useless checks leftover from a previous patch. A title such as : remove useless checks in 'release' method of KVM device would be more appropriate. I can send a v2 with Alexey's rb. Thanks, C. >> >> Signed-off-by: Cédric Le Goater <clg@kaod.org> >> --- >> >> Fixes http://patchwork.ozlabs.org/patch/1087506/ >> https://git.kernel.org/pub/scm/linux/kernel/git/paulus/powerpc.git/commit/?h=kvm-ppc-next&id=2bde9b3ec8bdf60788e9e2ce8c07a2f8d6003dbd >> >> virt/kvm/kvm_main.c | 6 ------ >> 1 file changed, 6 deletions(-) >> >> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c >> index 161830ec0aa5..ac15b8fd8399 100644 >> --- a/virt/kvm/kvm_main.c >> +++ b/virt/kvm/kvm_main.c >> @@ -2939,12 +2939,6 @@ static int kvm_device_release(struct inode *inode, struct file *filp) >> struct kvm_device *dev = filp->private_data; >> struct kvm *kvm = dev->kvm; >> >> - if (!dev) >> - return -ENODEV; >> - >> - if (dev->kvm != kvm) >> - return -EPERM; >> - >> if (dev->ops->release) { >> mutex_lock(&kvm->lock); >> list_del(&dev->vm_node); >> > >
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index 161830ec0aa5..ac15b8fd8399 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -2939,12 +2939,6 @@ static int kvm_device_release(struct inode *inode, struct file *filp) struct kvm_device *dev = filp->private_data; struct kvm *kvm = dev->kvm; - if (!dev) - return -ENODEV; - - if (dev->kvm != kvm) - return -EPERM; - if (dev->ops->release) { mutex_lock(&kvm->lock); list_del(&dev->vm_node);
There is no need to test for the device pointer validity when releasing a KVM device. The file descriptor should identify it safely. Signed-off-by: Cédric Le Goater <clg@kaod.org> --- Fixes http://patchwork.ozlabs.org/patch/1087506/ https://git.kernel.org/pub/scm/linux/kernel/git/paulus/powerpc.git/commit/?h=kvm-ppc-next&id=2bde9b3ec8bdf60788e9e2ce8c07a2f8d6003dbd virt/kvm/kvm_main.c | 6 ------ 1 file changed, 6 deletions(-)