Re: [PATCH v9 1/7] include: add zoned device structs

[Stefan Hajnoczi <stefanha@redhat.com>]

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

On Thu, Sep 15, 2022 at 06:06:38PM +0800, Sam Li wrote:
> Eric Blake <eblake@redhat.com> 于2022年9月15日周四 16:05写道：
> >
> > On Sat, Sep 10, 2022 at 01:27:53PM +0800, Sam Li wrote:
> > > Signed-off-by: Sam Li <faithilikerun@gmail.com>
> > > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> > > Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
> > > ---
> > >  include/block/block-common.h | 43 ++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 43 insertions(+)
> > >
> > > diff --git a/include/block/block-common.h b/include/block/block-common.h
> > > index fdb7306e78..36bd0e480e 100644
> > > --- a/include/block/block-common.h
> > > +++ b/include/block/block-common.h
> > > @@ -49,6 +49,49 @@ typedef struct BlockDriver BlockDriver;
> > >  typedef struct BdrvChild BdrvChild;
> > >  typedef struct BdrvChildClass BdrvChildClass;
> > >
> > > +typedef enum BlockZoneOp {
> > > +    BLK_ZO_OPEN,
> > > +    BLK_ZO_CLOSE,
> > > +    BLK_ZO_FINISH,
> > > +    BLK_ZO_RESET,
> > > +} BlockZoneOp;
> > > +
> > > +typedef enum BlockZoneModel {
> > > +    BLK_Z_NONE = 0x0, /* Regular block device */
> > > +    BLK_Z_HM = 0x1, /* Host-managed zoned block device */
> > > +    BLK_Z_HA = 0x2, /* Host-aware zoned block device */
> > > +} BlockZoneModel;
> > > +
> > > +typedef enum BlockZoneCondition {
> > > +    BLK_ZS_NOT_WP = 0x0,
> > > +    BLK_ZS_EMPTY = 0x1,
> > > +    BLK_ZS_IOPEN = 0x2,
> > > +    BLK_ZS_EOPEN = 0x3,
> > > +    BLK_ZS_CLOSED = 0x4,
> > > +    BLK_ZS_RDONLY = 0xD,
> > > +    BLK_ZS_FULL = 0xE,
> > > +    BLK_ZS_OFFLINE = 0xF,
> > > +} BlockZoneCondition;
> > > +
> > > +typedef enum BlockZoneType {
> > > +    BLK_ZT_CONV = 0x1, /* Conventional random writes supported */
> > > +    BLK_ZT_SWR = 0x2, /* Sequential writes required */
> > > +    BLK_ZT_SWP = 0x3, /* Sequential writes preferred */
> > > +} BlockZoneType;
> > > +
> > > +/*
> > > + * Zone descriptor data structure.
> > > + * Provides information on a zone with all position and size values in bytes.
> >
> > I'm glad that you chose bytes here for use in qemu.  But since the
> > kernel struct blk_zone uses sectors instead of bytes, is it worth
> > adding a sentence that we intentionally use bytes here, different from
> > Linux, to make it easier for reviewers to realize that scaling when
> > translating between qemu and kernel is necessary?
> 
> Sorry about the unit mistake. The zone information is in sectors which
> is the same as kernel struct blk_zone. I think adding a sentence to
> inform the sector unit makes it clear what the zone descriptor is.

I'd make the units bytes for consistency with the rest of the QEMU block
layer. For example, the MapEntry structure that "qemu-img map" reports
has names with similar fields and they are in bytes:

  struct MapEntry {
      int64_t start;
      int64_t length;

Stefan

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