Re: [PATCH 1/1] ndctl/namespace: Fix disable-namespace accounting relative to seed devices

From: Dan Williams <dan.j.williams@intel.com>
To: "Li, Redhairer" <redhairer.li@intel.com>
Cc: linux-nvdimm <linux-nvdimm@lists.01.org>
Subject: Re: [PATCH 1/1] ndctl/namespace: Fix disable-namespace accounting relative to seed devices
Date: Tue, 12 Jan 2021 23:07:50 -0800	[thread overview]
Message-ID: <CAPcyv4gupvktDG2PdCL6SyO4gwC1WoP8PKmPv1gP6pp8i10esQ@mail.gmail.com> (raw)
In-Reply-To: <BL0PR11MB3281CBD1CC9B64389731ECE492AC0@BL0PR11MB3281.namprd11.prod.outlook.com>

Hi Redhairer, sorry for the long delay circling back to this...

On Mon, Apr 27, 2020 at 6:10 PM Li, Redhairer <redhairer.li@intel.com> wrote:
>
> 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.

I think returning 1 is the right answer, because this isn't a failure
and there are several other places that call
ndctl_namespace_disable_safe() that need to be updated to treat rc < 0
as failure and rc >= 0 as success / no disable necessary.

ndctl/namespace.c:1127: rc = ndctl_namespace_disable_safe(ndns);
ndctl/namespace.c-1128- if (rc)
--
ndctl/namespace.c:1433: rc = ndctl_namespace_disable_safe(ndns);
ndctl/namespace.c-1434- if (rc) {
--
ndctl/namespace.c:1457: rc = ndctl_namespace_disable_safe(ndns);
ndctl/namespace.c-1458- if (rc) {
--
ndctl/namespace.c:1480: rc = ndctl_namespace_disable_safe(ndns);
ndctl/namespace.c-1481- if (rc) {
--
ndctl/namespace.c:2215:                                 rc =
ndctl_namespace_disable_safe(ndns);
ndctl/namespace.c-2216-                                 if (rc == 0)
--
ndctl/region.c:72:                      rc = ndctl_namespace_disable_safe(ndns);
ndctl/region.c-73-                      if (rc)

> 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

Why is that confusing isn't the namespace already disabled?

>
> 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

region.c and several other places need to be updated. I would split
this into 2 patches.

The first patch updates all callers of ndctl_disable_namespace_safe()
to treat rc > 0 as success.

Then the second patch can treat cmd_disable_namespace() to not count
rc > 0 as error, but also don't count it as disabled.
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org