Re: [ndctl PATCH v3 2/4] test: Don't skip tests if nfit modules are missing
"Verma, Vishal L" writes:
> On Fri, 2021-03-19 at 11:20 +0530, Santosh Sivaraj wrote:
>> "Verma, Vishal L" writes:
>
> [..]
>
>> >
>> > fix multi line comment to the right formatting:
>> > /*
>> > * line 1, etc
>> > */
>> >
>>
>> Will fix that.
>>
>> > > +if (access("/sys/bus/acpi", F_OK) == -1) {
>> > > +if (errno == ENOENT)
>> > > +family = NVDIMM_FAMILY_PAPR;
>> > > +}
>> >
>> > Instead of a blind default, can we perform a similar check for presence of
>> > PAPR too?
>> >
>>
>> Yes, I wanted to do that, but there is no reliable way of check that; there
>> is
>> no ofnode before module load, and there won't be any PAPR specific DT
>> entries if
>> the platform is not Power.
>>
>> I also test the 'ndtest' module on x86 with NDCTL_TEST_FAMILY environment
>> variable. I can let the default be nfit_test (NVDIMM_FAMILY_INTEL) and only
>> load
>> PAPR module when the environment variable is set. Thoughts?
>>
> The env variable seems reasonable to me. If there is ever a third
> 'family' adding tests, having an arbitrary default might be awkward.
> I may suggest - if acpi is detected, use NFIT. If env has something that
> is known, e.g. PAPR, use that. If env is unset or doesn't match anything
> we know about, then bail with an error message. Does that sound
> reasonable?
Yes, that sounds too to me. I will send in the next version soon.
Thanks for the review!
Thanks,
Santosh
___
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-le...@lists.01.org
Re: [ndctl PATCH v3 2/4] test: Don't skip tests if nfit modules are missing
On Fri, 2021-03-19 at 11:20 +0530, Santosh Sivaraj wrote:
> "Verma, Vishal L" writes:
[..]
> >
> > fix multi line comment to the right formatting:
> > /*
> > * line 1, etc
> > */
> >
>
> Will fix that.
>
> > > + if (access("/sys/bus/acpi", F_OK) == -1) {
> > > + if (errno == ENOENT)
> > > + family = NVDIMM_FAMILY_PAPR;
> > > + }
> >
> > Instead of a blind default, can we perform a similar check for presence of
> > PAPR too?
> >
>
> Yes, I wanted to do that, but there is no reliable way of check that; there is
> no ofnode before module load, and there won't be any PAPR specific DT entries
> if
> the platform is not Power.
>
> I also test the 'ndtest' module on x86 with NDCTL_TEST_FAMILY environment
> variable. I can let the default be nfit_test (NVDIMM_FAMILY_INTEL) and only
> load
> PAPR module when the environment variable is set. Thoughts?
>
The env variable seems reasonable to me. If there is ever a third
'family' adding tests, having an arbitrary default might be awkward.
I may suggest - if acpi is detected, use NFIT. If env has something that
is known, e.g. PAPR, use that. If env is unset or doesn't match anything
we know about, then bail with an error message. Does that sound
reasonable?
___
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-le...@lists.01.org
Re: [ndctl PATCH v3 2/4] test: Don't skip tests if nfit modules are missing
"Verma, Vishal L" writes:
> On Thu, 2021-03-11 at 13:16 +0530, Santosh Sivaraj wrote:
>> For NFIT to be available ACPI is a must, so don't fail when nfit modules
>> are missing on a platform that doesn't support ACPI.
>>
>> Signed-off-by: Santosh Sivaraj
>> ---
>> test.h| 2 +-
>> test/ack-shutdown-count-set.c | 2 +-
>> test/blk_namespaces.c | 2 +-
>> test/core.c | 23 +--
>> test/dpa-alloc.c | 2 +-
>> test/dsm-fail.c | 2 +-
>> test/libndctl.c | 2 +-
>> test/multi-pmem.c | 2 +-
>> test/parent-uuid.c| 2 +-
>> test/pmem_namespaces.c| 2 +-
>> 10 files changed, 30 insertions(+), 11 deletions(-)
>>
>> diff --git a/test.h b/test.h
>> index cba8d41..7de13fe 100644
>> --- a/test.h
>> +++ b/test.h
>> @@ -20,7 +20,7 @@ void builtin_xaction_namespace_reset(void);
>>
>>
>> struct kmod_ctx;
>> struct kmod_module;
>> -int nfit_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
>> +int ndctl_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
>> struct ndctl_ctx *nd_ctx, int log_level,
>> struct ndctl_test *test);
>>
>>
>> diff --git a/test/ack-shutdown-count-set.c b/test/ack-shutdown-count-set.c
>> index fb1d82b..c561ff3 100644
>> --- a/test/ack-shutdown-count-set.c
>> +++ b/test/ack-shutdown-count-set.c
>> @@ -99,7 +99,7 @@ static int test_ack_shutdown_count_set(int loglevel,
>> struct ndctl_test *test,
>> int result = EXIT_FAILURE, err;
>>
>>
>> ndctl_set_log_priority(ctx, loglevel);
>> -err = nfit_test_init(_ctx, , NULL, loglevel, test);
>> +err = ndctl_test_init(_ctx, , NULL, loglevel, test);
>> if (err < 0) {
>> result = 77;
>> ndctl_test_skip(test);
>> diff --git a/test/blk_namespaces.c b/test/blk_namespaces.c
>> index d7f00cb..f076e85 100644
>> --- a/test/blk_namespaces.c
>> +++ b/test/blk_namespaces.c
>> @@ -228,7 +228,7 @@ int test_blk_namespaces(int log_level, struct ndctl_test
>> *test,
>>
>>
>> if (!bus) {
>> fprintf(stderr, "ACPI.NFIT unavailable falling back to
>> nfit_test\n");
>> -rc = nfit_test_init(_ctx, , NULL, log_level, test);
>> +rc = ndctl_test_init(_ctx, , NULL, log_level, test);
>> ndctl_invalidate(ctx);
>> bus = ndctl_bus_get_by_provider(ctx, "nfit_test.0");
>> if (rc < 0 || !bus) {
>> diff --git a/test/core.c b/test/core.c
>> index cc7d8d9..903034a 100644
>> --- a/test/core.c
>> +++ b/test/core.c
>> @@ -11,6 +11,7 @@
>> #include
>> #include
>> #include
>> +#include
>> #include
>>
>>
>> #define KVER_STRLEN 20
>> @@ -106,11 +107,11 @@ int ndctl_test_get_skipped(struct ndctl_test *test)
>> return test->skip;
>> }
>>
>>
>> -int nfit_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
>> +int ndctl_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
>> struct ndctl_ctx *nd_ctx, int log_level,
>> struct ndctl_test *test)
>> {
>> -int rc;
>> +int rc, family = NVDIMM_FAMILY_INTEL;
>> unsigned int i;
>> const char *name;
>> struct ndctl_bus *bus;
>> @@ -127,6 +128,19 @@ int nfit_test_init(struct kmod_ctx **ctx, struct
>> kmod_module **mod,
>> "nd_e820",
>> "nd_pmem",
>> };
>> +char *test_env;
>> +
>> +/* Do we want to force test PAPR? */
>> +test_env = getenv("NDCTL_TEST_FAMILY");
>> +if (test_env && strcmp(test_env, "PAPR") == 0)
>> +family = NVDIMM_FAMILY_PAPR;
>> +
>> +/* ACPI is a must for nfit, so if ACPI is not available let's default to
>> + * PAPR */
>
> fix multi line comment to the right formatting:
> /*
> * line 1, etc
> */
>
Will fix that.
>> +if (access("/sys/bus/acpi", F_OK) == -1) {
>> +if (errno == ENOENT)
>> +family = NVDIMM_FAMILY_PAPR;
>> +}
>
> Instead of a blind default, can we perform a similar check for presence of
> PAPR too?
>
Yes, I wanted to do that, but there is no reliable way of check that; there is
no ofnode before module load, and there won't be any PAPR specific DT entries if
the platform is not Power.
I also test the 'ndtest' module on x86 with NDCTL_TEST_FAMILY environment
variable. I can let the default be nfit_test (NVDIMM_FAMILY_INTEL) and only load
PAPR module when the environment variable is set. Thoughts?
Thanks,
Santosh
>>
>>
>> log_init(_ctx, "test/init", "NDCTL_TEST");
>> log_ctx.log_priority = log_level;
>> @@ -185,6 +199,11 @@ retry:
>>
>>
>> path = kmod_module_get_path(*mod);
>> if (!path) {
>> +if (family == NVDIMM_FAMILY_PAPR &&
>> +(strcmp(name, "nfit") == 0 ||
>> + strcmp(name, "nd_e820") == 0))
>> +continue;
>> +
>> log_err(_ctx, "%s.ko: failed to
Re: [ndctl PATCH v3 2/4] test: Don't skip tests if nfit modules are missing
On Thu, 2021-03-11 at 13:16 +0530, Santosh Sivaraj wrote:
> For NFIT to be available ACPI is a must, so don't fail when nfit modules
> are missing on a platform that doesn't support ACPI.
>
> Signed-off-by: Santosh Sivaraj
> ---
> test.h| 2 +-
> test/ack-shutdown-count-set.c | 2 +-
> test/blk_namespaces.c | 2 +-
> test/core.c | 23 +--
> test/dpa-alloc.c | 2 +-
> test/dsm-fail.c | 2 +-
> test/libndctl.c | 2 +-
> test/multi-pmem.c | 2 +-
> test/parent-uuid.c| 2 +-
> test/pmem_namespaces.c| 2 +-
> 10 files changed, 30 insertions(+), 11 deletions(-)
>
> diff --git a/test.h b/test.h
> index cba8d41..7de13fe 100644
> --- a/test.h
> +++ b/test.h
> @@ -20,7 +20,7 @@ void builtin_xaction_namespace_reset(void);
>
>
> struct kmod_ctx;
> struct kmod_module;
> -int nfit_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
> +int ndctl_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
> struct ndctl_ctx *nd_ctx, int log_level,
> struct ndctl_test *test);
>
>
> diff --git a/test/ack-shutdown-count-set.c b/test/ack-shutdown-count-set.c
> index fb1d82b..c561ff3 100644
> --- a/test/ack-shutdown-count-set.c
> +++ b/test/ack-shutdown-count-set.c
> @@ -99,7 +99,7 @@ static int test_ack_shutdown_count_set(int loglevel, struct
> ndctl_test *test,
> int result = EXIT_FAILURE, err;
>
>
> ndctl_set_log_priority(ctx, loglevel);
> - err = nfit_test_init(_ctx, , NULL, loglevel, test);
> + err = ndctl_test_init(_ctx, , NULL, loglevel, test);
> if (err < 0) {
> result = 77;
> ndctl_test_skip(test);
> diff --git a/test/blk_namespaces.c b/test/blk_namespaces.c
> index d7f00cb..f076e85 100644
> --- a/test/blk_namespaces.c
> +++ b/test/blk_namespaces.c
> @@ -228,7 +228,7 @@ int test_blk_namespaces(int log_level, struct ndctl_test
> *test,
>
>
> if (!bus) {
> fprintf(stderr, "ACPI.NFIT unavailable falling back to
> nfit_test\n");
> - rc = nfit_test_init(_ctx, , NULL, log_level, test);
> + rc = ndctl_test_init(_ctx, , NULL, log_level, test);
> ndctl_invalidate(ctx);
> bus = ndctl_bus_get_by_provider(ctx, "nfit_test.0");
> if (rc < 0 || !bus) {
> diff --git a/test/core.c b/test/core.c
> index cc7d8d9..903034a 100644
> --- a/test/core.c
> +++ b/test/core.c
> @@ -11,6 +11,7 @@
> #include
> #include
> #include
> +#include
> #include
>
>
> #define KVER_STRLEN 20
> @@ -106,11 +107,11 @@ int ndctl_test_get_skipped(struct ndctl_test *test)
> return test->skip;
> }
>
>
> -int nfit_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
> +int ndctl_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
> struct ndctl_ctx *nd_ctx, int log_level,
> struct ndctl_test *test)
> {
> - int rc;
> + int rc, family = NVDIMM_FAMILY_INTEL;
> unsigned int i;
> const char *name;
> struct ndctl_bus *bus;
> @@ -127,6 +128,19 @@ int nfit_test_init(struct kmod_ctx **ctx, struct
> kmod_module **mod,
> "nd_e820",
> "nd_pmem",
> };
> + char *test_env;
> +
> + /* Do we want to force test PAPR? */
> + test_env = getenv("NDCTL_TEST_FAMILY");
> + if (test_env && strcmp(test_env, "PAPR") == 0)
> + family = NVDIMM_FAMILY_PAPR;
> +
> + /* ACPI is a must for nfit, so if ACPI is not available let's default to
> + * PAPR */
fix multi line comment to the right formatting:
/*
* line 1, etc
*/
> + if (access("/sys/bus/acpi", F_OK) == -1) {
> + if (errno == ENOENT)
> + family = NVDIMM_FAMILY_PAPR;
> + }
Instead of a blind default, can we perform a similar check for presence of
PAPR too?
>
>
> log_init(_ctx, "test/init", "NDCTL_TEST");
> log_ctx.log_priority = log_level;
> @@ -185,6 +199,11 @@ retry:
>
>
> path = kmod_module_get_path(*mod);
> if (!path) {
> + if (family == NVDIMM_FAMILY_PAPR &&
> + (strcmp(name, "nfit") == 0 ||
> + strcmp(name, "nd_e820") == 0))
> + continue;
> +
> log_err(_ctx, "%s.ko: failed to get path\n", name);
> break;
> }
> diff --git a/test/dpa-alloc.c b/test/dpa-alloc.c
> index e922009..0b3bb7a 100644
> --- a/test/dpa-alloc.c
> +++ b/test/dpa-alloc.c
> @@ -289,7 +289,7 @@ int test_dpa_alloc(int loglevel, struct ndctl_test *test,
> struct ndctl_ctx *ctx)
> return 77;
>
>
> ndctl_set_log_priority(ctx, loglevel);
> - err = nfit_test_init(_ctx, , NULL, loglevel, test);
> + err = ndctl_test_init(_ctx, , NULL, loglevel, test);
> if (err < 0) {
>
[ndctl PATCH v3 2/4] test: Don't skip tests if nfit modules are missing
For NFIT to be available ACPI is a must, so don't fail when nfit modules
are missing on a platform that doesn't support ACPI.
Signed-off-by: Santosh Sivaraj
---
test.h| 2 +-
test/ack-shutdown-count-set.c | 2 +-
test/blk_namespaces.c | 2 +-
test/core.c | 23 +--
test/dpa-alloc.c | 2 +-
test/dsm-fail.c | 2 +-
test/libndctl.c | 2 +-
test/multi-pmem.c | 2 +-
test/parent-uuid.c| 2 +-
test/pmem_namespaces.c| 2 +-
10 files changed, 30 insertions(+), 11 deletions(-)
diff --git a/test.h b/test.h
index cba8d41..7de13fe 100644
--- a/test.h
+++ b/test.h
@@ -20,7 +20,7 @@ void builtin_xaction_namespace_reset(void);
struct kmod_ctx;
struct kmod_module;
-int nfit_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
+int ndctl_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
struct ndctl_ctx *nd_ctx, int log_level,
struct ndctl_test *test);
diff --git a/test/ack-shutdown-count-set.c b/test/ack-shutdown-count-set.c
index fb1d82b..c561ff3 100644
--- a/test/ack-shutdown-count-set.c
+++ b/test/ack-shutdown-count-set.c
@@ -99,7 +99,7 @@ static int test_ack_shutdown_count_set(int loglevel, struct
ndctl_test *test,
int result = EXIT_FAILURE, err;
ndctl_set_log_priority(ctx, loglevel);
- err = nfit_test_init(_ctx, , NULL, loglevel, test);
+ err = ndctl_test_init(_ctx, , NULL, loglevel, test);
if (err < 0) {
result = 77;
ndctl_test_skip(test);
diff --git a/test/blk_namespaces.c b/test/blk_namespaces.c
index d7f00cb..f076e85 100644
--- a/test/blk_namespaces.c
+++ b/test/blk_namespaces.c
@@ -228,7 +228,7 @@ int test_blk_namespaces(int log_level, struct ndctl_test
*test,
if (!bus) {
fprintf(stderr, "ACPI.NFIT unavailable falling back to
nfit_test\n");
- rc = nfit_test_init(_ctx, , NULL, log_level, test);
+ rc = ndctl_test_init(_ctx, , NULL, log_level, test);
ndctl_invalidate(ctx);
bus = ndctl_bus_get_by_provider(ctx, "nfit_test.0");
if (rc < 0 || !bus) {
diff --git a/test/core.c b/test/core.c
index cc7d8d9..903034a 100644
--- a/test/core.c
+++ b/test/core.c
@@ -11,6 +11,7 @@
#include
#include
#include
+#include
#include
#define KVER_STRLEN 20
@@ -106,11 +107,11 @@ int ndctl_test_get_skipped(struct ndctl_test *test)
return test->skip;
}
-int nfit_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
+int ndctl_test_init(struct kmod_ctx **ctx, struct kmod_module **mod,
struct ndctl_ctx *nd_ctx, int log_level,
struct ndctl_test *test)
{
- int rc;
+ int rc, family = NVDIMM_FAMILY_INTEL;
unsigned int i;
const char *name;
struct ndctl_bus *bus;
@@ -127,6 +128,19 @@ int nfit_test_init(struct kmod_ctx **ctx, struct
kmod_module **mod,
"nd_e820",
"nd_pmem",
};
+ char *test_env;
+
+ /* Do we want to force test PAPR? */
+ test_env = getenv("NDCTL_TEST_FAMILY");
+ if (test_env && strcmp(test_env, "PAPR") == 0)
+ family = NVDIMM_FAMILY_PAPR;
+
+ /* ACPI is a must for nfit, so if ACPI is not available let's default to
+* PAPR */
+ if (access("/sys/bus/acpi", F_OK) == -1) {
+ if (errno == ENOENT)
+ family = NVDIMM_FAMILY_PAPR;
+ }
log_init(_ctx, "test/init", "NDCTL_TEST");
log_ctx.log_priority = log_level;
@@ -185,6 +199,11 @@ retry:
path = kmod_module_get_path(*mod);
if (!path) {
+ if (family == NVDIMM_FAMILY_PAPR &&
+ (strcmp(name, "nfit") == 0 ||
+strcmp(name, "nd_e820") == 0))
+ continue;
+
log_err(_ctx, "%s.ko: failed to get path\n", name);
break;
}
diff --git a/test/dpa-alloc.c b/test/dpa-alloc.c
index e922009..0b3bb7a 100644
--- a/test/dpa-alloc.c
+++ b/test/dpa-alloc.c
@@ -289,7 +289,7 @@ int test_dpa_alloc(int loglevel, struct ndctl_test *test,
struct ndctl_ctx *ctx)
return 77;
ndctl_set_log_priority(ctx, loglevel);
- err = nfit_test_init(_ctx, , NULL, loglevel, test);
+ err = ndctl_test_init(_ctx, , NULL, loglevel, test);
if (err < 0) {
ndctl_test_skip(test);
fprintf(stderr, "nfit_test unavailable skipping tests\n");
diff --git a/test/dsm-fail.c b/test/dsm-fail.c
index 9dfd8b0..0a6383d 100644
--- a/test/dsm-fail.c
+++ b/test/dsm-fail.c
@@ -346,7 +346,7 @@ int test_dsm_fail(int loglevel, struct ndctl_test *test,
struct ndctl_ctx *ctx)
int result = EXIT_FAILURE, err;
ndctl_set_log_priority(ctx, loglevel);
-
