| Message ID | 20250117190938.93793-5-imbrenda@linux.ibm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | KVM: s390: Stop using page->index and other things | expand |
On 17.01.25 20:09, Claudio Imbrenda wrote: > With the latest patch, attempting to create a memslot from userspace > will result in an EEXIST error for UCONTROL VMs, instead of EINVAL, > since the new memslot will collide with the internal memslot. There is > no simple way to bring back the previous behaviour. > > This is not a problem, but the test needs to be fixed accordingly. > > Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com> > --- > tools/testing/selftests/kvm/s390x/ucontrol_test.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/tools/testing/selftests/kvm/s390x/ucontrol_test.c b/tools/testing/selftests/kvm/s390x/ucontrol_test.c > index 135ee22856cf..ca18736257f8 100644 > --- a/tools/testing/selftests/kvm/s390x/ucontrol_test.c > +++ b/tools/testing/selftests/kvm/s390x/ucontrol_test.c > @@ -459,10 +459,12 @@ TEST_F(uc_kvm, uc_no_user_region) > }; > > ASSERT_EQ(-1, ioctl(self->vm_fd, KVM_SET_USER_MEMORY_REGION, ®ion)); > - ASSERT_EQ(EINVAL, errno); > + if (errno != EEXIST) > + ASSERT_EQ(EINVAL, errno); ASSERT_TRUE(errno == EEXIST || errno == EINVAL)' ?
On Mon, 20 Jan 2025 13:12:31 +0100 David Hildenbrand <david@redhat.com> wrote: > On 17.01.25 20:09, Claudio Imbrenda wrote: > > With the latest patch, attempting to create a memslot from userspace > > will result in an EEXIST error for UCONTROL VMs, instead of EINVAL, > > since the new memslot will collide with the internal memslot. There is > > no simple way to bring back the previous behaviour. > > > > This is not a problem, but the test needs to be fixed accordingly. > > > > Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com> > > --- > > tools/testing/selftests/kvm/s390x/ucontrol_test.c | 6 ++++-- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/tools/testing/selftests/kvm/s390x/ucontrol_test.c b/tools/testing/selftests/kvm/s390x/ucontrol_test.c > > index 135ee22856cf..ca18736257f8 100644 > > --- a/tools/testing/selftests/kvm/s390x/ucontrol_test.c > > +++ b/tools/testing/selftests/kvm/s390x/ucontrol_test.c > > @@ -459,10 +459,12 @@ TEST_F(uc_kvm, uc_no_user_region) > > }; > > > > ASSERT_EQ(-1, ioctl(self->vm_fd, KVM_SET_USER_MEMORY_REGION, ®ion)); > > - ASSERT_EQ(EINVAL, errno); > > + if (errno != EEXIST) > > + ASSERT_EQ(EINVAL, errno); > > ASSERT_TRUE(errno == EEXIST || errno == EINVAL)' > > ? > I had thought about that, but in case of failure it won't print the failing value. It's probably more readable with the ASSERT_EQ, I will change it.
On Mon Jan 20, 2025 at 1:25 PM CET, Claudio Imbrenda wrote: > On Mon, 20 Jan 2025 13:12:31 +0100 > David Hildenbrand <david@redhat.com> wrote: > > > On 17.01.25 20:09, Claudio Imbrenda wrote: > > > With the latest patch, attempting to create a memslot from userspace > > > will result in an EEXIST error for UCONTROL VMs, instead of EINVAL, > > > since the new memslot will collide with the internal memslot. There is > > > no simple way to bring back the previous behaviour. > > > > > > This is not a problem, but the test needs to be fixed accordingly. > > > > > > Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com> > > > --- > > > tools/testing/selftests/kvm/s390x/ucontrol_test.c | 6 ++++-- > > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > > > diff --git a/tools/testing/selftests/kvm/s390x/ucontrol_test.c b/tools/testing/selftests/kvm/s390x/ucontrol_test.c > > > index 135ee22856cf..ca18736257f8 100644 > > > --- a/tools/testing/selftests/kvm/s390x/ucontrol_test.c > > > +++ b/tools/testing/selftests/kvm/s390x/ucontrol_test.c > > > @@ -459,10 +459,12 @@ TEST_F(uc_kvm, uc_no_user_region) > > > }; > > > > > > ASSERT_EQ(-1, ioctl(self->vm_fd, KVM_SET_USER_MEMORY_REGION, ®ion)); > > > - ASSERT_EQ(EINVAL, errno); > > > + if (errno != EEXIST) > > > + ASSERT_EQ(EINVAL, errno); > > > > ASSERT_TRUE(errno == EEXIST || errno == EINVAL)' > > > > ? > > How about this? ASSERT_TRUE(errno == EEXIST || errno == EINVAL) TH_LOG("errno %s (%i) not expected for ioctl KVM_SET_USER_MEMORY_REGION", strerror(errno), errno); > > I had thought about that, but in case of failure it won't print the > failing value. > > It's probably more readable with the ASSERT_EQ, I will change it.
diff --git a/tools/testing/selftests/kvm/s390x/ucontrol_test.c b/tools/testing/selftests/kvm/s390x/ucontrol_test.c index 135ee22856cf..ca18736257f8 100644 --- a/tools/testing/selftests/kvm/s390x/ucontrol_test.c +++ b/tools/testing/selftests/kvm/s390x/ucontrol_test.c @@ -459,10 +459,12 @@ TEST_F(uc_kvm, uc_no_user_region) }; ASSERT_EQ(-1, ioctl(self->vm_fd, KVM_SET_USER_MEMORY_REGION, ®ion)); - ASSERT_EQ(EINVAL, errno); + if (errno != EEXIST) + ASSERT_EQ(EINVAL, errno); ASSERT_EQ(-1, ioctl(self->vm_fd, KVM_SET_USER_MEMORY_REGION2, ®ion2)); - ASSERT_EQ(EINVAL, errno); + if (errno != EEXIST) + ASSERT_EQ(EINVAL, errno); } TEST_F(uc_kvm, uc_map_unmap)
With the latest patch, attempting to create a memslot from userspace will result in an EEXIST error for UCONTROL VMs, instead of EINVAL, since the new memslot will collide with the internal memslot. There is no simple way to bring back the previous behaviour. This is not a problem, but the test needs to be fixed accordingly. Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com> --- tools/testing/selftests/kvm/s390x/ucontrol_test.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)