| Message ID | 1457685810-4103-1-git-send-email-jthumshirn@suse.de (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 864f617c9a3f |
| Headers | show |
On Fri, Mar 11, 2016 at 12:43 AM, Johannes Thumshirn <jthumshirn@suse.de> wrote: > Grab the kernel version used for tests dynamically via utsname() instead of > hardcoding the version of the build host. > > Otherwise tests will be skipped if the build host had a too old kernel > version. > > flodin:~ # ./ndctl test > __ndctl_test_attempt: skip test_libndctl:1950 requires: 4.2.0 current: 4.1.0 > test-libndctl: SKIP > __ndctl_test_attempt: skip test_dpa_alloc:300 requires: 4.2.0 current: 4.1.0 > test-dpa-alloc: SKIP > __ndctl_test_attempt: skip test_parent_uuid:230 requires: 4.3.0 current: 4.1.0 > test-parent-uuid: SKIP > attempted: 3 skipped: 3 > > Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de> > --- > test/core.c | 15 ++++++++++++++- > 1 file changed, 14 insertions(+), 1 deletion(-) > > Changes to v1: > Use utsname.release which is obviously correct. > On my test system utsname.release = "4.4.4-default" > > I can only imagine the sscanf() failing and on stack garbage in a, b, c being > greater than KERNEL_VERSION(4, 3, 0). Looks good to me, applied for v52. I've pushed it out on the 'pending' branch. It will move to 'master' after the v4.6-rc1 kernel is released.
On Fri, Mar 11, 2016 at 10:28:27AM -0800, Dan Williams wrote: > On Fri, Mar 11, 2016 at 12:43 AM, Johannes Thumshirn <jthumshirn@suse.de> wrote: > > Grab the kernel version used for tests dynamically via utsname() instead of > > hardcoding the version of the build host. > > > > Otherwise tests will be skipped if the build host had a too old kernel > > version. > > > > flodin:~ # ./ndctl test > > __ndctl_test_attempt: skip test_libndctl:1950 requires: 4.2.0 current: 4.1.0 > > test-libndctl: SKIP > > __ndctl_test_attempt: skip test_dpa_alloc:300 requires: 4.2.0 current: 4.1.0 > > test-dpa-alloc: SKIP > > __ndctl_test_attempt: skip test_parent_uuid:230 requires: 4.3.0 current: 4.1.0 > > test-parent-uuid: SKIP > > attempted: 3 skipped: 3 > > > > Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de> > > --- > > test/core.c | 15 ++++++++++++++- > > 1 file changed, 14 insertions(+), 1 deletion(-) > > > > Changes to v1: > > Use utsname.release which is obviously correct. > > On my test system utsname.release = "4.4.4-default" > > > > I can only imagine the sscanf() failing and on stack garbage in a, b, c being > > greater than KERNEL_VERSION(4, 3, 0). > > Looks good to me, applied for v52. I've pushed it out on the > 'pending' branch. It will move to 'master' after the v4.6-rc1 kernel > is released. Thanks. Though I'm still not too happy with the general case of version checking in the tests at all. Actually it should check for the presence of a feature and not when a feature was introduced in the mainline kernel. Some distro kernels tend to be an rather old version with havily backported features *cough* ours *cough*. I'll see if I can come up with something, maybe for the 4.7 window.
On Mon, Mar 14, 2016 at 1:31 AM, Johannes Thumshirn <jthumshirn@suse.de> wrote: > On Fri, Mar 11, 2016 at 10:28:27AM -0800, Dan Williams wrote: >> On Fri, Mar 11, 2016 at 12:43 AM, Johannes Thumshirn <jthumshirn@suse.de> wrote: >> > Grab the kernel version used for tests dynamically via utsname() instead of >> > hardcoding the version of the build host. >> > >> > Otherwise tests will be skipped if the build host had a too old kernel >> > version. >> > >> > flodin:~ # ./ndctl test >> > __ndctl_test_attempt: skip test_libndctl:1950 requires: 4.2.0 current: 4.1.0 >> > test-libndctl: SKIP >> > __ndctl_test_attempt: skip test_dpa_alloc:300 requires: 4.2.0 current: 4.1.0 >> > test-dpa-alloc: SKIP >> > __ndctl_test_attempt: skip test_parent_uuid:230 requires: 4.3.0 current: 4.1.0 >> > test-parent-uuid: SKIP >> > attempted: 3 skipped: 3 >> > >> > Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de> >> > --- >> > test/core.c | 15 ++++++++++++++- >> > 1 file changed, 14 insertions(+), 1 deletion(-) >> > >> > Changes to v1: >> > Use utsname.release which is obviously correct. >> > On my test system utsname.release = "4.4.4-default" >> > >> > I can only imagine the sscanf() failing and on stack garbage in a, b, c being >> > greater than KERNEL_VERSION(4, 3, 0). >> >> Looks good to me, applied for v52. I've pushed it out on the >> 'pending' branch. It will move to 'master' after the v4.6-rc1 kernel >> is released. > > Thanks. Though I'm still not too happy with the general case of version > checking in the tests at all. Actually it should check for the presence of a > feature and not when a feature was introduced in the mainline kernel. Some > distro kernels tend to be an rather old version with havily backported > features *cough* ours *cough*. I'll see if I can come up with something, maybe > for the 4.7 window. > For backport kernels the idea was to use "ndctl test -f" to turn on all tests regardless of nominal version. The automatic version determination was only meant for use in the 0day-kbuild-robot that might be running bisects.
On Mon, Mar 14, 2016 at 08:11:10AM -0700, Dan Williams wrote: > On Mon, Mar 14, 2016 at 1:31 AM, Johannes Thumshirn <jthumshirn@suse.de> wrote: > > On Fri, Mar 11, 2016 at 10:28:27AM -0800, Dan Williams wrote: > >> On Fri, Mar 11, 2016 at 12:43 AM, Johannes Thumshirn <jthumshirn@suse.de> wrote: > >> > Grab the kernel version used for tests dynamically via utsname() instead of > >> > hardcoding the version of the build host. > >> > > >> > Otherwise tests will be skipped if the build host had a too old kernel > >> > version. > >> > > >> > flodin:~ # ./ndctl test > >> > __ndctl_test_attempt: skip test_libndctl:1950 requires: 4.2.0 current: 4.1.0 > >> > test-libndctl: SKIP > >> > __ndctl_test_attempt: skip test_dpa_alloc:300 requires: 4.2.0 current: 4.1.0 > >> > test-dpa-alloc: SKIP > >> > __ndctl_test_attempt: skip test_parent_uuid:230 requires: 4.3.0 current: 4.1.0 > >> > test-parent-uuid: SKIP > >> > attempted: 3 skipped: 3 > >> > > >> > Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de> > >> > --- > >> > test/core.c | 15 ++++++++++++++- > >> > 1 file changed, 14 insertions(+), 1 deletion(-) > >> > > >> > Changes to v1: > >> > Use utsname.release which is obviously correct. > >> > On my test system utsname.release = "4.4.4-default" > >> > > >> > I can only imagine the sscanf() failing and on stack garbage in a, b, c being > >> > greater than KERNEL_VERSION(4, 3, 0). > >> > >> Looks good to me, applied for v52. I've pushed it out on the > >> 'pending' branch. It will move to 'master' after the v4.6-rc1 kernel > >> is released. > > > > Thanks. Though I'm still not too happy with the general case of version > > checking in the tests at all. Actually it should check for the presence of a > > feature and not when a feature was introduced in the mainline kernel. Some > > distro kernels tend to be an rather old version with havily backported > > features *cough* ours *cough*. I'll see if I can come up with something, maybe > > for the 4.7 window. > > > > For backport kernels the idea was to use "ndctl test -f" to turn on > all tests regardless of nominal version. The automatic version > determination was only meant for use in the 0day-kbuild-robot that > might be running bisects. OK, but having a ioctl or file in sysfs with a bit mask flipping "feature bits" on and off won't break the 0day bot and enhance the "user experience" (not really the right term I know). Oh and btw, thanks for having unit tests for the nvdimm subsystem. Johannes
On Mon, Mar 14, 2016 at 8:25 AM, Johannes Thumshirn <jthumshirn@suse.de> wrote: > On Mon, Mar 14, 2016 at 08:11:10AM -0700, Dan Williams wrote: >> On Mon, Mar 14, 2016 at 1:31 AM, Johannes Thumshirn <jthumshirn@suse.de> wrote: >> > On Fri, Mar 11, 2016 at 10:28:27AM -0800, Dan Williams wrote: >> >> On Fri, Mar 11, 2016 at 12:43 AM, Johannes Thumshirn <jthumshirn@suse.de> wrote: >> >> > Grab the kernel version used for tests dynamically via utsname() instead of >> >> > hardcoding the version of the build host. >> >> > >> >> > Otherwise tests will be skipped if the build host had a too old kernel >> >> > version. >> >> > >> >> > flodin:~ # ./ndctl test >> >> > __ndctl_test_attempt: skip test_libndctl:1950 requires: 4.2.0 current: 4.1.0 >> >> > test-libndctl: SKIP >> >> > __ndctl_test_attempt: skip test_dpa_alloc:300 requires: 4.2.0 current: 4.1.0 >> >> > test-dpa-alloc: SKIP >> >> > __ndctl_test_attempt: skip test_parent_uuid:230 requires: 4.3.0 current: 4.1.0 >> >> > test-parent-uuid: SKIP >> >> > attempted: 3 skipped: 3 >> >> > >> >> > Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de> >> >> > --- >> >> > test/core.c | 15 ++++++++++++++- >> >> > 1 file changed, 14 insertions(+), 1 deletion(-) >> >> > >> >> > Changes to v1: >> >> > Use utsname.release which is obviously correct. >> >> > On my test system utsname.release = "4.4.4-default" >> >> > >> >> > I can only imagine the sscanf() failing and on stack garbage in a, b, c being >> >> > greater than KERNEL_VERSION(4, 3, 0). >> >> >> >> Looks good to me, applied for v52. I've pushed it out on the >> >> 'pending' branch. It will move to 'master' after the v4.6-rc1 kernel >> >> is released. >> > >> > Thanks. Though I'm still not too happy with the general case of version >> > checking in the tests at all. Actually it should check for the presence of a >> > feature and not when a feature was introduced in the mainline kernel. Some >> > distro kernels tend to be an rather old version with havily backported >> > features *cough* ours *cough*. I'll see if I can come up with something, maybe >> > for the 4.7 window. >> > >> >> For backport kernels the idea was to use "ndctl test -f" to turn on >> all tests regardless of nominal version. The automatic version >> determination was only meant for use in the 0day-kbuild-robot that >> might be running bisects. > > OK, but having a ioctl or file in sysfs with a bit mask flipping "feature > bits" on and off won't break the 0day bot and enhance the "user experience" > (not really the right term I know). Sounds interesting in theory, but how do you have a bit mask for behavior changes? In other words, these compatibility points aren't delineated on clear feature boundaries. While a behavior change will/must not break old binaries, new tests check the new behavior out of existing interfaces. > Oh and btw, thanks for having unit tests for the nvdimm subsystem. :-)
--- a/test/core.c +++ b/test/core.c @@ -1,4 +1,5 @@ #include <linux/version.h> +#include <sys/utsname.h> #include <stdlib.h> #include <stdio.h> #include <test.h> @@ -11,6 +12,18 @@ struct ndctl_test { int skip; }; +static unsigned int get_system_kver(void) +{ + struct utsname utsname; + int a, b, c; + + uname(&utsname); + + sscanf(utsname.release, "%d.%d.%d", &a, &b, &c); + + return KERNEL_VERSION(a,b,c); +} + struct ndctl_test *ndctl_test_new(unsigned int kver) { struct ndctl_test *test = calloc(1, sizeof(*test)); @@ -19,7 +32,7 @@ struct ndctl_test *ndctl_test_new(unsign return NULL; if (!kver) - test->kver = LINUX_VERSION_CODE; + test->kver = get_system_kver(); else test->kver = kver;
Grab the kernel version used for tests dynamically via utsname() instead of hardcoding the version of the build host. Otherwise tests will be skipped if the build host had a too old kernel version. flodin:~ # ./ndctl test __ndctl_test_attempt: skip test_libndctl:1950 requires: 4.2.0 current: 4.1.0 test-libndctl: SKIP __ndctl_test_attempt: skip test_dpa_alloc:300 requires: 4.2.0 current: 4.1.0 test-dpa-alloc: SKIP __ndctl_test_attempt: skip test_parent_uuid:230 requires: 4.3.0 current: 4.1.0 test-parent-uuid: SKIP attempted: 3 skipped: 3 Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de> --- test/core.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) Changes to v1: Use utsname.release which is obviously correct. On my test system utsname.release = "4.4.4-default" I can only imagine the sscanf() failing and on stack garbage in a, b, c being greater than KERNEL_VERSION(4, 3, 0).