All of lore.kernel.org
 help / color / mirror / Atom feed
From: Klaus Jensen <its@irrelevant.dk>
To: Keith Busch <kbusch@kernel.org>
Cc: "Kevin Wolf" <kwolf@redhat.com>, "Fam Zheng" <fam@euphon.net>,
	"Damien Le Moal" <damien.lemoal@wdc.com>,
	qemu-block@nongnu.org, "Niklas Cassel" <niklas.cassel@wdc.com>,
	"Klaus Jensen" <k.jensen@samsung.com>,
	qemu-devel@nongnu.org, "Max Reitz" <mreitz@redhat.com>,
	"Alistair Francis" <alistair.francis@wdc.com>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>,
	"Matias Bjorling" <matias.bjorling@wdc.com>
Subject: Re: [PATCH v9 08/12] hw/block/nvme: Support Zoned Namespace Command Set
Date: Wed, 25 Nov 2020 22:12:05 +0100	[thread overview]
Message-ID: <X77IpabPCv1BXirj@apples.localdomain> (raw)
In-Reply-To: <20201118003234.GA2719991@dhcp-10-100-145-180.wdc.com>

[-- Attachment #1: Type: text/plain, Size: 1357 bytes --]

On Nov 17 16:32, Keith Busch wrote:
> On Thu, Nov 12, 2020 at 08:36:39PM +0100, Klaus Jensen wrote:
> > On Nov  5 11:53, Dmitry Fomichev wrote:
> > > @@ -133,6 +300,12 @@ static Property nvme_ns_props[] = {
> > >      DEFINE_BLOCK_PROPERTIES(NvmeNamespace, blkconf),
> > >      DEFINE_PROP_UINT32("nsid", NvmeNamespace, params.nsid, 0),
> > >      DEFINE_PROP_UUID("uuid", NvmeNamespace, params.uuid),
> > > +    DEFINE_PROP_BOOL("zoned", NvmeNamespace, params.zoned, false),
> > 
> > I disagree on this. Using the "magic" value ensures that only one
> > command set can be selected. We can do a custom property so we can set
> > `iocs=zoned` as well as `iocs=0x2` if that makes it more user friendly?
> 
> IMO, 'iocs' is a rather difficult parameter name for a user to remember
> compared to 'zoned=true'. While 'iocs' is a spec field, the spec isn't
> very user friendly either, and these user parameters can hide the spec
> terms behind human comprehensible names.
> 

I'm okay with a "zoned" bool parameter and having zone size and capacity
in bytes. But parameters such as ZASL, MAR and MOR are "expert"
parameters so I think its better to use the spec field names and
meanings for those. The nvme device already has precedence for using
spec field names (and meanings, e.g. zeroes based) for "expert"
parameters (mdts, aerl).

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

  reply	other threads:[~2020-11-25 21:15 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-05  2:53 [PATCH v9 00/12] hw/block/nvme: Support Namespace Types and Zoned Namespace Command Set Dmitry Fomichev
2020-11-05  2:53 ` [PATCH v9 01/12] hw/block/nvme: Add Commands Supported and Effects log Dmitry Fomichev
2020-11-05  2:53 ` [PATCH v9 02/12] hw/block/nvme: Generate namespace UUIDs Dmitry Fomichev
2020-11-05  6:36   ` Klaus Jensen
2020-11-05  2:53 ` [PATCH v9 03/12] hw/block/nvme: Separate read and write handlers Dmitry Fomichev
2020-11-05  2:53 ` [PATCH v9 04/12] hw/block/nvme: Merge nvme_write_zeroes() with nvme_write() Dmitry Fomichev
2020-11-05  2:53 ` [PATCH v9 05/12] hw/block/nvme: Add support for Namespace Types Dmitry Fomichev
2020-11-05  2:53 ` [PATCH v9 06/12] hw/block/nvme: Support allocated CNS command variants Dmitry Fomichev
2020-11-05  2:53 ` [PATCH v9 07/12] block/nvme: Make ZNS-related definitions Dmitry Fomichev
2020-11-05  2:53 ` [PATCH v9 08/12] hw/block/nvme: Support Zoned Namespace Command Set Dmitry Fomichev
2020-11-06 11:59   ` Niklas Cassel
2020-11-06 23:07     ` Dmitry Fomichev
2020-11-12 19:36   ` Klaus Jensen
2020-11-18  0:32     ` Keith Busch
2020-11-25 21:12       ` Klaus Jensen [this message]
2020-12-08 20:02     ` Dmitry Fomichev
2020-12-08 20:20       ` Klaus Jensen
2020-11-05  2:53 ` [PATCH v9 09/12] hw/block/nvme: Introduce max active and open zone limits Dmitry Fomichev
2020-11-05  2:53 ` [PATCH v9 10/12] hw/block/nvme: Support Zone Descriptor Extensions Dmitry Fomichev
2020-11-05  2:53 ` [PATCH v9 11/12] hw/block/nvme: Add injection of Offline/Read-Only zones Dmitry Fomichev
2020-11-05  2:53 ` [PATCH v9 12/12] hw/block/nvme: Document zoned parameters in usage text Dmitry Fomichev

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=X77IpabPCv1BXirj@apples.localdomain \
    --to=its@irrelevant.dk \
    --cc=alistair.francis@wdc.com \
    --cc=damien.lemoal@wdc.com \
    --cc=fam@euphon.net \
    --cc=k.jensen@samsung.com \
    --cc=kbusch@kernel.org \
    --cc=kwolf@redhat.com \
    --cc=matias.bjorling@wdc.com \
    --cc=mreitz@redhat.com \
    --cc=niklas.cassel@wdc.com \
    --cc=philmd@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.