Re: [PATCH v4 08/10] KVM: s390: add functions to (un)register GISC with GISA

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/12/2018 13:15, Michael Mueller wrote:


On 10.12.18 10:44, Cornelia Huck wrote:
On Mon, 10 Dec 2018 10:27:54 +0100
Pierre Morel <pmorel@xxxxxxxxxxxxx> wrote:

On 03/12/2018 10:48, Michael Mueller wrote:


On 03.12.18 10:21, Cornelia Huck wrote:
On Fri, 30 Nov 2018 15:32:13 +0100
Michael Mueller <mimu@xxxxxxxxxxxxx> wrote:
+int kvm_s390_gisc_unregister(struct kvm *kvm, u32 gisc)
+{
+    int rc = 0;
+
+    if (!kvm->arch.gib_in_use)
+        return -ENODEV;
+    if (gisc > MAX_ISC)
+        return -EINVAL;
+
+    spin_lock(&kvm->arch.iam_ref_lock);
+    if (kvm->arch.iam_ref_count[gisc] == 0) {
+        rc = -EFAULT;
-EFAULT looks odd. -EINVAL?
sure

Can we find some errno return value which is not already used in this
function?

I'm not sure what would be a better fit here (-EINVAL if you try to
unregister something that has never been registered seems reasonable).

Also, I think it is quite reasonable if a function returns the same
error for different reasons, as long as they are all the same class of
error. In this case, both the caller using an out-of-rage gisc or a
gisc that has never been registered indicate some internal mixup in how
the caller handles the gisc.

I think -EPERM is the right choice here. It is not permitted to
decrease the ref counter below zero.



To be anoying I would prefer
-ERANGE if gisc > MAX_ISC

-EINVAL in the second case is OK for me

and I also can agree with the argumentation of Conny.


--
Pierre Morel
Linux/KVM/QEMU in Böblingen - Germany




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux