| Message ID | 20210208003256.9280-1-dmitry.fomichev@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/block/nvme: fix Close Zone | expand |
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.
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; >
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!
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?
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.
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.
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 :)
> -----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 :)
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;
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(-)