If make this return 0 in the case when size is zero, then seed device will be
counted in due to
case ACTION_DISABLE:
rc = ndctl_namespace_disable_safe(ndns);
if (rc == 0)
(*processed)++;
break;
I ever think make it return 1.
It also can return correct number which not count in seed device.
But there will be no error msg when I disable seed device.
It will make user confuse.
Eg.
root@ubuntu-red:~/git/ndctl# ndctl/ndctl disable-namespace namespce0.0
disabled 0 namespace
So I follow enable-namespace to return -ENXIO and it will look like
root@ubuntu-red:~/git/ndctl# ndctl/ndctl disable-namespace namespce0.0
error disabling namespaces: No such device or address
disabled 0 namespaces
But no matter return -ENXIO or 1, it will make test/create.sh fail. This is why
I modify region.c
+ modprobe nfit_test
+ ../ndctl/ndctl disable-region -b nfit_test.0 all
disabled 0 regions
+ ../ndctl/ndctl zero-labels -b nfit_test.0 all
nmem1: regions active, abort label write
nmem3: regions active, abort label write
nmem0: regions active, abort label write
nmem2: regions active, abort label write
zeroed 0 nmem
++ err 28
+++ basename ./create.sh
++ echo test/create.sh: failed at line 28
-----Original Message-----
From: Dan Williams <[email protected]>
Sent: Tuesday, April 28, 2020 4:31 AM
To: Li, Redhairer <[email protected]>
Cc: linux-nvdimm <[email protected]>
Subject: Re: [PATCH 1/1] ndctl/namespace: Fix disable-namespace accounting
relative to seed devices
On Sun, Apr 26, 2020 at 2:53 AM Redhairer Li <[email protected]> wrote:
>
> Seed namespaces are included in "ndctl disable-namespace all". However
> since the user never "creates" them it is surprising to see
> "disable-namespace" report 1 more namespace relative to the number
> that have been created. Catch attempts to disable a zero-sized namespace:
>
> Before:
> {
> "dev":"namespace1.0",
> "size":"492.00 MiB (515.90 MB)",
> "blockdev":"pmem1"
> }
> {
> "dev":"namespace1.1",
> "size":"492.00 MiB (515.90 MB)",
> "blockdev":"pmem1.1"
> }
> {
> "dev":"namespace1.2",
> "size":"492.00 MiB (515.90 MB)",
> "blockdev":"pmem1.2"
> }
> disabled 4 namespaces
>
> After:
> {
> "dev":"namespace1.0",
> "size":"492.00 MiB (515.90 MB)",
> "blockdev":"pmem1"
> }
> {
> "dev":"namespace1.3",
> "size":"492.00 MiB (515.90 MB)",
> "blockdev":"pmem1.3"
> }
> {
> "dev":"namespace1.1",
> "size":"492.00 MiB (515.90 MB)",
> "blockdev":"pmem1.1"
> }
> disabled 3 namespaces
>
> Signed-off-by: Redhairer Li <[email protected]>
> ---
> ndctl/lib/libndctl.c | 11 ++++++++---
> ndctl/region.c | 4 +++-
> 2 files changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c index
> ee737cb..49f362b 100644
> --- a/ndctl/lib/libndctl.c
> +++ b/ndctl/lib/libndctl.c
> @@ -4231,6 +4231,7 @@ NDCTL_EXPORT int ndctl_namespace_disable_safe(struct
> ndctl_namespace *ndns)
> const char *bdev = NULL;
> char path[50];
> int fd;
> + unsigned long long size = ndctl_namespace_get_size(ndns);
>
> if (pfn && ndctl_pfn_is_enabled(pfn))
> bdev = ndctl_pfn_get_block_device(pfn); @@ -4260,9
> +4261,13 @@ NDCTL_EXPORT int ndctl_namespace_disable_safe(struct
> ndctl_namespace *ndns)
> devname, bdev, strerror(errno));
> return -errno;
> }
> - } else
> - ndctl_namespace_disable_invalidate(ndns);
> -
> + } else {
> + if (size == 0)
> + /* Don't try to disable idle namespace (no capacity
> allocated) */
> + return -ENXIO;
> + else
> + ndctl_namespace_disable_invalidate(ndns);
> + }
Maybe make this return 0 in the case when size is zero since by definition the
namespace must already be disabled if it has zero size.
} else if (size)
ndctl_namespace_disable_invalidate(ndns);
return 0;
>
> diff --git a/ndctl/region.c b/ndctl/region.c index 7945007..0014bb9
> 100644
> --- a/ndctl/region.c
> +++ b/ndctl/region.c
> @@ -72,6 +72,7 @@ static int region_action(struct ndctl_region
> *region, enum device_action mode) {
> struct ndctl_namespace *ndns;
> int rc = 0;
> + unsigned long long size;
>
> switch (mode) {
> case ACTION_ENABLE:
> @@ -80,7 +81,8 @@ static int region_action(struct ndctl_region *region, enum
> device_action mode)
> case ACTION_DISABLE:
> ndctl_namespace_foreach(region, ndns) {
> rc = ndctl_namespace_disable_safe(ndns);
> - if (rc)
> + size = ndctl_namespace_get_size(ndns);
> + if (rc && size != 0)
> return rc;
...then you wouldn't need to have this special case here since
ndctl_namespace_disable_safe() will not fail.
_______________________________________________
Linux-nvdimm mailing list -- [email protected]
To unsubscribe send an email to [email protected]
