| Message ID | 20250217204822.136437-1-olekstysh@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | tests/resource: Verify that Xen can deal with invalid resource type | expand |
On 17/02/2025 8:48 pm, Oleksandr Tyshchenko wrote: > From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> > > The sign of the presence of a corresponding bugfix is an error > returned on querying the size of an unknown for Xen resource type. > > Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> > --- > Refer https://patchew.org/Xen/20250217102741.4122367-1-olekstysh@gmail.com/ > Current patch should go in with Xen fix from the link above. > > 1. w/ fix in Xen: > > + ./tests/resource/test-resource > XENMEM_acquire_resource tests > Test x86 PV > Created d1 > Test grant table > (XEN) [ 8.702293] common/grant_table.c:1909:d0v1 Expanding d1 grant table from 1 to 32 frames > (XEN) [ 8.704499] common/grant_table.c:1909:d0v1 Expanding d1 grant table from 32 to 40 frames > Test x86 PVH > Created d2 > Test grant table > (XEN) [ 8.717210] common/grant_table.c:1909:d0v1 Expanding d2 grant table from 1 to 32 frames > (XEN) [ 8.719477] common/grant_table.c:1909:d0v1 Expanding d2 grant table from 32 to 40 frames > [ ok ] > [ ok ] > Welcome to Alpine Linux 3.18 > Kernel 6.1.19 on an x86_64 (/dev/hvc0) > > 2. w/o fix in Xen: > > + ./tests/resource/test-resource > XENMEM_acquire_resource tests > Test x86 PV(XEN) [ 9.839691] common/grant_table.c:1909:d0v0 Expanding d0 grant table from 1 to 2 frames > Created d1 > Test grant table > (XEN) [ 9.846621] common/grant_table.c:1909:d0v1 Expanding d1 grant table from 1 to 32 frames > (XEN) [ 9.848796] common/grant_table.c:1909:d0v1 Expanding d1 grant table from 32 to 40 frames > Fail: Expected error on an invalid resource type, got success > Test x86 PVH > Created d2 > Test grant table > (XEN) [ 9.865235] common/grant_table.c:1909:d0v1 Expanding d2 grant table from 1 to 32 frames > (XEN) [ 9.867403] common/grant_table.c:1909:d0v1 Expanding d2 grant table from 32 to 40 frames > Fail: Expected error on an invalid resource type, got success > * Execution of "/etc/local.d/xen.start" failed. > [ !! ] > [ !! ] > Welcome to Alpine Linux 3.18 > Kernel 6.1.19 on an x86_64 (/dev/hvc0) > --- > --- > tools/tests/resource/test-resource.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/tools/tests/resource/test-resource.c b/tools/tests/resource/test-resource.c > index 1b10be16a6..3f5479cf78 100644 > --- a/tools/tests/resource/test-resource.c > +++ b/tools/tests/resource/test-resource.c > @@ -123,6 +123,17 @@ static void test_gnttab(uint32_t domid, unsigned int nr_frames, > fail(" Fail: Managed to map gnttab v2 status frames in v1 mode\n"); > xenforeignmemory_unmap_resource(fh, res); > } > + > + /* > + * While at it, verify that an attempt to query the size of an unknown > + * for Xen resource type fails. "If this check starts failing, you've find the right place to test your addition to the Acquire Resource infrastructure." Or something suitable. There needs to be a hint of why 3 was picked here, and part of this sentence will show up in the context of the patch bumping 3 to 4, which also helps remind reviewers to ask if a change isn't somewhere in the submitted series. Anyway, LGTM now. Personally, I'd merge this into the bugfix patch. This improved test wants backporting along with the fix, and the end result is still only 3 hunks. Then, I'd suggest posting the combined result for-4.20 (cc Oleksii). It's not a major bug, but it's also a very simple and low risk fix too. ~Andrew > + */ > + rc = xenforeignmemory_resource_size( > + fh, domid, 3, 0, &size); P.S. Now it's simple, fold this back into 1 line. It causes an extra line of the comment to be visible in context.
On 17.02.25 23:09, Andrew Cooper wrote: Hello. > On 17/02/2025 8:48 pm, Oleksandr Tyshchenko wrote: >> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> >> >> The sign of the presence of a corresponding bugfix is an error >> returned on querying the size of an unknown for Xen resource type. >> >> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> >> --- >> Refer https://patchew.org/Xen/20250217102741.4122367-1-olekstysh@gmail.com/ >> Current patch should go in with Xen fix from the link above. >> >> 1. w/ fix in Xen: >> >> + ./tests/resource/test-resource >> XENMEM_acquire_resource tests >> Test x86 PV >> Created d1 >> Test grant table >> (XEN) [ 8.702293] common/grant_table.c:1909:d0v1 Expanding d1 grant table from 1 to 32 frames >> (XEN) [ 8.704499] common/grant_table.c:1909:d0v1 Expanding d1 grant table from 32 to 40 frames >> Test x86 PVH >> Created d2 >> Test grant table >> (XEN) [ 8.717210] common/grant_table.c:1909:d0v1 Expanding d2 grant table from 1 to 32 frames >> (XEN) [ 8.719477] common/grant_table.c:1909:d0v1 Expanding d2 grant table from 32 to 40 frames >> [ ok ] >> [ ok ] >> Welcome to Alpine Linux 3.18 >> Kernel 6.1.19 on an x86_64 (/dev/hvc0) >> >> 2. w/o fix in Xen: >> >> + ./tests/resource/test-resource >> XENMEM_acquire_resource tests >> Test x86 PV(XEN) [ 9.839691] common/grant_table.c:1909:d0v0 Expanding d0 grant table from 1 to 2 frames >> Created d1 >> Test grant table >> (XEN) [ 9.846621] common/grant_table.c:1909:d0v1 Expanding d1 grant table from 1 to 32 frames >> (XEN) [ 9.848796] common/grant_table.c:1909:d0v1 Expanding d1 grant table from 32 to 40 frames >> Fail: Expected error on an invalid resource type, got success >> Test x86 PVH >> Created d2 >> Test grant table >> (XEN) [ 9.865235] common/grant_table.c:1909:d0v1 Expanding d2 grant table from 1 to 32 frames >> (XEN) [ 9.867403] common/grant_table.c:1909:d0v1 Expanding d2 grant table from 32 to 40 frames >> Fail: Expected error on an invalid resource type, got success >> * Execution of "/etc/local.d/xen.start" failed. >> [ !! ] >> [ !! ] >> Welcome to Alpine Linux 3.18 >> Kernel 6.1.19 on an x86_64 (/dev/hvc0) >> --- >> --- >> tools/tests/resource/test-resource.c | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/tools/tests/resource/test-resource.c b/tools/tests/resource/test-resource.c >> index 1b10be16a6..3f5479cf78 100644 >> --- a/tools/tests/resource/test-resource.c >> +++ b/tools/tests/resource/test-resource.c >> @@ -123,6 +123,17 @@ static void test_gnttab(uint32_t domid, unsigned int nr_frames, >> fail(" Fail: Managed to map gnttab v2 status frames in v1 mode\n"); >> xenforeignmemory_unmap_resource(fh, res); >> } >> + >> + /* >> + * While at it, verify that an attempt to query the size of an unknown >> + * for Xen resource type fails. > > "If this check starts failing, you've find the right place to test your > addition to the Acquire Resource infrastructure." > > Or something suitable. There needs to be a hint of why 3 was picked > here, and part of this sentence will show up in the context of the patch > bumping 3 to 4, which also helps remind reviewers to ask if a change > isn't somewhere in the submitted series. > > Anyway, LGTM now. Thanks > > Personally, I'd merge this into the bugfix patch. This improved test > wants backporting along with the fix, and the end result is still only 3 > hunks. > > Then, I'd suggest posting the combined result for-4.20 (cc Oleksii). > It's not a major bug, but it's also a very simple and low risk fix too. > > ~Andrew > >> + */ >> + rc = xenforeignmemory_resource_size( >> + fh, domid, 3, 0, &size); > > P.S. Now it's simple, fold this back into 1 line. It causes an extra > line of the comment to be visible in context. I agree with the comments.
diff --git a/tools/tests/resource/test-resource.c b/tools/tests/resource/test-resource.c index 1b10be16a6..3f5479cf78 100644 --- a/tools/tests/resource/test-resource.c +++ b/tools/tests/resource/test-resource.c @@ -123,6 +123,17 @@ static void test_gnttab(uint32_t domid, unsigned int nr_frames, fail(" Fail: Managed to map gnttab v2 status frames in v1 mode\n"); xenforeignmemory_unmap_resource(fh, res); } + + /* + * While at it, verify that an attempt to query the size of an unknown + * for Xen resource type fails. + */ + rc = xenforeignmemory_resource_size( + fh, domid, 3, 0, &size); + + /* Check that Xen rejected the resource type. */ + if ( !rc ) + fail(" Fail: Expected error on an invalid resource type, got success\n"); } static void test_domain_configurations(void)