[PATCH] hw/block/nvme: fix Close Zone

[PATCH] hw/block/nvme: fix Close Zone

[Dmitry Fomichev]

Implicitly and Explicitly Open zones can be closed by Close Zone
management function. This got broken by a recent commit and now such
commands fail with Invalid Zone State Transition status.

Modify nvm_zrm_close() function to make Close Zone work correctly.

Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
Fixes: 053b5a302c3("hw/block/nvme: refactor zone resource management")
---
 hw/block/nvme.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 6b84e34843..c2f0c88fbf 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -1308,14 +1308,13 @@ static uint16_t nvme_zrm_finish(NvmeNamespace *ns, NvmeZone *zone)
 static uint16_t nvme_zrm_close(NvmeNamespace *ns, NvmeZone *zone)
 {
     switch (nvme_get_zone_state(zone)) {
-    case NVME_ZONE_STATE_CLOSED:
-        return NVME_SUCCESS;
-
     case NVME_ZONE_STATE_EXPLICITLY_OPEN:
     case NVME_ZONE_STATE_IMPLICITLY_OPEN:
         nvme_aor_dec_open(ns);
         nvme_assign_zone_state(ns, zone, NVME_ZONE_STATE_CLOSED);
         /* fall through */
+    case NVME_ZONE_STATE_CLOSED:
+        return NVME_SUCCESS;
 
     default:
         return NVME_ZONE_INVAL_TRANSITION;
-- 
2.28.0

Re: [PATCH] hw/block/nvme: fix Close Zone

[Klaus Jensen]

On Feb  8 09:32, Dmitry Fomichev wrote:
> Implicitly and Explicitly Open zones can be closed by Close Zone
> management function. This got broken by a recent commit and now such
> commands fail with Invalid Zone State Transition status.
> 
> Modify nvm_zrm_close() function to make Close Zone work correctly.
> 
> Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
> Fixes: 053b5a302c3("hw/block/nvme: refactor zone resource management")
> ---
>  hw/block/nvme.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 6b84e34843..c2f0c88fbf 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -1308,14 +1308,13 @@ static uint16_t nvme_zrm_finish(NvmeNamespace *ns, NvmeZone *zone)
>  static uint16_t nvme_zrm_close(NvmeNamespace *ns, NvmeZone *zone)
>  {
>      switch (nvme_get_zone_state(zone)) {
> -    case NVME_ZONE_STATE_CLOSED:
> -        return NVME_SUCCESS;
> -
>      case NVME_ZONE_STATE_EXPLICITLY_OPEN:
>      case NVME_ZONE_STATE_IMPLICITLY_OPEN:
>          nvme_aor_dec_open(ns);
>          nvme_assign_zone_state(ns, zone, NVME_ZONE_STATE_CLOSED);
>          /* fall through */
> +    case NVME_ZONE_STATE_CLOSED:
> +        return NVME_SUCCESS;
>  
>      default:
>          return NVME_ZONE_INVAL_TRANSITION;
> -- 
> 2.28.0
> 

Thanks, applied!

The commit ids change when this is merged with master, so I removed the
"Fixes" tag and made a reference to the commit title in the commit
message instead.

Re: [PATCH] hw/block/nvme: fix Close Zone

[Philippe Mathieu-Daudé]

Hi Dmitry, Klaus.

On 2/8/21 1:32 AM, Dmitry Fomichev wrote:
> Implicitly and Explicitly Open zones can be closed by Close Zone
> management function. This got broken by a recent commit and now such
> commands fail with Invalid Zone State Transition status.
> 
> Modify nvm_zrm_close() function to make Close Zone work correctly.
> 
> Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
> Fixes: 053b5a302c3("hw/block/nvme: refactor zone resource management")

'053b5a302c3': unknown revision or path not in the working tree.

If you point at an unmerged commit, why not fix it directly
before merging?

> ---
>  hw/block/nvme.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 6b84e34843..c2f0c88fbf 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -1308,14 +1308,13 @@ static uint16_t nvme_zrm_finish(NvmeNamespace *ns, NvmeZone *zone)
>  static uint16_t nvme_zrm_close(NvmeNamespace *ns, NvmeZone *zone)
>  {
>      switch (nvme_get_zone_state(zone)) {
> -    case NVME_ZONE_STATE_CLOSED:
> -        return NVME_SUCCESS;
> -
>      case NVME_ZONE_STATE_EXPLICITLY_OPEN:
>      case NVME_ZONE_STATE_IMPLICITLY_OPEN:
>          nvme_aor_dec_open(ns);
>          nvme_assign_zone_state(ns, zone, NVME_ZONE_STATE_CLOSED);
>          /* fall through */
> +    case NVME_ZONE_STATE_CLOSED:
> +        return NVME_SUCCESS;
>  
>      default:
>          return NVME_ZONE_INVAL_TRANSITION;
> 

Re: [PATCH] hw/block/nvme: fix Close Zone

[Klaus Jensen]

On Feb  8 10:03, Philippe Mathieu-Daudé wrote:
> Hi Dmitry, Klaus.
> 
> On 2/8/21 1:32 AM, Dmitry Fomichev wrote:
> > Implicitly and Explicitly Open zones can be closed by Close Zone
> > management function. This got broken by a recent commit and now such
> > commands fail with Invalid Zone State Transition status.
> > 
> > Modify nvm_zrm_close() function to make Close Zone work correctly.
> > 
> > Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
> > Fixes: 053b5a302c3("hw/block/nvme: refactor zone resource management")
> 
> '053b5a302c3': unknown revision or path not in the working tree.
> 
> If you point at an unmerged commit, why not fix it directly
> before merging?
> 

Sure, I can squash it in. That's why we have a staging tree I guess ;)

Thanks Philippe!

Re: [PATCH] hw/block/nvme: fix Close Zone

[Klaus Jensen]

On Feb  8 10:03, Philippe Mathieu-Daudé wrote:
> Hi Dmitry, Klaus.
> 
> On 2/8/21 1:32 AM, Dmitry Fomichev wrote:
> > Implicitly and Explicitly Open zones can be closed by Close Zone
> > management function. This got broken by a recent commit and now such
> > commands fail with Invalid Zone State Transition status.
> > 
> > Modify nvm_zrm_close() function to make Close Zone work correctly.
> > 
> > Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
> > Fixes: 053b5a302c3("hw/block/nvme: refactor zone resource management")
> 
> '053b5a302c3': unknown revision or path not in the working tree.
> 
> If you point at an unmerged commit, why not fix it directly
> before merging?
> 

Dmitry, you OK with me squashing this fix and appending

    [dmitry: fix broken Close Zone]
    Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>

to the commit message?

Re: [PATCH] hw/block/nvme: fix Close Zone

[Keith Busch]

On Mon, Feb 08, 2021 at 10:20:51AM +0100, Klaus Jensen wrote:
> On Feb  8 10:03, Philippe Mathieu-Daudé wrote:
> > Hi Dmitry, Klaus.
> > 
> > On 2/8/21 1:32 AM, Dmitry Fomichev wrote:
> > > Implicitly and Explicitly Open zones can be closed by Close Zone
> > > management function. This got broken by a recent commit and now such
> > > commands fail with Invalid Zone State Transition status.
> > > 
> > > Modify nvm_zrm_close() function to make Close Zone work correctly.
> > > 
> > > Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
> > > Fixes: 053b5a302c3("hw/block/nvme: refactor zone resource management")
> > 
> > '053b5a302c3': unknown revision or path not in the working tree.
> > 
> > If you point at an unmerged commit, why not fix it directly
> > before merging?
> > 
> 
> Dmitry, you OK with me squashing this fix and appending
> 
>     [dmitry: fix broken Close Zone]
>     Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
> 
> to the commit message?

IMO, we should avoid the habit of rebasing and force pushes on staging
trees once they're public.

Re: [PATCH] hw/block/nvme: fix Close Zone

[Philippe Mathieu-Daudé]

Hi Keith,

On 2/8/21 4:54 PM, Keith Busch wrote:
> On Mon, Feb 08, 2021 at 10:20:51AM +0100, Klaus Jensen wrote:
>> On Feb  8 10:03, Philippe Mathieu-Daudé wrote:
>>> Hi Dmitry, Klaus.
>>>
>>> On 2/8/21 1:32 AM, Dmitry Fomichev wrote:
>>>> Implicitly and Explicitly Open zones can be closed by Close Zone
>>>> management function. This got broken by a recent commit and now such
>>>> commands fail with Invalid Zone State Transition status.
>>>>
>>>> Modify nvm_zrm_close() function to make Close Zone work correctly.
>>>>
>>>> Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
>>>> Fixes: 053b5a302c3("hw/block/nvme: refactor zone resource management")
>>>
>>> '053b5a302c3': unknown revision or path not in the working tree.
>>>
>>> If you point at an unmerged commit, why not fix it directly
>>> before merging?
>>>
>>
>> Dmitry, you OK with me squashing this fix and appending
>>
>>     [dmitry: fix broken Close Zone]
>>     Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
>>
>> to the commit message?
> 
> IMO, we should avoid the habit of rebasing and force pushes on staging
> trees once they're public.

Well I had not information this patch was targeting another tree.

If you don't want to send regular pull request, it would be useful
to ask the NVMe contributors to provide an information on which
tree their patch is based.

Regards,

Phil.

Re: [PATCH] hw/block/nvme: fix Close Zone

[Klaus Jensen]

On Feb  8 17:19, Philippe Mathieu-Daudé wrote:
> Hi Keith,
> 
> On 2/8/21 4:54 PM, Keith Busch wrote:
> > On Mon, Feb 08, 2021 at 10:20:51AM +0100, Klaus Jensen wrote:
> >> On Feb  8 10:03, Philippe Mathieu-Daudé wrote:
> >>> Hi Dmitry, Klaus.
> >>>
> >>> On 2/8/21 1:32 AM, Dmitry Fomichev wrote:
> >>>> Implicitly and Explicitly Open zones can be closed by Close Zone
> >>>> management function. This got broken by a recent commit and now such
> >>>> commands fail with Invalid Zone State Transition status.
> >>>>
> >>>> Modify nvm_zrm_close() function to make Close Zone work correctly.
> >>>>
> >>>> Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
> >>>> Fixes: 053b5a302c3("hw/block/nvme: refactor zone resource management")
> >>>
> >>> '053b5a302c3': unknown revision or path not in the working tree.
> >>>
> >>> If you point at an unmerged commit, why not fix it directly
> >>> before merging?
> >>>
> >>
> >> Dmitry, you OK with me squashing this fix and appending
> >>
> >>     [dmitry: fix broken Close Zone]
> >>     Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
> >>
> >> to the commit message?
> > 
> > IMO, we should avoid the habit of rebasing and force pushes on staging
> > trees once they're public.
> 
> Well I had not information this patch was targeting another tree.
> 
> If you don't want to send regular pull request, it would be useful
> to ask the NVMe contributors to provide an information on which
> tree their patch is based.
> 

I'm just behind on sending a that pull request. I'll do that :)

RE: [PATCH] hw/block/nvme: fix Close Zone

[Dmitry Fomichev]

> -----Original Message-----
> From: Klaus Jensen <k.jensen@samsung.com>
> Sent: Monday, February 8, 2021 12:54 PM
> To: Philippe Mathieu-Daudé <philmd@redhat.com>
> Cc: Keith Busch <kbusch@kernel.org>; Dmitry Fomichev
> <Dmitry.Fomichev@wdc.com>; Kevin Wolf <kwolf@redhat.com>; Max Reitz
> <mreitz@redhat.com>; qemu-devel@nongnu.org; Niklas Cassel
> <Niklas.Cassel@wdc.com>
> Subject: Re: [PATCH] hw/block/nvme: fix Close Zone
> 
> On Feb  8 17:19, Philippe Mathieu-Daudé wrote:
> > Hi Keith,
> >
> > On 2/8/21 4:54 PM, Keith Busch wrote:
> > > On Mon, Feb 08, 2021 at 10:20:51AM +0100, Klaus Jensen wrote:
> > >> On Feb  8 10:03, Philippe Mathieu-Daudé wrote:
> > >>> Hi Dmitry, Klaus.
> > >>>
> > >>> On 2/8/21 1:32 AM, Dmitry Fomichev wrote:
> > >>>> Implicitly and Explicitly Open zones can be closed by Close Zone
> > >>>> management function. This got broken by a recent commit and now
> such
> > >>>> commands fail with Invalid Zone State Transition status.
> > >>>>
> > >>>> Modify nvm_zrm_close() function to make Close Zone work
> correctly.
> > >>>>
> > >>>> Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
> > >>>> Fixes: 053b5a302c3("hw/block/nvme: refactor zone resource
> management")
> > >>>
> > >>> '053b5a302c3': unknown revision or path not in the working tree.
> > >>>
> > >>> If you point at an unmerged commit, why not fix it directly
> > >>> before merging?
> > >>>
> > >>
> > >> Dmitry, you OK with me squashing this fix and appending
> > >>
> > >>     [dmitry: fix broken Close Zone]
> > >>     Signed-off-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>
> > >>
> > >> to the commit message?
> > >

Sure. I should perhaps have added Based-on tag to the commit.

> > > IMO, we should avoid the habit of rebasing and force pushes on staging
> > > trees once they're public.
> >
> > Well I had not information this patch was targeting another tree.
> >
> > If you don't want to send regular pull request, it would be useful
> > to ask the NVMe contributors to provide an information on which
> > tree their patch is based.
> >
> 
> I'm just behind on sending a that pull request. I'll do that :)